From f06b76e14ffdb1cb6e8671d81abfa0f72267e7e7 Mon Sep 17 00:00:00 2001 From: Jonas Wrede Date: Mon, 12 Oct 2026 22:04:54 +0200 Subject: [PATCH] fix: number a profile added after a named multi-profile extrude after body_name (#373) (#385) --- crates/doc/src/document.rs | 5 ++++- crates/doc/src/eval.rs | 10 ++++++++++ crates/engine/src/cmd/features.rs | 29 +++++++++-------------------- crates/engine/src/tests.rs | 27 ++++++++++++++++++++++++++- 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index 8423a25d..bc107929 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -1544,7 +1544,8 @@ pub struct Feature { pub name: String, #[serde(default)] pub suppressed: bool, - /// Names for the bodies this feature creates (in order); missing ones get `BodyN`. + /// Names for the bodies this feature creates (in order); missing ones are numbered after the + /// first (`Ribs (4)`), or get `BodyN` when there is none (or see `shell_piece_order`). #[serde(default, skip_serializing_if = "Vec::is_empty")] pub body_names: Vec, /// Parameter names of the feature's inputs (see `FeatureKind::inputs`), e.g. `d3`. @@ -1570,6 +1571,8 @@ pub struct Feature { /// The pieces this feature splits a body into take its body names in the kernel's shell order, /// as builds before format 14 named them, instead of after their faces (`lineage::order_lumps`). /// Set on the features of designs saved before then, whose stored body names mean that order. + /// Such features also name the bodies past their `body_names` `BodyN`, as those builds did, + /// where newer ones number them after the first name (`Ribs (4)`, #373). #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub shell_piece_order: bool, #[serde(flatten)] diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index f71b9efb..df660ad1 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -1054,6 +1054,16 @@ fn new_name(state: &mut ModelState, f: &Feature, k: usize) -> String { { return n.clone(); } + // Bodies past the stored names (a profile added after the feature was made) are numbered + // after the first name: `Ribs (4)` (#373). Features saved before format 14 keep `BodyN`, + // which later features of those designs may name. + if !f.shell_piece_order + && k >= f.body_names.len() + && let Some(base) = f.body_names.first().map(|n| n.trim()).filter(|n| !n.is_empty()) + { + let n = format!("{base} ({})", k.saturating_add(1)); + return if state.body(&n).is_none() { n } else { unique_body_name(state, base) }; + } loop { state.body_counter += 1; let n = format!("Body{}", state.body_counter); diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index 186b635e..5aa7c4a3 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -453,26 +453,15 @@ fn add_feature_in(s: &mut Session, p: &Value, kind: FeatureKind, world: bool) -> s.active_sketch = None; s.refresh(); // A feature that makes more bodies than it was given names (a multi-profile extrude with one - // `body_name`) names the rest after the first: `Ribs`, `Ribs (2)`, `Ribs (3)`… (#266). The - // names are stored on the feature, so recomputes and saved designs keep them. - if let Some(base) = names.first().map(|n| n.trim().to_string()).filter(|n| !n.is_empty()) { - let st = s.model.state(); - let made = st.bodies.iter().filter(|b| b.feature == id).count(); - if made > names.len() { - let mut taken: std::collections::BTreeSet = st.bodies.iter().map(|b| b.name.clone()).collect(); - taken.extend(names.iter().cloned()); - let mut i = 1usize; - while names.len() < made { - i += 1; - let n = format!("{base} ({i})"); - if taken.insert(n.clone()) { - names.push(n); - } - } - if let Some(f) = s.doc_mut().feature_mut(id) { - f.body_names = names; - } - s.refresh(); + // `body_name`) has the rest numbered after the first by the evaluator: `Ribs`, `Ribs (2)`, + // `Ribs (3)`… (#266, #373). They are stored on the feature, so recomputes and saved designs + // keep them even when the profiles are later picked in another order. + if names.first().is_some_and(|n| !n.trim().is_empty()) { + let made: Vec = s.model.state().bodies.iter().filter(|b| b.feature == id).map(|b| b.name.clone()).collect(); + if made.len() > names.len() + && let Some(f) = s.doc_mut().feature_mut(id) + { + f.body_names = made; } } let r = s.model.result(id).cloned(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index a6c63917..6fe54465 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2735,7 +2735,6 @@ fn raising_a_pattern_count_after_a_rename_numbers_the_new_copy_after_the_others( /// A profile added to the sketch after a named multi-profile extrude is named after `body_name` /// too (#373). #[test] -#[ignore = "fails until #373 is fixed"] fn a_profile_added_later_is_named_after_body_name() { let mut s = Session::default(); run(&mut s, "sketch.create", json!({"plane": "XY", "name": "R"})); @@ -2765,3 +2764,29 @@ fn extrude_reply_lists_the_bodies_it_created() { let r = run(&mut s, "solid.extrude", json!({"sketch": "R", "distance": "10", "operation": "new_body", "body_name": "Ribs"})); assert_eq!(r["created_bodies"], json!(["Ribs", "Ribs (2)", "Ribs (3)"]), "{r}"); } + +/// A design saved before format 14 names the bodies past its stored names `BodyN`, as it did when +/// it was saved, so later features that name them still find them (#373). +#[test] +fn a_design_saved_before_format_14_keeps_generic_names_past_body_name() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "R"})); + for x in [0, 10, 20] { + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [x, 0], "p1": [x + 5, 30]})); + } + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"sketch": "R", "distance": "10", "operation": "new_body", "body_name": "Ribs"})); + let mut legacy: Value = serde_json::from_str(&s.doc.to_json()).unwrap(); + legacy["format"] = json!("solvecraft/13"); + let extrude = legacy["features"].as_array_mut().unwrap().iter_mut().find(|f| f["type"] == "extrude").unwrap(); + extrude["body_names"] = json!(["Ribs"]); + let dir = std::env::temp_dir().join(format!("solvecraft-373-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let design = dir.join("ribs.solvecraft"); + std::fs::write(&design, legacy.to_string()).unwrap(); + let mut old = Session::default(); + run(&mut old, "doc.open", json!({"path": design.to_string_lossy()})); + let names: Vec = old.model.state().bodies.iter().map(|b| b.name.clone()).collect(); + assert_eq!(names, ["Ribs", "Body1", "Body2"]); + let _ = std::fs::remove_dir_all(&dir); +}