Merge pull request #1759

crates/engine/src/commands/animation.rs

@@ -10,7 +10,9 @@ use effectcraft_project::{GroupKind, Layer, LayerId, LayerSource, Node, Project,
 use effectcraft_time::Tick;
 use serde_json::{Value, json};
 
-use super::{CommandSpec, bad, frontend, has_comp, has_keys, has_layers, layer_mut, layers_p, match_path_of, selected_leaf_props, str_p};
+use super::{
+    CommandSpec, bad, frontend, has_comp, has_keys, has_layers, layer_mut, layers_p, match_path_of, resolve_effect_path, selected_leaf_props, str_p,
+};
 use crate::{EngineError, KeyRef, Result, Session, cmd};
 
 /// Preset file format version tag (`*.ecpreset`, JSON).
@@ -147,7 +149,8 @@ fn apply_preset(s: &mut Session, p: &Value) -> Result<Value> {
                 }
             }
             for (path, src) in &props {
-                match l.props.prop_mut(path) {
+                let resolved = resolve_effect_path(l, path);
+                match l.props.prop_mut(&resolved) {
                     Some(pr) if std::mem::discriminant(&pr.value) == std::mem::discriminant(&src.value) => {
                         pr.value = src.value.clone();
                         pr.keys = src

crates/engine/src/commands/edit.rs

@@ -333,7 +333,8 @@ fn paste_properties(s: &mut Session, p: &Value) -> Result<Value> {
             }
             let lt = l.layer_time(t);
             for (path, v) in &values {
-                if let Some(pr) = l.props.prop_mut(path)
+                let path = super::resolve_effect_path(l, path);
+                if let Some(pr) = l.props.prop_mut(&path)
                     && let Some(v) = pr.value.coerce_json(&v.to_json())
                 {
                     pr.set_value_at(lt, v);
@@ -463,7 +464,8 @@ fn paste_links(s: &mut Session, clip: LinkClip) -> Result<Value> {
         for lid in &ids {
             let l = super::layer_mut(proj, cid, *lid)?;
             for (path, e) in &pairs {
-                if let Some(pr) = l.props.prop_mut(path) {
+                let path = super::resolve_effect_path(l, path);
+                if let Some(pr) = l.props.prop_mut(&path) {
                     pr.expr = Some(e.clone());
                     n += 1;
                 }

crates/engine/src/commands/mod.rs

@@ -442,6 +442,24 @@ pub fn match_path_of(g: &effectcraft_project::PropGroup, uid: effectcraft_projec
     None
 }
 
+/// Turn `effects/<match name>[#n]/<property>` (as [`match_path_of`] writes it) into a uid path on
+/// `l`. Path parsing splits on `.`, which would cut a match name like `ec.control.slider` apart.
+/// Any other path, or an effect that is not on the layer, comes back unchanged.
+pub(crate) fn resolve_effect_path(l: &Layer, path: &str) -> String {
+    let Some((seg, tail)) = path.strip_prefix("effects/").and_then(|r| r.split_once('/')) else { return path.to_string() };
+    let (match_id, nth) = match seg.rsplit_once('#') {
+        Some((m, n)) if !m.is_empty() => match n.parse::<usize>() {
+            Ok(n) if n >= 1 => (m, n),
+            _ => (seg, 1),
+        },
+        _ => (seg, 1),
+    };
+    match l.effects().and_then(|fx| fx.groups().filter(|g| g.match_id == match_id).nth(nth - 1)) {
+        Some(g) => format!("effects/@{}/{tail}", g.uid),
+        None => path.to_string(),
+    }
+}
+
 /// Selected properties (leaves) of the active comp as (layer, uid); selected groups expand to
 /// their properties.
 pub(crate) fn selected_leaf_props(s: &Session) -> Vec<(LayerId, effectcraft_project::Uid)> {

crates/engine/src/tests_effects.rs

@@ -741,3 +741,56 @@ fn effect_colour_labels_set_clear_undo_and_round_trip() {
     let l = old.comp(s.active_comp_id().unwrap()).unwrap().layer(LayerId(a)).unwrap().clone();
     assert_eq!(l.effects().unwrap().groups().next().unwrap().label, None);
 }
+
+/// #1746: a preset saved from an effect property (`effects/ec.control.slider/slider`, `#2` for the second
+/// instance) applies back to the same layer instead of landing in `skipped`.
+#[test]
+fn preset_round_trips_effect_properties_by_match_name() {
+    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 (n, value) in [(1, 30.0), (2, 70.0)] {
+        s.execute("prop.set", json!({"layer": a, "path": format!("effects/#{n}/slider"), "value": value})).unwrap();
+    }
+    for n in [1, 2] {
+        let path = std::env::temp_dir().join(format!("ec-1746-{}-{n}.ecpreset", std::process::id())).to_string_lossy().to_string();
+        s.execute("prop.select", json!({"layer": a, "path": format!("effects/#{n}/slider")})).unwrap();
+        s.execute("anim.savePreset", json!({"path": path})).unwrap();
+        let saved = std::fs::read_to_string(&path).unwrap();
+        let want = if n == 1 { "effects/ec.control.slider/slider" } else { "effects/ec.control.slider#2/slider" };
+        assert!(saved.contains(want), "{saved}");
+        s.execute("prop.set", json!({"layer": a, "path": format!("effects/#{n}/slider"), "value": 0.0})).unwrap();
+        let r = s.execute("anim.applyPreset", json!({"path": path, "layers": [a]})).unwrap();
+        let _ = std::fs::remove_file(&path);
+        assert_eq!(r["applied"], 1, "{r}");
+        assert_eq!(r["skipped"], json!([]), "{r}");
+        assert_eq!(slider(&s, n - 1), if n == 1 { 30.0 } else { 70.0 });
+    }
+}
+
+/// #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]
+fn copied_effect_property_pastes_onto_the_same_effect() {
+    let (mut s, a, b) = comp_with_two_solids();
+    apply(&mut s, a, "Gaussian Blur");
+    apply(&mut s, b, "Gaussian Blur");
+    s.execute("prop.set", json!({"layer": a, "path": "effects/#1/blurriness", "value": 12.5})).unwrap();
+    s.execute("prop.select", json!({"layer": a, "path": "effects/#1/blurriness"})).unwrap();
+    s.execute("edit.copy", json!({})).unwrap();
+    s.execute("layer.select", json!({"layers": [b]})).unwrap();
+    let r = s.execute("edit.paste", json!({})).unwrap();
+    assert_eq!(r["values"], 1, "{r}");
+    assert_eq!(layer(&s, b).props.prop("effects/#1/blurriness").unwrap().value, KV::Scalar(12.5));
+    s.execute("prop.setExpression", json!({"layer": a, "path": "effects/#1/blurriness", "expression": "time * 10"})).unwrap();
+    s.execute("prop.select", json!({"layer": a, "path": "effects/#1/blurriness"})).unwrap();
+    s.execute("edit.copyExpressionOnly", json!({})).unwrap();
+    s.execute("layer.select", json!({"layers": [b]})).unwrap();
+    let r = s.execute("edit.paste", json!({})).unwrap();
+    assert_eq!(r["expressions"], 1, "{r}");
+    assert_eq!(layer(&s, b).props.prop("effects/#1/blurriness").unwrap().expr.as_ref().unwrap().text, "time * 10");
+}