From c41f1be5b727c2d4044ccb9409b82fd711f595fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Tue, 30 Apr 2024 16:24:31 +0200
Subject: [PATCH] Anim: RNA, refuse to set AnimData.binding_handle when there
 is no animation

Refuse to set the RNA property `AnimData.animation_binding_handle` when
`AnimData.animation` is nil.

When in a hypothetical future `AnimData.animation` would be set, the
binding handle would be ignored anyway, as it's only meaningful within
the context of a known animation. Refusing to set the binding handle in
cases where it is known to be meaningless may help to find bugs in
Python scripts.

Pull Request: https://projects.blender.org/blender/blender/pulls/121268
---
 .../blender/makesrna/intern/rna_animation.cc  |  8 +++--
 tests/python/bl_animation_id.py               | 33 +++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/source/blender/makesrna/intern/rna_animation.cc b/source/blender/makesrna/intern/rna_animation.cc
index bd18df0d6f6..8840fba1048 100644
--- a/source/blender/makesrna/intern/rna_animation.cc
+++ b/source/blender/makesrna/intern/rna_animation.cc
@@ -241,8 +241,12 @@ static void rna_AnimData_animation_binding_handle_set(
 
   blender::animrig::Animation *anim = blender::animrig::get_animation(animated_id);
   if (!anim) {
-    /* No animation to verify the binding handle is valid. Just set it, it'll be ignored anyway. */
-    adt->binding_handle = new_binding_handle;
+    /* No animation to verify the binding handle is valid. As the binding handle
+     * will be completely ignored when re-assigning an Animation, better to
+     * refuse setting it altogether. This will make bugs in Python code more obvious. */
+    WM_reportf(RPT_ERROR,
+               "Data-block '%s' does not have an animation, cannot set binding handle",
+               animated_id.name + 2);
     return;
   }
 
diff --git a/tests/python/bl_animation_id.py b/tests/python/bl_animation_id.py
index 248cab5a554..dd5399af7bb 100644
--- a/tests/python/bl_animation_id.py
+++ b/tests/python/bl_animation_id.py
@@ -40,6 +40,39 @@ class AnimationIDAssignmentTest(unittest.TestCase):
         bpy.data.objects.remove(camera)
         self.assertEqual(0, anim.users)
 
+    def test_animation_binding_assignment(self):
+        # Create new animation datablock.
+        anim = bpy.data.animations.new('TestAnim')
+        self.assertEqual(0, anim.users)
+
+        # Assign the animation to the cube,
+        cube = bpy.data.objects['Cube']
+        cube_adt = cube.animation_data_create()
+        cube_adt.animation = anim
+        bind_cube = anim.bindings.new(for_id=cube)
+        cube_adt.animation_binding_handle = bind_cube.handle
+        self.assertEqual(cube_adt.animation_binding_handle, bind_cube.handle)
+
+        # Assign the animation to the camera as well.
+        camera = bpy.data.objects['Camera']
+        bind_camera = anim.bindings.new(for_id=camera)
+        camera_adt = camera.animation_data_create()
+        camera_adt.animation = anim
+        self.assertEqual(camera_adt.animation_binding_handle, bind_camera.handle)
+
+        # Unassigning should keep the binding name.
+        cube_adt.animation = None
+        self.assertEqual(cube_adt.animation_binding_name, bind_cube.name)
+
+        # It should not be possible to set the binding handle while the animation is unassigned.
+        bind_extra = anim.bindings.new()
+        cube_adt.animation_binding_handle = bind_extra.handle
+        self.assertEqual(cube_adt.animation_binding_handle, bind_cube.handle)
+
+        # Clearing out the binding handle should be fine though.
+        cube_adt.animation_binding_handle = 0
+        self.assertEqual(cube_adt.animation_binding_handle, 0)
+
 
 class LimitationsTest(unittest.TestCase):
     """Test artificial limitations for the Animation data-block.