diff --git a/source/blender/animrig/ANIM_action.hh b/source/blender/animrig/ANIM_action.hh index d511a6011df..21ac7954898 100644 --- a/source/blender/animrig/ANIM_action.hh +++ b/source/blender/animrig/ANIM_action.hh @@ -220,6 +220,20 @@ class Action : public ::bAction { */ void slot_display_name_set(Main &bmain, Slot &slot, StringRefNull new_display_name); + /** + * Set the slot's target ID type, updating the identifier prefix to match and + * ensuring that the resulting identifier is unique. + * + * This has to be done on the Action level to ensure each slot has a unique + * identifier within the Action. + * + * \note This does NOT propagate the identifier to the slot's users. That is + * the caller's responsibility. + * + * \see #Action::slot_identifier_propagate + */ + void slot_idtype_define(Slot &slot, ID_Type idtype); + /** * Set the slot identifier, ensure it is unique, and propagate the new identifier to * all data-blocks that use it. diff --git a/source/blender/animrig/intern/action.cc b/source/blender/animrig/intern/action.cc index 0e097f29468..6be6ed53fd6 100644 --- a/source/blender/animrig/intern/action.cc +++ b/source/blender/animrig/intern/action.cc @@ -410,6 +410,13 @@ void Action::slot_display_name_set(Main &bmain, Slot &slot, StringRefNull new_di this->slot_identifier_propagate(bmain, slot); } +void Action::slot_idtype_define(Slot &slot, ID_Type idtype) +{ + slot.idtype = idtype; + slot.identifier_ensure_prefix(); + slot_identifier_ensure_unique(*this, slot); +} + void Action::slot_identifier_set(Main &bmain, Slot &slot, const StringRefNull new_identifier) { /* TODO: maybe this function should only set the 'identifier without prefix' aka the 'display diff --git a/source/blender/makesrna/intern/rna_action.cc b/source/blender/makesrna/intern/rna_action.cc index 854aedcf9ba..db15906e6fa 100644 --- a/source/blender/makesrna/intern/rna_action.cc +++ b/source/blender/makesrna/intern/rna_action.cc @@ -1554,6 +1554,24 @@ static const EnumPropertyItem *rna_ActionSlot_target_id_type_itemf(bContext * /* return items; } +static void rna_ActionSlot_target_id_type_set(PointerRNA *ptr, int value) +{ + animrig::Action &action = reinterpret_cast(ptr->owner_id)->wrap(); + animrig::Slot &slot = reinterpret_cast(ptr->data)->wrap(); + + if (slot.idtype != 0) { + /* Ignore the assignment. */ + printf( + "WARNING: ignoring assignment to target_id_type of Slot '%s' in Action '%s'. A Slot's " + "target_id_type can only be changed when currently 'UNSPECIFIED'.\n", + slot.identifier, + action.id.name); + return; + } + + action.slot_idtype_define(slot, ID_Type(value)); +} + #else static void rna_def_dopesheet(BlenderRNA *brna) @@ -2007,11 +2025,14 @@ static void rna_def_action_slot(BlenderRNA *brna) prop = RNA_def_property(srna, "target_id_type", PROP_ENUM, PROP_NONE); RNA_def_property_enum_sdna(prop, nullptr, "idtype"); RNA_def_property_enum_items(prop, default_ActionSlot_target_id_type_items); - RNA_def_property_enum_funcs(prop, nullptr, nullptr, "rna_ActionSlot_target_id_type_itemf"); + RNA_def_property_enum_funcs( + prop, nullptr, "rna_ActionSlot_target_id_type_set", "rna_ActionSlot_target_id_type_itemf"); + RNA_def_property_update(prop, NC_ANIMATION | ND_ANIMCHAN, "rna_ActionSlot_identifier_update"); RNA_def_property_flag(prop, PROP_ENUM_NO_CONTEXT); - RNA_def_property_clear_flag(prop, PROP_EDITABLE); - RNA_def_property_ui_text( - prop, "Target ID Type", "Type of data-block that this slot is intended to animate"); + RNA_def_property_ui_text(prop, + "Target ID Type", + "Type of data-block that this slot is intended to animate; can be set " + "when 'UNSPECIFIED' but is otherwise read-only"); RNA_def_property_translation_context(prop, BLT_I18NCONTEXT_ID_ID); prop = RNA_def_property(srna, "target_id_type_icon", PROP_INT, PROP_NONE); diff --git a/tests/python/bl_animation_action.py b/tests/python/bl_animation_action.py index fefa9fc1b8d..92b868b4239 100644 --- a/tests/python/bl_animation_action.py +++ b/tests/python/bl_animation_action.py @@ -191,6 +191,60 @@ class ActionSlotAssignmentTest(unittest.TestCase): "After assignment, the ID type should remain UNSPECIFIED when the Action is linked.") self.assertEqual("XXLegacy Slot", slot.identifier) + def test_untyped_slot_target_id_writing(self): + """Test writing to the target id type of an untyped slot.""" + + action = self._load_legacy_action(link=False) + + slot = action.slots[0] + self.assertEqual('UNSPECIFIED', slot.target_id_type) + self.assertEqual("XXLegacy Slot", slot.identifier) + + slot.target_id_type = 'OBJECT' + + self.assertEqual( + 'OBJECT', + slot.target_id_type, + "Should be able to write to target_id_type of a slot when not yet specified.") + self.assertEqual("OBLegacy Slot", slot.identifier) + + slot.target_id_type = 'MATERIAL' + + self.assertEqual( + 'OBJECT', + slot.target_id_type, + "Should NOT be able to write to target_id_type of a slot when already specified.") + self.assertEqual("OBLegacy Slot", slot.identifier) + + def test_untyped_slot_target_id_writing_with_duplicate_identifier(self): + """Test that writing to the target id type a slot appropriately renames + it when that would otherwise cause its identifier to collide with an + already existing slot.""" + + action = self._load_legacy_action(link=False) + + slot = action.slots[0] + + # Create soon-to-collide slot. + other_slot = action.slots.new('OBJECT', "Legacy Slot") + + # Ensure the setup is correct. + self.assertEqual('UNSPECIFIED', slot.target_id_type) + self.assertEqual("XXLegacy Slot", slot.identifier) + self.assertEqual('OBJECT', other_slot.target_id_type) + self.assertEqual("OBLegacy Slot", other_slot.identifier) + + # Assign the colliding target id type. + slot.target_id_type = 'OBJECT' + + self.assertEqual('OBJECT', slot.target_id_type) + self.assertEqual( + "OBLegacy Slot.001", + slot.identifier, + "Should get renamed to not conflict with existing slots.") + self.assertEqual('OBJECT', other_slot.target_id_type) + self.assertEqual("OBLegacy Slot", other_slot.identifier) + @staticmethod def _load_legacy_action(*, link: bool) -> bpy.types.Action: # At the moment of writing, the only way to create an untyped slot is to