From 963f489eab4947936cf770f8e01710c50298f69b Mon Sep 17 00:00:00 2001 From: Madhav Agarwal Date: Thu, 22 Oct 2026 18:47:59 +0530 Subject: [PATCH] engine: resolve effect match-name paths in prop_ref (#2064) --- crates/engine/src/commands/prop.rs | 7 +++++-- crates/engine/src/tests_effects.rs | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/commands/prop.rs b/crates/engine/src/commands/prop.rs index 3af877d34..596164b42 100644 --- a/crates/engine/src/commands/prop.rs +++ b/crates/engine/src/commands/prop.rs @@ -22,8 +22,11 @@ pub(crate) fn prop_ref(s: &Session, p: &Value, cmd: &str) -> Result<(ItemId, Lay }); } let path = str_p(p, "path").ok_or_else(|| bad(cmd, "missing `path` (e.g. transform/position) or `prop` uid"))?; - let pr = layer.props.prop(path).ok_or_else(|| { - let group = layer.props.group(path).or_else(|| path.trim().strip_prefix('@')?.parse().ok().and_then(|uid| layer.props.find_group(uid))); + // `effects/[#n]/` is how `essential.list`, presets and Copy write an + // effect property; turn it into the uid path `PropGroup::prop` parses (#2064). + let resolved = super::resolve_effect_path(layer, path); + let pr = layer.props.prop(&resolved).ok_or_else(|| { + let group = layer.props.group(&resolved).or_else(|| path.trim().strip_prefix('@')?.parse().ok().and_then(|uid| layer.props.find_group(uid))); if group.is_some() { bad(cmd, format!("`{path}` is a property group; use layer.tree (MCP: get_layer) to inspect its children, then select a leaf property")) } else { diff --git a/crates/engine/src/tests_effects.rs b/crates/engine/src/tests_effects.rs index 18fcc6f84..1ea57f4cf 100644 --- a/crates/engine/src/tests_effects.rs +++ b/crates/engine/src/tests_effects.rs @@ -772,6 +772,27 @@ fn preset_round_trips_effect_properties_by_match_name() { } } +/// #2064: `prop.addKey` accepts the `effects/[#n]/` path that +/// `essential.list` reports. +#[test] +fn add_key_resolves_effect_match_name_paths() { + let (mut s, a, _) = comp_with_two_solids(); + apply(&mut s, a, "Slider Control"); + apply(&mut s, a, "Slider Control"); + let slider = |s: &Session, n: usize| match layer(s, a).effects().unwrap().groups().nth(n).unwrap().get("slider").unwrap().value { + KV::Scalar(v) => v, + ref other => panic!("not a scalar: {other:?}"), + }; + for (path, n, value) in [("effects/ec.control.slider/slider", 0, 30.0), ("effects/ec.control.slider#2/slider", 1, 70.0)] { + let reply = s.execute("prop.addKey", json!({"layer": a, "path": path, "time": 0, "value": value})).unwrap(); + assert_eq!(reply, json!(1), "{path}"); + let got = s.execute("prop.get", json!({"layer": a, "path": path, "time": 0})).unwrap(); + assert_eq!(got["value"], json!(value), "{path}"); + assert_eq!(got["keys"].as_array().unwrap().len(), 1, "{path}"); + assert_eq!(slider(&s, n), 0.0, "the static value stays put; the key carries {value}"); + } +} + /// #1746: Edit ▸ Copy of an effect property writes the same `effects/ec.blur.gaussian/blurriness` path, /// and Paste (the value, or Copy Expression Only) sets that property of the other layer's effect. #[test]