feat: Animation property locking prevents conflicting animations (closes #120)
Add AnimationConflictMode enum with three modes: - REPLACE (default): Complete existing animation and start new one - QUEUE: Wait for existing animation to complete before starting - ERROR: Raise RuntimeError if property is already being animated Changes: - AnimationManager now tracks property locks per (target, property) pair - Animation.start() accepts optional conflict_mode parameter - Queued animations start automatically when property becomes free - Updated type stubs with ConflictMode type alias 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
366ccecb7d
commit
58efffd2fd
6 changed files with 522 additions and 38 deletions
|
|
@ -631,21 +631,122 @@ AnimationManager& AnimationManager::getInstance() {
|
|||
return instance;
|
||||
}
|
||||
|
||||
void AnimationManager::addAnimation(std::shared_ptr<Animation> animation) {
|
||||
if (animation && animation->hasValidTarget()) {
|
||||
if (isUpdating) {
|
||||
// Defer adding during update to avoid iterator invalidation
|
||||
pendingAnimations.push_back(animation);
|
||||
void* AnimationManager::getAnimationTarget(const std::shared_ptr<Animation>& anim) const {
|
||||
return anim ? anim->getTargetPtr() : nullptr;
|
||||
}
|
||||
|
||||
bool AnimationManager::isPropertyAnimating(void* target, const std::string& property) const {
|
||||
if (!target) return false;
|
||||
PropertyKey key{target, property};
|
||||
auto it = propertyLocks.find(key);
|
||||
if (it == propertyLocks.end()) return false;
|
||||
// Check if the animation is still valid
|
||||
return !it->second.expired();
|
||||
}
|
||||
|
||||
void AnimationManager::cleanupPropertyLocks() {
|
||||
// Remove expired locks
|
||||
for (auto it = propertyLocks.begin(); it != propertyLocks.end(); ) {
|
||||
if (it->second.expired()) {
|
||||
it = propertyLocks.erase(it);
|
||||
} else {
|
||||
activeAnimations.push_back(animation);
|
||||
++it;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void AnimationManager::processQueue() {
|
||||
// Try to start queued animations whose properties are now free
|
||||
for (auto it = animationQueue.begin(); it != animationQueue.end(); ) {
|
||||
const auto& key = it->first;
|
||||
auto& anim = it->second;
|
||||
|
||||
// Check if property is now free
|
||||
auto lockIt = propertyLocks.find(key);
|
||||
bool propertyFree = (lockIt == propertyLocks.end()) || lockIt->second.expired();
|
||||
|
||||
if (propertyFree && anim && anim->hasValidTarget()) {
|
||||
// Property is free, start the animation
|
||||
propertyLocks[key] = anim;
|
||||
activeAnimations.push_back(anim);
|
||||
it = animationQueue.erase(it);
|
||||
} else if (!anim || !anim->hasValidTarget()) {
|
||||
// Animation target was destroyed, remove from queue
|
||||
it = animationQueue.erase(it);
|
||||
} else {
|
||||
++it;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void AnimationManager::addAnimation(std::shared_ptr<Animation> animation,
|
||||
AnimationConflictMode conflict_mode) {
|
||||
if (!animation || !animation->hasValidTarget()) {
|
||||
return;
|
||||
}
|
||||
|
||||
void* target = getAnimationTarget(animation);
|
||||
std::string property = animation->getTargetProperty();
|
||||
PropertyKey key{target, property};
|
||||
|
||||
// Check for existing animation on this property (#120)
|
||||
auto existingIt = propertyLocks.find(key);
|
||||
bool hasExisting = (existingIt != propertyLocks.end()) && !existingIt->second.expired();
|
||||
|
||||
if (hasExisting) {
|
||||
auto existingAnim = existingIt->second.lock();
|
||||
|
||||
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()
|
||||
);
|
||||
}
|
||||
// Fall through to add the new animation
|
||||
break;
|
||||
|
||||
case AnimationConflictMode::QUEUE:
|
||||
// Add to queue - will start when existing completes
|
||||
if (isUpdating) {
|
||||
// Also defer queue additions during update
|
||||
pendingAnimations.push_back(animation);
|
||||
} else {
|
||||
animationQueue.emplace_back(key, animation);
|
||||
}
|
||||
return; // Don't add to active animations yet
|
||||
|
||||
case AnimationConflictMode::ERROR:
|
||||
// Raise Python exception
|
||||
PyGILState_STATE gstate = PyGILState_Ensure();
|
||||
PyErr_Format(PyExc_RuntimeError,
|
||||
"Animation conflict: property '%s' is already being animated on this target. "
|
||||
"Use conflict_mode='replace' to override or 'queue' to wait.",
|
||||
property.c_str());
|
||||
PyGILState_Release(gstate);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Register property lock and add animation
|
||||
propertyLocks[key] = animation;
|
||||
|
||||
if (isUpdating) {
|
||||
// Defer adding during update to avoid iterator invalidation
|
||||
pendingAnimations.push_back(animation);
|
||||
} else {
|
||||
activeAnimations.push_back(animation);
|
||||
}
|
||||
}
|
||||
|
||||
void AnimationManager::update(float deltaTime) {
|
||||
// Set flag to defer new animations
|
||||
isUpdating = true;
|
||||
|
||||
|
||||
// Remove completed or invalid animations
|
||||
activeAnimations.erase(
|
||||
std::remove_if(activeAnimations.begin(), activeAnimations.end(),
|
||||
|
|
@ -654,15 +755,37 @@ void AnimationManager::update(float deltaTime) {
|
|||
}),
|
||||
activeAnimations.end()
|
||||
);
|
||||
|
||||
|
||||
// Clear update flag
|
||||
isUpdating = false;
|
||||
|
||||
|
||||
// Clean up expired property locks (#120)
|
||||
cleanupPropertyLocks();
|
||||
|
||||
// Process queued animations - start any that are now unblocked (#120)
|
||||
processQueue();
|
||||
|
||||
// Add any animations that were created during update
|
||||
if (!pendingAnimations.empty()) {
|
||||
activeAnimations.insert(activeAnimations.end(),
|
||||
pendingAnimations.begin(),
|
||||
pendingAnimations.end());
|
||||
// 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
|
||||
auto lockIt = propertyLocks.find(key);
|
||||
if (lockIt == propertyLocks.end() || lockIt->second.expired()) {
|
||||
propertyLocks[key] = anim;
|
||||
activeAnimations.push_back(anim);
|
||||
} else {
|
||||
// Property still locked, re-queue
|
||||
animationQueue.emplace_back(key, anim);
|
||||
}
|
||||
}
|
||||
}
|
||||
pendingAnimations.clear();
|
||||
}
|
||||
}
|
||||
|
|
@ -678,4 +801,7 @@ void AnimationManager::clear(bool completeAnimations) {
|
|||
}
|
||||
}
|
||||
activeAnimations.clear();
|
||||
pendingAnimations.clear();
|
||||
animationQueue.clear();
|
||||
propertyLocks.clear();
|
||||
}
|
||||
|
|
@ -12,6 +12,15 @@
|
|||
class UIDrawable;
|
||||
class UIEntity;
|
||||
|
||||
/**
|
||||
* ConflictMode - How to handle multiple animations on the same property (#120)
|
||||
*/
|
||||
enum class AnimationConflictMode {
|
||||
REPLACE, // Stop/complete existing animation, start new one (default)
|
||||
QUEUE, // Queue new animation to run after existing one completes
|
||||
ERROR // Raise an error if property is already being animated
|
||||
};
|
||||
|
||||
// Forward declare namespace
|
||||
namespace EasingFunctions {
|
||||
float linear(float t);
|
||||
|
|
@ -71,6 +80,13 @@ public:
|
|||
float getElapsed() const { return elapsed; }
|
||||
bool isComplete() const { return elapsed >= duration; }
|
||||
bool isDelta() const { return delta; }
|
||||
|
||||
// Get raw target pointer for property locking (#120)
|
||||
void* getTargetPtr() const {
|
||||
if (auto sp = targetWeak.lock()) return sp.get();
|
||||
if (auto sp = entityTargetWeak.lock()) return sp.get();
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
private:
|
||||
std::string targetProperty; // Property name to animate (e.g., "x", "color.r", "sprite_number")
|
||||
|
|
@ -157,19 +173,64 @@ namespace EasingFunctions {
|
|||
class AnimationManager {
|
||||
public:
|
||||
static AnimationManager& getInstance();
|
||||
|
||||
|
||||
// Add an animation to be managed
|
||||
void addAnimation(std::shared_ptr<Animation> animation);
|
||||
|
||||
// conflict_mode determines behavior when property is already animated (#120)
|
||||
void addAnimation(std::shared_ptr<Animation> animation,
|
||||
AnimationConflictMode conflict_mode = AnimationConflictMode::REPLACE);
|
||||
|
||||
// Update all animations
|
||||
void update(float deltaTime);
|
||||
|
||||
|
||||
// Clear all animations (optionally completing them first)
|
||||
void clear(bool completeAnimations = false);
|
||||
|
||||
|
||||
// Get/set default conflict mode
|
||||
AnimationConflictMode getDefaultConflictMode() const { return defaultConflictMode; }
|
||||
void setDefaultConflictMode(AnimationConflictMode mode) { defaultConflictMode = mode; }
|
||||
|
||||
// Check if a property is currently being animated on a target
|
||||
bool isPropertyAnimating(void* target, const std::string& property) const;
|
||||
|
||||
// Get active animation count (for debugging/testing)
|
||||
size_t getActiveAnimationCount() const { return activeAnimations.size(); }
|
||||
|
||||
private:
|
||||
AnimationManager() = default;
|
||||
std::vector<std::shared_ptr<Animation>> activeAnimations;
|
||||
std::vector<std::shared_ptr<Animation>> pendingAnimations; // Animations to add after update
|
||||
bool isUpdating = false; // Flag to track if we're in update loop
|
||||
AnimationConflictMode defaultConflictMode = AnimationConflictMode::REPLACE;
|
||||
|
||||
// Property lock tracking for conflict detection (#120)
|
||||
// Key: (target_ptr, property_name) -> weak reference to active animation
|
||||
struct PropertyKey {
|
||||
void* target;
|
||||
std::string property;
|
||||
|
||||
bool operator==(const PropertyKey& other) const {
|
||||
return target == other.target && property == other.property;
|
||||
}
|
||||
};
|
||||
|
||||
struct PropertyKeyHash {
|
||||
size_t operator()(const PropertyKey& key) const {
|
||||
return std::hash<void*>()(key.target) ^
|
||||
(std::hash<std::string>()(key.property) << 1);
|
||||
}
|
||||
};
|
||||
|
||||
std::unordered_map<PropertyKey, std::weak_ptr<Animation>, PropertyKeyHash> propertyLocks;
|
||||
|
||||
// Queued animations waiting for property to become available
|
||||
std::vector<std::pair<PropertyKey, std::shared_ptr<Animation>>> animationQueue;
|
||||
|
||||
// Helper to get target pointer from animation
|
||||
void* getAnimationTarget(const std::shared_ptr<Animation>& anim) const;
|
||||
|
||||
// Clean up expired property locks
|
||||
void cleanupPropertyLocks();
|
||||
|
||||
// Process queued animations
|
||||
void processQueue();
|
||||
};
|
||||
|
|
@ -8,7 +8,7 @@
|
|||
#include "UIGrid.h"
|
||||
#include "UIEntity.h"
|
||||
#include "UI.h" // For the PyTypeObject definitions
|
||||
#include <cstring>
|
||||
#include <cstring> // For strcmp in parseConflictMode
|
||||
|
||||
PyObject* PyAnimation::create(PyTypeObject* type, PyObject* args, PyObject* kwds) {
|
||||
PyAnimationObject* self = (PyAnimationObject*)type->tp_alloc(type, 0);
|
||||
|
|
@ -133,27 +133,53 @@ PyObject* PyAnimation::get_is_delta(PyAnimationObject* self, void* closure) {
|
|||
return PyBool_FromLong(self->data->isDelta());
|
||||
}
|
||||
|
||||
PyObject* PyAnimation::start(PyAnimationObject* self, PyObject* args) {
|
||||
// Helper to convert Python string to AnimationConflictMode
|
||||
static bool parseConflictMode(const char* mode_str, AnimationConflictMode& mode) {
|
||||
if (!mode_str || strcmp(mode_str, "replace") == 0) {
|
||||
mode = AnimationConflictMode::REPLACE;
|
||||
} else if (strcmp(mode_str, "queue") == 0) {
|
||||
mode = AnimationConflictMode::QUEUE;
|
||||
} else if (strcmp(mode_str, "error") == 0) {
|
||||
mode = AnimationConflictMode::ERROR;
|
||||
} else {
|
||||
PyErr_Format(PyExc_ValueError,
|
||||
"Invalid conflict_mode '%s'. Must be 'replace', 'queue', or 'error'.", mode_str);
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
PyObject* PyAnimation::start(PyAnimationObject* self, PyObject* args, PyObject* kwds) {
|
||||
static const char* kwlist[] = {"target", "conflict_mode", nullptr};
|
||||
PyObject* target_obj;
|
||||
if (!PyArg_ParseTuple(args, "O", &target_obj)) {
|
||||
const char* conflict_mode_str = nullptr;
|
||||
|
||||
if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|s", const_cast<char**>(kwlist),
|
||||
&target_obj, &conflict_mode_str)) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
||||
// Parse conflict mode
|
||||
AnimationConflictMode conflict_mode;
|
||||
if (!parseConflictMode(conflict_mode_str, conflict_mode)) {
|
||||
return NULL; // Error already set
|
||||
}
|
||||
|
||||
// Get type objects from the module to ensure they're initialized
|
||||
PyObject* frame_type = PyObject_GetAttrString(McRFPy_API::mcrf_module, "Frame");
|
||||
PyObject* caption_type = PyObject_GetAttrString(McRFPy_API::mcrf_module, "Caption");
|
||||
PyObject* sprite_type = PyObject_GetAttrString(McRFPy_API::mcrf_module, "Sprite");
|
||||
PyObject* grid_type = PyObject_GetAttrString(McRFPy_API::mcrf_module, "Grid");
|
||||
PyObject* entity_type = PyObject_GetAttrString(McRFPy_API::mcrf_module, "Entity");
|
||||
|
||||
|
||||
bool handled = false;
|
||||
|
||||
|
||||
// Use PyObject_IsInstance to support inheritance
|
||||
if (frame_type && PyObject_IsInstance(target_obj, frame_type)) {
|
||||
PyUIFrameObject* frame = (PyUIFrameObject*)target_obj;
|
||||
if (frame->data) {
|
||||
self->data->start(frame->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data, conflict_mode);
|
||||
handled = true;
|
||||
}
|
||||
}
|
||||
|
|
@ -161,7 +187,7 @@ PyObject* PyAnimation::start(PyAnimationObject* self, PyObject* args) {
|
|||
PyUICaptionObject* caption = (PyUICaptionObject*)target_obj;
|
||||
if (caption->data) {
|
||||
self->data->start(caption->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data, conflict_mode);
|
||||
handled = true;
|
||||
}
|
||||
}
|
||||
|
|
@ -169,7 +195,7 @@ PyObject* PyAnimation::start(PyAnimationObject* self, PyObject* args) {
|
|||
PyUISpriteObject* sprite = (PyUISpriteObject*)target_obj;
|
||||
if (sprite->data) {
|
||||
self->data->start(sprite->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data, conflict_mode);
|
||||
handled = true;
|
||||
}
|
||||
}
|
||||
|
|
@ -177,7 +203,7 @@ PyObject* PyAnimation::start(PyAnimationObject* self, PyObject* args) {
|
|||
PyUIGridObject* grid = (PyUIGridObject*)target_obj;
|
||||
if (grid->data) {
|
||||
self->data->start(grid->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data, conflict_mode);
|
||||
handled = true;
|
||||
}
|
||||
}
|
||||
|
|
@ -186,23 +212,28 @@ PyObject* PyAnimation::start(PyAnimationObject* self, PyObject* args) {
|
|||
PyUIEntityObject* entity = (PyUIEntityObject*)target_obj;
|
||||
if (entity->data) {
|
||||
self->data->startEntity(entity->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data);
|
||||
AnimationManager::getInstance().addAnimation(self->data, conflict_mode);
|
||||
handled = true;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// Clean up references
|
||||
Py_XDECREF(frame_type);
|
||||
Py_XDECREF(caption_type);
|
||||
Py_XDECREF(sprite_type);
|
||||
Py_XDECREF(grid_type);
|
||||
Py_XDECREF(entity_type);
|
||||
|
||||
|
||||
if (!handled) {
|
||||
PyErr_SetString(PyExc_TypeError, "Target must be a Frame, Caption, Sprite, Grid, or Entity (or a subclass of these)");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
||||
// Check if an error was set (e.g., from ERROR conflict mode)
|
||||
if (PyErr_Occurred()) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
Py_RETURN_NONE;
|
||||
}
|
||||
|
||||
|
|
@ -276,14 +307,19 @@ PyGetSetDef PyAnimation::getsetters[] = {
|
|||
};
|
||||
|
||||
PyMethodDef PyAnimation::methods[] = {
|
||||
{"start", (PyCFunction)start, METH_VARARGS,
|
||||
{"start", (PyCFunction)start, METH_VARARGS | METH_KEYWORDS,
|
||||
MCRF_METHOD(Animation, start,
|
||||
MCRF_SIG("(target: UIDrawable)", "None"),
|
||||
MCRF_SIG("(target: UIDrawable, conflict_mode: str = 'replace')", "None"),
|
||||
MCRF_DESC("Start the animation on a target UI element."),
|
||||
MCRF_ARGS_START
|
||||
MCRF_ARG("target", "The UI element to animate (Frame, Caption, Sprite, Grid, or Entity)")
|
||||
MCRF_ARG("conflict_mode", "How to handle conflicts if property is already animating: "
|
||||
"'replace' (default) - complete existing animation and start new one; "
|
||||
"'queue' - wait for existing animation to complete; "
|
||||
"'error' - raise RuntimeError if property is busy")
|
||||
MCRF_RETURNS("None")
|
||||
MCRF_NOTE("The animation will automatically stop if the target is destroyed. Call AnimationManager.update(delta_time) each frame to progress animations.")
|
||||
MCRF_RAISES("RuntimeError", "When conflict_mode='error' and property is already animating")
|
||||
MCRF_NOTE("The animation will automatically stop if the target is destroyed.")
|
||||
)},
|
||||
{"update", (PyCFunction)update, METH_VARARGS,
|
||||
MCRF_METHOD(Animation, update,
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@ public:
|
|||
static PyObject* get_is_delta(PyAnimationObject* self, void* closure);
|
||||
|
||||
// Methods
|
||||
static PyObject* start(PyAnimationObject* self, PyObject* args);
|
||||
static PyObject* start(PyAnimationObject* self, PyObject* args, PyObject* kwds);
|
||||
static PyObject* update(PyAnimationObject* self, PyObject* args);
|
||||
static PyObject* get_current_value(PyAnimationObject* self, PyObject* args);
|
||||
static PyObject* complete(PyAnimationObject* self, PyObject* args);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue