fix: raising a pattern's count after a rename numbers the new copies after the others (#372) (#540)

A copy added by raising a body pattern's count after a rename is numbered after the highest stored copy, so it no longer takes the number the rename freed. Un-ignores the #372 regression test and adds one for undo/redo and save/reopen. Fixes #372.

Co-authored-by: jonaswre <[email protected]>
This commit is contained in:
Jonas Wrede authored and GitHub committed 2026-10-13 04:04:44 -04:00
1 parent 711e5508b5
commit f9c19ade16
2 files changed
+41 -2

No files matched your search

+20 -1
View File
@@ -1143,6 +1143,25 @@ fn new_name(state: &mut ModelState, f: &Feature, k: usize) -> String {
}
}
/// The k-th body of a body pattern: its stored name, else a copy of `source`. Once `body.rename`
/// has stored this feature's names, a copy beyond them (the count was raised) is numbered after
/// the highest stored `source (n)`, so it never takes a number freed by a rename (#372).
/// Features without stored names keep the first free `source (n)`.
fn pattern_copy_name(state: &ModelState, f: &Feature, k: usize, source: &str) -> String {
if f.body_names.get(k).is_some_and(|name| !name.trim().is_empty()) {
return copy_body_name(state, f, k, source);
}
let base = if source.trim().is_empty() { "Body" } else { source.trim() };
let prefix = format!("{base} (");
let highest = f.body_names.iter().filter_map(|n| n.trim().strip_prefix(prefix.as_str())?.strip_suffix(')')?.parse::<u32>().ok()).max();
match highest {
Some(h) => {
(u64::from(h) + 1..).map(|i| format!("{base} ({i})")).find(|n| state.body(n).is_none()).unwrap_or_else(|| unique_body_name(state, base))
}
None => unique_body_name(state, base),
}
}
fn copy_body_name(state: &ModelState, f: &Feature, k: usize, source: &str) -> String {
let base = f.body_names.get(k).filter(|name| !name.trim().is_empty()).map(String::as_str).unwrap_or(source);
unique_body_name(state, base)
@@ -3570,7 +3589,7 @@ fn eval_feature(doc: &Document, vals: &BTreeMap<String, Value>, f: &Feature, st:
let mb = st.body(n).cloned().ok_or_else(|| DocError::Unknown(format!("body `{n}`")))?;
for (k, m) in mats.iter().enumerate() {
let copy = moved_copy(&mb, kernel::transform_matrix(&mb.body, *m)?, k + 1);
let name = copy_body_name(st, f, output, &mb.name);
let name = pattern_copy_name(st, f, output, &mb.name);
st.bodies.push(ModelBody::traced_new(name, copy, f.id));
output += 1;
}
+21 -1
View File
@@ -2806,7 +2806,6 @@ fn a_pattern_repeats_a_fillet_on_the_edges_it_found() {
/// A copy added by raising the count after a rename takes the next number, not the freed one (#372).
#[test]
#[ignore = "fails until #372 is fixed"]
fn raising_a_pattern_count_after_a_rename_numbers_the_new_copy_after_the_others() {
let mut s = Session::default();
run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "operation": "new_body", "body_name": "X"}));
@@ -2966,3 +2965,24 @@ fn draft_on_a_cylinder_side_makes_a_frustum() {
let expect = PI * h / 3.0 * (big * big + big * small + small * small);
assert!(rel(volume(&mut s), expect) < 1e-3, "{} vs {expect}", volume(&mut s));
}
/// The copies added after a rename keep their numbers through undo/redo, save/reopen and a
/// further count change; lowering the count drops the last copies (#372).
#[test]
fn pattern_copies_added_after_a_rename_keep_their_numbers() {
let mut s = Session::default();
let names = |s: &Session| -> Vec<String> { s.model.state().bodies.iter().map(|b| b.name.clone()).collect() };
run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "operation": "new_body", "body_name": "X"}));
run(&mut s, "solid.pattern.rectangular", json!({"bodies": ["X"], "dir1": [1, 0, 0], "count1": 3, "spacing1": 20, "name": "Pat"}));
run(&mut s, "body.rename", json!({"body": "X (2)", "name": "Second"}));
run(&mut s, "timeline.edit", json!({"feature": "Pat", "set": {"pattern": {"count1": "5"}}}));
assert_eq!(names(&s), ["X", "Second", "X (3)", "X (4)", "X (5)"]);
run(&mut s, "edit.undo", json!({}));
assert_eq!(names(&s), ["X", "Second", "X (3)"]);
run(&mut s, "edit.redo", json!({}));
assert_eq!(names(&s), ["X", "Second", "X (3)", "X (4)", "X (5)"]);
let reopened = Session::new(solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap());
assert_eq!(names(&reopened), ["X", "Second", "X (3)", "X (4)", "X (5)"]);
run(&mut s, "timeline.edit", json!({"feature": "Pat", "set": {"pattern": {"count1": "2"}}}));
assert_eq!(names(&s), ["X", "Second"]);
}