Join from a sketch on a body's face joins that body (#1349) (#1526)

Thanks @SychPL for the fix.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/face_join_tests.rs

@@ -125,3 +125,44 @@ fn face_join_of_a_step_import_keeps_all_32_neighbours_unchanged() {
     let FeatureKind::Extrude { targets, .. } = &s.doc.feature(result["feature"].as_u64().unwrap()).unwrap().kind else { panic!("extrude") };
     assert_eq!(targets, &["Wall"]);
 }
+
+/// #1349: a profile sketched on a body's face joins that body, and leaves alone a neighbour
+/// whose face it lies on too: a rib on the side of a bracket, drawn down over the motor the
+/// bracket sits on, welded the motor in (a join that ran for minutes on the imported motor and
+/// failed). Explicit targets still join both.
+#[test]
+fn join_of_a_profile_sketched_on_a_face_leaves_a_neighbour_it_lies_on() {
+    for targets in [None, Some(json!(["Bracket", "Motor"]))] {
+        let mut s = Session::default();
+        run(&mut s, "solid.box", json!({"length":40,"width":30,"height":20,"body_name":"Motor"}));
+        run(&mut s, "solid.box", json!({"corner":[0,0,20],"length":40,"width":30,"height":5,"body_name":"Bracket"}));
+        let sketch = run(&mut s, "sketch.create", json!({"plane": {"face": [40, 15, 22.5]}}))["sketch"].as_u64().unwrap();
+        let plane = s.model.state().sketch(sketch).unwrap().plane;
+        let at = |y: f64, z: f64| {
+            let p = plane.to_local(Vec3::new(40.0, y, z));
+            json!([p.x, p.y])
+        };
+        // On the bracket's side and down over the motor's.
+        run(&mut s, "sketch.rectangle.two_point", json!({"p0": at(5.0, 10.0), "p1": at(25.0, 25.0)}));
+        run(&mut s, "sketch.finish", json!({}));
+        let mut params =
+            json!({"sketch": sketch, "profiles": [{"point": at(15.0, 22.5)}, {"point": at(15.0, 15.0)}], "distance": 1, "operation": "join"});
+        if let Some(t) = &targets {
+            params["targets"] = t.clone();
+        }
+        let result = run(&mut s, "solid.extrude", params);
+        assert!(s.model.results.iter().all(|r| r.error.is_none()), "{:?}", s.model.results);
+        let state = s.model.state();
+        let volume = |name: &str| solidcraft_kernel::measure(&state.body(name).unwrap().body).unwrap().volume;
+        if targets.is_some() {
+            assert_eq!(state.bodies.len(), 1);
+            assert!((volume("Bracket") - (24_000.0 + 6000.0 + 300.0)).abs() < 1e-6, "{}", volume("Bracket"));
+            continue;
+        }
+        assert_eq!(state.bodies.len(), 2);
+        assert!((volume("Motor") - 24_000.0).abs() < 1e-6, "the motor is untouched");
+        assert!((volume("Bracket") - (6000.0 + 300.0)).abs() < 1e-6, "{}", volume("Bracket"));
+        let FeatureKind::Extrude { targets, .. } = &s.doc.feature(result["feature"].as_u64().unwrap()).unwrap().kind else { panic!("extrude") };
+        assert_eq!(targets, &["Bracket"]);
+    }
+}

crates/engine/src/cmd/features.rs

