Merge pull request #7044 from dotCooCoo/fix/effect-apply-strict-params

Effects: effect.apply refuses a wrongly typed parameter that has no default

crates/engine/src/cmd/effectcmd.rs

@@ -16,7 +16,7 @@ use vectorcraft_render::effects;
 use super::appearance::{ItemTarget, aligned, appearance_targets, index_param, item_indices, item_target, item_target_at};
 use super::expand::has_seeded;
 use super::rasterfx;
-use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, fileio, has_doc, str_param, switch_or};
+use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, check_switches, cmd, fileio, has_doc, str_param, switch_or};
 
 /// The name of the `svg.filter` effect drawing `xml`: its `<filter>` id. An effect names the
 /// document's filter of that name, so `xml` that differs from the document's filter of the same
@@ -76,7 +76,7 @@ fn effect_specs() -> Vec<CommandSpec> {
             "Effect Options",
             [],
             None,
-            "{index: int, params?: {…} (merged into the current parameters), warpStyle?: string (an existing Warp style suffix, e.g. arc or flag; changes only an applied Warp in place), threeDType?: plane|extrude|revolve|inflate (the 3D and Materials panel's object type: changes only an applied 3D effect in place into Rotate, Extrude & Bevel, Revolve or Inflate; the settings the new effect also takes keep their values, rotation angles wrapped into -180..180 outside Revolve, and the others start at its defaults; params then merge into the new effect's; the type the effect already has, with nothing else given, changes nothing and records no undo step), visible?: bool, item?: appearance item index|null (as effect.remove), ids?|id?: [..], target?: \"object\"|\"contents\", aligned?: bool (index and item are rows of the first target's stack: each other target edits its equal one, as the Appearance panel does for a mixed selection)} → {ids}",
+            "{index: int, params?: {…} (merged into the current parameters), warpStyle?: string (an existing Warp style suffix, e.g. arc or flag; changes only an applied Warp in place; params are checked against the new style), threeDType?: plane|extrude|revolve|inflate (the 3D and Materials panel's object type: changes only an applied 3D effect in place into Rotate, Extrude & Bevel, Revolve or Inflate; the settings the new effect also takes keep their values, rotation angles wrapped into -180..180 outside Revolve, and the others start at its defaults; params then merge into the new effect's; the type the effect already has, with nothing else given, changes nothing and records no undo step), visible?: bool (any other value but null is refused, when a step plays too), item?: appearance item index|null (as effect.remove), ids?|id?: [..], target?: \"object\"|\"contents\", aligned?: bool (index and item are rows of the first target's stack: each other target edits its equal one, as the Appearance panel does for a mixed selection)} → {ids}",
             has_doc,
             set_params
         ),
@@ -649,15 +649,28 @@ pub(crate) fn set_params_effects(s: &Session, p: &Value) -> Vec<String> {
         return vec![];
     };
     let Ok(targets) = effect_targets(s, p, item, index, C) else { return vec![] };
-    // A 3D effect whose type `threeDType` changes takes the new effect's parameters.
+    // A 3D effect whose type `threeDType` changes, and a Warp whose style `warpStyle` changes, take
+    // the new effect's parameters.
     let kind = three_d_type(p, C).ok().flatten();
+    let warp = p.get("warpStyle").and_then(Value::as_str).filter(|style| effects::WarpStyle::from_id(style).is_some());
     let effect = |&(id, at, index): &(NodeId, Option<usize>, usize)| {
         let id = &st.doc.node(id)?.appearance.effects_at(at)?.get(index)?.id;
-        Some(kind.filter(|_| effects::is_three_d(id)).map_or_else(|| id.clone(), str::to_string))
+        Some(edited_effect_id(id, kind, warp))
     };
     targets.iter().filter_map(effect).collect()
 }
 
