Anim: change RNA Action.id_root to have backwards-compatible behavior

Most of the old Animato properties on an Action (e.g. FCurve list, Channel
Groups) already act as proxies for the data for the first slot in the first
strip of the first layer. (Say that three times fast!) However, this was not yet
the case for `Action.id_root`.

This PR changes `Action.id_root` to act as a proxy for the first Slot's
`target_id_type` property, both for reading and writing.

If the Action has no Slots, then reading always returns 'UNSPECIFIED', and
writing will create a Slot and set its `target_id_type`.

Note that the ability to write to the first Slot's `target_id_type` via
`Action.id_root` conflicts with `target_id_type` supposedly only being writable
when it's still 'UNSPECIFIED' (#133883). Although that's certainly a little
weird, practically speaking this doesn't break anything for now, and is a
temporary kludge to keep `id_root` working until we can remove it in Blender
5.0. `id_root` will be removed entirely in 5.0, resolving this inconsistency.

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

source/blender/animrig/ANIM_action_legacy.hh

@@ -21,6 +21,14 @@ namespace blender::animrig::legacy {
 constexpr const char *DEFAULT_LEGACY_SLOT_NAME = "Legacy Slot";
 constexpr const char *DEFAULT_LEGACY_LAYER_NAME = "Legacy Layer";
 
+/**
+ * Ensure that a Slot exists, for legacy Python API shims that need one.
+ *
+ * \return The first Slot if one already exists, or a newly created "Legacy
+ * Slot" otherwise.
+ */
+Slot &slot_ensure(Action &action);
+
 /**
  * Return the Channelbag for compatibility with the legacy Python API.
  *

source/blender/animrig/intern/action_legacy.cc

@@ -26,6 +26,19 @@ static Strip *first_keyframe_strip(Action &action)
   return nullptr;
 }
 
+Slot &slot_ensure(Action &action)
+{
+  assert_baklava_phase_1_invariants(action);
+
+  if (!action.slots().is_empty()) {
+    return *action.slot(0);
+  }
+
+  Slot &slot = action.slot_add();
+  action.slot_display_name_define(slot, DATA_(DEFAULT_LEGACY_SLOT_NAME));
+  return slot;
+}
+
 Channelbag *channelbag_get(Action &action)
 {
   if (action.slots().is_empty()) {
@@ -44,15 +57,7 @@ Channelbag &channelbag_ensure(Action &action)
 {
   assert_baklava_phase_1_invariants(action);
 
-  /* Ensure a Slot. */
-  Slot *slot;
-  if (action.slots().is_empty()) {
-    slot = &action.slot_add();
-    action.slot_display_name_define(*slot, DATA_(DEFAULT_LEGACY_SLOT_NAME));
-  }
-  else {
-    slot = action.slot(0);
-  }
+  Slot &slot = slot_ensure(action);
 
   /* Ensure a Layer + keyframe Strip.
    *
@@ -68,7 +73,7 @@ Channelbag &channelbag_ensure(Action &action)
   Strip &keystrip = *action.layer(0)->strip(0);
 
   /* Ensure a Channelbag. */
-  return keystrip.data<StripKeyframeData>(action).channelbag_for_slot_ensure(*slot);
+  return keystrip.data<StripKeyframeData>(action).channelbag_for_slot_ensure(slot);
 }
 
 /* Lots of template args to support transparent non-const and const versions. */

source/blender/makesrna/intern/rna_action.cc

@@ -1571,6 +1571,52 @@ static void rna_ActionSlot_target_id_type_set(PointerRNA *ptr, int value)
   action.slot_idtype_define(slot, ID_Type(value));
 }
 
+/* For API backwards compatability with pre-layered-actions (Blender 4.3 and
+ * earlier), we treat `Action.id_root` as a proxy for the `target_id_type`
+ * property (`idtype` in DNA) of the Action's first Slot.
+ *
+ * If the Action has no slots, then we fallback to returning 'unspecified' (0).
+ */
+static int rna_Action_id_root_get(PointerRNA *ptr)
+{
+  animrig::Action &action = reinterpret_cast<bAction *>(ptr->owner_id)->wrap();
+
+  if (action.slots().is_empty()) {
+    return 0;
+  }
+
+  return action.slot(0)->idtype;
+}
+
+/* For API backwards compatability with pre-layered-actions (Blender 4.3 and
+ * earlier), we treat `Action.id_root` as a proxy for the `target_id_type`
+ * property (`idtype` in DNA) of the Action's first Slot.
+ *
+ * If the Action has no slots, then a legacy slot is created and its
+ * `target_id_type` is set. */
+static void rna_Action_id_root_set(PointerRNA *ptr, int value)
+{
+  animrig::Action &action = reinterpret_cast<bAction *>(ptr->owner_id)->wrap();
+
+  animrig::Slot &slot = animrig::legacy::slot_ensure(action);
+  action.slot_idtype_define(slot, ID_Type(value));
+}
+
+static void rna_Action_id_root_update(Main *bmain, Scene *, PointerRNA *ptr)
+{
+  animrig::Action &action = rna_action(ptr);
+
+  if (action.slots().is_empty()) {
+    /* Nothing to do: id_root can't be set without at least one slot, so no
+     * change was possible that would necessitate an update. */
+    return;
+  }
+
+  /* Setting id_root actually sets the target ID type of the first slot, so it's
+   * the resulting changes to the first slot that we need to propagate. */
+  action.slot_identifier_propagate(*bmain, *action.slot(0));
+}
+
 #else
 
 static void rna_def_dopesheet(BlenderRNA *brna)
