Expressions: pick-whip references to effect properties by index (refs #1072)

In After Effects, holding Alt while dropping the expression pick whip
refers to an effect's property by its index, effect("Color Control")(1),
instead of its name, ("Color").

prop.pickWhip takes byIndex: the effect keeps its name and each
property inside it is selected by its one-based place among its group's
children (nested groups too), which is what an expression's (1) reads.
Other properties, dimensions ([1]), insertion at the cursor and match
names (Compact English off) are as before. An index still reads a
renamed property, but another one if properties are reordered.

Tests: pick_whip_by_index_selects_effect_properties_by_their_index
(engine, the references); effectcraft-host pick_whip (by index and by
name read the same values, nested and per dimension).

crates/engine/src/commands/link.rs

@@ -102,11 +102,39 @@ fn quote(s: &str) -> String {
 
 /// The AE expression reference to property `uid` of `target` as seen from a property of `from`.
 pub(crate) fn reference(comp: &Comp, from: &Layer, target: &Layer, uid: Uid, compact: bool) -> Option<String> {
+    reference_with(comp, from, target, uid, compact, false)
+}
+
+/// [`reference`], with the properties inside an effect selected by their index (`by_index`, an
+/// Alt pick whip in After Effects): `effect("Color Control")(1)` rather than `("Color")`. The
+/// effect keeps its name. An index is the property's place among its group's children, as an
+/// expression's `(1)` reads it: it follows a renamed parameter, not one moved to another place.
+pub(crate) fn reference_with(comp: &Comp, from: &Layer, target: &Layer, uid: Uid, compact: bool, by_index: bool) -> Option<String> {
     let chain = target.props.node_chain(uid)?;
+    // Each node's one-based index among its group's children, and whether it lies inside an
+    // effect (below the effect itself): those are selected by index with `by_index`.
+    let mut parent = &target.props;
+    let mut depth_in_effects: Option<usize> = None;
+    let mut by_number: Vec<Option<usize>> = vec![];
+    for n in &chain {
+        let at = parent.children.iter().position(|c| c.uid() == n.uid()).map(|i| i + 1);
+        by_number.push(at.filter(|_| by_index && depth_in_effects.is_some_and(|d| d >= 1)));
+        depth_in_effects = match depth_in_effects {
+            Some(d) => Some(d + 1),
+            None => (n.match_id() == "effects").then_some(0),
+        };
+        if let Node::Group(g) = n {
+            parent = g;
+        }
+    }
+    let selector = |i: usize, label: &str| match by_number.get(i).copied().flatten() {
+        Some(at) => format!("({at})"),
+        None => format!("({})", quote(label)),
+    };
     let mut parts: Vec<String> = vec![];
     let mut pending: Option<&str> = None;
     let mut in_effect = false;
-    for n in chain {
+    for (i, n) in chain.iter().copied().enumerate() {
         if let Some(f) = pending.take() {
             parts.push(format!("{f}({})", quote(n.name())));
             in_effect |= f == "effect";
@@ -119,7 +147,7 @@ pub(crate) fn reference(comp: &Comp, from: &Layer, target: &Layer, uid: Uid, com
             (Node::Group(_), "transform") if !in_effect => parts.push("transform".into()),
             (Node::Group(_), "text") if !in_effect => parts.push("text".into()),
             (Node::Prop(_), "sourceText") => parts.push("sourceText".into()),
-            _ if in_effect => parts.push(format!("({})", quote(n.name()))),
+            _ if in_effect => parts.push(selector(i, n.name())),
             _ => parts.push(camel(n.name())),
         }
     }
@@ -127,10 +155,9 @@ pub(crate) fn reference(comp: &Comp, from: &Layer, target: &Layer, uid: Uid, com
     let _ = comp;
     if !compact {
         // Match names: language-independent, like After Effects with Compact English off.
-        let chain = target.props.node_chain(uid)?;
         let mut out = if from.id == target.id { "thisLayer".to_string() } else { format!("thisComp.layer({})", quote(&target.name)) };
-        for n in chain {
-            out.push_str(&format!("({})", quote(n.match_id())));
+        for (i, n) in chain.iter().copied().enumerate() {
+            out.push_str(&selector(i, n.match_id()));
         }
         return Some(out);
     }
@@ -161,7 +188,9 @@ fn pick_whip(s: &mut Session, p: &Value) -> Result<Value> {
     let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?;
     let from = comp.layer(lid).ok_or(EngineError::NoComp)?;
     let target = comp.layer(tl).ok_or(EngineError::NoComp)?;
-    let r = reference(comp, from, target, tu, s.prefs.general.expression_pick_whip_compact).ok_or_else(|| bad("prop.pickWhip", "no such target property"))?;
+    let by_index = b_p(p, "byIndex").unwrap_or(false);
+    let r = reference_with(comp, from, target, tu, s.prefs.general.expression_pick_whip_compact, by_index)
+        .ok_or_else(|| bad("prop.pickWhip", "no such target property"))?;
     let (me, them) = (from.props.find(uid).ok_or(EngineError::NoComp)?, target.props.find(tu).ok_or(EngineError::NoComp)?);
     let (dm, dt) = (dims(from, me), dims(target, them));
     // Dropped on one of the target's values: that dimension (`position[0]`), as in After Effects.
@@ -206,7 +235,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Pick Whip (Link Property)",
             [],
             None,
-            "{layer?, path|prop, target: {layer, path|prop, dimension?}, expression?, range?: [start, end]} → sets an AE reference expression (`dimension`: one of the target's values, `position[0]`; with `expression`: inserts it there, replacing the characters in `range`, default the end)",
+            "{layer?, path|prop, target: {layer, path|prop, dimension?}, expression?, range?: [start, end], byIndex?: bool} → sets an AE reference expression (`dimension`: one of the target's values, `position[0]`; with `expression`: inserts it there, replacing the characters in `range`, default the end; `byIndex`: an effect's properties by their index, `effect(\"Color Control\")(1)`, as an Alt pick whip does)",
             has_layers,
             pick_whip
         ),

crates/engine/src/tests_timeline.rs

@@ -570,6 +570,59 @@ fn pick_whip_generates_ae_reference_expressions() {
     assert!(s.execute("prop.pickWhip", json!({"layer": l, "path": "transform/opacity", "target": {"layer": l, "path": "transform/opacity"}})).is_err());
 }
 
+/// #1072: an Alt pick whip (`byIndex`) selects an effect's properties by their index,
+/// `effect("Color Control")(1)`, its place among its group's children (nested groups too). The
+/// effect keeps its name; other properties, dimensions, insertion into an expression and match
+/// names (Compact English off) work as before. (`effectcraft-host`'s `pick_whip` tests check the
+/// references read the right values.)
+#[test]
+fn pick_whip_by_index_selects_effect_properties_by_their_index() {
+    let (mut s, l) = setup();
+    let o = s.execute("layer.newSolid", json!({"name": "Other", "color": "#00ff00"})).unwrap()["layer"].as_u64().unwrap();
+    for fx in ["Color Control", "Fractal Noise", "Point Control"] {
+        s.execute("effect.apply", json!({"layer": o, "effect": fx})).unwrap();
+    }
+    // Fractal Noise ▸ Transform ▸ Scale, by its places in the property tree.
+    let fx = s.active_comp().unwrap().layer(effectcraft_project::LayerId(o)).unwrap().props.sub("effects").unwrap().groups().nth(1).unwrap().clone();
+    let group = fx.children.iter().position(|c| c.match_id() == "transform").unwrap() + 1;
+    let scale = fx.sub("transform").unwrap().children.iter().position(|c| c.match_id() == "scale").unwrap() + 1;
+    let whip = |s: &mut Session, path: &str, target: Value, extra: Value| {
+        let mut p = json!({"layer": l, "path": path, "target": target, "byIndex": true});
+        p.as_object_mut().unwrap().extend(extra.as_object().unwrap().clone());
+        s.execute("prop.pickWhip", p).unwrap()
+    };
+    let other = "thisComp.layer(\"Other\")";
+    let undo = s.history.undo.len();
+    assert_eq!(
+        whip(&mut s, "transform/rotation", json!({"layer": o, "path": "effects/#1/color"}), json!({})),
+        json!(format!("{other}.effect(\"Color Control\")(1)[0]")),
+        "a colour into one value: its first"
+    );
+    assert_eq!(s.history.undo.len(), undo + 1, "one undo step");
+    let nested = format!("{other}.effect(\"Fractal Noise\")({group})({scale})");
+    assert_eq!(whip(&mut s, "transform/rotation", json!({"layer": o, "path": "effects/#2/transform/scale"}), json!({})), json!(nested));
+    assert_eq!(
+        whip(&mut s, "transform/opacity", json!({"layer": o, "path": "effects/#3/point", "dimension": 1}), json!({})),
+        json!(format!("{other}.effect(\"Point Control\")(1)[1]"))
+    );
+    assert_eq!(
+        whip(&mut s, "transform/rotation", json!({"layer": o, "path": "effects/#2/transform/scale"}), json!({"expression": "1 + ", "range": [4, 4]})),
+        json!(format!("1 + {nested}")),
+        "inserted at the cursor"
+    );
+    assert_eq!(
+        whip(&mut s, "transform/opacity", json!({"layer": o, "path": "transform/opacity"}), json!({})),
+        json!(format!("{other}.transform.opacity")),
+        "only an effect's properties go by index"
+    );
+    // Match names (Compact English off): the effect by its match name, its properties by index.
+    s.execute("prefs.set", json!({"key": "general.expressionPickWhipCompact", "value": false})).unwrap();
+    let e = whip(&mut s, "transform/rotation", json!({"layer": o, "path": "effects/#2/transform/scale"}), json!({}));
+    assert!(e.as_str().unwrap().ends_with(&format!("({group})({scale})")), "{e}");
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(prop(&s, l, "transform/rotation").expr.unwrap().text, format!("1 + {nested}"), "one undo per pick");
+}
+
 /// #407: Reset without a value gives a property its own default (a Transform property's, an
 /// effect parameter's) besides removing its keyframes and expression.
 #[test]

crates/host/tests/pick_whip.rs

@@ -0,0 +1,38 @@
+//! The pick whip's references read the property they were made from (#1072): an Alt pick whip's
+//! (`byIndex`) select an effect's properties by index, `effect("Fractal Noise")(i)(j)`, and must
+//! read the same value as the reference by name.
+//!
+//! (With Compact English off, a reference to another layer, `thisComp.layer("Other")("effects")…`,
+//! doesn't evaluate yet, by name or by index: "not a callable function".)
+
+use serde_json::{Value, json};
+
+#[test]
+fn index_references_read_the_same_values_as_names() {
+    let mut s = effectcraft_host::session();
+    s.execute("comp.new", json!({"name": "T", "width": 64, "height": 64, "duration": 1})).unwrap();
+    let l = s.execute("layer.newSolid", json!({"name": "Reader", "color": "#ffffff"})).unwrap()["layer"].as_u64().unwrap();
+    let o = s.execute("layer.newSolid", json!({"name": "Other", "color": "#00ff00"})).unwrap()["layer"].as_u64().unwrap();
+    for fx in ["Fractal Noise", "Point Control"] {
+        s.execute("effect.apply", json!({"layer": o, "effect": fx})).unwrap();
+    }
+    s.execute("prop.set", json!({"layer": o, "path": "effects/#1/transform/scale", "value": 77})).unwrap();
+    s.execute("prop.set", json!({"layer": o, "path": "effects/#2/point", "value": [12, 34]})).unwrap();
+    let evaluated = |s: &mut effectcraft_engine::Session, path: &str| -> Value {
+        let v = s.execute("prop.get", json!({"layer": l, "path": path})).unwrap();
+        assert!(v.get("expressionError").is_none(), "{v}");
+        v["evaluated"].clone()
+    };
+    for (path, target, dimension, want) in
+        [("transform/rotation", "effects/#1/transform/scale", None, json!(77.0)), ("transform/opacity", "effects/#2/point", Some(1), json!(34.0))]
+    {
+        let mut t = json!({"layer": o, "path": target});
+        if let Some(d) = dimension {
+            t["dimension"] = json!(d);
+        }
+        for by_index in [false, true] {
+            let e = s.execute("prop.pickWhip", json!({"layer": l, "path": path, "target": t, "byIndex": by_index})).unwrap();
+            assert_eq!(evaluated(&mut s, path), want, "by index {by_index}: {e}");
+        }
+    }
+}

docs/agents.md

@@ -604,7 +604,9 @@ Extension points and the bundled extensions: [plugins.md](plugins.md).
   Display Style between Timecode and Frames: `file.projectSettings {"timeDisplay": "frames"}`.
 * `prop.pickWhip {layer, prop, target: {layer, prop, dimension?}}` links a property as its
   expression pick whip does; `dimension` picks one of the target's values (`position[0]`), as a
-  drop on that value field does.
+  drop on that value field does. `byIndex: true` (an Alt pick whip: Alt held as it drops) refers
+  to an effect's properties by their index, `effect("Color Control")(1)`: it still reads a
+  renamed property, but another one if properties are reordered.
 * Every property row has a property pick whip in the Parent & Link column
   (`timeline.prop.<uid>.propertyPickWhip`; the expression row's is `timeline.prop.<uid>.pickWhip`).
   Dragged onto another property's name or value, in the Timeline or Effect Controls