Anim: store Armature's active bone collection by name

Instead of storing the active bone collection as `BoneCollection*`, store
its name in DNA. This makes it impossible to point to non-owned bone
collections, makes it easier to handle copy-on-write copies, and should
make it possible to use library overrides on it.

This now also introduces a new requirement that bone collections always
have a name. If the name is set to the empty string, the default name
("Bones") will be used instead.

The `active_collection` pointer is still there for ease of access, but
it's been moved to the armature's `runtime` property.

The library overrides part isn't working yet, though.

source/blender/animrig/ANIM_bone_collections.h

@@ -94,6 +94,9 @@ void ANIM_armature_bonecoll_remove(struct bArmature *armature, struct BoneCollec
  * Set the given bone collection as the active one.
  *
  * Pass `nullptr` to clear the active bone collection.
+ *
+ * The bone collection MUST already be owned by this armature. If it is not,
+ * this function will simply clear the active bone collection.
  */
 void ANIM_armature_bonecoll_active_set(struct bArmature *armature, struct BoneCollection *bcoll);
 

source/blender/animrig/intern/bone_collections.cc

@@ -77,7 +77,9 @@ void ANIM_armature_runtime_refresh(bArmature *armature)
 {
   ANIM_armature_runtime_free(armature);
 
-  ANIM_armature_bonecoll_active_set(armature, armature->active_collection);
+  BoneCollection *active = ANIM_armature_bonecoll_get_by_name(armature,
+                                                              armature->active_collection_name);
+  ANIM_armature_bonecoll_active_set(armature, active);
 
   /* Construct the bone-to-collections mapping. */
   LISTBASE_FOREACH (BoneCollection *, bcoll, &armature->collections) {
@@ -142,7 +144,8 @@ BoneCollection *ANIM_armature_bonecoll_insert_copy_after(bArmature *armature,
 static void armature_bonecoll_active_clear(bArmature *armature)
 {
   armature->runtime.active_collection_index = -1;
-  armature->active_collection = nullptr;
+  armature->runtime.active_collection = nullptr;
+  armature->active_collection_name[0] = '\0';
 }
 
 void ANIM_armature_bonecoll_active_set(bArmature *armature, BoneCollection *bcoll)
@@ -159,8 +162,9 @@ void ANIM_armature_bonecoll_active_set(bArmature *armature, BoneCollection *bcol
     return;
   }
 
+  STRNCPY(armature->active_collection_name, bcoll->name);
   armature->runtime.active_collection_index = index;
-  armature->active_collection = bcoll;
+  armature->runtime.active_collection = bcoll;
 }
 
 void ANIM_armature_bonecoll_active_index_set(bArmature *armature, const int bone_collection_index)
@@ -178,8 +182,9 @@ void ANIM_armature_bonecoll_active_index_set(bArmature *armature, const int bone
     return;
   }
 
+  STRNCPY(armature->active_collection_name, bcoll->name);
   armature->runtime.active_collection_index = bone_collection_index;
-  armature->active_collection = bcoll;
+  armature->runtime.active_collection = bcoll;
 }
 
 bool ANIM_armature_bonecoll_move(bArmature *armature, BoneCollection *bcoll, const int step)
@@ -192,7 +197,7 @@ bool ANIM_armature_bonecoll_move(bArmature *armature, BoneCollection *bcoll, con
     return false;
   }
 
-  if (bcoll == armature->active_collection) {
+  if (bcoll == armature->runtime.active_collection) {
     armature->runtime.active_collection_index = BLI_findindex(&armature->collections, bcoll);
   }
   return true;
@@ -204,7 +209,15 @@ void ANIM_armature_bonecoll_name_set(bArmature *armature, BoneCollection *bcoll,
 
   STRNCPY(old_name, bcoll->name);
 
-  STRNCPY_UTF8(bcoll->name, name);
+  if (name[0] == '\0') {
+    /* Refuse to have nameless collections. The name of the active collection is stored in DNA, and
+     * an empty string means 'no active collection'. */
+    STRNCPY(bcoll->name, bonecoll_default_name);
+  }
+  else {
+    STRNCPY_UTF8(bcoll->name, name);
+  }
+
   bonecoll_ensure_name_unique(armature, bcoll);
 
   BKE_animdata_fix_paths_rename_all(&armature->id, "collections", old_name, bcoll->name);
@@ -420,7 +433,7 @@ void ANIM_bone_set_layer_ebone(EditBone *ebone, const int layer)
 
 void ANIM_armature_bonecoll_assign_active(const bArmature *armature, EditBone *ebone)
 {
-  if (armature->active_collection == nullptr) {
+  if (armature->runtime.active_collection == nullptr) {
     /* No active collection, do not assign to any. */
     printf("ANIM_armature_bonecoll_assign_active(%s, %s): no active collection\n",
            ebone->name,
@@ -428,7 +441,7 @@ void ANIM_armature_bonecoll_assign_active(const bArmature *armature, EditBone *e
     return;
   }
 
-  ANIM_armature_bonecoll_assign_editbone(armature->active_collection, ebone);
+  ANIM_armature_bonecoll_assign_editbone(armature->runtime.active_collection, ebone);
 }
 
 void ANIM_armature_bonecoll_show_from_bone(bArmature *armature, const Bone *bone)

source/blender/animrig/intern/bone_collections_test.cc

@@ -55,6 +55,13 @@ class ANIM_armature_bone_collections : public testing::Test {
     BLI_addtail(&arm.bonebase, &bone2);    /* bone2 is root bone. */
     BLI_addtail(&bone2.childbase, &bone3); /* bone3 has bone2 as parent. */
   }
+
+  void TearDown() override
+  {
+    LISTBASE_FOREACH_BACKWARD_MUTABLE (BoneCollection *, bcoll, &arm.collections) {
+      ANIM_armature_bonecoll_remove(&arm, bcoll);
+    }
+  }
 };
 
 TEST_F(ANIM_armature_bone_collections, armature_owned_collections)
@@ -112,4 +119,59 @@ TEST_F(ANIM_armature_bone_collections, bones_assign_remove)
          "removed";
 }
 
+TEST_F(ANIM_armature_bone_collections, active_set_clear_by_pointer)
+{
+  BoneCollection *bcoll1 = ANIM_armature_bonecoll_new(&arm, "Bones 1");
+  BoneCollection *bcoll2 = ANIM_armature_bonecoll_new(&arm, "Bones 2");
+  BoneCollection *bcoll3 = ANIM_bonecoll_new("Alien Bones");
+
+  ANIM_armature_bonecoll_active_set(&arm, bcoll1);
+  EXPECT_EQ(0, arm.runtime.active_collection_index);
+  EXPECT_EQ(bcoll1, arm.runtime.active_collection);
+  EXPECT_STREQ(bcoll1->name, arm.active_collection_name);
+
+  ANIM_armature_bonecoll_active_set(&arm, nullptr);
+  EXPECT_EQ(-1, arm.runtime.active_collection_index);
+  EXPECT_EQ(nullptr, arm.runtime.active_collection);
+  EXPECT_STREQ("", arm.active_collection_name);
+
+  ANIM_armature_bonecoll_active_set(&arm, bcoll2);
+  EXPECT_EQ(1, arm.runtime.active_collection_index);
+  EXPECT_EQ(bcoll2, arm.runtime.active_collection);
+  EXPECT_STREQ(bcoll2->name, arm.active_collection_name);
+
+  ANIM_armature_bonecoll_active_set(&arm, bcoll3);
+  EXPECT_EQ(-1, arm.runtime.active_collection_index);
+  EXPECT_EQ(nullptr, arm.runtime.active_collection);
+  EXPECT_STREQ("", arm.active_collection_name);
+
+  ANIM_bonecoll_free(bcoll3);
+}
+
+TEST_F(ANIM_armature_bone_collections, active_set_clear_by_index)
+{
+  BoneCollection *bcoll1 = ANIM_armature_bonecoll_new(&arm, "Bones 1");
+  BoneCollection *bcoll2 = ANIM_armature_bonecoll_new(&arm, "Bones 2");
+
+  ANIM_armature_bonecoll_active_index_set(&arm, 0);
+  EXPECT_EQ(0, arm.runtime.active_collection_index);
+  EXPECT_EQ(bcoll1, arm.runtime.active_collection);
+  EXPECT_STREQ(bcoll1->name, arm.active_collection_name);
+
+  ANIM_armature_bonecoll_active_index_set(&arm, -1);
+  EXPECT_EQ(-1, arm.runtime.active_collection_index);
+  EXPECT_EQ(nullptr, arm.runtime.active_collection);
+  EXPECT_STREQ("", arm.active_collection_name);
+
+  ANIM_armature_bonecoll_active_index_set(&arm, 1);
+  EXPECT_EQ(1, arm.runtime.active_collection_index);
+  EXPECT_EQ(bcoll2, arm.runtime.active_collection);
+  EXPECT_STREQ(bcoll2->name, arm.active_collection_name);
+
+  ANIM_armature_bonecoll_active_index_set(&arm, 47);
+  EXPECT_EQ(-1, arm.runtime.active_collection_index);
+  EXPECT_EQ(nullptr, arm.runtime.active_collection);
+  EXPECT_STREQ("", arm.active_collection_name);
+}
+
 }  // namespace blender::animrig::tests

source/blender/editors/armature/bone_collections.cc

@@ -99,7 +99,7 @@ static bool active_bone_collection_poll(bContext *C)
   }
 
   bArmature *armature = static_cast<bArmature *>(ob->data);
-  if (armature->active_collection == nullptr) {
+  if (armature->runtime.active_collection == nullptr) {
     CTX_wm_operator_poll_msg_set(C, "Armature has no active bone collection, select one first");
     return false;
   }
@@ -145,7 +145,7 @@ static int bone_collection_remove_exec(bContext *C, wmOperator * /* op */)
 
   /* The poll function ensures armature->active_collection is not NULL. */
   bArmature *armature = static_cast<bArmature *>(ob->data);
-  ANIM_armature_bonecoll_remove(armature, armature->active_collection);
+  ANIM_armature_bonecoll_remove(armature, armature->runtime.active_collection);
 
   /* notifiers for updates */
   WM_event_add_notifier(C, NC_OBJECT | ND_POSE, ob);
@@ -180,7 +180,8 @@ static int bone_collection_move_exec(bContext *C, wmOperator *op)
   /* Poll function makes sure this is valid. */
   bArmature *armature = static_cast<bArmature *>(ob->data);
 
-  const bool ok = ANIM_armature_bonecoll_move(armature, armature->active_collection, direction);
+  const bool ok = ANIM_armature_bonecoll_move(
+      armature, armature->runtime.active_collection, direction);
   if (!ok) {
     return OPERATOR_CANCELLED;
   }
@@ -233,7 +234,7 @@ static BoneCollection *get_bonecoll_named_or_active(bContext * /*C*/,
   RNA_string_get(op->ptr, "name", bcoll_name);
 
   if (bcoll_name[0] == '\0') {
-    return armature->active_collection;
+    return armature->runtime.active_collection;
   }
 
   BoneCollection *bcoll = ANIM_armature_bonecoll_get_by_name(armature, bcoll_name);

source/blender/editors/armature/editarmature_undo.cc

@@ -18,6 +18,7 @@
 #include "BLI_array_utils.h"
 #include "BLI_listbase.h"
 #include "BLI_map.hh"
+#include "BLI_string.h"
 
 #include "BKE_armature.h"
 #include "BKE_context.h"
@@ -70,7 +71,7 @@ static void remap_ebone_bone_collection_references(
 
 struct UndoArmature {
   EditBone *act_edbone;
-  BoneCollection *active_collection;
+  char active_collection_name[MAX_NAME];
   ListBase /* EditBone */ ebones;
   ListBase /* BoneCollection */ bone_collections;
   size_t undo_size;
@@ -98,7 +99,10 @@ static void undoarm_to_editarm(UndoArmature *uarm, bArmature *arm)
   ANIM_bonecoll_listbase_free(&arm->collections, true);
   auto bcoll_map = ANIM_bonecoll_listbase_copy_no_membership(
       &arm->collections, &uarm->bone_collections, true);
-  arm->active_collection = bcoll_map.lookup_default(uarm->active_collection, nullptr);
+
+  BoneCollection *active_bcoll = ANIM_armature_bonecoll_get_by_name(arm,
+                                                                    uarm->active_collection_name);
+  ANIM_armature_bonecoll_active_set(arm, active_bcoll);
 
   remap_ebone_bone_collection_references(arm->edbo, bcoll_map);
 
@@ -123,7 +127,7 @@ static void *undoarm_from_editarm(UndoArmature *uarm, bArmature *arm)
   /* Copy bone collections. */
   auto bcoll_map = ANIM_bonecoll_listbase_copy_no_membership(
       &uarm->bone_collections, &arm->collections, false);
-  uarm->active_collection = bcoll_map.lookup_default(arm->active_collection, nullptr);
+  STRNCPY(uarm->active_collection_name, arm->active_collection_name);
 
   /* Point the new edit bones at the new collections. */
   remap_ebone_bone_collection_references(&uarm->ebones, bcoll_map);

source/blender/makesdna/DNA_armature_types.h

@@ -147,6 +147,7 @@ typedef struct bArmature_Runtime {
    */
   int active_collection_index;
   uint8_t _pad0[4];
+  struct BoneCollection *active_collection;
 } bArmature_Runtime;
 
 typedef struct bArmature {
@@ -185,8 +186,12 @@ typedef struct bArmature {
 
   /* BoneCollection. */
   ListBase collections;
-  /* Do not directly assign, use `ANIM_armature_bonecoll_active_set` instead. */
-  struct BoneCollection *active_collection;
+
+  /** Do not directly assign, use `ANIM_armature_bonecoll_active_set` instead.
+   * This is stored as a string to make it possible for the library overrides system to understand
+   * when it actually changed (compared to a BoneCollection*, which would change on every load).
+   */
+  char active_collection_name[64]; /* MAX_NAME. */
 
   /** For UI, to show which layers are there. */
   unsigned int layer_used DNA_DEPRECATED;

source/blender/makesrna/intern/rna_armature.cc

@@ -185,6 +185,15 @@ static void rna_Armature_edit_bone_remove(bArmature *arm,
   RNA_POINTER_INVALIDATE(ebone_ptr);
 }
 
+static void rna_BoneCollections_active_set(PointerRNA *ptr,
+                                           PointerRNA value,
+                                           struct ReportList * /*reports*/)
+{
+  bArmature *arm = (bArmature *)ptr->data;
+  BoneCollection *bcoll = (BoneCollection *)value.data;
+  ANIM_armature_bonecoll_active_set(arm, bcoll);
+}
+
 static int rna_BoneCollections_active_index_get(PointerRNA *ptr)
 {
   bArmature *arm = (bArmature *)ptr->data;
@@ -210,6 +219,14 @@ static void rna_BoneCollections_active_index_range(
   *max = max_ii(0, BLI_listbase_count(&arm->collections) - 1);
 }
 
+static void rna_BoneCollections_active_name_set(PointerRNA *ptr, const char *name)
+{
+  bArmature *arm = (bArmature *)ptr->data;
+  BoneCollection *bcoll = ANIM_armature_bonecoll_get_by_name(arm, name);
+  ANIM_armature_bonecoll_active_set(arm, bcoll);
+  WM_main_add_notifier(NC_OBJECT | ND_BONE_COLLECTION, ptr->data);
+}
+
 static void rna_BoneCollections_move(bArmature *arm, ReportList *reports, int from, int to)
 {
   const int count = BLI_listbase_count(&arm->collections);
@@ -1791,13 +1808,16 @@ static void rna_def_armature_collections(BlenderRNA *brna, PropertyRNA *cprop)
 
   prop = RNA_def_property(srna, "active", PROP_POINTER, PROP_NONE);
   RNA_def_property_struct_type(prop, "BoneCollection");
-  RNA_def_property_pointer_sdna(prop, nullptr, "active_collection");
+  RNA_def_property_pointer_sdna(prop, nullptr, "runtime.active_collection");
+  RNA_def_property_override_flag(prop, PROPOVERRIDE_IGNORE);
   RNA_def_property_flag(prop, PROP_EDITABLE);
+  RNA_def_property_pointer_funcs(
+      prop, nullptr, "rna_BoneCollections_active_set", nullptr, nullptr);
   RNA_def_property_ui_text(prop, "Active Collection", "Armature's active bone collection");
 
   prop = RNA_def_property(srna, "active_index", PROP_INT, PROP_NONE);
   RNA_def_property_int_sdna(prop, nullptr, "runtime.active_collection_index");
-  RNA_def_property_flag(prop, PROP_EDITABLE);
+  RNA_def_property_override_flag(prop, PROPOVERRIDE_IGNORE);
   RNA_def_property_ui_text(prop,
                            "Active Collection Index",
                            "The index of the Armature's active bone collection; -1 when there "
@@ -1807,6 +1827,15 @@ static void rna_def_armature_collections(BlenderRNA *brna, PropertyRNA *cprop)
                              "rna_BoneCollections_active_index_set",
                              "rna_BoneCollections_active_index_range");
 
+  prop = RNA_def_property(srna, "active_name", PROP_STRING, PROP_NONE);
+  RNA_def_property_string_sdna(prop, nullptr, "active_collection_name");
+  RNA_def_property_override_flag(prop, PROPOVERRIDE_OVERRIDABLE_LIBRARY);
+  RNA_def_property_ui_text(prop,
+                           "Active Collection Name",
+                           "The name of the Armature's active bone collection; empty when there "
+                           "is no active collection");
+  RNA_def_property_string_funcs(prop, nullptr, nullptr, "rna_BoneCollections_active_name_set");
+
   /* Armature.collections.new(...) */
   func = RNA_def_function(srna, "new", "ANIM_armature_bonecoll_new");
   RNA_def_function_ui_description(func, "Add a new empty bone collection to the armature");