+/// The id effect `id` has once `effect.setParams` sets its 3D type to `kind` or its Warp style to
+/// `warp` (each when given and when `id` is an effect of that family).
+fn edited_effect_id(id: &str, kind: Option<&str>, warp: Option<&str>) -> String {
+    let is_warp = id.strip_prefix("warp.").and_then(effects::WarpStyle::from_id).is_some();
+    match (kind, warp) {
+        (Some(to), _) if effects::is_three_d(id) => to.to_string(),
+        (_, Some(style)) if is_warp => format!("warp.{style}"),
+        _ => id.to_string(),
+    }
+}
+
 /// The effect `threeDType` in `p` turns a 3D effect into ([`effects::THREE_D_TYPES`]), when given.
 fn three_d_type(p: &Value, cmd: &str) -> Result<Option<&'static str>> {
     let key = "threeDType";
@@ -718,6 +731,7 @@ fn set_params(s: &mut Session, p: &Value) -> Result<Value> {
             Ok(format!("warp.{style}"))
         })
         .transpose()?;
+    check_switches(p, &["visible"], C)?;
     let visible = p.get("visible").and_then(Value::as_bool);
     let item = item_target(s, p, C)?;
     let edited = set_params_effects(s, p);

crates/engine/src/cmd/effectcmd/tests.rs

@@ -626,6 +626,50 @@ fn warp_options_non_warp_target_rejection_is_atomic() {
     assert!(!state.is_dirty());
 }
 
+/// `effect.setParams` refuses a `visible` that is not true or false, when called and when a step
+/// plays, and the effect, the document and the undo history stay as they were. `null` leaves the
+/// visibility as it is.
+#[test]
+fn set_params_refuses_a_visible_that_is_not_true_or_false() {
+    let (mut s, id) = session_with_rect();
+    s.execute("effect.apply", &json!({"effect": "distort.roughen", "item": null})).unwrap();
+    s.doc_mut().unwrap().mark_saved();
+    let before = s.doc().unwrap().doc.clone();
+    let undo = s.doc().unwrap().history.undo.len();
+    for visible in [json!(1), json!(0), json!("false"), json!([])] {
+        let p = json!({"index": 0, "item": null, "visible": visible, "params": {"size": 9}});
+        for replay in [false, true] {
+            let r = if replay { s.replay("effect.setParams", &p) } else { s.execute("effect.setParams", &p) };
+            let err = r.err().map(|e| e.to_string()).unwrap_or_default();
+            assert!(err.contains("`visible` must be true or false"), "{visible} (replay {replay}): {err}");
+            let state = s.doc().unwrap();
+            assert_eq!(state.doc, before, "{visible} (replay {replay})");
+            assert_eq!(state.history.undo.len(), undo, "{visible} (replay {replay})");
+            assert!(!state.is_dirty());
+        }
+    }
+    s.execute("effect.setParams", &json!({"index": 0, "item": null, "visible": null, "params": {"size": 9}})).unwrap();
+    let n = node(&s, id);
+    assert!(n.appearance.effects[0].visible);
+    assert_eq!(n.appearance.effects[0].params["size"], 9);
+}
+
+/// The effect `effect.setParams` checks the parameters against is the one it leaves: a Warp's new
+/// style and a 3D effect's new type, and the effect as it is otherwise.
+#[test]
+fn set_params_checks_the_effect_it_leaves() {
+    use super::edited_effect_id;
+    assert_eq!(edited_effect_id("warp.arc", None, Some("flag")), "warp.flag");
+    assert_eq!(edited_effect_id("warp.arc", None, None), "warp.arc");
+    assert_eq!(edited_effect_id("distort.roughen", None, Some("flag")), "distort.roughen");
+    assert_eq!(edited_effect_id("threeD.extrude", Some("threeD.revolve"), None), "threeD.revolve");
+    assert_eq!(edited_effect_id("warp.arc", Some("threeD.revolve"), None), "warp.arc");
+    let (mut s, _) = session_with_rect();
+    s.execute("effect.apply", &json!({"effect": "warp.arc", "item": null})).unwrap();
+    let p = json!({"index": 0, "item": null, "warpStyle": "flag", "params": {"bend": 25}});
+    assert_eq!(super::set_params_effects(&s, &p), ["warp.flag"]);
+}
+
 #[test]
 fn list_remove_and_set_params() {
     let (mut s, id) = session_with_rect();

crates/engine/src/cmd/params.rs

@@ -15,8 +15,8 @@
 //! commands a command runs, the steps of a `command.batch` or an `action.batch` included: a
 //! misspelled `widht` no longer runs the command with its default. So are the `params` of an
 //! effect (#1592): `effect.apply` and
-//! `effect.setParams` answer a key the effect doesn't take, or a value of another type than its
+//! `effect.setParams` answer a key the effect doesn't take, or a value of another type than the
-//! default, with a parameter error ([`check_effect_params`]).
+//! effect reads, with a parameter error ([`check_effect_params`]).
 //!
 //! A command with a parameter schema (`CommandSpec::schema`) is also checked against the schema
 //! before it runs ([`crate::schemas::check`]): the schema refuses a value of the wrong type or out
@@ -263,15 +263,15 @@ pub(crate) fn unknown(id: &str, key: &str, keys: &[&str]) -> crate::EngineError
 }
 
 /// `params` of built-in effect `effect`, as command `id` was given them (#1592): only keys the
