diff --git a/crates/engine/src/commands/prop.rs b/crates/engine/src/commands/prop.rs index 8f263818..b2198972 100644 --- a/crates/engine/src/commands/prop.rs +++ b/crates/engine/src/commands/prop.rs @@ -987,6 +987,7 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result { let sel = s.state.selected_keys.clone(); s.edit("Delete Keyframes", None, |proj, st| { let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; + let mut removed = 0usize; for k in &sel { if let Some(pr) = comp.layer_mut(k.layer).and_then(|l| l.props.find_mut(k.prop)) && let Some(i) = key_at(&pr.keys, k.time) @@ -995,10 +996,13 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result { pr.value = pr.keys[0].value.clone(); } pr.keys.remove(i); + removed += 1; } } st.selected_keys.clear(); - Ok(json!(sel.len())) + // The keys actually removed: a selected key that is gone (or listed twice) isn't counted, + // so a caller can tell that a delete changed nothing. + Ok(json!(removed)) }) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index a5b016b7..a785f9bd 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1232,6 +1232,8 @@ mod tests_codec_options; #[cfg(test)] mod tests_color_view; #[cfg(test)] +mod tests_delete_keys; +#[cfg(test)] mod tests_disk_cache; #[cfg(test)] mod tests_effects; diff --git a/crates/engine/src/tests_delete_keys.rs b/crates/engine/src/tests_delete_keys.rs new file mode 100644 index 00000000..b1b9af31 --- /dev/null +++ b/crates/engine/src/tests_delete_keys.rs @@ -0,0 +1,45 @@ +//! `keys.delete` reports the keys it removed, not the keys it was asked to remove. + +use effectcraft_project::LayerId; +use serde_json::json; + +use crate::{KeyRef, Session}; + +fn setup() -> (Session, u64, u64) { + let mut s = Session::default(); + s.execute_checked("comp.new", json!({"width": 64, "height": 64, "frameRate": 30, "duration": 4})).unwrap(); + let layer = s.execute_checked("layer.newSolid", json!({"name": "Box", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for t in 0..3 { + s.execute_checked("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": t, "value": t * 25})).unwrap(); + } + let prop = s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.prop("transform/opacity").unwrap().uid; + (s, layer, prop) +} + +fn keys_of(s: &Session, layer: u64, prop: u64) -> Vec { + let keys = &s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys; + keys.iter().map(|k| KeyRef { layer: LayerId(layer), prop, time: k.time }).collect() +} + +#[test] +fn delete_keys_counts_only_the_keys_it_removed() { + let (mut s, layer, prop) = setup(); + let keys = keys_of(&s, layer, prop); + assert_eq!(keys.len(), 3); + // Two real keys, one of them listed twice, and a key of a property that does not exist. + let missing = KeyRef { layer: LayerId(layer), prop: u64::MAX, time: keys[1].time }; + s.state.selected_keys = vec![keys[0], keys[2], keys[0], missing]; + let removed = s.execute_checked("keys.delete", json!({})).unwrap(); + assert_eq!(removed, json!(2)); + assert_eq!(keys_of(&s, layer, prop), vec![keys[1]]); +} + +#[test] +fn delete_keys_reports_zero_when_no_selected_key_exists() { + let (mut s, layer, prop) = setup(); + let keys = keys_of(&s, layer, prop); + s.state.selected_keys = vec![KeyRef { layer: LayerId(layer), prop: u64::MAX, time: keys[0].time }]; + let removed = s.execute_checked("keys.delete", json!({})).unwrap(); + assert_eq!(removed, json!(0)); + assert_eq!(keys_of(&s, layer, prop), keys); +}