From f8021dae9c5e654a4e2475d8d73ab587c9429f82 Mon Sep 17 00:00:00 2001 From: Jonathan Nicholl Date: Sat, 11 Jun 2022 21:14:40 -0400 Subject: [PATCH] Add animation_changed signal to AnimationLibrary AnimationLibrary now listens for the animation_changed signal on its animations and emits this new signal, with the animation name added on. AnimationPlayer now connects to this signal rather than connecting to each individual animation, which was poor practice due to bypassing encapsulation. --- doc/classes/AnimationLibrary.xml | 7 +++++ scene/animation/animation_player.cpp | 42 ++------------------------- scene/animation/animation_player.h | 4 +-- scene/resources/animation_library.cpp | 13 +++++++++ scene/resources/animation_library.h | 2 ++ 5 files changed, 26 insertions(+), 42 deletions(-) diff --git a/doc/classes/AnimationLibrary.xml b/doc/classes/AnimationLibrary.xml index 769b338063e..9be4cda5d6a 100644 --- a/doc/classes/AnimationLibrary.xml +++ b/doc/classes/AnimationLibrary.xml @@ -65,6 +65,13 @@ Emitted when an [Animation] is added, under the key [param name]. + + + + Emitted when there's a change in one of the animations, e.g. tracks are added, moved or have changed paths. [param name] is the key of the animation that was changed. + See also [signal Resource.changed], which this acts as a relay for. + + diff --git a/scene/animation/animation_player.cpp b/scene/animation/animation_player.cpp index 85bc4e98145..abe8d8c8499 100644 --- a/scene/animation/animation_player.cpp +++ b/scene/animation/animation_player.cpp @@ -1274,23 +1274,6 @@ void AnimationPlayer::_animation_set_cache_update() { } void AnimationPlayer::_animation_added(const StringName &p_name, const StringName &p_library) { - { - int at_pos = -1; - - for (uint32_t i = 0; i < animation_libraries.size(); i++) { - if (animation_libraries[i].name == p_library) { - at_pos = i; - break; - } - } - - ERR_FAIL_COND(at_pos == -1); - - ERR_FAIL_COND(!animation_libraries[at_pos].library->animations.has(p_name)); - - _ref_anim(animation_libraries[at_pos].library->animations[p_name]); - } - _animation_set_cache_update(); } @@ -1301,11 +1284,6 @@ void AnimationPlayer::_animation_removed(const StringName &p_name, const StringN return; // No need to update because not the one from the library being used. } - AnimationData animation_data = animation_set[name]; - if (animation_data.animation_library == p_library) { - _unref_anim(animation_data.animation); - } - _animation_set_cache_update(); // Erase blends if needed @@ -1401,10 +1379,7 @@ Error AnimationPlayer::add_animation_library(const StringName &p_name, const Ref ald.library->connect(SNAME("animation_added"), callable_mp(this, &AnimationPlayer::_animation_added).bind(p_name)); ald.library->connect(SNAME("animation_removed"), callable_mp(this, &AnimationPlayer::_animation_removed).bind(p_name)); ald.library->connect(SNAME("animation_renamed"), callable_mp(this, &AnimationPlayer::_animation_renamed).bind(p_name)); - - for (const KeyValue> &K : ald.library->animations) { - _ref_anim(K.value); - } + ald.library->connect(SNAME("animation_changed"), callable_mp(this, &AnimationPlayer::_animation_changed)); _animation_set_cache_update(); @@ -1428,27 +1403,16 @@ void AnimationPlayer::remove_animation_library(const StringName &p_name) { animation_libraries[at_pos].library->disconnect(SNAME("animation_added"), callable_mp(this, &AnimationPlayer::_animation_added)); animation_libraries[at_pos].library->disconnect(SNAME("animation_removed"), callable_mp(this, &AnimationPlayer::_animation_removed)); animation_libraries[at_pos].library->disconnect(SNAME("animation_renamed"), callable_mp(this, &AnimationPlayer::_animation_renamed)); + animation_libraries[at_pos].library->disconnect(SNAME("animation_changed"), callable_mp(this, &AnimationPlayer::_animation_changed)); stop(); - for (const KeyValue> &K : animation_libraries[at_pos].library->animations) { - _unref_anim(K.value); - } - animation_libraries.remove_at(at_pos); _animation_set_cache_update(); notify_property_list_changed(); } -void AnimationPlayer::_ref_anim(const Ref &p_anim) { - Ref(p_anim)->connect("changed", callable_mp(this, &AnimationPlayer::_animation_changed), CONNECT_REFERENCE_COUNTED); -} - -void AnimationPlayer::_unref_anim(const Ref &p_anim) { - Ref(p_anim)->disconnect("changed", callable_mp(this, &AnimationPlayer::_animation_changed)); -} - void AnimationPlayer::rename_animation_library(const StringName &p_name, const StringName &p_new_name) { if (p_name == p_new_name) { return; @@ -1799,7 +1763,7 @@ double AnimationPlayer::get_current_animation_length() const { return playback.current.from->animation->get_length(); } -void AnimationPlayer::_animation_changed() { +void AnimationPlayer::_animation_changed(const StringName &p_name) { clear_caches(); emit_signal(SNAME("caches_cleared")); if (is_playing()) { diff --git a/scene/animation/animation_player.h b/scene/animation/animation_player.h index 4f32927d254..3fa7e0c8035 100644 --- a/scene/animation/animation_player.h +++ b/scene/animation/animation_player.h @@ -291,9 +291,7 @@ private: return ret; } - void _animation_changed(); - void _ref_anim(const Ref &p_anim); - void _unref_anim(const Ref &p_anim); + void _animation_changed(const StringName &p_name); void _set_process(bool p_process, bool p_force = false); diff --git a/scene/resources/animation_library.cpp b/scene/resources/animation_library.cpp index 427d4185515..b37bfbae629 100644 --- a/scene/resources/animation_library.cpp +++ b/scene/resources/animation_library.cpp @@ -52,11 +52,13 @@ Error AnimationLibrary::add_animation(const StringName &p_name, const Refdisconnect(SNAME("changed"), callable_mp(this, &AnimationLibrary::_animation_changed)); animations.erase(p_name); emit_signal(SNAME("animation_removed"), p_name); } animations.insert(p_name, p_animation); + animations.get(p_name)->connect(SNAME("changed"), callable_mp(this, &AnimationLibrary::_animation_changed).bind(p_name)); emit_signal(SNAME("animation_added"), p_name); notify_property_list_changed(); return OK; @@ -65,6 +67,7 @@ Error AnimationLibrary::add_animation(const StringName &p_name, const Refdisconnect(SNAME("changed"), callable_mp(this, &AnimationLibrary::_animation_changed)); animations.erase(p_name); emit_signal(SNAME("animation_removed"), p_name); notify_property_list_changed(); @@ -75,6 +78,8 @@ void AnimationLibrary::rename_animation(const StringName &p_name, const StringNa ERR_FAIL_COND_MSG(!is_valid_animation_name(p_new_name), "Invalid animation name: '" + String(p_new_name) + "'."); ERR_FAIL_COND_MSG(animations.has(p_new_name), vformat("Animation name \"%s\" already exists in library.", p_new_name)); + animations.get(p_name)->disconnect(SNAME("changed"), callable_mp(this, &AnimationLibrary::_animation_changed)); + animations.get(p_name)->connect(SNAME("changed"), callable_mp(this, &AnimationLibrary::_animation_changed).bind(p_new_name)); animations.insert(p_new_name, animations[p_name]); animations.erase(p_name); emit_signal(SNAME("animation_renamed"), p_name, p_new_name); @@ -100,6 +105,10 @@ TypedArray AnimationLibrary::_get_animation_list() const { return ret; } +void AnimationLibrary::_animation_changed(const StringName &p_name) { + emit_signal(SNAME("animation_changed"), p_name); +} + void AnimationLibrary::get_animation_list(List *p_animations) const { List anims; @@ -115,6 +124,9 @@ void AnimationLibrary::get_animation_list(List *p_animations) const } void AnimationLibrary::_set_data(const Dictionary &p_data) { + for (KeyValue> &K : animations) { + K.value->disconnect(SNAME("changed"), callable_mp(this, &AnimationLibrary::_animation_changed)); + } animations.clear(); List keys; p_data.get_key_list(&keys); @@ -146,6 +158,7 @@ void AnimationLibrary::_bind_methods() { ADD_SIGNAL(MethodInfo("animation_added", PropertyInfo(Variant::STRING_NAME, "name"))); ADD_SIGNAL(MethodInfo("animation_removed", PropertyInfo(Variant::STRING_NAME, "name"))); ADD_SIGNAL(MethodInfo("animation_renamed", PropertyInfo(Variant::STRING_NAME, "name"), PropertyInfo(Variant::STRING_NAME, "to_name"))); + ADD_SIGNAL(MethodInfo("animation_changed", PropertyInfo(Variant::STRING_NAME, "name"))); } AnimationLibrary::AnimationLibrary() { } diff --git a/scene/resources/animation_library.h b/scene/resources/animation_library.h index d63807b6d7b..54bd641b6d2 100644 --- a/scene/resources/animation_library.h +++ b/scene/resources/animation_library.h @@ -42,6 +42,8 @@ class AnimationLibrary : public Resource { TypedArray _get_animation_list() const; + void _animation_changed(const StringName &p_name); + friend class AnimationPlayer; //for faster access HashMap> animations;