-/// effect takes (those of its dialog defaults and of its params description), each of its
+/// effect takes (those of its dialog defaults and of its params description), each of the type
-/// default's type, a `color` the effect can draw ([`check_effect_color`]) and a Levels or Curves
+/// the effect reads ([`check_effect_types`]), a `color` the effect can draw ([`check_effect_color`])
-/// `channel` the effect has ([`check_effect_channel`]). Numbers out of range pass: the effect
+/// and a Levels or Curves `channel` the effect has ([`check_effect_channel`]). Numbers out of range
-/// clamps them when it draws. An effect plug-in resolves its own against its manifest, and an
+/// pass: the effect clamps them when it draws. An installed effect plug-in's are checked against its
-/// unknown effect is the command's to report.
+/// manifest, and an unknown effect is the command's to report.
 pub fn check_effect_params(id: &str, effect: &str, params: &Value) -> Result<()> {
     let Value::Object(given) = params else { return Ok(()) };
     if effect.starts_with(vectorcraft_plugins::effect::PREFIX) {
-        return Ok(());
+        return check_effect_types(id, effect, params);
     }
     let Some(info) = vectorcraft_render::effects::effect_info(effect) else { return Ok(()) };
     let defaults = info.defaults.as_object();
@@ -282,18 +282,10 @@ pub fn check_effect_params(id: &str, effect: &str, params: &Value) -> Result<()>
             keys.push(key);
         }
     }
-    for (key, v) in given {
+    if let Some(key) = given.keys().find(|k| !keys.contains(&k.as_str())) {
-        if !keys.contains(&key.as_str()) {
+        return Err(unknown(id, key, &keys));
-            return Err(unknown(id, key, &keys));
-        }
-        match defaults.and_then(|d| d.get(key)) {
-            Some(Value::Number(_)) if !v.is_number() => return Err(bad(id, format!("`{key}` must be a number"))),
-            Some(Value::Bool(_)) if !v.is_boolean() => return Err(bad(id, format!("`{key}` must be true or false"))),
-            // (A colour is also taken as `[r, g, b]` or as a colour object.)
-            Some(Value::String(_)) if v.is_number() || v.is_boolean() => return Err(bad(id, format!("`{key}` must be a string"))),
-            _ => {}
-        }
     }
+    check_effect_types(id, effect, params)?;
     // An SVG filter's XML is one drawable <filter> element.
     if effect == vectorcraft_render::effects::SVG_FILTER
         && let Some(xml) = given.get("xml")
@@ -329,6 +321,59 @@ pub fn check_effect_params(id: &str, effect: &str, params: &Value) -> Result<()>
     check_effect_channel(id, effect, params)
 }
 
