diff --git a/include/nodec_animation/animated_component_writer.hpp b/include/nodec_animation/animated_component_writer.hpp index 8ccf8e0..9f8fdd8 100644 --- a/include/nodec_animation/animated_component_writer.hpp +++ b/include/nodec_animation/animated_component_writer.hpp @@ -23,13 +23,55 @@ class AnimatedComponentWriter { std::unordered_map properties; }; + /** + * @brief Dummy OutputArchive paired with PropertyWriter. + * + * cereal's trait system requires input archives to have a corresponding output archive + * for load_minimal/save_minimal patterns (used by enums). This dummy archive satisfies + * that requirement without implementing actual serialization. + * + * The CEREAL_SETUP_ARCHIVE_TRAITS macro below links PropertyWriter to this archive, + * preventing "Could not find an associated output archive" errors. + */ + class DummyOutputArchive : public cereal::OutputArchive { + public: + DummyOutputArchive() : OutputArchive(this) {} + + template + void save_value(const T &) {} + }; + + /** + * @brief Custom cereal InputArchive for applying animation curve values to component properties. + * + * Unlike standard cereal archives (JSONInputArchive, BinaryInputArchive, etc.), + * this class can be reused via reset() because it does NOT use cereal's internal state: + * - itsPolymorphicTypeMap: Not used (no polymorphic types deserialized) + * - itsSharedPointerMap: Not used (shared_ptr is ignored, see CEREAL_LOAD_FUNCTION_NAME overloads) + * - itsVersionedTypes: Not used (no versioning) + * + * Standard cereal archives accumulate these states during deserialization, + * causing ID collisions and invalid references on reuse. + * PropertyWriter only maintains traversal state (name_stack_, current_property_name_) + * which is cleared in reset(). + */ class PropertyWriter : public cereal::InputArchive { public: - PropertyWriter(const nodec_animation::resources::AnimatedComponent &source, - float time, - AnimatedComponentWriter &owner, ComponentAnimationState *state, InternalTag) + // Constructor for reusable instance (call reset() before each use) + PropertyWriter(AnimatedComponentWriter &owner, InternalTag) : InputArchive(this), - source_(source), time_(time), owner_(owner), state_(state) {} + source_(nullptr), time_(0), owner_(owner), state_(nullptr) {} + + // Reset state for reuse with new parameters + void reset(const nodec_animation::resources::AnimatedComponent &source, + float time, + ComponentAnimationState *state) { + source_ = &source; + time_ = time; + state_ = state; + name_stack_.clear(); + current_property_name_.clear(); + } void load_value(std::string &) { // Ignore. @@ -37,8 +79,10 @@ class AnimatedComponentWriter { template::value> = cereal::traits::sfinae> void load_value(T &value) { - auto iter = source_.properties.find(current_property_name_); - if (iter == source_.properties.end()) return; + if (!source_) return; + + auto iter = source_->properties.find(current_property_name_); + if (iter == source_->properties.end()) return; auto *property_animation_state = [&]() -> PropertyAnimationState * { if (!state_) return nullptr; @@ -81,15 +125,16 @@ class AnimatedComponentWriter { } private: - const nodec_animation::resources::AnimatedComponent &source_; - const float time_; + const nodec_animation::resources::AnimatedComponent *source_; + float time_; AnimatedComponentWriter &owner_; std::vector name_stack_; std::string current_property_name_; ComponentAnimationState *state_; }; - AnimatedComponentWriter() { + AnimatedComponentWriter() + : writer_(*this, InternalTag{}) { } /** @@ -102,10 +147,12 @@ class AnimatedComponentWriter { void write(const nodec_animation::resources::AnimatedComponent &source, float time, Component &dest, ComponentAnimationState *state = nullptr) { - PropertyWriter writer(source, time, *this, state, InternalTag{}); - - writer(dest); + writer_.reset(source, time, state); + writer_(dest); } + +private: + PropertyWriter writer_; }; // --- PropertyWriter --- @@ -156,8 +203,30 @@ inline void CEREAL_LOAD_FUNCTION_NAME(AnimatedComponentWriter::PropertyWriter &, // Ignore. } +// --- DummyOutputArchive minimal save functions (for trait lookup only) --- + +// save_minimal for enums on DummyOutputArchive (required for cereal trait resolution) +template::value> = cereal::traits::sfinae> +inline typename std::underlying_type::type +CEREAL_SAVE_MINIMAL_FUNCTION_NAME(const AnimatedComponentWriter::DummyOutputArchive &, const T &e) { + return static_cast::type>(e); +} + +// load_minimal for enums on PropertyWriter (ignores the value - enums not animatable) +template::value> = cereal::traits::sfinae> +inline void CEREAL_LOAD_MINIMAL_FUNCTION_NAME(const AnimatedComponentWriter::PropertyWriter &, + T &, + const typename std::underlying_type::type &) { + // Ignore - enums are not animatable +} + } // namespace nodec_animation CEREAL_REGISTER_ARCHIVE(nodec_animation::AnimatedComponentWriter::PropertyWriter) +// Link PropertyWriter (input) with DummyOutputArchive (output) for load_minimal/save_minimal trait lookup +// NOTE: DummyOutputArchive is NOT registered with CEREAL_REGISTER_ARCHIVE to avoid polymorphic binding issues +CEREAL_SETUP_ARCHIVE_TRAITS(nodec_animation::AnimatedComponentWriter::PropertyWriter, + nodec_animation::AnimatedComponentWriter::DummyOutputArchive) + #endif \ No newline at end of file diff --git a/include/nodec_animation/animation_curve.hpp b/include/nodec_animation/animation_curve.hpp index 5dca60c..a802625 100644 --- a/include/nodec_animation/animation_curve.hpp +++ b/include/nodec_animation/animation_curve.hpp @@ -101,6 +101,9 @@ class AnimationCurve { assert(0 <= hint && hint < keyframes_.size() - 1); if (current.time < keyframes_[hint].time) { + if (hint == 0) { + return keyframes_.begin(); + } if (keyframes_[hint - 1].time <= current.time) { return keyframes_.begin() + hint; } diff --git a/include/nodec_animation/component_registry.hpp b/include/nodec_animation/component_registry.hpp index 0577350..c65c93f 100644 --- a/include/nodec_animation/component_registry.hpp +++ b/include/nodec_animation/component_registry.hpp @@ -35,9 +35,11 @@ class ComponentRegistry { auto *component = registry.try_get_component(entity); if (!component) return; - AnimatedComponentWriter writer; - writer.write(source, time, *component, state); + writer_.write(source, time, *component, state); } + + private: + mutable AnimatedComponentWriter writer_; }; public: diff --git a/include/nodec_animation/components/impl/animated_data.hpp b/include/nodec_animation/components/impl/animated_data.hpp index 33aab27..edbd247 100644 --- a/include/nodec_animation/components/impl/animated_data.hpp +++ b/include/nodec_animation/components/impl/animated_data.hpp @@ -13,16 +13,28 @@ struct AnimatedData { const resources::AnimatedEntity *animated_entity) { clip_ = clip; animated_entity_ = animated_entity; + cached_clip_version_ = clip ? clip->version() : 0; } std::shared_ptr clip() const noexcept { return clip_; } + /// Returns the cached animated entity pointer, or nullptr if invalid. + /// The pointer becomes invalid when the clip's version has changed + /// (e.g., when the clip is updated via the editor API). const resources::AnimatedEntity *animated_entity() const noexcept { + if (!is_valid()) { + return nullptr; + } return animated_entity_; } + /// Check if the cached animated entity pointer is still valid. + bool is_valid() const noexcept { + return clip_ && clip_->version() == cached_clip_version_ && animated_entity_ != nullptr; + } + std::unordered_map component_animation_states; @@ -31,6 +43,7 @@ struct AnimatedData { private: std::shared_ptr clip_; const resources::AnimatedEntity *animated_entity_{nullptr}; + std::uint32_t cached_clip_version_{0}; }; } // namespace impl } // namespace components diff --git a/include/nodec_animation/resources/animation_clip.hpp b/include/nodec_animation/resources/animation_clip.hpp index 44287af..720687c 100644 --- a/include/nodec_animation/resources/animation_clip.hpp +++ b/include/nodec_animation/resources/animation_clip.hpp @@ -26,6 +26,10 @@ struct AnimatedComponent { }; struct AnimatedEntity { + // Using std::map instead of std::unordered_map for pointer stability. + // std::map guarantees that pointers/references to elements remain valid + // after insert/erase operations (node-based container), which allows + // AnimatedData to safely cache pointers to AnimatedEntity nodes. std::map children; std::unordered_map components; }; @@ -66,6 +70,7 @@ class AnimationClip { if (property_name.empty()) return; entity.components[nodec::type_id()].properties[property_name].curve = curve; + ++version_; } const AnimatedEntity &root_entity() const { @@ -74,10 +79,16 @@ class AnimationClip { void set_root_entity(AnimatedEntity &&entity) { root_entity_ = std::move(entity); + ++version_; + } + + std::uint32_t version() const noexcept { + return version_; } private: AnimatedEntity root_entity_; + std::uint32_t version_{0}; }; } // namespace resources diff --git a/include/nodec_animation/systems/animator_system.hpp b/include/nodec_animation/systems/animator_system.hpp index 5775915..4cce034 100644 --- a/include/nodec_animation/systems/animator_system.hpp +++ b/include/nodec_animation/systems/animator_system.hpp @@ -71,7 +71,14 @@ class AnimatorSystem { } registry.view().each([&](SceneEntity entity, AnimatedData &animated_data) { - for (auto &component : animated_data.animated_entity()->components) { + // Skip if the cached pointer is invalid (e.g., clip was updated via editor API). + // Auto-rebind is not implemented yet; the animation simply stops until manually restarted. + const auto *animated_entity = animated_data.animated_entity(); + if (!animated_entity) { + return; + } + + for (auto &component : animated_entity->components) { auto &animated_component = component.second; auto &type_info = component.first;