source/blender/animrig/ANIM_animdata.hh

@@ -31,7 +31,10 @@ class Animation;
 bAction *id_action_ensure(Main *bmain, ID *id);
 
 /**
- * Get or create the animation datablock for the given ID. Doesn NOT create a binding.
+ * Get the animation datablock for the given ID, creating one if it doesn't
+ * already exist.
+ *
+ * Note: this does NOT create a binding if one doesn't already exist.
  */
 Animation *id_animation_ensure(Main *bmain, ID *id);
 

source/blender/animrig/ANIM_keyframing.hh

@@ -62,7 +62,7 @@ class CombinedKeyingResult {
 
   void add(const SingleKeyingResult result);
 
-  void add_multiple(const SingleKeyingResult result, int count);
+  void add(const SingleKeyingResult result, int count);
 
   /* Add values of the given result to this result. */
   void merge(const CombinedKeyingResult &combined_result);

source/blender/animrig/intern/animdata.cc

@@ -79,13 +79,13 @@ Animation *id_animation_ensure(Main *bmain, ID *id)
 {
   BLI_assert(id != nullptr);
 
-  AnimData *adt = BKE_animdata_from_id(id);
+  AnimData *adt = BKE_animdata_ensure_id(id);
   if (adt == nullptr) {
-    adt = BKE_animdata_ensure_id(id);
-  }
-
-  if (adt == nullptr) {
-    /* If still none (as not allowed to add, or ID doesn't have animdata for some reason) */
+    /* TODO: this printf is copied from the corresponding code in
+     * `id_action_ensure()`.  Is this really the right way to handle reporting
+     * this?  If it's never supposed to happen, an assert would be better.  If
+     * it *is* supposed to happen, then printf doesn't seem like the right way
+     * to report it. */
     printf("ERROR: Couldn't add AnimData (ID = %s)\n", (id) ? (id->name) : "<None>");
     return nullptr;
   }

source/blender/animrig/intern/keyframing.cc

@@ -59,7 +59,7 @@ void CombinedKeyingResult::add(const SingleKeyingResult result)
   result_counter[int(result)]++;
 }
 
-void CombinedKeyingResult::add_multiple(const SingleKeyingResult result, int count)
+void CombinedKeyingResult::add(const SingleKeyingResult result, int count)
 {
   result_counter[int(result)] += count;
 }
@@ -1134,7 +1134,7 @@ static CombinedKeyingResult insert_key_anim(Animation &anim,
     const bool success = anim.assign_id(binding, *id);
     if (!success) {
       /* TODO: count the rna paths properly (e.g. accounting for multi-element properties). */
-      combined_result.add_multiple(SingleKeyingResult::NO_VALID_BINDING, rna_paths.size());
+      combined_result.add(SingleKeyingResult::NO_VALID_BINDING, rna_paths.size());
       return combined_result;
     }
   }
@@ -1149,7 +1149,7 @@ static CombinedKeyingResult insert_key_anim(Animation &anim,
 
   if (layer == nullptr) {
     /* TODO: count the rna paths properly (e.g. accounting for multi-element properties). */
-    combined_result.add_multiple(SingleKeyingResult::NO_VALID_LAYER, rna_paths.size());
+    combined_result.add(SingleKeyingResult::NO_VALID_LAYER, rna_paths.size());
     return combined_result;
   }
 
@@ -1189,14 +1189,13 @@ CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer,
                                     const AnimationEvalContext &anim_eval_context)
 {
   ID *id = rna_pointer->owner_id;
-  AnimData *adt;
   CombinedKeyingResult combined_result;
 
-  /* init animdata if none available yet */
-  adt = BKE_animdata_ensure_id(id);
+  /* Init animdata if none available yet. */
+  AnimData *adt = BKE_animdata_ensure_id(id);
   if (adt == nullptr) {
     /* TODO: count the rna paths properly (e.g. accounting for multi-element properties). */
-    combined_result.add_multiple(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size());
+    combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size());
     return combined_result;
   }
 
@@ -1204,14 +1203,15 @@ CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer,
     /* TODO: Don't hardcode key settings. */
     Animation *anim = id_animation_ensure(bmain, id);
     if (anim == nullptr) {
-      printf(
-          "Could not insert keyframe, as this type does not support animation data (ID = "
-          "%s)",
-          id->name);
-      /* TODO: count the rna paths properly (e.g. accounting for multi-element properties). */
-      combined_result.add_multiple(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size());
+      /* TODO: count the rna paths properly (e.g. accounting for multi-element
+       * properties). */
+      /* TODO: is ID_NOT_ANIMATABLE the right result?  If that were the case,
+       * presumably we would have returned on the failed attempt to get the
+       * AnimData above.  But what *does* cause this case? */
+      combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size());
       return combined_result;
     }
+
     KeyframeSettings key_settings;
     key_settings.keyframe_type = key_type;
     key_settings.handle = HD_AUTO_ANIM;
@@ -1221,10 +1221,13 @@ CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer,
   }
 
   bAction *action = id_action_ensure(bmain, id);
-
   if (action == nullptr) {
-    /* TODO: count the rna paths properly (e.g. accounting for multi-element properties). */
-    combined_result.add_multiple(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size());
+    /* TODO: count the rna paths properly (e.g. accounting for multi-element
+     * properties). */
+    /* TODO: is ID_NOT_ANIMATABLE the right result?  If that were the case,
+     * presumably we would have returned on the failed attempt to get the
+     * AnimData above.  But what *does* cause this case? */
+    combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size());
     return combined_result;
   }