@@ -2728,7 +2774,11 @@ static void rna_def_action_legacy(BlenderRNA *brna, StructRNA *srna)
   prop = RNA_def_property(srna, "id_root", PROP_ENUM, PROP_NONE);
   RNA_def_property_enum_sdna(prop, nullptr, "idroot");
   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,
+                              "rna_Action_id_root_get",
+                              "rna_Action_id_root_set",
+                              "rna_ActionSlot_target_id_type_itemf");
+  RNA_def_property_update(prop, NC_ANIMATION | ND_ANIMCHAN, "rna_Action_id_root_update");
   RNA_def_property_flag(prop, PROP_ENUM_NO_CONTEXT);
   RNA_def_property_ui_text(prop,
                            "ID Root Type",

tests/python/bl_animation_action.py

@@ -324,7 +324,7 @@ class LegacyAPIOnLayeredActionTest(unittest.TestCase):
     - curve_frame_range
     - fcurves
     - groups
-    - id_root (should always be 0 for layered Actions)
+    - id_root
     - flip_with_pose(object)
     """
 
@@ -450,6 +450,68 @@ class LegacyAPIOnLayeredActionTest(unittest.TestCase):
 
         self.assertEqual([group], channelbag.groups[:])
 
+    def test_id_root_on_layered_action(self) -> None:
+        # When there's at least one slot, action.id_root should simply act as a
+        # proxy for the first slot's target_id_type. This should work for both
+        # reading and writing.
+
+        slot_1 = self.action.slots.new('OBJECT', "Slot 1")
+        slot_2 = self.action.slots.new('CAMERA', "Slot 2")
+        bpy.data.objects['Cube'].animation_data_create()
+        bpy.data.objects['Cube'].animation_data.action = self.action
+        bpy.data.objects['Cube'].animation_data.action_slot = slot_1
+
+        self.assertEqual(self.action.id_root, 'OBJECT')
+        self.assertEqual(self.action.slots[0].target_id_type, 'OBJECT')
+        self.assertEqual(self.action.slots[0].identifier, 'OBSlot 1')
+        self.assertEqual(self.action.slots[1].target_id_type, 'CAMERA')
+        self.assertEqual(self.action.slots[1].identifier, 'CASlot 2')
+        self.assertEqual(bpy.data.objects['Cube'].animation_data.last_slot_identifier, 'OBSlot 1')
+
+        self.action.id_root = 'MATERIAL'
+
+        self.assertEqual(self.action.id_root, 'MATERIAL')
+        self.assertEqual(self.action.slots[0].target_id_type, 'MATERIAL')
+        self.assertEqual(self.action.slots[0].identifier, 'MASlot 1')
+        self.assertEqual(self.action.slots[1].target_id_type, 'CAMERA')
+        self.assertEqual(self.action.slots[1].identifier, 'CASlot 2')
+        self.assertEqual(bpy.data.objects['Cube'].animation_data.last_slot_identifier, 'MASlot 1')
+
+    def test_id_root_on_layered_action_for_identifier_uniqueness(self) -> None:
+        # When setting id_root such that the first slot's identifier would
+        # become a duplicate, the name portion of the identifier should be
+        # automatically renamed to be unique.
+
+        slot_1 = self.action.slots.new('OBJECT', "Foo")
+        slot_2 = self.action.slots.new('CAMERA', "Foo")
+
+        self.assertEqual(self.action.id_root, 'OBJECT')
+        self.assertEqual(self.action.slots[0].target_id_type, 'OBJECT')
+        self.assertEqual(self.action.slots[0].identifier, 'OBFoo')
+        self.assertEqual(self.action.slots[1].target_id_type, 'CAMERA')
+        self.assertEqual(self.action.slots[1].identifier, 'CAFoo')
+
+        self.action.id_root = 'CAMERA'
+
+        self.assertEqual(self.action.id_root, 'CAMERA')
+        self.assertEqual(self.action.slots[0].target_id_type, 'CAMERA')
+        self.assertEqual(self.action.slots[0].identifier, 'CAFoo.001')
+        self.assertEqual(self.action.slots[1].target_id_type, 'CAMERA')
+        self.assertEqual(self.action.slots[1].identifier, 'CAFoo')
+
+    def test_id_root_on_empty_action(self) -> None:
+        # When there are no slots, setting action.id_root should create a legacy
+        # slot and set its target_id_type.
+
+        self.assertEqual(self.action.id_root, 'UNSPECIFIED')
+        self.assertEqual(len(self.action.slots), 0)
+
+        self.action.id_root = 'OBJECT'
+
+        self.assertEqual(self.action.id_root, 'OBJECT')
+        self.assertEqual(len(self.action.slots), 1)
+        self.assertEqual(self.action.slots[0].target_id_type, 'OBJECT')
+
 
 class ChannelbagsTest(unittest.TestCase):
     def setUp(self):