Anim: make RNA Slot.target_id_type writable when not yet specified

When loading old blend files, versioned Actions can end up having a slot with an
'UNSPECIFIED' `target_id_type`. Assigning the slot to an ID will then set the
slot's `target_id_type` to match the type of the ID, but there was previously no
way to directly set it via Python if needed.

This PR changes the writability of `target_id_type` to match the extent of its
mutability when assigning a slot to an IDs. Which is to say, it can be set if
it's 'UNSPECIFIED', but not otherwise.

RNA doesn't have a good way to represent this, so we accomplish this with a
custom set function that simply ignores the write if the slot's `target_id_type`
isn't 'UNSPECIFIED'. This isn't ideal, but is the least-bad solution at the
moment.

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

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.

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

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<bAction *>(ptr->owner_id)->wrap();
+  animrig::Slot &slot = reinterpret_cast<ActionSlot *>(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);

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