diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index feee6913f..9f1e20121 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -304,8 +304,8 @@ fn move_effect(s: &mut Session, p: &Value) -> Result { if aligned(p) && super::appearance::appearances_differ(&s.doc()?.doc, &roots) { return Err(bad(C, "effects of objects whose appearances differ can't be moved together")); } - // Where the effect landed in the first object. - let mut landed = None; + // Where the effect landed in each object it moved in, in the order of `ids`. + let mut landed = vec![]; let ids = s.edit(if copy { "Duplicate Effect" } else { "Move Effect" }, |d, _| { let mut done = vec![]; for id in &roots { @@ -317,7 +317,7 @@ fn move_effect(s: &mut Session, p: &Value) -> Result { let at = to.min(fx.len()); fx.insert(at, e); check_revolve_count(n, C)?; - landed.get_or_insert((at, di)); + landed.push((at, di)); done.push(*id); } if done.is_empty() { @@ -325,7 +325,10 @@ fn move_effect(s: &mut Session, p: &Value) -> Result { } Ok(done) })?; - let (index, item) = landed.unzip(); + // The reply names the object whose row the destination is: the first selected object for the + // active item (which need not paint first), as `effect.apply` does; the first target for an + // explicit `toItem` or the object's effects. + let (index, item) = landed.get(dst.anchor(s, &ids)).copied().unzip(); Ok(json!({ "ids": ids_json(&ids), "index": index, "item": item.flatten() })) } diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index e1728c1ad..a49bae19a 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -534,6 +534,43 @@ fn apply_reports_the_index_in_the_object_whose_row_it_names() { assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(1)), "a's row 0 holds two effects"); } +/// `effect.move`'s reply names where the effect landed in the object whose row the active item is +/// (the first selected object), also when that object paints above the others. +#[test] +fn move_reports_the_index_in_the_object_whose_row_it_names() { + let mut s = session(); + let (a, b) = reversed_pair(&mut s); + run(&mut s, "select.set", json!({"ids": [a.0]})); + run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 0})); + run(&mut s, "select.set", json!({"ids": [b.0]})); + run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 0})); + run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 0})); + run(&mut s, "select.set", json!({"ids": [b.0, a.0]})); + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + // Row 0's first effect goes to the end of the row: index 1 in b, 0 in a, which paints first. + let r = run(&mut s, "effect.move", json!({"from": 0, "to": 9})); + assert_eq!((node(&s, a).appearance.items[0].effects().len(), node(&s, b).appearance.items[0].effects().len()), (1, 2)); + assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(1)), "b's row 0 holds two effects"); + // An explicit item is a row of the first target, a. + let r = run(&mut s, "effect.move", json!({"from": 0, "to": 9, "fromItem": 0})); + assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(0)), "a's row 0 holds one effect"); + // When the first selected object has no effect to move, the reply names the first object that moved. + run(&mut s, "select.set", json!({"ids": [a.0, b.0]})); + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + let r = run(&mut s, "effect.move", json!({"from": 1, "to": 0})); + assert_eq!(r["ids"], json!([b.0]), "only b has a second effect in row 0"); + assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(0)), "b's effect moved to the start of its row 0"); + // From the active row to an explicit destination (here the object's effects, where b already + // has one), the reply names the first target, a. + run(&mut s, "select.set", json!({"ids": [b.0]})); + run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": null})); + run(&mut s, "select.set", json!({"ids": [b.0, a.0]})); + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + let r = run(&mut s, "effect.move", json!({"from": 0, "to": 9, "toItem": null})); + assert_eq!((node(&s, a).appearance.effects.len(), node(&s, b).appearance.effects.len()), (1, 2)); + assert_eq!((r["item"].clone(), r["index"].clone()), (json!(null), json!(0)), "a's effects hold one effect"); +} + /// How many steps there are to undo and to redo, and whether the document has unsaved changes. fn history(s: &Session) -> (usize, usize, bool) { let st = s.doc().unwrap();