From 651ee5a78e1892a609bf1e32536a8fb34bf6c458 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 18 Oct 2026 01:04:01 +0200 Subject: [PATCH] Resolve effects/ property paths in anim.applyPreset (#1746) --- crates/engine/src/commands/animation.rs | 7 +++- crates/engine/src/commands/edit.rs | 6 ++- crates/engine/src/commands/mod.rs | 18 +++++++++ crates/engine/src/tests_effects.rs | 53 +++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/commands/animation.rs b/crates/engine/src/commands/animation.rs index a556d9a4..ae8184b5 100644 --- a/crates/engine/src/commands/animation.rs +++ b/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 { } } 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 diff --git a/crates/engine/src/commands/edit.rs b/crates/engine/src/commands/edit.rs index 422728ea..73702665 100644 --- a/crates/engine/src/commands/edit.rs +++ b/crates/engine/src/commands/edit.rs @@ -328,7 +328,8 @@ fn paste_properties(s: &mut Session, p: &Value) -> Result { } 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); @@ -458,7 +459,8 @@ fn paste_links(s: &mut Session, clip: LinkClip) -> Result { 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; } diff --git a/crates/engine/src/commands/mod.rs b/crates/engine/src/commands/mod.rs index 685d4dad..23388594 100644 --- a/crates/engine/src/commands/mod.rs +++ b/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/[#n]/` (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::() { + 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)> { diff --git a/crates/engine/src/tests_effects.rs b/crates/engine/src/tests_effects.rs index 6f87b4ae..18fcc6f8 100644 --- a/crates/engine/src/tests_effects.rs +++ b/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"); +}