From 18510b77afae83ec1ec614218d9d9ddd7006cb50 Mon Sep 17 00:00:00 2001
From: dotCooCoo <RobertLeeLW@gmail.com>
Date: Sat, 31 Oct 2026 12:06:47 -0700
Subject: [PATCH] Appearance: effect.apply reports the new effect's index in
 the object whose row it names

The reply's index came from the last object the effect was added to, while
its item came from the first selected object (or the first target for an
explicit item). When those objects held different numbers of effects on the
row, the reply named no effect of that object. The index is now read from
that object's effect list.
---
 crates/engine/src/cmd/effectcmd.rs    |  5 ++---
 crates/engine/src/tests/appearance.rs | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs
index 8010931a7..feee6913f 100644
--- a/crates/engine/src/cmd/effectcmd.rs
+++ b/crates/engine/src/cmd/effectcmd.rs
@@ -213,16 +213,15 @@ pub(crate) fn apply(s: &mut Session, p: &Value) -> Result<Value> {
             }
         }
     }
-    let mut index = 0;
     let ids = edit_effects(s, p, item, C, &label, "Apply Effect: select objects", 0, |fx, _| {
         fx.push(effect.clone());
-        index = fx.len() - 1;
         true
     })?;
     // The item the effect landed on in the object whose row the item is (the first selected object
-    // for the active item, which applies where it fits).
+    // for the active item, which applies where it fits), and the effect's index in that object.
     let first = ids.get(item.anchor(s, &ids)).and_then(|id| s.doc().ok()?.doc.node(*id));
     let landed = first.and_then(|n| item.effects_item(&n.appearance, C).ok().flatten());
+    let index = first.and_then(|n| n.appearance.effects_at(landed)).map_or(0, |fx| fx.len().saturating_sub(1));
     Ok(json!({ "ids": ids_json(&ids), "index": index, "item": landed }))
 }
 
diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs
index e36871188..e1728c1ad 100644
--- a/crates/engine/src/tests/appearance.rs
+++ b/crates/engine/src/tests/appearance.rs
@@ -513,6 +513,27 @@ fn edits_change_the_active_row_of_the_first_selected_object() {
     assert!(!e.contains("internal error"), "{e}");
 }
 
+/// `effect.apply`'s reply names the new effect by its index in the object whose row `item` is,
+/// also when the objects painted after it already have effects there.
+#[test]
+fn apply_reports_the_index_in_the_object_whose_row_it_names() {
+    let mut s = session();
+    let (a, b) = reversed_pair(&mut s);
+    let c = rect(&mut s, 350.0);
+    run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 0}));
+    // b's red fill row is not shared, so each object's row 0 takes the effect: c's second one.
+    run(&mut s, "select.set", json!({"ids": [b.0, a.0, c.0]}));
+    run(&mut s, "appearance.setActiveItem", json!({"index": 0}));
+    let r = run(&mut s, "effect.apply", json!({"effect": "distort.twist"}));
+    assert_eq!(node(&s, c).appearance.items[0].effects().len(), 2);
+    assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(0)), "b's row 0 holds one effect");
+    // An explicit item is a row of the first target, a, which paints below c.
+    run(&mut s, "select.set", json!({"ids": [c.0, a.0]}));
+    let r = run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 0}));
+    assert_eq!(node(&s, c).appearance.items[0].effects().len(), 3);
+    assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(1)), "a's row 0 holds two effects");
+}
+
 /// 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();