fix: number a profile added after a named multi-profile extrude after body_name (#373) (#385)

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<String>,
     /// 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)]

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);

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<String> = 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<String> = 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();

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<String> = old.model.state().bodies.iter().map(|b| b.name.clone()).collect();
+    assert_eq!(names, ["Ribs", "Body1", "Body2"]);
+    let _ = std::fs::remove_dir_all(&dir);
+}