diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index 8a9ba805..1364d920 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -416,6 +416,32 @@ fn high_level_tools_add_effect_and_report_state() { assert!(err && c[0]["text"].as_str().unwrap().contains("unknown effect"), "{c:?}"); } +/// add_effect is one undo step, and a call that fails leaves the layer and the history as they +/// were (#175). +#[test] +fn add_effect_is_one_undo_step_and_all_or_nothing() { + let mut s = server(); + call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "A", "width": 64, "height": 64}})); + call_json(&mut s, "execute_command", json!({"command": "layer.newSolid", "params": {"name": "S", "color": "#ff0000"}})); + let undo = |s: &mut McpServer| call_json(s, "get_project", json!({}))["undo"].as_array().unwrap().clone(); + let effects = |s: &mut McpServer| { + let l = call_json(s, "get_layer", json!({"layer": "S", "flat": true})); + l["properties"].as_array().unwrap().iter().filter(|p| p["path"].as_str().unwrap().starts_with("effects/")).count() + }; + let before = undo(&mut s); + let (c, err) = call(&mut s, "add_effect", json!({"layer": "S", "effect": "Gaussian Blur", "values": {"blurriness": 8, "repeatEdgePixels": true}})); + assert!(err && c[0]["text"].as_str().unwrap().contains("effects/#1/repeatEdgePixels"), "{c:?}"); + assert_eq!((undo(&mut s), effects(&mut s)), (before.clone(), 0), "nothing applied"); + // The corrected retry lands at #1, in one undo step named like effect.apply's. + let r = call_json(&mut s, "add_effect", json!({"layer": "S", "effect": "Gaussian Blur", "values": {"blurriness": 8, "repeatEdge": true}})); + assert_eq!((&r["path"], &r["set"]), (&json!("effects/#1"), &json!(["effects/#1/blurriness", "effects/#1/repeatEdge"])), "{r}"); + let after = undo(&mut s); + assert_eq!(after.len(), before.len() + 1, "{after:?}"); + assert_eq!(after.last(), Some(&json!("Apply Gaussian Blur"))); + call_json(&mut s, "undo", json!({})); + assert_eq!((undo(&mut s), effects(&mut s)), (before, 0), "one undo removes the effect and its values"); +} + #[test] fn list_fonts_lists_families_with_styles() { let mut s = server(); diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 8a1ae2be..87315767 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -154,7 +154,8 @@ fn list_fonts(b: &mut Backend, a: &Value) -> Result { json_reply(b.exec("text.fonts", obj(&[("query", get(a, "query")), ("rescan", get(a, "rescan"))]))?) } -/// `effect.apply` on one layer, then set parameters on the new instance by param id. +/// `effect.apply` on one layer, then set parameters on the new instance by param id: one batch, +/// so one undo step, and a failing value leaves nothing applied. fn add_effect(b: &mut Backend, a: &Value) -> Result { let (layer, effect, comp) = (need(a, "layer")?, need(a, "effect")?, get(a, "comp")); let count = |t: &Value| effects_group(t).and_then(|g| g.get("children")).and_then(Value::as_array).map_or(0, Vec::len); @@ -162,16 +163,22 @@ fn add_effect(b: &mut Backend, a: &Value) -> Result { let index = count(&before) + 1; let mut p = obj(&[("effect", Some(effect)), ("comp", comp)]); p["layers"] = json!([layer]); - b.exec("effect.apply", p)?; + let mut steps = vec![json!({"command": "effect.apply", "params": p})]; let prefix = format!("effects/#{index}"); let mut set = vec![]; if let Some(vals) = get(a, "values").and_then(Value::as_object) { for (k, v) in vals { let path = format!("{prefix}/{k}"); - b.exec("prop.set", obj(&[("layer", Some(layer)), ("comp", comp), ("path", Some(&json!(path))), ("value", Some(v))]))?; + steps.push( + json!({"command": "prop.set", "params": obj(&[("layer", Some(layer)), ("comp", comp), ("path", Some(&json!(path))), ("value", Some(v))])}), + ); set.push(path); } } + // The undo step is named like effect.apply's own ("Apply Gaussian Blur"). + let name = effect.as_str().map(|e| effectcraft_engine::effects::lookup(e).map_or(e, |s| s.name)); + let label = name.map(|n| json!(format!("Apply {n}"))); + b.exec("engine.batch", obj(&[("steps", Some(&json!(steps))), ("label", label.as_ref())]))?; let tree = b.exec("layer.tree", obj(&[("layer", Some(layer)), ("comp", comp)]))?; let inst = effects_group(&tree).and_then(|g| g.get("children")).and_then(Value::as_array).and_then(|c| c.get(index - 1)).cloned(); let mut params = vec![]; @@ -590,7 +597,7 @@ static TOOLS: &[ToolDef] = &[ }, ToolDef { name: "add_effect", - description: "Apply an effect to a layer and optionally set its parameters in one undoable call. `effect` is an id (`ec.blur.gaussian`) or After Effects name (`Gaussian Blur`; see list_effects); `values` maps parameter ids to values, e.g. {\"blurriness\": 12}. Returns the instance `path` (`effects/#n`, for set_property / add_keyframe) and its parameters with their paths and current values.", + description: "Apply an effect to a layer and optionally set its parameters in one undoable call. `effect` is an id (`ec.blur.gaussian`) or After Effects name (`Gaussian Blur`; see list_effects); `values` maps parameter ids to values, e.g. {\"blurriness\": 12}. One undo step; if a value fails, nothing is applied. Returns the instance `path` (`effects/#n`, for set_property / add_keyframe) and its parameters with their paths and current values.", bridge_only: false, schema: || { schema( diff --git a/docs/agents.md b/docs/agents.md index 376475e0..b4ea4763 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -59,7 +59,7 @@ The server speaks JSON-RPC 2.0 over stdio, one message per line, and supports MC | `add_keyframe {layer, path, time+value \| keys:[...], interpolation?, comp?}` | Adds keys, then optionally applies linear/bezier/hold/easyEase. | | `list_effects {filter?}` | Effect ids, names, categories, GPU / 32-bpc support and parameters. | | `list_fonts {query?, rescan?}` | Font families text layers can use, bundled and installed, with their styles, origin and own-language name; `rescan` picks up fonts installed since launch. | -| `add_effect {layer, effect, values?, comp?}` | Apply an effect and set its parameters in one call; returns the instance path (`effects/#n`) and its parameter paths. | +| `add_effect {layer, effect, values?, comp?}` | Apply an effect and set its parameters in one call and one undo step (a failing value leaves nothing applied); returns the instance path (`effects/#n`) and its parameter paths. | | `render_frame {comp?, time?, max_side?, path?, inline?, transparent?}` | Returns a PNG image of a frame; `transparent: true` keeps the alpha (as an RGB + Alpha render writes it) instead of compositing over the comp background. | | `open_project {path \| demo \| new}` / `save_project {path?}` | Open and save files. | | `undo {steps?}` / `redo {steps?}` | History. |