+/// The type of the values in `params` of effect `effect`, as command `id` was given them: a key
+/// the effect has a dialog default for takes a value of its default's type, a key only the
+/// effect's params description names takes the type the effect reads ([`undefaulted_type`]), and
+/// an installed effect plug-in's take the types its manifest gives them. The `style` of Crop Marks
+/// is `roman` or `japanese`. A number given as text (`"10 pt"`), `true` for a number, `1` for a
+/// switch and `null` for either are refused.
+fn check_effect_types(id: &str, effect: &str, params: &Value) -> Result<()> {
+    let Value::Object(given) = params else { return Ok(()) };
+    if effect.starts_with(vectorcraft_plugins::effect::PREFIX) {
+        return match vectorcraft_plugins::effect::installed(effect) {
+            Some(plugin) => plugin.manifest().resolve_params(params).map(drop).map_err(|why| bad(id, why.to_string())),
+            None => Ok(()),
+        };
+    }
+    let Some(info) = vectorcraft_render::effects::effect_info(effect) else { return Ok(()) };
+    for (key, v) in given {
+        let want = info.defaults.get(key).cloned().or_else(|| undefaulted_type(effect, key));
+        match want {
+            Some(Value::Number(_)) if !v.is_number() => return Err(bad(id, format!("`{key}` must be a number"))),
+            Some(Value::Bool(_)) if !v.is_boolean() => return Err(bad(id, format!("`{key}` must be true or false"))),
+            // (A colour is also taken as `[r, g, b]` or as a colour object.)
+            Some(Value::String(_)) if v.is_number() || v.is_boolean() => return Err(bad(id, format!("`{key}` must be a string"))),
+            _ => {}
+        }
+    }
+    // Crop Marks draw one of two styles.
+    if effect == vectorcraft_render::effects::CROP_MARKS
+        && let Some(style) = given.get("style")
+        && style.as_str().and_then(vectorcraft_doc::marks::MarkStyle::parse).is_none()
+    {
+        return Err(bad(id, "`style` must be roman or japanese"));
+    }
+    Ok(())
+}
+
+/// A value of the type effect `effect` reads for `key`, a parameter its params description names
+/// without a dialog default: the earlier 3D Perspective and light (`perspective`, `lightAzimuth`,
+/// `lightElevation`) and Scribble `variation` are numbers, an Outline Stroke's `width` is a number
+/// and a Gaussian Blur's `linear` is true or false. A Glass or Texturizer `textureImage`, an SVG
+/// filter's `name` and `xml`, a Warp's `version` and the `style` of Crop Marks have checks of their
+/// own; they aren't listed.
+pub(crate) fn undefaulted_type(effect: &str, key: &str) -> Option<Value> {
+    use vectorcraft_render::effects;
+    let number = || Some(Value::from(0.0));
+    match key {
+        "perspective" | "lightAzimuth" | "lightElevation" if effects::is_three_d(effect) => number(),
+        "variation" if effect == "stylize.scribble" => number(),
+        "width" if effect == "path.outlineStroke" => number(),
+        "linear" if effect == "blur.gaussian" => Some(Value::Bool(false)),
+        _ => None,
+    }
+}
+
 /// Can an effect read `v` as its `color`: a string [`Color::from_hex`] reads (`#rrggbb`, `#rgb`,
 /// `#rrggbbaa`, black, white, red, green, blue), `[r, g, b]` whose first three entries are numbers
 /// (read on a 0..1 scale) or a color object (`{"model": "rgb", "r": …}`)? The effect draws its

crates/engine/src/tests/params.rs

@@ -600,6 +600,105 @@ fn effect_params_are_the_effects_own() {
     assert!(s.check_params("effect.setParams", &json!({"index": 7, "params": {"blurr": 3}})).is_ok());
 }
 
