fix: animations modifying animations during callback is now safe

src/Animation.cpp

@@ -977,14 +977,21 @@ void AnimationManager::addAnimation(std::shared_ptr<Animation> animation,
 
         switch (conflict_mode) {
             case AnimationConflictMode::REPLACE:
-                // Complete the existing animation and replace it
                 if (existingAnim) {
-                    existingAnim->complete();
-                    // Remove from active animations
-                    activeAnimations.erase(
-                        std::remove(activeAnimations.begin(), activeAnimations.end(), existingAnim),
-                        activeAnimations.end()
-                    );
+                    if (isUpdating) {
+                        // During update, just stop the animation without completing
+                        // to avoid recursive callback issues and iterator invalidation.
+                        // The update loop will clean up stopped animations.
+                        existingAnim->stop();
+                    } else {
+                        // Outside update loop, complete the animation (jump to final value)
+                        // and remove it safely
+                        existingAnim->complete();
+                        activeAnimations.erase(
+                            std::remove(activeAnimations.begin(), activeAnimations.end(), existingAnim),
+                            activeAnimations.end()
+                        );
+                    }
                 }
                 // Fall through to add the new animation
                 break;
@@ -1046,21 +1053,26 @@ void AnimationManager::update(float deltaTime) {
 
     // Add any animations that were created during update
     if (!pendingAnimations.empty()) {
-        // Re-add pending animations through addAnimation to handle conflicts properly
         for (auto& anim : pendingAnimations) {
             if (anim && anim->hasValidTarget()) {
-                // Check if this was a queued animation or a new one
                 void* target = getAnimationTarget(anim);
                 std::string property = anim->getTargetProperty();
                 PropertyKey key{target, property};
 
-                // If not already locked, add it
+                // Check if this animation is already the property lock holder
+                // (this happens when addAnimation was called during update)
                 auto lockIt = propertyLocks.find(key);
-                if (lockIt == propertyLocks.end() || lockIt->second.expired()) {
+                bool isLockHolder = (lockIt != propertyLocks.end()) &&
+                                   (lockIt->second.lock() == anim);
+                bool propertyFree = (lockIt == propertyLocks.end()) ||
+                                   lockIt->second.expired();
+
+                if (isLockHolder || propertyFree) {
+                    // This animation owns the lock or property is free - add it
                     propertyLocks[key] = anim;
                     activeAnimations.push_back(anim);
                 } else {
-                    // Property still locked, re-queue
+                    // Property still locked by another animation, re-queue
                     animationQueue.emplace_back(key, anim);
                 }
             }

tests/regression/recursive_animation_callback_segfault.py

@@ -0,0 +1,46 @@
+# Regression test: Recursive animation callbacks
+# Issue: Animation callbacks that started new animations caused segfault in AnimationManager::update
+# Fixed by: Deferring animation removal during update loop iteration
+#
+# This test verifies that:
+# 1. Animation callbacks can start new animations on the same property (REPLACE mode)
+# 2. No segfault occurs from iterator invalidation
+# 3. The animation chain completes properly
+
+import mcrfpy
+import sys
+
+scene = mcrfpy.Scene("test")
+mcrfpy.current_scene = scene
+
+frame = mcrfpy.Frame(pos=(100, 100), size=(50, 50), fill_color=mcrfpy.Color(255, 0, 0))
+scene.children.append(frame)
+
+callback_count = 0
+MAX_CALLBACKS = 10
+
+def recursive_callback(target, prop, value):
+    """Animation callback that starts another animation on the same property"""
+    global callback_count
+    callback_count += 1
+
+    if callback_count >= MAX_CALLBACKS:
+        print(f"PASS - {callback_count} recursive animation callbacks completed without segfault")
+        sys.exit(0)
+
+    # Chain another animation - this used to cause segfault due to iterator invalidation
+    target.animate("x", 100 + (callback_count * 20), 0.1, mcrfpy.Easing.LINEAR, callback=recursive_callback)
+
+# Start the chain
+frame.animate("x", 200, 0.1, mcrfpy.Easing.LINEAR, callback=recursive_callback)
+
+def timeout_check(timer, runtime):
+    """Safety timeout"""
+    if callback_count >= MAX_CALLBACKS:
+        print(f"PASS - {callback_count} callbacks completed")
+        sys.exit(0)
+    else:
+        print(f"FAIL - only {callback_count}/{MAX_CALLBACKS} callbacks executed")
+        sys.exit(1)
+
+safety_timer = mcrfpy.Timer("safety", timeout_check, 5000, once=True)