Merge pull request #61958 from jtnicholl/animation_connections

Add `animation_changed` signal to `AnimationLibrary`, have `AnimationPlayer` connect to it instead of `Animation`'s `changed`

doc/classes/AnimationLibrary.xml

@@ -65,6 +65,13 @@
 				Emitted when an [Animation] is added, under the key [param name].
 			</description>
 		</signal>
+		<signal name="animation_changed">
+			<param index="0" name="name" type="StringName" />
+			<description>
+				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.
+			</description>
+		</signal>
 		<signal name="animation_removed">
 			<param index="0" name="name" type="StringName" />
 			<description>

scene/animation/animation_player.cpp

@@ -1286,23 +1286,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();
 }
 
@@ -1313,11 +1296,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
@@ -1413,10 +1391,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));
-
+	ald.library->connect(SNAME("animation_changed"), callable_mp(this, &AnimationPlayer::_animation_changed));
-	for (const KeyValue<StringName, Ref<Animation>> &K : ald.library->animations) {
-		_ref_anim(K.value);
-	}
 
 	_animation_set_cache_update();
 
@@ -1440,27 +1415,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<StringName, Ref<Animation>> &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<Animation> &p_anim) {
-	Ref<Animation>(p_anim)->connect("changed", callable_mp(this, &AnimationPlayer::_animation_changed), CONNECT_REFERENCE_COUNTED);
-}
-
-void AnimationPlayer::_unref_anim(const Ref<Animation> &p_anim) {
-	Ref<Animation>(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;
@@ -1811,7 +1775,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();
 	if (is_playing()) {
 		playback.seeked = true; //need to restart stuff, like audio

scene/animation/animation_player.h

@@ -291,9 +291,7 @@ private:
 		return ret;
 	}
 
-	void _animation_changed();
+	void _animation_changed(const StringName &p_name);
-	void _ref_anim(const Ref<Animation> &p_anim);
-	void _unref_anim(const Ref<Animation> &p_anim);
 
 	void _set_process(bool p_process, bool p_force = false);
 

scene/resources/animation_library.cpp

@@ -52,11 +52,13 @@ Error AnimationLibrary::add_animation(const StringName &p_name, const Ref<Animat
 	ERR_FAIL_COND_V(p_animation.is_null(), ERR_INVALID_PARAMETER);
 
 	if (animations.has(p_name)) {
+		animations.get(p_name)->disconnect(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 Ref<Animat
 void AnimationLibrary::remove_animation(const StringName &p_name) {
 	ERR_FAIL_COND_MSG(!animations.has(p_name), vformat("Animation not found: %s.", p_name));
 
+	animations.get(p_name)->disconnect(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<StringName> 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<StringName> *p_animations) const {
 	List<StringName> anims;
 
@@ -115,6 +124,9 @@ void AnimationLibrary::get_animation_list(List<StringName> *p_animations) const
 }
 
 void AnimationLibrary::_set_data(const Dictionary &p_data) {
+	for (KeyValue<StringName, Ref<Animation>> &K : animations) {
+		K.value->disconnect(SNAME("changed"), callable_mp(this, &AnimationLibrary::_animation_changed));
+	}
 	animations.clear();
 	List<Variant> 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() {
 }

scene/resources/animation_library.h

@@ -42,6 +42,8 @@ class AnimationLibrary : public Resource {
 
 	TypedArray<StringName> _get_animation_list() const;
 
+	void _animation_changed(const StringName &p_name);
+
 	friend class AnimationPlayer; //for faster access
 	HashMap<StringName, Ref<Animation>> animations;