+/// An effect parameter that only the effect's params description names, with no dialog default,
+/// is held to the type the effect reads: the earlier 3D Perspective and light, Scribble's
+/// `variation` and an Outline Stroke's `width` are numbers, and a Gaussian Blur's `linear` is true
+/// or false. The `style` of Crop Marks is `roman` or `japanese`. Every such key of every effect is
+/// listed, or has a check of its own. A key left out keeps its default.
+#[test]
+fn an_effect_parameter_without_a_default_is_held_to_its_type() {
+    use crate::cmd::params::undefaulted_type;
+    use vectorcraft_render::effects::effect_catalog;
+    // Read by checks of their own: a stored texture or a texture to load, an SVG filter, a Warp's
+    // map and the style of Crop Marks.
+    let own_check = |effect: &str, key: &str| match key {
+        "textureImage" | "name" | "xml" => true,
+        "version" => effect.starts_with("warp."),
+        "style" => effect == "cropMarks",
+        _ => false,
+    };
+    let mut unlisted = vec![];
+    for e in effect_catalog() {
+        for key in doc_keys(e.params).unwrap().keys {
+            if e.defaults.get(key).is_none() && undefaulted_type(e.id, key).is_none() && !own_check(e.id, key) {
+                unlisted.push(format!("{}.{key}", e.id));
+            }
+        }
+    }
+    assert!(unlisted.is_empty(), "parameters without a default or a listed type: {unlisted:?}");
+    let apply = |effect: &str, params| json!({"effect": effect, "params": params});
+    for (effect, params, why) in [
+        ("threeD.revolve", json!({"perspective": true}), "`perspective` must be a number"),
+        ("threeD.extrude", json!({"lightAzimuth": "30"}), "`lightAzimuth` must be a number"),
+        ("threeD.rotate", json!({"lightElevation": null}), "`lightElevation` must be a number"),
+        ("threeD.inflate", json!({"perspective": "50%"}), "`perspective` must be a number"),
+        ("path.outlineStroke", json!({"width": "10 pt"}), "`width` must be a number"),
+        ("stylize.scribble", json!({"variation": false}), "`variation` must be a number"),
+        ("blur.gaussian", json!({"radius": 2, "linear": 1}), "`linear` must be true or false"),
+        ("cropMarks", json!({"style": "bogus"}), "`style` must be roman or japanese"),
+        ("cropMarks", json!({"style": 1}), "`style` must be roman or japanese"),
+        ("cropMarks", json!({"style": null}), "`style` must be roman or japanese"),
+    ] {
+        assert_eq!(refusal("effect.apply", apply(effect, params.clone())), why, "{effect} {params}");
+        assert_eq!(refusal("appearance.addEffect", apply(effect, params.clone())), why, "{effect} {params}");
+    }
+    for (effect, params) in [
+        ("threeD.revolve", json!({"perspective": 40, "lightAzimuth": 30.5, "lightElevation": 0})),
+        ("path.outlineStroke", json!({"width": 10})),
+        ("stylize.scribble", json!({"variation": 2})),
+        ("blur.gaussian", json!({"linear": true})),
+        ("blur.gaussian", json!({})),
+        ("cropMarks", json!({"style": "japanese"})),
+        ("cropMarks", json!({})),
+    ] {
+        assert!(check_params("effect.apply", &apply(effect, params.clone())).is_ok(), "{effect} {params}");
+    }
+}
+
+/// `effect.apply`, `appearance.addEffect` and `effect.setParams` refuse such a parameter of the
+/// wrong type when they run, and so does a step of a `command.batch` whose effect a step before it
+/// added, with the document and its undo history as they were. A saved step still plays with it,
+/// as with a key the effect no longer takes.
+#[test]
+fn effect_commands_refuse_an_undefaulted_parameter_of_the_wrong_type() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({})).unwrap();
+    s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 40})).unwrap();
+    let state = |s: &Session| {
+        let st = s.doc().unwrap();
+        (st.history.undo.len(), st.doc.clone())
+    };
+    let before = state(&s);
+    let apply = |effect: &str, params| json!({"effect": effect, "params": params});
+    for (id, params, why) in [
+        ("effect.apply", apply("path.outlineStroke", json!({"width": "10 pt"})), "`width` must be a number"),
+        ("appearance.addEffect", apply("blur.gaussian", json!({"linear": 1})), "`linear` must be true or false"),
+        ("effect.apply", apply("threeD.revolve", json!({"perspective": "50"})), "`perspective` must be a number"),
+    ] {
+        let e = s.execute(id, &params).unwrap_err().to_string();
+        assert!(e.contains(why), "{id} {params}: {e}");
+        assert_eq!(state(&s), before, "{id} {params}");
+    }
+    let batch = json!({"commands": [
+        {"command": "effect.apply", "params": apply("path.outlineStroke", json!({"width": 3}))},
+        {"command": "effect.setParams", "params": {"index": 0, "params": {"width": "10 pt"}}},
+    ]});
+    let e = s.execute("command.batch", &batch).unwrap_err().to_string();
+    assert!(e.contains("`width` must be a number"), "{e}");
+    assert_eq!(state(&s), before);
+    s.execute("effect.apply", &apply("path.outlineStroke", json!({"width": 3}))).unwrap();
+    let applied = state(&s);
+    for params in [json!({"width": "10 pt"}), json!({"width": null}), json!({"width": true})] {
+        let e = s.execute("effect.setParams", &json!({"index": 0, "params": params.clone()})).unwrap_err().to_string();
+        assert!(e.contains("`width` must be a number"), "{params}: {e}");
+        assert_eq!(state(&s), applied, "{params}");
+    }
+    s.execute("effect.setParams", &json!({"index": 0, "params": {"width": 4}})).unwrap();
+    s.replay("effect.setParams", &json!({"index": 0, "params": {"retired": "x", "width": "10 pt"}})).unwrap();
+    let st = s.doc().unwrap();
+    assert_eq!(st.doc.layers[0].children().unwrap()[0].appearance.effects[0].params["width"], json!("10 pt"));
+}
+
 /// The Transform effect (`distort.transform`) is held to its params description: `effect.apply`
 /// and `effect.setParams` refuse each of its on/off options given as text, such as `reflectX:
 /// "false"`, and each of its numbers given as text, such as `copies: "0"`, and leave the effect

