From dde6d48dd02da0acce957fdbb23b284d2c567616 Mon Sep 17 00:00:00 2001 From: NOWN-DEV <257771410+NOWN-DEV@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:51:12 +0200 Subject: [PATCH] fix: renaming a body keeps the names of its duplicates (#1056) Bodies imported with the same label come in as X, X (2), X (3). Renaming one of them no longer renumbers the others: body.rename now pins the current names of the feature's other bodies when it stores the new one, so later renames, materials, appearances and features still find them. --- crates/engine/src/cmd/browser.rs | 8 ++++- crates/engine/src/tests.rs | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/browser.rs b/crates/engine/src/cmd/browser.rs index a022baf9..3e5faf19 100644 --- a/crates/engine/src/cmd/browser.rs +++ b/crates/engine/src/cmd/browser.rs @@ -489,7 +489,10 @@ fn rename_body(s: &mut Session, p: &Value) -> Result { return Err(bad(cmd, format!("another body is called `{name}`"))); } // The body is the k-th new body of its feature; its siblings keep the names they have now - // (unpinned pattern copies would otherwise be renumbered into the freed name, #265). + // (unpinned pattern copies would otherwise be renumbered into the freed name, #265). That + // includes the siblings whose stored name is shared: the solids of a STEP file with one label + // are stored as `X`, `X`, `X` and built as `X`, `X (2)`, `X (3)`, so the stored names are set + // to the numbered ones, or `X (3)` would turn into `X (2)` once the second solid is renamed. let fid = b.feature; let siblings: Vec = st.bodies.iter().filter(|x| x.feature == fid).map(|x| x.name.clone()).collect(); let k = siblings.iter().position(|n| *n == old).unwrap_or(0); @@ -499,6 +502,9 @@ fn rename_body(s: &mut Session, p: &Value) -> Result { let i = f.body_names.len(); f.body_names.push(siblings.get(i).cloned().unwrap_or_default()); } + for (slot, current) in f.body_names.iter_mut().zip(&siblings) { + slot.clone_from(current); + } if let Some(slot) = f.body_names.get_mut(k) { *slot = name.to_string(); } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 163cdd93..dd10facc 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2880,6 +2880,67 @@ fn renaming_a_pattern_copy_keeps_the_other_copies_names() { assert_eq!(names, ["X", "Second", "Third"]); } +/// Three solids in one STEP file that carry the same label: the file written for three boxes, each +/// named `X`. +fn step_three_solids_labelled_x(tag: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("solidcraft-step-{tag}-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let mut s = Session::default(); + for (i, label) in ["Aa", "Bb", "Cc"].iter().enumerate() { + let corner = [30.0 * i as f64, 0.0, 0.0]; + run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "corner": corner, "operation": "new_body", "body_name": label})); + } + let p = dir.join("Three.step"); + run(&mut s, "file.export", json!({"path": p.to_string_lossy()})); + let mut text = std::fs::read_to_string(&p).unwrap(); + for label in ["Aa", "Bb", "Cc"] { + assert!(text.contains(&format!("MANIFOLD_SOLID_BREP('{label}'")), "the file names its solids: {label}"); + text = text.replace(&format!("MANIFOLD_SOLID_BREP('{label}'"), "MANIFOLD_SOLID_BREP('X'"); + } + std::fs::write(&p, text).unwrap(); + p +} + +/// Renaming one of several imported bodies that share a label leaves the names of the others alone: +/// the model lists them as `X`, `X (2)`, `X (3)`, and `X (3)` is still there after `X (2)` is renamed. +#[test] +fn renaming_an_imported_body_keeps_the_names_of_its_duplicates() { + let p = step_three_solids_labelled_x("duplicate-labels"); + let mut s = Session::default(); + run(&mut s, "file.insert_step", json!({"path": p.to_string_lossy()})); + let names = |s: &Session| s.model.state().bodies.iter().map(|b| b.name.clone()).collect::>(); + assert_eq!(names(&s), ["X", "X (2)", "X (3)"]); + let x_of = + |s: &mut Session, body: &str| run(s, "inspect.measure", json!({"bodies": [body]}))["bodies"][0]["bbox"]["min"][0].as_f64().map(f64::round); + assert_eq!(x_of(&mut s, "X (3)"), Some(60.0)); + run(&mut s, "body.rename", json!({"body": "X (2)", "name": "Second"})); + assert_eq!(names(&s), ["X", "Second", "X (3)"]); + assert_eq!(x_of(&mut s, "X (3)"), Some(60.0), "X (3) is still the solid at x = 60"); + run(&mut s, "body.rename", json!({"body": "X (3)", "name": "Third"})); + assert_eq!(names(&s), ["X", "Second", "Third"]); + assert_eq!(x_of(&mut s, "Third"), Some(60.0)); + // The names survive a rebuild and a reopen of the saved design. + let design = p.with_file_name("Three.solidcraft"); + run(&mut s, "file.save_as", json!({"path": design.to_string_lossy()})); + let mut r = Session::default(); + run(&mut r, "doc.open", json!({"path": design.to_string_lossy()})); + assert_eq!(names(&r), ["X", "Second", "Third"]); +} + +/// The same when the first of the three is renamed: `X (2)` and `X (3)` do not slide down to `X` and +/// `X (2)`, and what is set on one of them (a material) stays with it. +#[test] +fn renaming_the_first_imported_duplicate_keeps_the_numbers_of_the_others() { + let p = step_three_solids_labelled_x("duplicate-labels-first"); + let mut s = Session::default(); + run(&mut s, "file.insert_step", json!({"path": p.to_string_lossy()})); + run(&mut s, "material.assign", json!({"bodies": ["X (3)"], "material": "Steel"})); + run(&mut s, "body.rename", json!({"body": "X", "name": "First"})); + let names: Vec = s.model.state().bodies.iter().map(|b| b.name.clone()).collect(); + assert_eq!(names, ["First", "X (2)", "X (3)"]); + assert!(s.doc.materials.contains_key("X (3)"), "the material is still on X (3): {:?}", s.doc.materials.keys().collect::>()); +} + /// Parameters that work are listed in the command's params doc, which is all agents see (#271). #[test] fn params_docs_list_working_parameters() {