From a8989aff6dc8bdcc64fce808e1d10dfd99b18823 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 18:22:01 -0800 Subject: [PATCH] Commands: app and control channel params of the wrong type are refused view.setZoom, view.screenMode, window.panel, file.revert, file.close, ui.newSwatch, ui.symbolOptions, ui.batchDialog and file.exportSelection read a number or switch of the wrong type as its default and went on, so `view.setZoom {"zoom": "200"}` set 100%. They now refuse it and change nothing. The control channel's ui.key, ui.move, ui.wheel, ui.click, ui.drag, ui.pointer, ui.resize, ui.screenshot, ui.render and ui.set, and the headless session's key, pointer and render calls, do the same for their numbers, switches and mods, before any input is queued. Omitting a param or giving null keeps its default. --- crates/mcp/src/headless.rs | 38 +++++++-- crates/mcp/src/tests.rs | 28 +++++++ crates/ui-egui/src/control.rs | 125 +++++++++++++++++++++------- crates/ui-egui/src/control/tests.rs | 86 +++++++++++++++++++ crates/ui-egui/src/menus.rs | 2 +- crates/ui-egui/src/menus/run.rs | 82 ++++++++++++++---- docs/mcp.md | 13 +++ 7 files changed, 317 insertions(+), 57 deletions(-) diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index 00e914ddb..0377cd5ae 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -56,6 +56,24 @@ fn s<'a>(p: &'a Value, k: &str) -> Option<&'a str> { p.get(k).and_then(Value::as_str) } +/// An optional param read with `read`: none when it is omitted or null, else an error naming the +/// key when `read` gets nothing from its value. +fn opt(p: &Value, k: &str, read: impl Fn(&Value) -> Option, what: &str) -> Result, String> { + match p.get(k) { + None | Some(Value::Null) => Ok(None), + Some(v) => read(v).map(Some).ok_or_else(|| format!("`{k}` must be {what}")), + } +} + +fn opt_bool(p: &Value, k: &str) -> Result, String> { + opt(p, k, Value::as_bool, "true or false") +} + +/// A pointer packet's `mods`: none when omitted or null, else an object of booleans. +fn packet_mods(p: &Value) -> Result, String> { + opt(p, "mods", |m| serde_json::from_value(m.clone()).ok(), "an object of true or false values") +} + pub(crate) fn pointer_kind(k: &str) -> Option { Some(match k { "down" => PointerKind::Down, @@ -210,12 +228,13 @@ impl Headless { fn pointer(&mut self, p: &Value) -> Result { let events = p.get("events").and_then(Value::as_array).ok_or("missing `events`")?; - let base: Mods = p.get("mods").and_then(|m| serde_json::from_value(m.clone()).ok()).unwrap_or_default(); + let base = packet_mods(p)?.unwrap_or_default(); let default = PointerEvent::json_backquote(p, false)?; let held = events.iter().map(|e| PointerEvent::json_backquote(e, default)).collect::, _>>()?; + let mods = events.iter().map(|e| Ok(packet_mods(e)?.unwrap_or(base))).collect::, String>>()?; self.ensure_document(); let mut requests = vec![]; - for (e, backquote) in events.iter().zip(held) { + for ((e, backquote), mods) in events.iter().zip(held).zip(mods) { let kind = s(e, "kind").unwrap_or(""); let kind = pointer_kind(kind).ok_or_else(|| format!("unknown pointer kind `{kind}` (down|drag|up|move|doubleclick)"))?; if s(e, "space") == Some("screen") { @@ -223,7 +242,6 @@ impl Headless { } let x = e.get("x").and_then(Value::as_f64).ok_or("pointer event needs numeric `x`")?; let y = e.get("y").and_then(Value::as_f64).ok_or("pointer event needs numeric `y`")?; - let mods = e.get("mods").and_then(|m| serde_json::from_value(m.clone()).ok()).unwrap_or(base); let ev = PointerEvent { kind, pos: Point::new(x, y), @@ -253,8 +271,8 @@ impl Headless { fn key(&mut self, p: &Value) -> Result { let key = s(p, "key").ok_or("missing `key`")?; - let b = |n: &str| p.get(n).and_then(Value::as_bool).unwrap_or(false); - let mods = Mods { shift: b("shift"), alt: b("alt"), cmd: b("cmd"), ctrl: b("ctrl"), ..Mods::default() }; + let b = |n: &str| opt_bool(p, n).map(|on| on.unwrap_or(false)); + let mods = Mods { shift: b("shift")?, alt: b("alt")?, cmd: b("cmd")?, ctrl: b("ctrl")?, ..Mods::default() }; // A letter reaches the tool only when the tool claims it without Cmd, Ctrl or Alt (E while // a selection marquee is dragged); otherwise it is a shortcut. let mut tk = tool_key(key); @@ -296,16 +314,18 @@ impl Headless { } fn render(&mut self, p: &Value) -> Result { + let idx = opt(p, "artboard", |v| v.as_u64().and_then(|i| usize::try_from(i).ok()), "a whole number of 0 or more")?.unwrap_or(0); + let scale = opt(p, "scale", Value::as_f64, "a number")?.unwrap_or(1.0).clamp(0.01, 16.0); + let trace_views = opt_bool(p, "traceViews")? == Some(true); + let data = opt_bool(p, "data")? == Some(true); let st = self.session.active().ok_or("no document")?; let doc = st.doc.clone(); - let idx = p.get("artboard").and_then(Value::as_u64).unwrap_or(0) as usize; let r = doc.artboards.get(idx).map(|a| a.rect).ok_or("no such artboard")?; - let scale = p.get("scale").and_then(Value::as_f64).unwrap_or(1.0).clamp(0.01, 16.0); vectorcraft_render::raster_size(r, scale)?; // Rendered with the options of a PNG export on white, so its pixels are the exported file's. // `traceViews: true` draws Image Trace objects as their View shows them, as the canvas does. let mut opts = RasterExportOptions { background: Some([255; 3]), ..Default::default() }.render_options(RasterFormat::Png); - opts.trace_views = p.get("traceViews").and_then(Value::as_bool) == Some(true); + opts.trace_views = trace_views; let img = self.renderer.render_region_with(&doc, r, scale, &opts); // A cancelled render stops drawing: the picture is not whole and is not written. vectorcraft_engine::cancel::check().map_err(|e| e.to_string())?; @@ -317,7 +337,7 @@ impl Headless { vectorcraft_format::write_atomic(std::path::Path::new(path), &png).map_err(|e| format!("write {path}: {e}"))?; out["path"] = json!(path); // `data: true` asks for the image as well as the file. - if p.get("data").and_then(Value::as_bool) == Some(true) { + if data { out["pngBase64"] = json!(vectorcraft_format::base64_encode(&png)); } } diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index d7de8dd7f..dbe429a28 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -1452,6 +1452,34 @@ fn headless_backend_methods() { assert!(h.call("ui.resize", json!({})).is_err()); } +/// The headless key, pointer and screenshot calls refuse a value of the wrong type and change +/// nothing; omitting it or giving null keeps its default. +#[test] +fn headless_methods_refuse_params_of_the_wrong_type() { + let mut h = Headless::new(); + h.call("engine.execute", json!({"command": "file.new", "params": {"width": 300, "height": 200}})).unwrap(); + h.call("ui.tool.select", json!({"tool": "rectangle"})).unwrap(); + let before = h.call("document.json", json!({})).unwrap(); + let drag = |mods: Value| json!({"events": [{"kind": "down", "x": 10, "y": 10}, {"kind": "drag", "x": 60, "y": 60, "mods": mods}, {"kind": "up", "x": 60, "y": 60}]}); + for (method, params, key) in [ + ("ui.render", json!({"artboard": "0"}), "artboard"), + ("ui.render", json!({"scale": "2"}), "scale"), + ("ui.render", json!({"traceViews": 1}), "traceViews"), + ("ui.screenshot", json!({"data": "yes", "path": "x.png"}), "data"), + ("ui.key", json!({"key": "Delete", "shift": "true"}), "shift"), + ("ui.pointer", drag(json!({"shift": "yes"})), "mods"), + ("ui.pointer", json!({"mods": 1, "events": []}), "mods"), + ] { + let e = h.call(method, params.clone()).unwrap_err(); + assert!(e.contains(&format!("`{key}`")), "{method} {params}: {e}"); + } + assert_eq!(h.call("document.json", json!({})).unwrap(), before, "a refused call changes nothing"); + let r = h.call("ui.render", json!({"artboard": null, "scale": null, "traceViews": null})).unwrap(); + assert_eq!((r["width"].as_u64(), r["height"].as_u64()), (Some(300), Some(200))); + h.call("ui.pointer", drag(Value::Null)).unwrap(); + assert_ne!(h.call("document.json", json!({})).unwrap(), before, "null mods keep the packet's"); +} + // ---------- workflow tools ---------- fn id_of(result: &Value) -> u64 { diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index b02c3b6e1..77e931bdc 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -494,9 +494,23 @@ fn key_from(name: &str) -> Option { /// The modifier keys a request holds down (`cmd`, `ctrl`, `alt`, `shift`; `cmd` is Command on /// macOS, Ctrl elsewhere). -fn modifiers(p: &Value) -> egui::Modifiers { - let b = |n: &str| p.get(n).and_then(Value::as_bool).unwrap_or(false); - egui::Modifiers { alt: b("alt"), ctrl: b("ctrl"), shift: b("shift"), mac_cmd: b("cmd") && cfg!(target_os = "macos"), command: b("cmd") } +fn modifiers(p: &Value) -> Result { + let b = |n: &str| crate::menus::opt_bool(p, n).map(|on| on.unwrap_or(false)); + let cmd = b("cmd")?; + Ok(egui::Modifiers { alt: b("alt")?, ctrl: b("ctrl")?, shift: b("shift")?, mac_cmd: cmd && cfg!(target_os = "macos"), command: cmd }) +} + +/// An optional number param of a control request as `f32`, `default` when it is omitted or null. +fn coord(p: &Value, key: &str, default: f32) -> Result { + crate::menus::opt_number(p, key).map(|n| n.map_or(default, |n| n as f32)) +} + +/// A pointer packet's `mods`: none when omitted or null, else an object of booleans. +fn packet_mods(value: Option<&Value>) -> Result, String> { + match value { + None | Some(Value::Null) => Ok(None), + Some(m) => serde_json::from_value(m.clone()).map(Some).map_err(|e| format!("`mods` must be an object of true or false values: {e}")), + } } pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome { @@ -532,10 +546,13 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlReques "ui.pointer" => pointer_events(app, ctx, req), "ui.key" => { let Some(k) = s("key").and_then(key_from) else { return err("unknown or missing `key`") }; + let m = match modifiers(p) { + Ok(m) => m, + Err(e) => return err(e), + }; if let Err(e) = reserve_synthetic(app, 2 + usize::from(s("text").is_some())) { return err(e); } - let m = modifiers(p); app.synthetic.push(egui::Event::Key { key: k, physical_key: None, pressed: true, repeat: false, modifiers: m }); // Typed text comes with the press, so it shares the key's frame and modifiers. if let Some(t) = s("text") { @@ -546,31 +563,39 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlReques ok(Value::Null) } "ui.move" => { + let pos = match coord(p, "x", 0.0).and_then(|x| Ok(egui::pos2(x, coord(p, "y", 0.0)?))) { + Ok(pos) => pos, + Err(e) => return err(e), + }; if let Err(e) = reserve_synthetic(app, 1) { return err(e); } - let pos = egui::pos2(p.get("x").and_then(Value::as_f64).unwrap_or(0.0) as f32, p.get("y").and_then(Value::as_f64).unwrap_or(0.0) as f32); app.synthetic.push(egui::Event::PointerMoved(pos)); ctx.request_repaint(); ok(Value::Null) } "ui.click" | "ui.drag" => synthetic_click(app, ctx, req), "ui.wheel" => { + // Mouse wheel notches over screen point (x, y): `dy` up (+) or down, `dx` sideways + // (`unit: "point"` for trackpad-like points), with modifiers as `ui.click` takes them. + let given = + (|| Ok::<_, String>(([coord(p, "dx", 0.0)?, coord(p, "dy", 0.0)?, coord(p, "x", 0.0)?, coord(p, "y", 0.0)?], modifiers(p)?)))(); + let ([dx, dy, x, y], modifiers) = match given { + Ok(given) => given, + Err(e) => return err(e), + }; if let Err(e) = reserve_synthetic(app, 2) { return err(e); } - // Mouse wheel notches over screen point (x, y): `dy` up (+) or down, `dx` sideways - // (`unit: "point"` for trackpad-like points), with modifiers as `ui.click` takes them. - let f = |k: &str| p.get(k).and_then(Value::as_f64).unwrap_or(0.0) as f32; let unit = if s("unit") == Some("point") { egui::MouseWheelUnit::Point } else { egui::MouseWheelUnit::Line }; // Notches, not a flood: a turn stays within what a real wheel sends. - let delta = egui::vec2(f("dx"), f("dy")).clamp(egui::Vec2::splat(-100.0), egui::Vec2::splat(100.0)); - app.synthetic.push(egui::Event::PointerMoved(egui::pos2(f("x"), f("y")))); + let delta = egui::vec2(dx, dy).clamp(egui::Vec2::splat(-100.0), egui::Vec2::splat(100.0)); + app.synthetic.push(egui::Event::PointerMoved(egui::pos2(x, y))); app.synthetic.push(egui::Event::MouseWheel { unit, delta, phase: egui::TouchPhase::Move, - modifiers: modifiers(p), + modifiers, source: egui::MouseWheelSource::Unknown, }); ctx.request_repaint(); @@ -635,8 +660,10 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlReques ok(Value::Null) } "ui.resize" => { - let w = p.get("width").and_then(Value::as_f64).unwrap_or(1440.0) as f32; - let h = p.get("height").and_then(Value::as_f64).unwrap_or(900.0) as f32; + let (w, h) = match coord(p, "width", 1440.0).and_then(|w| Ok((w, coord(p, "height", 900.0)?))) { + Ok(size) => size, + Err(e) => return err(e), + }; ctx.send_viewport_cmd(egui::ViewportCommand::InnerSize(egui::vec2(w, h))); ok(Value::Null) } @@ -649,32 +676,48 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlReques if let Some(Err(e)) = s("path").map(vectorcraft_engine::file_access::check_write) { return err(e); } - if p.get("focus").and_then(Value::as_bool).unwrap_or(true) { + let flags = crate::menus::opt_bool(p, "focus").and_then(|focus| Ok((focus, crate::menus::opt_bool(p, "data")?))); + let (focus, data) = match flags { + Ok(flags) => flags, + Err(e) => return err(e), + }; + if focus.unwrap_or(true) { ctx.send_viewport_cmd(egui::ViewportCommand::Focus); } ctx.request_repaint(); - Outcome::Screenshot { path: s("path").map(str::to_string), data: p.get("data").and_then(Value::as_bool) == Some(true) } + Outcome::Screenshot { path: s("path").map(str::to_string), data: data == Some(true) } } "ui.render" => { + let given = (|| { + Ok::<_, String>(( + crate::menus::opt_index(p, "artboard")?, + crate::menus::opt_number(p, "scale")?, + crate::menus::opt_bool(p, "traceViews")?, + crate::menus::opt_bool(p, "data")?, + )) + })(); + let (index, scale, trace_views, data) = match given { + Ok(given) => given, + Err(e) => return err(e), + }; let Some(st) = app.session.active() else { return err("no document") }; let doc = st.doc.clone(); // The artboard asked for (0-based, as headless), else the first. - let index = p.get("artboard").and_then(Value::as_u64).map_or(Some(0), |i| usize::try_from(i).ok()); - let Some(r) = index.and_then(|i| doc.artboards.get(i)).map(|a| a.rect) else { return err("no such artboard") }; - let scale = p.get("scale").and_then(Value::as_f64).unwrap_or(1.0); + let Some(r) = doc.artboards.get(index.unwrap_or(0)).map(|a| a.rect) else { return err("no such artboard") }; + let scale = scale.unwrap_or(1.0); if let Err(e) = vectorcraft_render::raster_size(r, scale) { return err(e); } // Rendered with the options of a PNG export on white, so its pixels are the exported file's. // `traceViews: true` draws Image Trace objects as their View shows them, as the canvas does. let mut opts = RasterExportOptions { background: Some([255; 3]), ..Default::default() }.render_options(RasterFormat::Png); - opts.trace_views = p.get("traceViews").and_then(Value::as_bool) == Some(true); + opts.trace_views = trace_views == Some(true); let img = app.canvas.renderer.render_region_with(&doc, r, scale, &opts); let png = match img.to_png() { Ok(png) => png, Err(e) => return err(e), }; - let data = p.get("data").and_then(Value::as_bool) == Some(true); + let data = data == Some(true); match s("path") { Some(path) => match app.services.write.as_mut() { Some(w) => wrap( @@ -926,7 +969,21 @@ fn pointer_events_inner(app: &mut VectorcraftApp, ctx: &egui::Context, req: &Con if rotating && let Err(error) = validate_rotate_view_events(p, events) { return err(error); } - let base_mods: Mods = p.get("mods").and_then(|m| serde_json::from_value(m.clone()).ok()).unwrap_or_default(); + // Every event is read before any is sent, so a malformed packet sends none of them. + let read = packet_mods(p.get("mods")).and_then(|base| { + let base = base.unwrap_or_default(); + events + .iter() + .map(|e| { + let at = |k: &str| crate::menus::opt_number(e, k).map(|n| n.unwrap_or(0.0)); + Ok((at("x")?, at("y")?, packet_mods(e.get("mods"))?.unwrap_or(base))) + }) + .collect::, String>>() + }); + let read = match read { + Ok(read) => read, + Err(e) => return err(e), + }; let view = app.view_info(); let xf = app.canvas_rect.zip(app.view().copied()).map(|(rect, v)| Xf::new(rect, &v)); let from = app.session.input_count(); @@ -936,7 +993,7 @@ fn pointer_events_inner(app: &mut VectorcraftApp, ctx: &egui::Context, req: &Con Ok(held) => held, Err(e) => return err(e), }; - for (e, backquote) in events.iter().zip(held) { + for ((e, backquote), (x, y, mods)) in events.iter().zip(held).zip(read) { let kind = match e.get("kind").and_then(Value::as_str).unwrap_or("") { "down" => PointerKind::Down, "drag" => PointerKind::Drag, @@ -945,10 +1002,7 @@ fn pointer_events_inner(app: &mut VectorcraftApp, ctx: &egui::Context, req: &Con "doubleclick" | "dblclick" => PointerKind::DoubleClick, other => return err(format!("unknown pointer kind `{other}`")), }; - let x = e.get("x").and_then(Value::as_f64).unwrap_or(0.0); - let y = e.get("y").and_then(Value::as_f64).unwrap_or(0.0); let screen = e.get("space").and_then(Value::as_str) == Some("screen"); - let mods = e.get("mods").and_then(|m| serde_json::from_value(m.clone()).ok()).unwrap_or(base_mods); if rotating { if let Err(e) = rotate_view_pointer(app, kind, vectorcraft_geom::Point::new(x, y), screen, mods.shift) { return err(e); @@ -1007,18 +1061,24 @@ fn synthetic_click(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlR let Some(events) = count.checked_mul(2).and_then(|n| n.checked_add(steps)).and_then(|n| n.checked_add(1)) else { return err("synthetic input event count is out of range"); }; + // Screen-space (egui points) pointer input through egui itself: reaches every widget. + let drag = req.method == "ui.drag"; + let given = (|| { + let a = egui::pos2(coord(p, "x", 0.0)?, coord(p, "y", 0.0)?); + let end = if drag { egui::pos2(coord(p, "toX", 0.0)?, coord(p, "toY", 0.0)?) } else { a }; + Ok::<_, String>((a, end, modifiers(p)?)) + })(); + let (a, end, modifiers) = match given { + Ok(given) => given, + Err(e) => return err(e), + }; if let Err(e) = reserve_synthetic(app, events) { return err(e); } - // Screen-space (egui points) pointer input through egui itself: reaches every widget. - let f = |k: &str| p.get(k).and_then(Value::as_f64).unwrap_or(0.0) as f32; let button = match s("button") { Some("right") | Some("secondary") => egui::PointerButton::Secondary, _ => egui::PointerButton::Primary, }; - let modifiers = modifiers(p); - let a = egui::pos2(f("x"), f("y")); - let end = if req.method == "ui.drag" { egui::pos2(f("toX"), f("toY")) } else { a }; app.synthetic.push(egui::Event::PointerMoved(a)); app.synthetic.push(egui::Event::PointerButton { pos: a, button, pressed: true, modifiers }); if req.method == "ui.drag" { @@ -1040,6 +1100,11 @@ fn synthetic_click(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlR fn set_ui(app: &mut VectorcraftApp, req: &ControlRequest) -> Outcome { let p = &req.params; let s = |k: &str| p.get(k).and_then(Value::as_str); + // A flag of the wrong type is refused before anything is set. + let flags = ["rulers", "outline", "grid", "smartGuides", "boundingBox", "controlBar", "imageTracePreview"]; + if let Some(e) = flags.iter().find_map(|k| crate::menus::opt_bool(p, k).err()) { + return err(e); + } // Outline is a setting of the active document's view: without one, nothing is set. if p.get("outline").is_some_and(Value::is_boolean) && app.session.active().is_none() { return err("no document"); diff --git a/crates/ui-egui/src/control/tests.rs b/crates/ui-egui/src/control/tests.rs index 0a9c042a1..2cdf13a4d 100644 --- a/crates/ui-egui/src/control/tests.rs +++ b/crates/ui-egui/src/control/tests.rs @@ -314,6 +314,92 @@ fn render_takes_the_artboard_asked_for() { assert_eq!(got, size.map(Some), "{params}: {r}"); } assert!(request("ui.render", json!({"artboard": 2}))["error"].is_string()); + for (params, key) in [ + (json!({"artboard": "1"}), "artboard"), + (json!({"artboard": -1}), "artboard"), + (json!({"scale": "0.5"}), "scale"), + (json!({"traceViews": 1}), "traceViews"), + (json!({"data": "yes"}), "data"), + ] { + let r = request("ui.render", params.clone()); + assert!(r["error"].as_str().is_some_and(|e| e.contains(&format!("`{key}`"))), "{params}: {r}"); + } + let r = request("ui.render", json!({"artboard": null, "scale": null})); + assert_eq!([&r["result"]["width"], &r["result"]["height"]].map(Value::as_u64), [Some(64), Some(64)], "null keeps the default: {r}"); +} + +/// The control channel's input methods refuse a number or switch of the wrong type before they +/// queue any event; omitting it keeps its default. +#[test] +fn control_input_refuses_params_of_the_wrong_type() { + let move_to = |mods: Value| json!({"events": [{"kind": "move", "x": 5, "y": 5}, {"kind": "move", "x": 6, "y": 6, "mods": mods}]}); + for (method, params, key) in [ + ("ui.move", json!({"x": "10"}), "x"), + ("ui.move", json!({"y": true}), "y"), + ("ui.wheel", json!({"dy": "1"}), "dy"), + ("ui.wheel", json!({"dy": 1, "shift": "yes"}), "shift"), + ("ui.click", json!({"x": [1]}), "x"), + ("ui.click", json!({"cmd": 1}), "cmd"), + ("ui.drag", json!({"toX": "5"}), "toX"), + ("ui.key", json!({"key": "Enter", "alt": "true"}), "alt"), + ("ui.resize", json!({"width": "800"}), "width"), + ("ui.screenshot", json!({"focus": "no"}), "focus"), + ("ui.screenshot", json!({"data": 1}), "data"), + ("ui.set", json!({"rulers": "true"}), "rulers"), + ("ui.set", json!({"grid": true, "controlBar": 0}), "controlBar"), + ("ui.pointer", json!({"events": [{"kind": "move", "x": "5", "y": 0}]}), "x"), + ("ui.pointer", move_to(json!({"shift": "yes"})), "mods"), + ("ui.pointer", json!({"mods": "shift", "events": [{"kind": "move", "x": 1, "y": 1}]}), "mods"), + ] { + let mut app = VectorcraftApp::new(Session::new(), Services::default()); + app.synthetic.push(egui::Event::Text("pending".into())); + let before = (format!("{:?}", app.synthetic), app.ui.view.rulers, app.ui.view.grid, app.ui.control_bar); + let r = input_request(&mut app, method, params.clone()); + assert_eq!(r["ok"], false, "{method} {params}: {r}"); + assert!(r["error"].as_str().is_some_and(|e| e.contains(&format!("`{key}`"))), "{method} {params}: {r}"); + let after = (format!("{:?}", app.synthetic), app.ui.view.rulers, app.ui.view.grid, app.ui.control_bar); + assert_eq!(after, before, "{method} {params}: nothing is queued or set"); + } + let mut app = VectorcraftApp::new(Session::new(), Services::default()); + assert_eq!(input_request(&mut app, "ui.move", json!({"x": null, "y": 4}))["ok"], true); + assert!(matches!(app.synthetic.first(), Some(egui::Event::PointerMoved(p)) if *p == egui::pos2(0.0, 4.0))); +} + +/// The app's own commands refuse a param of the wrong type and change nothing; omitting it or +/// giving null keeps its default. +#[test] +fn ui_commands_refuse_params_of_the_wrong_type() { + let mut app = VectorcraftApp::new(Session::new(), Services::default()); + app.run("file.new", json!({"width": 200, "height": 200})).unwrap(); + let id = app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 40, "height": 40})).unwrap()["id"].as_u64().unwrap(); + app.run("select.set", json!({"ids": [id]})).unwrap(); + app.run("view.setZoom", json!({"zoom": 300, "center": [5, 5]})).unwrap(); + let view = |app: &VectorcraftApp| app.view().map(|v| (v.zoom, v.center.x, v.center.y)); + let before = (view(&app), app.ui.screen_mode, app.session.documents().len(), app.ui.dialog.is_some(), app.ui.open_panel.clone()); + for (id, params, key) in [ + ("view.setZoom", json!({"zoom": "200"}), "zoom"), + ("view.setZoom", json!({"zoom": 200, "center": [1]}), "center"), + ("view.setZoom", json!({"zoom": 200, "center": "0,0"}), "center"), + ("view.screenMode", json!({"mode": "2"}), "mode"), + ("view.screenMode", json!({"mode": 1.5}), "mode"), + ("window.panel", json!({"panel": "layers", "toggle": "true"}), "toggle"), + ("window.panel", json!({"panel": "layers", "toggle": true, "toggleGroup": 1}), "toggleGroup"), + ("file.revert", json!({"confirmed": "yes"}), "confirmed"), + ("file.close", json!({"index": "0"}), "index"), + ("ui.newSwatch", json!({"spot": "true"}), "spot"), + ("ui.newSwatch", json!({"global": 1}), "global"), + ("ui.symbolOptions", json!({"edit": 1}), "edit"), + ("ui.batchDialog", json!({"set": "1"}), "set"), + ("ui.batchDialog", json!({"action": -1}), "action"), + ("file.exportSelection", json!({"multiple": "true"}), "multiple"), + ] { + let e = app.run(id, params.clone()).unwrap_err(); + assert!(e.contains(&format!("`{key}`")), "{id} {params}: {e}"); + let after = (view(&app), app.ui.screen_mode, app.session.documents().len(), app.ui.dialog.is_some(), app.ui.open_panel.clone()); + assert_eq!(after, before, "{id} {params}: nothing changes"); + } + app.run("view.setZoom", json!({"zoom": null, "center": null})).unwrap(); + assert_eq!(app.view().map(|v| v.zoom), Some(crate::state::zoom_from_display_percent(100.0, app.session.prefs.display_print_size))); } #[test] diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 3d3f2a8c0..7326a3157 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -75,7 +75,7 @@ pub(crate) use labels::lists_plugin; use labels::shows_a_name; pub use labels::{CONTEXT_LABELS, DYNAMIC_LABELS, display_label, dynamic_label, item_shortcut, menu_strings}; pub use run::{check_params, run_ui_command}; -pub(crate) use run::{normalize_panel, opt_bool}; +pub(crate) use run::{normalize_panel, opt_bool, opt_index, opt_number}; #[cfg(test)] use slots::RECENT_FONT_IDS; pub use state::{checked, enabled, plugin_revision, shortcut_of, shown_state}; diff --git a/crates/ui-egui/src/menus/run.rs b/crates/ui-egui/src/menus/run.rs index 1223e1e42..febb8313e 100644 --- a/crates/ui-egui/src/menus/run.rs +++ b/crates/ui-egui/src/menus/run.rs @@ -25,6 +25,34 @@ pub(crate) fn opt_bool(p: &Value, key: &str) -> Result, String> { } } +/// An optional `{key?: number}` param: none when it is omitted or null. +pub(crate) fn opt_number(p: &Value, key: &str) -> Result, String> { + match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(v) => v.as_f64().map(Some).ok_or_else(|| format!("`{key}` must be a number")), + } +} + +/// An optional `{key?: index}` param, a whole number of 0 or more: none when it is omitted or null. +pub(crate) fn opt_index(p: &Value, key: &str) -> Result, String> { + match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(v) => v.as_u64().and_then(|i| usize::try_from(i).ok()).map(Some).ok_or_else(|| format!("`{key}` must be a whole number of 0 or more")), + } +} + +/// An optional `{key?: [x, y]}` param: none when it is omitted or null. +fn opt_point(p: &Value, key: &str) -> Result, String> { + match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(v) => match v.as_array().map(Vec::as_slice) { + Some([x, y]) => x.as_f64().zip(y.as_f64()).map(|(x, y)| Some(vectorcraft_geom::Point::new(x, y))), + _ => None, + } + .ok_or_else(|| format!("`{key}` must be [x, y] numbers")), + } +} + /// Handle a UI command. `None` = not a UI command (the engine handles it). /// `params` has only keys the params description of engine command `id` names, or the parameter /// error naming the one it doesn't (#1442). The UI's own commands aren't checked yet: their @@ -48,13 +76,18 @@ pub fn check_params(session: &vectorcraft_engine::Session, id: &str, params: &Va pub fn run_ui_command(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option> { // Window menu items and the named panel function keys toggle against the checked state; // other commands that open panels show them. - if id == "window.panel" && p.get("toggle").and_then(Value::as_bool) == Some(true) && checked(app, id, p) == Some(true) { + let toggles = if id == "window.panel" { opt_bool(p, "toggle").and_then(|t| Ok((t, opt_bool(p, "toggleGroup")?))) } else { Ok((None, None)) }; + let (toggle, toggle_group) = match toggles { + Ok(flags) => flags, + Err(e) => return Some(Err(e)), + }; + if toggle == Some(true) && checked(app, id, p) == Some(true) { let panel = p.get("panel").and_then(Value::as_str).and_then(normalize_panel).unwrap_or_default(); if app.ui.open_panel.as_deref() == Some(panel) { app.ui.open_panel = None; return Some(Ok(json!({"open": null}))); } - if p.get("toggleGroup").and_then(Value::as_bool) == Some(true) { + if toggle_group == Some(true) { return Some(crate::panel_docking::hide_group(app, panel)); } return Some(app.run("window.panel.close", json!({"panel": panel}))); @@ -131,7 +164,7 @@ fn run_ui_part1(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option io::new_from_template(app, s("path")), - "file.revert" if p.get("confirmed").and_then(Value::as_bool) != Some(true) => io::ask_revert(app), + "file.revert" if opt_bool(p, "confirmed") != Ok(Some(true)) => opt_bool(p, "confirmed").and_then(|_| io::ask_revert(app)), "file.reveal" => io::reveal(app), "file.cancelExport" => Ok(io::cancel_exports(app)), "file.exportInBackground" => Ok(io::hide_exports(app)), @@ -386,9 +419,11 @@ fn run_ui_part5(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option { let print = app.session.prefs.display_print_size; - let z = crate::state::zoom_from_display_percent(p.get("zoom").and_then(Value::as_f64).unwrap_or(100.0), print); - let center = - p.get("center").and_then(Value::as_array).and_then(|a| Some(vectorcraft_geom::Point::new(a.first()?.as_f64()?, a.get(1)?.as_f64()?))); + let (zoom, center) = match opt_number(p, "zoom").and_then(|z| Ok((z, opt_point(p, "center")?))) { + Ok(given) => given, + Err(e) => return Some(Err(e)), + }; + let z = crate::state::zoom_from_display_percent(zoom.unwrap_or(100.0), print); match app.view_mut() { Some(v) => { v.zoom = z.clamp(0.0313, 640.0); @@ -417,7 +452,11 @@ fn run_ui_part5(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option { - app.ui.screen_mode = match p.get("mode").and_then(Value::as_u64) { + let mode = match opt_index(p, "mode") { + Ok(mode) => mode, + Err(e) => return Some(Err(e)), + }; + app.ui.screen_mode = match mode { Some(m) => m.min(3) as u8, // F cycles the three screen modes. Without a mode, the command also goes from // Presentation Mode (not in the cycle) back to Normal. @@ -664,9 +703,10 @@ fn run_ui_part7(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option match p.get("index").and_then(Value::as_u64).map(|i| i as usize) { - Some(i) => crate::unsaved::close(app, i), - None => { + "file.close" => match opt_index(p, "index") { + Err(e) => Err(e), + Ok(Some(i)) => crate::unsaved::close(app, i), + Ok(None) => { app.session.active_index()?; crate::doc_windows::close_current(app) } @@ -678,8 +718,10 @@ fn run_ui_part7(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option Err("missing `name`".into()), }, "ui.newSwatch" => { - let on = |k: &str| p.get(k).and_then(Value::as_bool).unwrap_or(false); - crate::dialogs::new_swatch::open(app, on("spot"), on("global"), s("group").as_deref()) + let on = |k: &str| opt_bool(p, k).map(|on| on.unwrap_or(false)); + on("spot") + .and_then(|spot| Ok((spot, on("global")?))) + .and_then(|(spot, global)| crate::dialogs::new_swatch::open(app, spot, global, s("group").as_deref())) } "ui.newColorGroup" => { let names = p.get("swatches").and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_str).map(str::to_string).collect()); @@ -697,7 +739,10 @@ fn run_ui_part7(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option crate::panels::text_styles::open_new(app, crate::panels::text_styles::Kind::Para), "ui.symbolOptions" => { let name = s("name"); - let edit = p.get("edit").and_then(Value::as_bool).unwrap_or(false); + let edit = match opt_bool(p, "edit") { + Ok(edit) => edit.unwrap_or(false), + Err(e) => return Some(Err(e)), + }; if edit || name.is_some() { crate::dialogs::symbol_options::open_edit(app, name.as_deref()) } else { @@ -899,9 +944,8 @@ fn run_ui_part11(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option crate::panels::actions::continue_playback(app, p), crate::panels::actions::DELETE_STEP => crate::panels::actions::delete_step(app, p), "ui.batchDialog" => { - let set = p.get("set").and_then(Value::as_u64).unwrap_or(0) as usize; - let action = p.get("action").and_then(Value::as_u64).unwrap_or(0) as usize; - crate::dialogs::action_batch::open(app, set, action) + let index = |k: &str| opt_index(p, k).map(|i| i.unwrap_or(0)); + index("set").and_then(|set| Ok((set, index("action")?))).and_then(|(set, action)| crate::dialogs::action_batch::open(app, set, action)) } crate::panels::actions::SAVE_SET => crate::panels::actions::save_set(app, p), crate::panels::actions::LOAD_SET => crate::panels::actions::load_set(app, p), @@ -934,7 +978,11 @@ fn run_ui_part12(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option { - app.run("assets.add", json!({ "multiple": p.get("multiple").and_then(Value::as_bool).unwrap_or(false) })).map(|r| { + let multiple = match opt_bool(p, "multiple") { + Ok(multiple) => multiple.unwrap_or(false), + Err(e) => return Some(Err(e)), + }; + app.run("assets.add", json!({ "multiple": multiple })).map(|r| { let ids: Vec = r["assets"].as_array().into_iter().flatten().filter_map(Value::as_u64).collect(); crate::dialogs::open_export_for_screens_assets(app, Some(&ids)); json!({ "assets": ids }) diff --git a/docs/mcp.md b/docs/mcp.md index 5826c5c8c..7ced9eb15 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1265,6 +1265,19 @@ fails with no change. A description with a `…` entry takes more keys than it l object can carry several formats' options). A command that takes its own reply back (`document.setup`, `text.areaOptions`) says so the same way. +The app's own commands are not checked against their descriptions, but those listed here refuse a value of the wrong +type and change nothing: `view.setZoom` (`zoom`, `center`), `view.screenMode` (`mode`), `window.panel` (`toggle`, +`toggleGroup`), `file.revert` (`confirmed`), `file.close` (`index`), `ui.newSwatch` (`spot`, `global`), +`ui.symbolOptions` (`edit`), `ui.batchDialog` (`set`, `action`) and `file.exportSelection` (`multiple`). So do the +control channel's `ui.key`, `ui.move`, `ui.wheel`, `ui.click`, `ui.drag`, `ui.pointer`, `ui.resize`, `ui.screenshot`, +`ui.render` and `ui.set` for their numbers, switches and `mods`, and the headless session's key, pointer and screenshot +calls. Omitting a param or giving null keeps its default: + +```json +{"id": "view.setZoom", "params": {"zoom": "200"}} +→ `zoom` must be a number +``` + Saved actions aren't checked this way. When the Actions panel plays an action (Play at any speed, an action's function key, the Batch dialog or `ui.action.play`), a key a command no longer reads, recorded by an older version, is ignored and the action still plays. `Session::replay` plays a journal the same way. The command still refuses an effect