diff --git a/source/blender/editors/armature/armature_add.cc b/source/blender/editors/armature/armature_add.cc index e6c96df8ffc..14394fa85ae 100644 --- a/source/blender/editors/armature/armature_add.cc +++ b/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(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); diff --git a/tests/python/bl_rigging_symmetrize.py b/tests/python/bl_rigging_symmetrize.py index 898dcf684da..413ba26cec7 100644 --- a/tests/python/bl_rigging_symmetrize.py +++ b/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