@@ -15,7 +15,7 @@ pub static COMMANDS: &[CommandSpec] = &[
         .at("SOLID", "CREATE")
         .icon("extrude")
         .key("E")
-        .params("distance: expr (or through_all: true); taper?: angle expr; sketch?: id|name (default: active or last sketch); profiles?: all | [index] | [{curves: [ids]}] (the profile whose outer loop is made of exactly these curves, or pieces of them; found again on every recompute, so it follows parameter changes; islands inside are its holes; an error when no profile or several match) | [{point:[x,y]}] (index and point picks are stored by their curves when that finds the same profile); face?: [x,y,z] (extrude a planar body face instead of a sketch profile); face_body?: name (whose face, where faces of several bodies meet at the point); direction?: positive|negative|symmetric (distance is total thickness, split evenly around the start plane); convention?: total_distance|legacy_per_side (default total_distance; legacy preserves older symmetric extents); distance2?; start_offset?: expr (along the sketch normal, also with direction: negative); to?: [x,y,z] (To Object: a face, vertex or point; to_offset?: expr past it); from?: [x,y,z] (From Object: start at that face or point); operation?: new|join|cut|intersect|auto (cut into a body, otherwise join existing solids; new in an empty component); targets?: [body] (Join from a face defaults to that face's body; explicit targets or participants override); name?; body_name?; thin?: true (Thin: a wall along sketch curves instead of the profiles) with curves?: [curve ids] (open chains and closed loops, joined where they meet end to end; default: every drawn curve of the sketch), thickness: expr, side?: center|inside|outside|two_sides (a closed loop's inside is the area it encloses, an open chain's the side it bends toward, a straight line's the left of its first curve as drawn; default center), thickness2?: expr (two_sides: the thickness outside); corners are mitred, crossing walls are one"),
+        .params("distance: expr (or through_all: true); taper?: angle expr; sketch?: id|name (default: active or last sketch); profiles?: all | [index] | [{curves: [ids]}] (the profile whose outer loop is made of exactly these curves, or pieces of them; found again on every recompute, so it follows parameter changes; islands inside are its holes; an error when no profile or several match) | [{point:[x,y]}] (index and point picks are stored by their curves when that finds the same profile); face?: [x,y,z] (extrude a planar body face instead of a sketch profile); face_body?: name (whose face, where faces of several bodies meet at the point); direction?: positive|negative|symmetric (distance is total thickness, split evenly around the start plane); convention?: total_distance|legacy_per_side (default total_distance; legacy preserves older symmetric extents); distance2?; start_offset?: expr (along the sketch normal, also with direction: negative); to?: [x,y,z] (To Object: a face, vertex or point; to_offset?: expr past it); from?: [x,y,z] (From Object: start at that face or point); operation?: new|join|cut|intersect|auto (cut into a body, otherwise join existing solids; new in an empty component); targets?: [body] (Join from a face, or from a sketch on a body's face, defaults to that face's body; explicit targets or participants override); name?; body_name?; thin?: true (Thin: a wall along sketch curves instead of the profiles) with curves?: [curve ids] (open chains and closed loops, joined where they meet end to end; default: every drawn curve of the sketch), thickness: expr, side?: center|inside|outside|two_sides (a closed loop's inside is the area it encloses, an open chain's the side it bends toward, a straight line's the left of its first curve as drawn; default center), thickness2?: expr (two_sides: the thickness outside); corners are mitred, crossing walls are one"),
     CommandSpec::new("solid.revolve", "Revolve", revolve)
         .at("SOLID", "CREATE")
         .icon("revolve")
@@ -552,10 +552,13 @@ fn extrude(s: &mut Session, p: &Value) -> Result<Value> {
     };
     let mut targets = string_list(p, "targets");
     if operation == Operation::Join && targets.is_empty() && string_list(p, "participants").is_empty() {
-        // A face's outward extension joins that body. Assembly neighbours can touch only an
-        // edge of the tool: trying to join them first fails, or absorbs unrelated parts (#226).
+        // A face's outward extension joins that body, and so does a profile sketched on a body's
+        // face. Assembly neighbours can touch only an edge of the tool: trying to join them
+        // first fails, or absorbs unrelated parts (#226). A profile drawn past the face can lie
+        // on a neighbour's face too: a rib on a bracket's side welded in the motor the bracket
+        // holds, a join that ran for minutes and failed (#1349).
         // Store the resolved target so recomputes and reopening keep the same scope.
-        targets.extend(face_body);
+        targets.extend(face_body.or_else(|| sketch_face_body(s, sketch)));
     }
     let kind = FeatureKind::Extrude {
         sketch,
@@ -572,6 +575,17 @@ fn extrude(s: &mut Session, p: &Value) -> Result<Value> {
     add_feature(s, p, kind)
 }
 
+/// The body whose face `sketch` is attached to (on the face itself, not offset from it), when
+/// it is a body of the active component.
+fn sketch_face_body(s: &Session, sketch: u64) -> Option<String> {
+    let FeatureKind::Sketch { plane: solidcraft_doc::PlaneRef::Face { anchor: Some(anchor), .. }, .. } = &s.doc.feature(sketch)?.kind else {
+        return None;
+    };
+    let st = s.model.state();
+    let body = st.body(&anchor.target.body)?;
+    (s.doc.model_body_component(body) == s.active_component).then(|| body.name.clone())
+}
+
 /// A thin extrude's wall from its parameters: the sketch's curves it follows (every drawn
 /// curve when none are given), its thickness and which side of them it lies on.
 fn thin_param(s: &Session, p: &Value, sketch: u64, cmd: &str) -> Result<ThinWall> {