Fix #129094: Sub-targets not symmetrized if from different armature

When symmetrizing a bone (in edit mode) any constraint subtargets were name flipped,
if possible and they exist. This only worked if the target is the armature of the bone
being flipped.
This patch changes that so a subtarget flip is always attempted for targets that are an armature.

Pull Request: https://projects.blender.org/blender/blender/pulls/129169

source/blender/editors/armature/armature_add.cc

@@ -295,11 +295,13 @@ EditBone *add_points_bone(Object *obedit, float head[3], float tail[3])
 
 static EditBone *get_named_editbone(ListBase *edbo, const char *name)
 {
-  if (name) {
-    LISTBASE_FOREACH (EditBone *, eBone, edbo) {
-      if (STREQ(name, eBone->name)) {
-        return eBone;
-      }
+  if (!edbo || !name) {
+    return nullptr;
+  }
+
+  LISTBASE_FOREACH (EditBone *, eBone, edbo) {
+    if (STREQ(name, eBone->name)) {
+      return eBone;
     }
   }
 
@@ -387,7 +389,6 @@ static void pose_edit_bone_duplicate(ListBase *editbones, Object *ob)
 }
 
 static void update_duplicate_subtarget(EditBone *dup_bone,
-                                       ListBase *editbones,
                                        Object *ob,
                                        const bool lookup_mirror_subtarget)
 {
@@ -402,6 +403,7 @@ static void update_duplicate_subtarget(EditBone *dup_bone,
 
   EditBone *oldtarget, *newtarget;
   ListBase *conlist = &pchan->constraints;
+  char name_flipped[MAX_ID_NAME - 2];
   LISTBASE_FOREACH (bConstraint *, curcon, conlist) {
     /* does this constraint have a subtarget in
      * this armature?
@@ -412,30 +414,28 @@ static void update_duplicate_subtarget(EditBone *dup_bone,
       continue;
     }
     LISTBASE_FOREACH (bConstraintTarget *, ct, &targets) {
-      if ((ct->tar != ob) && (!ct->subtarget[0])) {
+      if (!ct->tar || !ct->subtarget[0]) {
         continue;
       }
-      oldtarget = get_named_editbone(editbones, ct->subtarget);
-      if (!oldtarget) {
+      Object *target_ob = ct->tar;
+      if (target_ob->type != OB_ARMATURE || !target_ob->data) {
+        /* Can only mirror armature. */
         continue;
       }
-      /* was the subtarget bone duplicated too? If
+      bArmature *target_armature = static_cast<bArmature *>(target_ob->data);
+      /* Was the subtarget bone duplicated too? If
        * so, update the constraint to point at the
        * duplicate of the old subtarget.
        */
-      if (oldtarget->temp.ebone) {
+      oldtarget = get_named_editbone(&target_armature->bonebase, ct->subtarget);
+      if (oldtarget && oldtarget->temp.ebone) {
         newtarget = oldtarget->temp.ebone;
         STRNCPY(ct->subtarget, newtarget->name);
       }
       else if (lookup_mirror_subtarget) {
-        /* The subtarget was not selected for duplication, try to see if a mirror bone of
-         * the current target exists */
-        char name_flip[MAXBONENAME];
-
-        BLI_string_flip_side_name(name_flip, oldtarget->name, false, sizeof(name_flip));
-        newtarget = get_named_editbone(editbones, name_flip);
-        if (newtarget) {
-          STRNCPY(ct->subtarget, newtarget->name);
+        BLI_string_flip_side_name(name_flipped, ct->subtarget, false, sizeof(name_flipped));
+        if (bPoseChannel *flipped_bone = BKE_pose_channel_find_name(ct->tar->pose, name_flipped)) {
+          STRNCPY(ct->subtarget, flipped_bone->name);
         }
       }
     }
@@ -1161,7 +1161,7 @@ static int armature_duplicate_selected_exec(bContext *C, wmOperator *op)
         }
 
         /* Lets try to fix any constraint sub-targets that might have been duplicated. */
-        update_duplicate_subtarget(ebone, arm->edbo, ob, false);
+        update_duplicate_subtarget(ebone, ob, false);
       }
     }
 
@@ -1414,7 +1414,7 @@ static int armature_symmetrize_exec(bContext *C, wmOperator *op)
         ebone->bbone_next_flag = ebone_iter->bbone_next_flag;
 
         /* Lets try to fix any constraint sub-targets that might have been duplicated. */
-        update_duplicate_subtarget(ebone, arm->edbo, obedit, true);
+        update_duplicate_subtarget(ebone, obedit, true);
         /* Try to update constraint options so that they are mirrored as well
          * (need to supply bone_iter as well in case we are working with existing bones) */
         update_duplicate_constraint_settings(ebone, ebone_iter, obedit);

tests/python/bl_rigging_symmetrize.py

@@ -221,6 +221,109 @@ class ArmatureSymmetrizeTest(AbstractAnimationTest, unittest.TestCase):
                 value, vec2[idx], 3, "%s does not match with expected value on bone %s" % (check_str, bone_name))
 
 
+def create_armature() -> tuple[bpy.types.Object, bpy.types.Armature]:
+    arm = bpy.data.armatures.new('Armature')
+    arm_ob = bpy.data.objects.new('ArmObject', arm)
+
+    # Link to the scene just for giggles. And ease of debugging when things
+    # go bad.
+    bpy.context.scene.collection.objects.link(arm_ob)
+
+    return arm_ob, arm
+
+
+def set_edit_bone_selected(ebone: bpy.types.EditBone, selected: bool):
+    # Helper to select all parts of an edit bone.
+    ebone.select = selected
+    ebone.select_tail = selected
+    ebone.select_head = selected
+
+
+def create_copy_loc_constraint(pose_bone, target_ob, subtarget):
+    pose_bone.constraints.new("COPY_LOCATION")
+    pose_bone.constraints[0].target = target_ob
+    pose_bone.constraints[0].subtarget = subtarget
+
+
+class ArmatureSymmetrizeTargetsTest(unittest.TestCase):
+    arm_ob: bpy.types.Object
+    arm: bpy.types.Armature
+
+    def setUp(self):
+        bpy.ops.wm.read_homefile(use_factory_startup=True)
+        self.arm_ob, self.arm = create_armature()
+        bpy.context.view_layer.objects.active = self.arm_ob
+        bpy.ops.object.mode_set(mode='EDIT')
+        ebone = self.arm.edit_bones.new(name="test.l")
+        ebone.tail = (1, 0, 0)
+
+    def test_symmetrize_selection(self):
+        # Only selected things are symmetrized.
+        set_edit_bone_selected(self.arm.edit_bones["test.l"], False)
+        bpy.ops.armature.symmetrize()
+        self.assertEqual(len(self.arm.edit_bones), 1, "If nothing is selected, no bone is symmetrized")
+
+        set_edit_bone_selected(self.arm.edit_bones["test.l"], True)
+        bpy.ops.armature.symmetrize()
+        self.assertEqual(len(self.arm.edit_bones), 2, "Selected EditBone should have been symmetrized")
+        self.assertTrue("test.r" in self.arm.edit_bones)
+        self.assertTrue("test.l" in self.arm.edit_bones)
+
+    def test_symmetrize_constraint_sub_target(self):
+        # Explicitly test that constraints targeting another armature are symmetrized.
+        bpy.ops.object.mode_set(mode='OBJECT')
+        target_arm_ob, target_arm = create_armature()
+        bpy.context.view_layer.objects.active = target_arm_ob
+        bpy.ops.object.mode_set(mode='EDIT')
+        target_arm.edit_bones.new("target.l")
+        target_arm.edit_bones.new("target.r")
+        target_arm.edit_bones["target.l"].tail = (1, 0, 0)
+        target_arm.edit_bones["target.r"].tail = (1, 0, 0)
+
+        bpy.ops.object.mode_set(mode='OBJECT')
+        bpy.context.view_layer.objects.active = self.arm_ob
+
+        bpy.ops.object.mode_set(mode='POSE')
+        pose_bone_l = self.arm_ob.pose.bones["test.l"]
+        create_copy_loc_constraint(pose_bone_l, target_arm_ob, "target.l")
+
+        bpy.ops.object.mode_set(mode='EDIT')
+        set_edit_bone_selected(self.arm.edit_bones["test.l"], True)
+        bpy.ops.armature.symmetrize()
+        self.assertEqual(len(self.arm.edit_bones), 2, "Bone should have been symmetrized")
+        self.assertTrue("test.r" in self.arm.edit_bones)
+        self.assertTrue("test.l" in self.arm.edit_bones)
+
+        bpy.ops.object.mode_set(mode='POSE')
+        self.assertEqual(len(self.arm_ob.pose.bones["test.r"].constraints), 1, "Constraint should have been copied")
+        symm_constraint = self.arm_ob.pose.bones["test.r"].constraints[0]
+        self.assertEqual(symm_constraint.subtarget, "target.r")
+
+    def test_symmetrize_invalid_subtarget(self):
+        # Blender shouldn't crash when there is an invalid subtarget specified.
+        bpy.ops.object.mode_set(mode='OBJECT')
+        target_ob = bpy.data.objects.new("target", None)
+        bpy.context.scene.collection.objects.link(target_ob)
+
+        bpy.context.view_layer.objects.active = self.arm_ob
+
+        bpy.ops.object.mode_set(mode='POSE')
+        pose_bone_l = self.arm_ob.pose.bones["test.l"]
+        create_copy_loc_constraint(pose_bone_l, target_ob, "invalid_subtarget")
+
+        bpy.ops.object.mode_set(mode='EDIT')
+        set_edit_bone_selected(self.arm.edit_bones["test.l"], True)
+        bpy.ops.armature.symmetrize()
+        self.assertEqual(len(self.arm.edit_bones), 2, "Bone should have been symmetrized")
+        self.assertTrue("test.r" in self.arm.edit_bones)
+        self.assertTrue("test.l" in self.arm.edit_bones)
+
+        bpy.ops.object.mode_set(mode='POSE')
+        self.assertEqual(len(self.arm_ob.pose.bones["test.r"].constraints), 1, "Constraint should have been copied")
+        symm_constraint = self.arm_ob.pose.bones["test.r"].constraints[0]
+        self.assertEqual(symm_constraint.subtarget, "invalid_subtarget")
+
+
 def main():
     global args
     import argparse