diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index a31f4158..d626e758 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -4,7 +4,7 @@ use serde_json::{Value, json}; use solidcraft_doc::{AxisRef, Direction, Extent, FeatureKind, Operation, PointExpr, ProfileSel, expr::Kind, point_expr}; use solidcraft_geom::Vec3; -use super::CommandSpec; +use super::{CommandSpec, not_in_sketch}; use crate::params::{bad, bool_, expr, req_expr, str_, string_list, vec2, vec3}; use crate::{EngineError, Result, Session}; @@ -107,7 +107,7 @@ pub static COMMANDS: &[CommandSpec] = &[ .icon("presspull") .key("Q") .params("face: [x,y,z] and distance: expr (positive adds material, negative cuts), or edges: [[x,y,z]…] and distance (a fillet)"), - CommandSpec::new("solid.move", "Move/Copy", move_bodies).at("SOLID", "MODIFY").icon("move").key("M").params("bodies: [names]; translate?: [x,y,z] (exprs or numbers); axis?: [x,y,z]; angle?: expr; copy?: bool (default false: keeps the original bodies and transforms a new copy instead of moving them in place)"), + CommandSpec::new("solid.move", "Move/Copy", move_bodies).at("SOLID", "MODIFY").icon("move").key("M").enabled(not_in_sketch).params("bodies: [names]; translate?: [x,y,z] (exprs or numbers); axis?: [x,y,z]; angle?: expr; copy?: bool (default false: keeps the original bodies and transforms a new copy instead of moving them in place)"), ]; /// Where an extrude starts and which way is positive: a point inside the (first) profile or @@ -1595,3 +1595,7 @@ mod stacked_face_tests; #[cfg(test)] #[path = "solid_move_copy_tests.rs"] mod solid_move_copy_tests; + +#[cfg(test)] +#[path = "move_in_sketch_tests.rs"] +mod move_in_sketch_tests; diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 21a33552..bdba3856 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -186,6 +186,12 @@ pub fn in_sketch(s: &Session) -> std::result::Result<(), String> { if s.active_sketch.is_some() { Ok(()) } else { Err("no sketch is being edited (Create Sketch or Edit Sketch first)".into()) } } +/// The mirror of [`in_sketch`], for a command that acts on bodies and has nothing to offer the +/// geometry under the cursor while a sketch is open. +pub fn not_in_sketch(s: &Session) -> std::result::Result<(), String> { + if s.active_sketch.is_none() { Ok(()) } else { Err("a sketch is being edited (this works on bodies: Finish Sketch first)".into()) } +} + /// Every command, in toolbar order. pub fn command_specs() -> Vec<&'static CommandSpec> { let mut v: Vec<&'static CommandSpec> = Vec::new(); diff --git a/crates/engine/src/cmd/move_in_sketch_tests.rs b/crates/engine/src/cmd/move_in_sketch_tests.rs new file mode 100644 index 00000000..33f4d03e --- /dev/null +++ b/crates/engine/src/cmd/move_in_sketch_tests.rs @@ -0,0 +1,56 @@ +//! `solid.move` moves bodies, so it has nothing to offer the geometry under the cursor while a +//! sketch is being edited. It is disabled there, with a reason, and comes back once the sketch +//! is finished. +use super::*; + +fn cube(s: &mut Session) { + s.execute("solid.box", &json!({"length": 20, "width": 20, "height": 20})).unwrap(); +} + +fn spec() -> &'static crate::cmd::CommandSpec { + crate::find_command("solid.move").unwrap() +} + +#[test] +fn move_is_disabled_while_a_sketch_is_being_edited_and_says_why() { + let mut s = Session::default(); + cube(&mut s); + assert!(spec().info(&s).enabled, "Move is enabled with no sketch open"); + + s.execute("sketch.create", &json!({"plane": "XY"})).unwrap(); + let info = spec().info(&s); + assert!(!info.enabled, "Move must be disabled while a sketch is being edited"); + let why = info.disabled_reason.unwrap_or_default(); + assert!(why.contains("sketch is being edited"), "the reason must name the sketch: {why}"); + assert!(why.contains("bodies"), "the reason must say what Move acts on: {why}"); + assert!(why.contains("Finish Sketch"), "the reason must say the way out: {why}"); +} + +#[test] +fn running_move_in_a_sketch_is_refused_and_changes_nothing() { + let mut s = Session::default(); + cube(&mut s); + s.execute("sketch.create", &json!({"plane": "XY"})).unwrap(); + let features = s.doc.features.len(); + + let e = s.execute("solid.move", &json!({"bodies": ["Body1"], "translate": [30, 0, 0]})).unwrap_err(); + assert!(matches!(e, EngineError::Disabled(ref id, _) if id == "solid.move"), "{e:?}"); + + assert_eq!(s.doc.features.len(), features, "a refused command adds no feature"); + assert!(s.active_sketch.is_some(), "the sketch is still being edited"); + let bounds = s.model.state().body("Body1").unwrap().mesh().bounds(); + assert!(bounds.min.dist(Vec3::ZERO) < 1e-6, "the body did not move: {bounds:?}"); +} + +#[test] +fn move_works_again_once_the_sketch_is_finished() { + let mut s = Session::default(); + cube(&mut s); + s.execute("sketch.create", &json!({"plane": "XY"})).unwrap(); + s.execute("sketch.finish", &json!({})).unwrap(); + assert!(spec().info(&s).enabled, "Move comes back once the sketch is finished"); + + s.execute("solid.move", &json!({"bodies": ["Body1"], "translate": [30, 0, 0]})).unwrap(); + let bounds = s.model.state().body("Body1").unwrap().mesh().bounds(); + assert!(bounds.min.dist(Vec3::new(30.0, 0.0, 0.0)) < 1e-6, "{bounds:?}"); +} diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index f4c5f3a1..efaa1ec3 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -137,6 +137,7 @@ pub fn handle(app: &mut SolveApp, ctx: &egui::Context, req: &ControlRequest) -> "camera": app.cam, "tool": app.tool.as_ref().map(|t| t.cmd), "dialog": app.dialog.as_ref().map(|d| format!("{d:?}")), + "status": app.status.as_ref().map(|(text, _, error)| json!({"text": text, "error": error})), "hover": app.viewport.hover.as_ref().map(|h| format!("{h:?}")), "renderer": if app.viewport.gpu.is_some() { "gpu" } else { "cpu" }, "frame_ms": app.frame_ms, diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 5e51d083..45e20438 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -530,6 +530,16 @@ impl SolveApp { if self.file_load.busy() { return; } + // A disabled command starts nothing, however it was reached. The toolbar greys its + // button, but a shortcut, the command palette and the control channel all land here. + // Its reason goes to the status line, so the refusal is never silent. + if let Some(info) = solidcraft_engine::find_command(id).map(|c| c.info(&self.session)) + && let Some(why) = info.disabled_reason + { + let label = self.tr_label(info.label).to_string(); + self.set_status(format!("{label}: {why}"), true); + return; + } // Another command ends Edit Text (its dialog is not a feature edit to keep). sketch_text_move::cancel_drag(self); if sketch_text::editing(self) || self.dialog.as_ref().is_some_and(|d| matches!(d.kind, dialogs::Kind::TextMove(_))) { diff --git a/crates/ui-egui/src/scenario.rs b/crates/ui-egui/src/scenario.rs index ba2275c8..06287899 100644 --- a/crates/ui-egui/src/scenario.rs +++ b/crates/ui-egui/src/scenario.rs @@ -34,6 +34,7 @@ //! - `{"note": "…"}`: a comment; `{"pending": "why"}`: the scenario waits for a fix (its test //! is ignored) //! - `{"expect": {…}}`: checks, see [`check`]; `{"until": {…}}` waits (frames) until they pass +//! (`"status"` takes a substring of the status line, or null for none) //! //! The same files drive the live app over the control channel for screenshots. @@ -562,7 +563,8 @@ fn approx(got: f64, want: &Value, what: &str) -> Result<(), String> { /// Checks: `bodies` (count), `volume` / `area` (total, number or [value, rel tol]), `faces`, /// `features` (count), `errors` (timeline features in error, default 0), `dialog` (null: none -/// open, or a word its kind must contain), `tool` (null or command id), `sketching` (bool), +/// open, or a word its kind must contain), `tool` (null or command id), `status` (null, or a +/// substring of the status line), `sketching` (bool), /// `selection` (count), `glyphs` ({constraint id: count}, none overlapping one another or a dimension value), `param` ({name: value}), `sketch_status` (solved|…), `dof`, `query` /// ({command, params?, path, value}: a command's result). pub fn check(h: &mut Harness, e: &Value) -> Result<(), String> { @@ -760,6 +762,18 @@ pub fn check(h: &mut Harness, e: &Value) -> Result<(), String> { return Err(format!("tool: got {}, want {v}", ui["tool"])); } } + "status" => { + // A substring of the status line's text, or null for no status at all. + let got = &ui["status"]["text"]; + let ok = match v { + Value::Null => ui["status"].is_null(), + Value::String(w) => got.as_str().is_some_and(|g| g.contains(w.as_str())), + _ => false, + }; + if !ok { + return Err(format!("status: got {}, want {v}", ui["status"])); + } + } "glyphs" => { // {constraint id: glyphs drawn for it}; glyphs never overlap one another. let g = crate::sketch_tools::glyph_positions(); diff --git a/crates/ui-egui/tests/scenarios/move_disabled_in_sketch.json b/crates/ui-egui/tests/scenarios/move_disabled_in_sketch.json new file mode 100644 index 00000000..e13d61b9 --- /dev/null +++ b/crates/ui-egui/tests/scenarios/move_disabled_in_sketch.json @@ -0,0 +1,37 @@ +[ + {"note": "solid.move moves bodies. While a sketch is being edited it can do nothing to the geometry under the cursor, so it is disabled there rather than opening a window that cannot act. The refusal says why, and the command comes back once the sketch is finished."}, + {"call": "engine.execute", "params": {"command": "solid.box", "params": {"length": 20, "width": 20, "height": 20}}}, + {"call": "ui.view", "params": {"view": "iso"}}, + + {"note": "With no sketch open it works as before: the window opens on a picked body."}, + {"start": "solid.move"}, + {"click": {"world": [10, 10, 20]}}, + {"expect": {"dialog": "Move", "bodies": 1}}, + {"key": "Escape"}, + {"expect": {"dialog": null}}, + + {"note": "Inside a sketch the same press opens nothing and leaves the sketch alone."}, + {"call": "engine.execute", "params": {"command": "sketch.create", "params": {"plane": "XZ"}}}, + {"call": "engine.execute", "params": {"command": "sketch.rectangle.two_point", "params": {"p0": [40, 0], "p1": [80, 20]}}}, + {"expect": {"sketching": true, "sketch_curves": {"own": 4}}}, + {"start": "solid.move"}, + {"expect": {"dialog": null, "tool": null, "sketching": true}}, + {"expect": {"status": "a sketch is being edited"}}, + {"expect": {"status": "Finish Sketch first"}}, + + {"note": "The engine refuses it too, so a script cannot walk around the greyed button."}, + {"call": "engine.execute", "params": {"command": "solid.move", "params": {"bodies": ["Body1"], "translate": [30, 0, 0]}}, "fail": true}, + {"expect": {"sketching": true, "bodies": 1, "errors": 0}}, + {"expect": {"bbox": {"body": "Body1", "min": [0, 0, 0], "max": [20, 20, 20]}}}, + + {"note": "Finish the sketch and Move is back, with the body still where it was."}, + {"start": "sketch.finish"}, + {"expect": {"sketching": false}}, + {"start": "solid.move"}, + {"click": {"world": [10, 10, 20]}}, + {"expect": {"dialog": "Move"}}, + {"key": "Escape"}, + {"expect": {"dialog": null, "bodies": 1, "errors": 0}}, + {"expect": {"bbox": {"body": "Body1", "min": [0, 0, 0], "max": [20, 20, 20]}}}, + {"shot": "move_disabled_in_sketch"} +] diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 72428150..970a1a1a 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -47,7 +47,7 @@ unit regardless of the design, pass an expression with one: `"value": "50 mm"` o | Method | Params | Notes | |---|---|---| -| `ui.inspect` | — | UI state, what is hidden (`visibility`: bodies, origin and construction items, sketches; set it with the `browser.visibility` command), viewport rect, camera, active tool/dialog, hover, renderer, timings, live preview (`active`, `busy`, `error`, `ms`, bodies it stands in for) | +| `ui.inspect` | — | UI state, what is hidden (`visibility`: bodies, origin and construction items, sketches; set it with the `browser.visibility` command), viewport rect, camera, active tool/dialog, the status line (`status`: text and whether it is an error), hover, renderer, timings, live preview (`active`, `busy`, `error`, `ms`, bodies it stands in for) | | `ui.set` | any `UiState` field (`tab`, `perspective`, `showGrid`, `dark`, …), and `hiddenBodies`, `hiddenOrigin`, `hiddenSketches`, `shownSketches` (whole lists; `browser.visibility` changes single items) | `dark: false` switches to the light theme | | `ui.view` | `view`: `front`, `back`, `top`, `bottom`, `left`, `right`, `iso`, `home`, `fit`; `animate` (bool, default false: snap) | | | `ui.start` | `command` | like a toolbar click: starts the sketch tool or dialog for the command |