Make SolveApp::start honour a command's enabled flag, and disable solid.move inside a sketch (#740)

`SolveApp::start` now honours a command's enabled flag, so shortcuts, the command palette and the control channel can't start a disabled command, and Move is disabled while a sketch is open.
This commit is contained in:
William Dunn authored and GitHub committed 2026-10-13 11:43:45 -04:00
1 parent 62a7f903f2
commit ff609bc320
8 files changed
+132 -4

No files matched your search

+6 -2
View File
@@ -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;
+6
View File
@@ -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();
@@ -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:?}");
}
+1
View File
@@ -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,
+10
View File
@@ -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(_))) {
+15 -1
View File
@@ -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();
@@ -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"}
]
+1 -1
View File
@@ -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 |