From 2880525e6b426db64a3cfc2ecc6e410602fe791b Mon Sep 17 00:00:00 2001 From: Mqrius Date: Tue, 13 Oct 2026 03:50:05 +0200 Subject: [PATCH] fix(ui): the Combine dialog previews without an error, so OK is enabled (#481) The live preview also evaluates each command as a new body, to show the tool of a join or cut (as_new_body sets every `operation` to "new"). A combine has no new-body form: its operation is between bodies that already exist, and solid.combine rejects "new" ("operation must be join, cut or intersect"). That error became the preview's error even when the combine itself worked, and since OK is greyed while the preview has an error, a combine could not be applied from its dialog at all. as_new_body now leaves solid.combine alone, so its preview shows the result. Co-authored-by: Claude Opus 5.5 --- crates/ui-egui/src/preview.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/preview.rs b/crates/ui-egui/src/preview.rs index fd851234..3ff37813 100644 --- a/crates/ui-egui/src/preview.rs +++ b/crates/ui-egui/src/preview.rs @@ -276,7 +276,13 @@ fn build_tool(before: &ModelState, after: &ModelState, c: Colors, op: &str) -> B fn as_new_body(cmds: &[(String, Value)]) -> Option<(Vec<(String, Value)>, String)> { let mut out = cmds.to_vec(); let mut changed: Option = None; - for (_, p) in &mut out { + for (command, p) in &mut out { + let inner = p.get("command").and_then(Value::as_str).map(str::to_string); + // A combine's operation is between bodies that already exist: it has no new-body form + // (and no tool body of its own to show). + if inner.as_deref().unwrap_or(command.as_str()) == "solid.combine" { + continue; + } let p = if p.get("command").is_some() { p.get_mut("params")? } else { p }; if let Some(op) = p.get_mut("operation") && let Some(o) = op.as_str().filter(|o| *o != "new").map(str::to_string) @@ -455,6 +461,22 @@ mod tests { assert_eq!(pv.error.as_deref(), Some("current invalid expression"), "current errors still prevent invalid Apply"); } + /// Combine previews its result (cut, join or intersect between existing bodies) without an + /// error, so its dialog's OK is enabled. Before, the preview also tried the command as a new + /// body, which a combine has no form of, and always reported "operation must be join, cut or + /// intersect". + #[test] + fn combine_previews_its_result_without_an_error() { + let mut s = solidcraft_engine::Session::default(); + s.execute("solid.box", &json!({"length": 20, "width": 20, "height": 20, "body_name": "Cube"})).unwrap(); + s.execute("solid.cylinder", &json!({"base": [10, 10, -5], "diameter": 6, "height": 30, "body_name": "Pin"})).unwrap(); + for op in ["cut", "join", "intersect"] { + let cmd = ("solid.combine".to_string(), json!({"target": "Cube", "tools": ["Pin"], "operation": op, "keep_tools": false})); + let built = compute(&s, &[cmd], colors()).unwrap_or_else(|e| panic!("{op}: {e}")); + assert!(built.error.is_none(), "{op}: {:?}", built.error); + } + } + fn count(bytes: &[u8], stride: usize, col: [u8; 4]) -> usize { bytes.chunks(stride).filter(|v| v.get(24..28) == Some(&col[..])).count() }