crates/engine/src/tests/plugins.rs

@@ -235,8 +235,16 @@ fn live_effect_plugins_reshape_editable_art_and_expand() {
     run(&mut s, "select.all", json!({}));
     run(&mut s, "object.move", json!({"dx": 50, "dy": 0}));
     assert_eq!(effected(&s), *node(&s, a).path_data().unwrap());
+    // A parameter of another type than the manifest gives it is refused.
+    let before = (s.doc().unwrap().doc.clone(), s.doc().unwrap().history.undo.len());
+    let wrong = json!({"ids": [a.0], "effect": "plugin.test.engine.tri", "params": {"size": "4 pt"}});
+    let e = s.execute("effect.apply", &wrong).unwrap_err().to_string();
+    assert!(e.contains("`size` must be a number"), "{e}");
+    assert_eq!((s.doc().unwrap().doc.clone(), s.doc().unwrap().history.undo.len()), before);
     run(&mut s, "effect.apply", json!({"ids": [a.0], "effect": "plugin.test.engine.tri", "params": {"size": 4}}));
     assert_eq!(effected(&s), triangle());
+    let e = s.execute("effect.setParams", &json!({"ids": [a.0], "index": 1, "params": {"size": true}})).unwrap_err().to_string();
+    assert!(e.contains("`size` must be a number"), "{e}");
     assert_eq!(node(&s, a).appearance.effects[1].params, json!({"size": 4}));
     // Uninstalled: the geometry is drawn as it is.
     run(&mut s, "plugin.remove", json!({"id": "test.engine.tri"}));

docs/mcp.md

@@ -1488,14 +1488,33 @@ refused before the command runs:
 Only the top level is checked, except for the export and save commands below: the keys of an object inside a value
 (`artboardLayout: {…}`) are the command's to read. Another exception is an effect's `params`: `effect.apply` (and `appearance.addEffect`) and `effect.setParams`
 check them against the effect's own entry in `effect.list`, its `defaults` and the keys its `params` text names. A key
-the effect doesn't take, or a value of another type than its default (a string for a number, a number for a switch),
+the effect doesn't take, or a value of another type than the effect reads (a string for a number, a number for a
-is refused the same way:
+switch), is refused the same way:
 
 ```json
 {"id": "effect.apply", "params": {"effect": "blur.gaussian", "params": {"blurr": 3}}}
-→ invalid parameters for `effect.apply`: unknown parameter `blurr`; the command takes: radius
+→ invalid parameters for `effect.apply`: unknown parameter `blurr`; the command takes: radius, linear
 ```
 
