diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index c7f70e3a..7b94485c 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -939,6 +939,66 @@ fn z_axis() -> Vec3 { } impl FeatureKind { + /// Sketches the feature is built from (profiles, paths, points): deleting one of them + /// deletes the feature. Every kind is listed, so a new one has to say what it uses. + pub fn input_sketches(&self) -> Vec { + use FeatureKind as K; + match self { + K::Extrude { sketch, .. } + | K::Revolve { sketch, .. } + | K::Emboss { sketch, .. } + | K::Rib { sketch, .. } + | K::SheetBase { sketch, .. } + | K::SheetContour { sketch, .. } => vec![*sketch], + K::Sweep { sketch, path_sketch, .. } => vec![*sketch, *path_sketch], + K::Pipe { path_sketch, .. } | K::Pattern { pattern: PatternKind::Path { path_sketch, .. }, .. } => vec![*path_sketch], + K::Loft { sections, .. } => sections.iter().map(|s| s.sketch).collect(), + K::SheetFold { sketch, .. } | K::SplitFace { sketch, .. } | K::Patch { sketch, .. } => sketch.iter().copied().collect(), + K::Hole { points, .. } => points.iter().map(|p| p.sketch).collect(), + K::Sketch { .. } + | K::Pattern { .. } + | K::Fillet { .. } + | K::Chamfer { .. } + | K::Box { .. } + | K::Cylinder { .. } + | K::Sphere { .. } + | K::Torus { .. } + | K::Combine { .. } + | K::Mirror { .. } + | K::Thread { .. } + | K::Shell { .. } + | K::Draft { .. } + | K::ConstructionPlane { .. } + | K::Split { .. } + | K::ConstructionAxis { .. } + | K::ConstructionPoint { .. } + | K::Scale { .. } + | K::OffsetFace { .. } + | K::DeleteFace { .. } + | K::BoundingSolid { .. } + | K::ReplaceFace { .. } + | K::Align { .. } + | K::Coil { .. } + | K::SheetFlange { .. } + | K::SheetHem { .. } + | K::SheetUnfold { .. } + | K::SheetConvert { .. } + | K::Boss { .. } + | K::Lip { .. } + | K::SnapFit { .. } + | K::Rest { .. } + | K::BoundaryFill { .. } + | K::Remove { .. } + | K::Stitch { .. } + | K::Thicken { .. } + | K::SurfaceExtend { .. } + | K::SurfaceTrim { .. } + | K::Move { .. } + | K::Import { .. } + | K::MeshImport { .. } => Vec::new(), + } + } + /// A sheet metal feature (its result depends on the sheet metal rules). pub fn is_plastic(&self) -> bool { matches!(self, FeatureKind::Boss { .. } | FeatureKind::Lip { .. } | FeatureKind::SnapFit { .. } | FeatureKind::Rest { .. }) @@ -1620,18 +1680,7 @@ impl Document { pub fn delete_feature(&mut self, id: u64) -> Result> { let idx = self.feature_index(id).ok_or_else(|| DocError::Unknown(format!("feature {id}")))?; let mut gone = vec![id]; - for f in &self.features { - match &f.kind { - FeatureKind::Extrude { sketch, .. } | FeatureKind::Revolve { sketch, .. } if *sketch == id => gone.push(f.id), - FeatureKind::Sweep { sketch, path_sketch, .. } if *sketch == id || *path_sketch == id => gone.push(f.id), - FeatureKind::Loft { sections, .. } if sections.iter().any(|s| s.sketch == id) => gone.push(f.id), - FeatureKind::Pattern { pattern: PatternKind::Path { path_sketch, .. }, .. } if *path_sketch == id => gone.push(f.id), - FeatureKind::Emboss { sketch, .. } | FeatureKind::Rib { sketch, .. } if *sketch == id => gone.push(f.id), - FeatureKind::SheetBase { sketch, .. } | FeatureKind::SheetContour { sketch, .. } if *sketch == id => gone.push(f.id), - FeatureKind::SheetFold { sketch: Some(s), .. } | FeatureKind::SplitFace { sketch: Some(s), .. } if *s == id => gone.push(f.id), - _ => {} - } - } + gone.extend(self.features.iter().filter(|f| f.kind.input_sketches().contains(&id)).map(|f| f.id)); self.features.retain(|f| !gone.contains(&f.id)); // Configuration columns of deleted features go with them. let keep: Vec = self.configs.columns.iter().map(|c| !matches!(c, crate::config::Column::Suppress(id) if gone.contains(id))).collect(); diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index b74f6da1..5a4b9016 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -295,3 +295,53 @@ fn parameter_cycles_are_found() { assert!(errs["a"].contains("circular reference: a → b → c → a"), "{}", errs["a"]); assert!(vals.contains_key("d") && errs.contains_key("a") && errs.contains_key("e")); } + +/// Features that use a sketch's points or regions go with it (not only profile and path users). +#[test] +fn input_sketches_cover_points_and_patches() { + let hole: FeatureKind = serde_json::from_value(serde_json::json!({ + "type": "hole", "position": [0, 0, 0], "direction": [0, 0, -1], "diameter": "3", + "points": {"sketch": 7, "ids": ["p1"]}, + })) + .unwrap(); + assert_eq!(hole.input_sketches(), vec![7]); + let patch: FeatureKind = serde_json::from_value(serde_json::json!({"type": "patch", "sketch": 9})).unwrap(); + assert_eq!(patch.input_sketches(), vec![9]); + let pipe: FeatureKind = serde_json::from_value(serde_json::json!({"type": "pipe", "path_sketch": 72, "path": ["l1"], "diameter": "4"})).unwrap(); + assert_eq!(pipe.input_sketches(), vec![72]); +} + +/// Guard for `FeatureKind::input_sketches`: a variant with a field that refers to a sketch +/// (a field named `*sketch*` holding an id, sketch points or loft sections) must not sit in the +/// arm that returns no sketches. New variants are caught by the compiler (the match has no +/// wildcard); this catches a sketch field added to an existing variant. +#[test] +fn input_sketches_lists_every_variant_with_a_sketch_field() { + let src = include_str!("document.rs"); + let body = src.split("pub enum FeatureKind {").nth(1).and_then(|s| s.split("\n}\n").next()).unwrap(); + let mut with_sketch = Vec::new(); + let mut variant = ""; + for line in body.lines() { + let t = line.trim(); + if line.starts_with(" ") && !line.starts_with(" ") && t.chars().next().is_some_and(|c| c.is_ascii_uppercase()) { + variant = t.split(|c: char| !c.is_alphanumeric()).next().unwrap_or(""); + continue; + } + let Some((name, ty)) = t.strip_prefix("pub ").unwrap_or(t).split_once(':') else { continue }; + let refers = (name.contains("sketch") && (ty.contains("u64") || ty.contains("SketchPoints"))) + || ty.contains("SketchPoints") + || ty.contains("LoftSection"); + if refers && !line.trim_start().starts_with("//") && !with_sketch.contains(&variant) { + with_sketch.push(variant); + } + } + assert!(with_sketch.len() >= 10, "the FeatureKind parse found too few sketch users: {with_sketch:?}"); + let f = src.split("pub fn input_sketches(&self)").nth(1).unwrap(); + let empty_arm = f.split("=> Vec::new()").next().and_then(|s| s.rsplit("=> ").next()).unwrap(); + for v in with_sketch { + assert!( + !empty_arm.contains(&format!("K::{v} {{")), + "FeatureKind::{v} has a sketch field but input_sketches says it uses no sketch: deleting the sketch would leave it broken" + ); + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3c4a6714..3a06c2e6 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -839,6 +839,22 @@ fn pipe_scale_offset_bounding_and_materials() { assert!(s.execute("material.assign", &json!({"bodies": ["Cube"], "material": "unobtainium"})).is_err()); } +/// Regression: deleting a pipe's path sketch left the pipe in the timeline ("unknown sketch"). +#[test] +fn deleting_a_path_sketch_deletes_its_pipe() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Path"})); + run(&mut s, "sketch.line", json!({"points": [[0, 0], [30, 0]], "ids": ["l1"]})); + run(&mut s, "sketch.finish", json!({})); + let pipe = run(&mut s, "solid.pipe", json!({"path_sketch": "Path", "path": ["l1"], "diameter": 4})); + let deps = run(&mut s, "timeline.dependents", json!({"feature": "Path"})); + assert_eq!(deps["deleted_with_it"].as_array().map(Vec::len), Some(1), "{deps} (pipe {pipe})"); + let r = run(&mut s, "timeline.delete", json!({"features": ["Path"]})); + assert_eq!(r["deleted"].as_array().map(Vec::len), Some(2), "{r}"); + assert!(s.doc.features.is_empty(), "{:?}", s.doc.features.iter().map(|f| &f.name).collect::>()); + assert!(s.model.results.iter().all(|r| r.error.is_none())); +} + #[test] fn components_occurrences_and_world_placement() { let mut s = Session::default();