From e67ae66f3e14930ea144ce09cebc10fff8366fd2 Mon Sep 17 00:00:00 2001 From: SS-EN <84559909+SS-EN@users.noreply.github.com> Date: Sat, 24 Oct 2026 07:35:31 +0000 Subject: [PATCH] Renaming a parameter follows it into sheet-metal and plastic rules and canvases (#4420) (#4423) * claim: #4420 * Renaming a parameter follows it into sheet-metal and plastic rules and canvases (#4420) Document::rename_param rewrote other parameters, feature inputs, dimensions, configurations and joints, but not the expressions of sheet metal and plastic rules or the plane a canvas lies on, so after a rename the sheet metal and plastic features using them failed with "unknown parameter" and the canvas left the view. It renames them there now, and param_users lists them (a rule's field, a canvas's input), so a parameter they use can't be deleted. Co-Authored-By: Claude Opus 5.5 * Parameters: rules' features in drives and the graph; shared rule field lists (#4420) A sheet metal or plastic rule that uses a parameter makes the sheet metal (or plastic) features rebuild when it changes, so parameters.users lists them in drives and parameters.graph links them. SheetRule::fields/fields_mut and PlasticRule::fields_mut replace the hand-written copies of the rules' fields in params.rs, eval.rs and sheet.manage_rules. Suggested by @CamWight. Co-Authored-By: Claude Opus 5.5 * fmt (#4420) Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/doc/src/eval.rs | 9 +--- crates/doc/src/params.rs | 55 +++++++++++++++++++++ crates/doc/src/plastic.rs | 16 ++++++ crates/doc/src/sheet.rs | 31 ++++++++++++ crates/engine/src/cmd/parameters_tests.rs | 59 +++++++++++++++++++++++ crates/engine/src/cmd/sheet.rs | 11 +---- 6 files changed, 164 insertions(+), 17 deletions(-) diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index 40dbac3ce7..bd81c6ed2f 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -1245,7 +1245,7 @@ fn chain_fingerprint( fp.hash(&mut h); serde_json::to_string(&doc.sheet).unwrap_or_default().hash(&mut h); for r in &doc.sheet.rules { - for e in [&r.thickness, &r.k_factor, &r.bend_radius, &r.relief_width, &r.relief_depth, &r.corner_relief, &r.hem_gap, &r.gap] { + for (_, e, _) in r.fields() { for n in expr::references(e) { vals.get(&n).map(|v| v.v.to_bits()).hash(&mut h); } @@ -1284,12 +1284,7 @@ fn context_digest(doc: &Document, vals: &BTreeMap) -> u64 { serde_json::to_string(&doc.plastic).unwrap_or_default().hash(&mut h); serde_json::to_string(&doc.components).unwrap_or_default().hash(&mut h); serde_json::to_string(&doc.body_components).unwrap_or_default().hash(&mut h); - for e in doc - .sheet - .rules - .iter() - .flat_map(|r| [&r.thickness, &r.k_factor, &r.bend_radius, &r.relief_width, &r.relief_depth, &r.corner_relief, &r.hem_gap, &r.gap]) - { + for e in doc.sheet.rules.iter().flat_map(|r| r.fields().map(|(_, e, _)| e)) { for n in expr::references(e) { vals.get(&n).map(|v| v.v.to_bits()).hash(&mut h); } diff --git a/crates/doc/src/params.rs b/crates/doc/src/params.rs index 5f4f0ddb54..d127b389a0 100644 --- a/crates/doc/src/params.rs +++ b/crates/doc/src/params.rs @@ -570,6 +570,10 @@ pub enum ParamUser { /// A joint's offset, angle or value, or a motion link's gear, rack or screw length (`input` /// says which; a link is named by the joint it drives). Joint { joint: u64, name: String, input: String }, + /// A sheet metal or plastic rule's value (`kind` says which, `field` which value). + Rule { kind: String, name: String, field: String }, + /// The plane a canvas lies on (`input` is its label, e.g. `Offset`). + Canvas { canvas: u64, name: String, input: String }, } impl ParamUser { @@ -583,6 +587,8 @@ impl ParamUser { None => format!("configuration column {column}"), }, ParamUser::Joint { name, input, .. } => format!("joint {name} ({input})"), + ParamUser::Rule { kind, name, field } => format!("{kind} rule {name} ({field})"), + ParamUser::Canvas { name, input, .. } => format!("canvas {name} ({input})"), } } } @@ -715,6 +721,24 @@ impl Document { out.push(ParamUser::Joint { joint, name: jname, input }); } } + let uses = |e: &str| expr::references(e).iter().any(|r| r == name); + for r in &self.sheet.rules { + for (field, _, _) in r.fields().into_iter().filter(|(_, e, _)| uses(e)) { + out.push(ParamUser::Rule { kind: "sheet metal".into(), name: r.name.clone(), field: field.into() }); + } + } + for r in &self.plastic.rules { + for (field, _, _) in r.fields().into_iter().filter(|(_, e, _)| uses(e)) { + out.push(ParamUser::Rule { kind: "plastic".into(), name: r.name.clone(), field: field.into() }); + } + } + for c in &self.canvases { + c.plane.walk_exprs(&mut |(label, e, _, _)| { + if uses(e) { + out.push(ParamUser::Canvas { canvas: c.id, name: c.name.clone(), input: label.into() }); + } + }); + } out } @@ -833,6 +857,24 @@ impl Document { n += 1; } } + // Sheet metal and plastic rules and canvas planes hold expressions too (#4420). + let rules = self.sheet.rules.iter_mut().flat_map(|r| r.fields_mut()).chain(self.plastic.rules.iter_mut().flat_map(|r| r.fields_mut())); + for (_, e) in rules { + let r = expr::rename_reference(e, old, new); + if r != *e { + *e = r; + n += 1; + } + } + for c in &mut self.canvases { + c.plane.walk_exprs_mut(&mut |(_, e, _, _)| { + let r = expr::rename_reference(e, old, new); + if r != *e { + *e = r.into(); + n += 1; + } + }); + } if let Some(c) = self.param_comments.remove(old) { self.param_comments.insert(new.to_string(), c); } @@ -870,6 +912,13 @@ impl Document { names.extend(sketch.text_exprs().into_iter().flat_map(|(_, e)| expr::references(e))); names.extend(sketch.patterns.iter().filter_map(|p| p.count_expression.as_deref()).flat_map(expr::references)); } + // Sheet metal and plastic features rebuild when their rules' parameters change (#4420). + if f.kind.is_sheet() { + names.extend(self.sheet.rules.iter().flat_map(|r| r.fields()).flat_map(|(_, e, _)| expr::references(e))); + } + if f.kind.is_plastic() { + names.extend(self.plastic.rules.iter().flat_map(|r| r.fields()).flat_map(|(_, e, _)| expr::references(e))); + } out.extend(names.into_iter().map(|n| (format!("Feature:{}", f.name), n))); } out @@ -890,9 +939,15 @@ impl Document { break; } } + // A rule that uses it changes every sheet metal (or plastic) feature: the evaluation + // hashes the rules into their fingerprint. + let uses = |e: &String| expr::references(e).iter().any(|r| affected.contains(r)); + let sheet = self.sheet.rules.iter().any(|r| r.fields().iter().any(|(_, e, _)| uses(e))); + let plastic = self.plastic.rules.iter().any(|r| r.fields().iter().any(|(_, e, _)| uses(e))); let mut out = Vec::new(); for f in &self.features { let mut hit = f.param_names.iter().any(|n| affected.contains(n)); + hit |= (sheet && f.kind.is_sheet()) || (plastic && f.kind.is_plastic()); hit |= f.kind.expressions().iter().any(|e| expr::references(e).iter().any(|r| affected.contains(r))); if let FeatureKind::Sketch { sketch, .. } = &f.kind { hit |= sketch.constraints.iter().any(|c| c.param.as_ref().is_some_and(|p| affected.contains(p))); diff --git a/crates/doc/src/plastic.rs b/crates/doc/src/plastic.rs index 2e765b813d..2645ddcc33 100644 --- a/crates/doc/src/plastic.rs +++ b/crates/doc/src/plastic.rs @@ -88,6 +88,22 @@ impl PlasticRule { _ => return None, }) } + + /// The expressions to change, by field name, in the order of [`PlasticRule::fields`]. + pub fn fields_mut(&mut self) -> [(&'static str, &mut String); 10] { + [ + ("thickness", &mut self.thickness), + ("nominal_radius", &mut self.nominal_radius), + ("clearance", &mut self.clearance), + ("knife_edge", &mut self.knife_edge), + ("reveal_height", &mut self.reveal_height), + ("thickness_variation", &mut self.thickness_variation), + ("draft", &mut self.draft), + ("max_thickness", &mut self.max_thickness), + ("min_thickness", &mut self.min_thickness), + ("min_draft", &mut self.min_draft), + ] + } } /// The design's own plastic rules, the active one and the rules assigned to bodies. diff --git a/crates/doc/src/sheet.rs b/crates/doc/src/sheet.rs index c506e4e04a..9a6e001812 100644 --- a/crates/doc/src/sheet.rs +++ b/crates/doc/src/sheet.rs @@ -261,6 +261,37 @@ pub struct SheetRule { pub gap: String, } +impl SheetRule { + /// The expressions, by field name (as commands name them), with the kind each evaluates to. + /// (`corner_relief_shape` is a choice, not an expression.) + pub fn fields(&self) -> [(&'static str, &String, Kind); 8] { + [ + ("thickness", &self.thickness, Kind::Length), + ("k_factor", &self.k_factor, Kind::Unitless), + ("bend_radius", &self.bend_radius, Kind::Length), + ("relief_width", &self.relief_width, Kind::Length), + ("relief_depth", &self.relief_depth, Kind::Length), + ("corner_relief", &self.corner_relief, Kind::Length), + ("hem_gap", &self.hem_gap, Kind::Length), + ("gap", &self.gap, Kind::Length), + ] + } + + /// The expressions to change, by field name, in the order of [`SheetRule::fields`]. + pub fn fields_mut(&mut self) -> [(&'static str, &mut String); 8] { + [ + ("thickness", &mut self.thickness), + ("k_factor", &mut self.k_factor), + ("bend_radius", &mut self.bend_radius), + ("relief_width", &mut self.relief_width), + ("relief_depth", &mut self.relief_depth), + ("corner_relief", &mut self.corner_relief), + ("hem_gap", &mut self.hem_gap), + ("gap", &mut self.gap), + ] + } +} + fn rule_gap() -> String { "Thickness".into() } diff --git a/crates/engine/src/cmd/parameters_tests.rs b/crates/engine/src/cmd/parameters_tests.rs index 27bc318f98..2871b24241 100644 --- a/crates/engine/src/cmd/parameters_tests.rs +++ b/crates/engine/src/cmd/parameters_tests.rs @@ -195,6 +195,65 @@ fn deleting_a_used_parameter_lists_its_users() { assert!(g["edges"].as_array().unwrap().iter().any(|e| e[0] == "Feature:Base" && e[1] == dim.as_str()), "{g}"); } +/// #4420: sheet metal and plastic rules and the plane of a canvas hold expressions too. Renaming a +/// parameter they use renames it there, and they count as its users (so it can't be deleted). +#[test] +fn rules_and_canvases_follow_a_renamed_parameter() { + let mut s = Session::default(); + for (name, expression) in [("gauge", "2 mm"), ("lift", "15 mm"), ("taper", "3 deg")] { + run(&mut s, "parameters.add", json!({"name": name, "expression": expression})); + } + let mut png = b"\x89PNG\r\n\x1a\n\0\0\0\rIHDR".to_vec(); + png.extend([0, 0, 0, 4, 0, 0, 0, 2, 8, 6, 0, 0, 0, 0, 0, 0, 0]); + run(&mut s, "canvas.insert", json!({"data": solidcraft_doc::canvas::base64_encode(&png), "plane": "XY", "offset": "lift"})); + run(&mut s, "sheet.manage_rules", json!({"name": "Steel", "thickness": "gauge", "active": true})); + run(&mut s, "solid.box", json!({"length": 60, "width": 40, "height": 2, "body_name": "Tray"})); + run(&mut s, "sheet.convert", json!({"body": "Tray", "face": [30, 20, 2], "rule": "Steel"})); + run(&mut s, "plastic.manage_rules", json!({"name": "Housing", "draft": "taper", "clearance": "gauge / 10"})); + let users = |s: &mut Session, name: &str| -> Vec { + let u = run(s, "parameters.users", json!({"name": name})); + u["users"].as_array().unwrap().iter().map(|x| x["type"].as_str().unwrap_or_default().to_string()).collect() + }; + assert_eq!(users(&mut s, "gauge"), ["rule", "rule"], "the sheet metal thickness and the plastic clearance"); + assert_eq!(users(&mut s, "lift"), ["canvas"]); + let e = err(&mut s, "parameters.delete", json!({"name": "taper"})); + assert!(e.contains("plastic rule Housing (draft)"), "{e}"); + // The sheet metal features rebuild when a rule's parameter changes, so they are what it drives. + let u = run(&mut s, "parameters.users", json!({"name": "gauge"})); + assert_eq!( + u["drives"].as_array().map(|d| d.iter().map(|x| x["name"].clone()).collect::>()), + Some(vec![json!("ConvertToSheetMetal1")]), + "{u}" + ); + let g = run(&mut s, "parameters.graph", json!({})); + assert!(g["edges"].as_array().unwrap().iter().any(|e| e[0] == "Feature:ConvertToSheetMetal1" && e[1] == "gauge"), "{g}"); + assert!(!g["edges"].as_array().unwrap().iter().any(|e| e[0] == "Feature:Box1" && e[1] == "gauge"), "{g}"); + // Every field of the shared list follows: `gap` was set through `sheet.manage_rules` too. + run(&mut s, "parameters.add", json!({"name": "seam", "expression": "0.5 mm"})); + run(&mut s, "sheet.manage_rules", json!({"name": "Steel", "gap": "seam"})); + run(&mut s, "parameters.rename", json!({"name": "seam", "new_name": "joint_gap"})); + assert_eq!(s.doc.sheet.rules[0].gap, "joint_gap"); + for (old, new) in [("gauge", "sheet_t"), ("lift", "raise"), ("taper", "draft_angle")] { + run(&mut s, "parameters.rename", json!({"name": old, "new_name": new})); + } + let failing: Vec = s.model.results.iter().filter_map(|r| r.error.clone()).collect(); + assert!(failing.is_empty(), "{failing:?}"); + assert_eq!(s.doc.sheet.rules[0].thickness, "sheet_t"); + let housing = s.doc.plastic.rules.iter().find(|r| r.name == "Housing").unwrap(); + assert_eq!((housing.draft.as_str(), housing.clearance.as_str()), ("draft_angle", "sheet_t / 10")); + assert!(run(&mut s, "plastic.rules", json!({}))["rules"].as_array().unwrap().iter().any(|r| r["name"] == "Housing" && !r["values"].is_null())); + match &s.doc.canvases[0].plane { + solidcraft_doc::PlaneRef::Offset { distance, .. } => assert_eq!(distance.as_ref(), "raise"), + other => panic!("{other:?}"), + } + assert_eq!(users(&mut s, "raise"), ["canvas"]); + // Undo takes the old names back. + for _ in 0..3 { + run(&mut s, "edit.undo", json!({})); + } + assert_eq!(s.doc.sheet.rules[0].thickness, "gauge"); +} + #[test] fn favorites_comments_and_completion() { let mut s = Session::default(); diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index fb01531518..86d89f1279 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -405,16 +405,7 @@ fn rules(s: &mut Session, p: &Value) -> Result { } let mut rule = s.doc.sheet.rules.iter().find(|r| r.name == name).cloned().unwrap_or_else(|| SheetRule { name: name.clone(), ..s.doc.sheet_rule(None) }); - for (k, slot) in [ - ("thickness", &mut rule.thickness), - ("k_factor", &mut rule.k_factor), - ("bend_radius", &mut rule.bend_radius), - ("relief_width", &mut rule.relief_width), - ("relief_depth", &mut rule.relief_depth), - ("corner_relief", &mut rule.corner_relief), - ("hem_gap", &mut rule.hem_gap), - ("gap", &mut rule.gap), - ] { + for (k, slot) in rule.fields_mut() { if let Some(e) = expr(p, k) { *slot = e.into(); }