+A key the effect has a default for takes a value of its default's type. A key only the `params` text names takes the
+type the effect reads: the earlier 3D `perspective`, `lightAzimuth` and `lightElevation` and Scribble's `variation` are
+numbers, an Outline Stroke's `width` is a number and a Gaussian Blur's `linear` is true or false. A number written as
+text (`"10 pt"`), `true` for a number, `1` for a switch and `null` for either are refused; a key left out keeps its
+default. The `style` of Crop Marks must be `roman` or `japanese`. An installed plug-in effect's parameters take the
+types its manifest gives them. A step of `command.batch` gets the same error, and the document and its undo history
+stay as they were:
+
+```json
+{"id": "effect.apply", "params": {"effect": "path.outlineStroke", "params": {"width": "10 pt"}}}
+→ invalid parameters for `effect.apply`: `width` must be a number
+```
+
+A replayed journal or action still plays such a value, as it plays a key the effect no longer takes.
+
+`effect.setParams` with `warpStyle` checks `params` against the new Warp style, and with `threeDType` against the new
+3D effect. Its own `visible` must be true or false (`null` leaves the visibility as it is); any other value is refused,
+in a replayed journal or action too, and nothing changes.
+
 The `color` of Drop Shadow, Inner Glow, Outer Glow and Shift to Color must be one the effect can read: a hex color
 (`#rrggbb`, `#rgb` or `#rrggbbaa`, with or without the `#`), `black`, `white`, `red`, `green` or `blue`, `[r, g, b]`
 (three numbers, each read on a 0 to 1 scale), or a color object (`{"model": "rgb", "r": 1, "g": 0, "b": 0}`).

docs/target-app-parity.md

@@ -796,6 +796,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
   - Plug-in, recovery and script report replies under `--automation-read-root`: `plugin.list`, `plugin.info` and `plugin.safeMode` name the plug-in folder, failed files, plug-in sources and disabled plug-ins' files outside the read roots by file name or `null`, `file.recovery.*` leave out a document's original file and the recovery folder outside them, and `ui.inspect`'s `scriptReport` names files a run wrote outside them by file name. `crates/engine/src/tests/plugins.rs` (`plugin_list_leaves_out_paths_outside_the_read_root`), `crates/engine/src/tests/recovery.rs` (`recovery_replies_leave_out_paths_outside_the_read_root`), `crates/ui-egui/src/tests/automation.rs` (`ui_inspect_script_report_leaves_out_files_outside_the_read_root`).
   - Scripts: `alert`, `confirm` and `prompt` open dialogs during a run, each with a Stop button that ends the run and puts every document back. In the app the run waits at the call for the person's answer; a run over MCP or the command line takes its answers in order from `script.run`'s `answers`, and the report lists each dialog with its answer. `crates/engine/src/tests/script_dialogs.rs`, `crates/ui-egui/src/tests/script_dialogs.rs`.
   - Parameter keys: the engine refuses a key that a command's params description doesn't name before the command runs, in every build and for every caller, including the app's own calls, the commands a command runs and the steps of `command.batch` and `action.batch`. An action recorded by an older version still plays from the Actions panel and the Batch dialog (`Session::replay`). `crates/engine/tests/it/param_keys.rs`.
+  - Effect parameter types: `effect.apply`, `appearance.addEffect` and `effect.setParams` refuse an effect parameter of another type than the effect reads, for keys only the params text names too (the earlier 3D `perspective`, `lightAzimuth` and `lightElevation`, Scribble's `variation`, Outline Stroke's `width`, Gaussian Blur's `linear`), a Crop Marks `style` other than `roman` or `japanese`, and an installed plug-in effect's parameter of another type than its manifest gives it, in a `command.batch` step too, with the document and undo history unchanged; a replayed action still plays. `effect.setParams` checks the parameters against a Warp's new style, and refuses a `visible` that is not true or false, in a replayed action too. `crates/engine/src/tests/params.rs` (`an_effect_parameter_without_a_default_is_held_to_its_type`, `effect_commands_refuse_an_undefaulted_parameter_of_the_wrong_type`), `crates/engine/src/tests/plugins.rs` (`live_effect_plugins_reshape_editable_art_and_expand`), `crates/engine/src/cmd/effectcmd/tests.rs` (`set_params_refuses_a_visible_that_is_not_true_or_false`, `set_params_checks_the_effect_it_leaves`).
   - MCP server with 33 tools (drawing, text, effects, Pathfinder, transforms, graphs, text wrap, export, screenshots, undo/redo, UI inspect/screenshot aliases, any command), attached to the running app or headless. Headless and remote `command_list` include desktop `UI_COMMANDS` (`ui: true`); a UI-only id still needs `--connect` to run. Eight prompts (poster, icon-set, recolor, trace-and-style, export-set, check-spelling, variables-bind, path-tools). Effect ids are not command ids. Prefer a CLI built from the checkout when a release binary lags (#6460). `vectorcraft://commands/reference` serves the API reference. Tool descriptions and replies agree with the engine: format lists come from its format table, `save_file` says it keeps the document's own format, `inspect_document` reports the paint type's characters show, and selection exports report the encoder's warnings. `--automation-read-root` / `--automation-write-root` (on `vectorcraft-cli mcp` and on `vectorcraft --control`) confine the files commands read and write to chosen folders, links followed, with the flags PhotoCraft takes (#832); each flag may be given more than once, a path inside any of its folders being allowed (#3546, `file_access::tests::several_roots_of_a_kind_each_grant_their_access`, `cli::mcp_automation_roots_take_several_folders`). A long export can be cancelled (`notifications/cancelled` stops it at the next page or group and writes nothing), reports page progress to a client that sent a progress token, and does not make other requests wait: engine-free ones answer at once, the rest are refused as busy after two seconds (#2165, #2171, #2172).
   - MCP protocol: five prompts (`prompts/list` / `prompts/get`: poster, icon set, recolour, trace-and-style, export set), `completion/complete` for prompt arguments and template variables read from the live catalogues (formats, effect ids, command ids, trace presets, swatch and object ids), four resource templates (`vectorcraft://object/{id}`, `//command/{id}`, `//effect/{id}`, `//swatch/{name}`) so an agent reads one thing instead of the whole document, and `logging/setLevel` with `notifications/message`. `window.panel` ids are case-insensitive (display labels work too), `vectorcraft://object/{id}` reads through `document.node {summary: true}` (the same reply bytes from one lookup instead of summarizing the whole document: a leaf read on a 2,000-object document drops from ~20 ms to ~0.06 ms), large containers slice with `document.node {depth?, childLimit?}` (truncated levels report `childCount`), `document.inspect` and the `inspect_document` tool slice the same way so a depth-0 read is the layer skeleton (~9.7 KB instead of ~2 MB on a 2,000-object document), `document.find` locates nodes by name/kind/text with ancestor paths, and the CLI sweeps the protocol surface over stdio against the compiled binary. `docs/mcp.md` has a Protocol section covering the capability table and what is deliberately absent (subscriptions, progress, cancellation, sampling, elicitation, Streamable HTTP — all of which need the transport to interleave reads).
   - Headless CLI (`vectorcraft-cli run`, `convert`, `info`, `bench`, `perf`, `mcp`). A `run` step, like a `command_batch` step over MCP, can use an earlier step's result (`"$1.id"`, #785); `run --commands FILE` (or `-` for standard input) reads the steps from a JSON array or JSON Lines, past the Windows command-line limit (#3147, `apps/vectorcraft-cli/tests/it/cli.rs` `run_reads_*`, `run_refuses_malformed_commands_naming_the_step`).