Merge pull request #65148 from Mickeon/animated-sprite-negative-speed-scale

Allow negative `speed_scale` in AnimatedSprite2D & 3D

6 files changed, 69 insertions, 67 deletions

diff --git a/doc/classes/AnimatedSprite2D.xml b/doc/classes/AnimatedSprite2D.xml
index b207eda27f..afbe34816a 100644
--- a/doc/classes/AnimatedSprite2D.xml
+++ b/doc/classes/AnimatedSprite2D.xml
@@ -5,6 +5,8 @@
 	</brief_description>
 	<description>
 		[AnimatedSprite2D] is similar to the [Sprite2D] node, except it carries multiple textures as animation frames. Animations are created using a [SpriteFrames] resource, which allows you to import image files (or a folder containing said files) to provide the animation frames for the sprite. The [SpriteFrames] resource can be configured in the editor via the SpriteFrames bottom panel.
+		After setting up [member frames], [method play] may be called. It's also possible to select an [member animation] and toggle [member playing], even within the editor.
+		To pause the current animation, call [method stop] or set [member playing] to [code]false[/code]. Alternatively, setting [member speed_scale] to [code]0[/code] also preserves the current frame's elapsed time.
 		[b]Note:[/b] You can associate a set of normal or specular maps by creating additional [SpriteFrames] resources with a [code]_normal[/code] or [code]_specular[/code] suffix. For example, having 3 [SpriteFrames] resources [code]run[/code], [code]run_normal[/code], and [code]run_specular[/code] will make it so the [code]run[/code] animation uses normal and specular maps.
 	</description>
 	<tutorials>
@@ -17,13 +19,14 @@
 			<param index="0" name="anim" type="StringName" default="&amp;&quot;&quot;" />
 			<param index="1" name="backwards" type="bool" default="false" />
 			<description>
-				Plays the animation named [param anim]. If no [param anim] is provided, the current animation is played. If [code]backwards[/code] is [code]true[/code], the animation will be played in reverse.
+				Plays the animation named [param anim]. If no [param anim] is provided, the current animation is played. If [param backwards] is [code]true[/code], the animation is played in reverse.
 			</description>
 		</method>
 		<method name="stop">
 			<return type="void" />
 			<description>
-				Stops the current animation (does not reset the frame counter).
+				Stops the current [member animation] at the current [member frame].
+				[b]Note:[/b] This method resets the current frame's elapsed time. If this behavior is undesired, consider setting [member speed_scale] to [code]0[/code], instead.
 			</description>
 		</method>
 	</methods>
@@ -50,10 +53,10 @@
 			The texture's drawing offset.
 		</member>
 		<member name="playing" type="bool" setter="set_playing" getter="is_playing" default="false">
-			If [code]true[/code], the [member animation] is currently playing.
+			If [code]true[/code], the [member animation] is currently playing. Setting this property to [code]false[/code] is the equivalent of calling [method stop].
 		</member>
 		<member name="speed_scale" type="float" setter="set_speed_scale" getter="get_speed_scale" default="1.0">
-			The animation speed is multiplied by this value.
+			The animation speed is multiplied by this value. If set to a negative value, the animation is played in reverse. If set to [code]0[/code], the animation is paused, preserving the current frame's elapsed time.
 		</member>
 	</members>
 	<signals>
diff --git a/doc/classes/AnimatedSprite3D.xml b/doc/classes/AnimatedSprite3D.xml
index 58d3ca6ad3..09baf882fb 100644
--- a/doc/classes/AnimatedSprite3D.xml
+++ b/doc/classes/AnimatedSprite3D.xml
@@ -4,7 +4,9 @@
 		2D sprite node in 3D world, that can use multiple 2D textures for animation.
 	</brief_description>
 	<description>
-		Animations are created using a [SpriteFrames] resource, which can be configured in the editor via the SpriteFrames panel.
+		[AnimatedSprite3D] is similar to the [Sprite3D] node, except it carries multiple textures as animation [member frames]. Animations are created using a [SpriteFrames] resource, which allows you to import image files (or a folder containing said files) to provide the animation frames for the sprite. The [SpriteFrames] resource can be configured in the editor via the SpriteFrames bottom panel.
+		After setting up [member frames], [method play] may be called. It's also possible to select an [member animation] and toggle [member playing], even within the editor.
+		To pause the current animation, call [method stop] or set [member playing] to [code]false[/code]. Alternatively, setting [member speed_scale] to [code]0[/code] also preserves the current frame's elapsed time.
 	</description>
 	<tutorials>
 		<link title="2D Sprite animation (also applies to 3D)">$DOCS_URL/tutorials/2d/2d_sprite_animation.html</link>
@@ -15,13 +17,14 @@
 			<param index="0" name="anim" type="StringName" default="&amp;&quot;&quot;" />
 			<param index="1" name="backwards" type="bool" default="false" />
 			<description>
-				Plays the animation named [param anim]. If no [param anim] is provided, the current animation is played. If [param backwards] is [code]true[/code], the animation will be played in reverse.
+				Plays the animation named [param anim]. If no [param anim] is provided, the current animation is played. If [param backwards] is [code]true[/code], the animation is played in reverse.
 			</description>
 		</method>
 		<method name="stop">
 			<return type="void" />
 			<description>
-				Stops the current animation (does not reset the frame counter).
+				Stops the current [member animation] at the current [member frame].
+				[b]Note:[/b] This method resets the current frame's elapsed time. If this behavior is undesired, consider setting [member speed_scale] to [code]0[/code], instead.
 			</description>
 		</method>
 	</methods>
@@ -36,10 +39,10 @@
 			The [SpriteFrames] resource containing the animation(s).
 		</member>
 		<member name="playing" type="bool" setter="set_playing" getter="is_playing" default="false">
-			If [code]true[/code], the [member animation] is currently playing.
+			If [code]true[/code], the [member animation] is currently playing. Setting this property to [code]false[/code] is the equivalent of calling [method stop].
 		</member>
 		<member name="speed_scale" type="float" setter="set_speed_scale" getter="get_speed_scale" default="1.0">
-			The animation speed is multiplied by this value.
+			The animation speed is multiplied by this value. If set to a negative value, the animation is played in reverse. If set to [code]0[/code], the animation is paused, preserving the current frame's elapsed time.
 		</member>
 	</members>
 	<signals>
diff --git a/scene/2d/animated_sprite_2d.cpp b/scene/2d/animated_sprite_2d.cpp
index 7fe464d2f4..afa4633848 100644
--- a/scene/2d/animated_sprite_2d.cpp
+++ b/scene/2d/animated_sprite_2d.cpp
@@ -63,9 +63,13 @@ Rect2 AnimatedSprite2D::_edit_get_rect() const {
 }
 
 bool AnimatedSprite2D::_edit_use_rect() const {
-	if (!frames.is_valid() || !frames->has_animation(animation) || frame < 0 || frame >= frames->get_frame_count(animation)) {
+	if (frames.is_null() || !frames->has_animation(animation)) {
 		return false;
 	}
+	if (frame < 0 || frame >= frames->get_frame_count(animation)) {
+		return false;
+	}
+
 	Ref<Texture2D> t;
 	if (animation) {
 		t = frames->get_frame(animation, frame);
@@ -79,7 +83,10 @@ Rect2 AnimatedSprite2D::get_anchorable_rect() const {
 }
 
 Rect2 AnimatedSprite2D::_get_rect() const {
-	if (!frames.is_valid() || !frames->has_animation(animation) || frame < 0 || frame >= frames->get_frame_count(animation)) {
+	if (frames.is_null() || !frames->has_animation(animation)) {
+		return Rect2();
+	}
+	if (frame < 0 || frame >= frames->get_frame_count(animation)) {
 		return Rect2();
 	}
 
@@ -161,29 +168,22 @@ void AnimatedSprite2D::_validate_property(PropertyInfo &p_property) const {
 void AnimatedSprite2D::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_INTERNAL_PROCESS: {
-			if (frames.is_null()) {
+			if (frames.is_null() || !frames->has_animation(animation)) {
 				return;
 			}
-			if (!frames->has_animation(animation)) {
-				return;
-			}
-			if (frame < 0) {
-				return;
+
+			double speed = frames->get_animation_speed(animation) * Math::abs(speed_scale);
+			if (speed == 0) {
+				return; // Do nothing.
 			}
+			int last_frame = frames->get_frame_count(animation) - 1;
 
 			double remaining = get_process_delta_time();
-
 			while (remaining) {
-				double speed = frames->get_animation_speed(animation) * speed_scale;
-				if (speed == 0) {
-					return; // Do nothing.
-				}
-
 				if (timeout <= 0) {
 					timeout = _get_frame_duration();
 
-					int last_frame = frames->get_frame_count(animation) - 1;
-					if (!backwards) {
+					if (!playing_backwards) {
 						// Forward.
 						if (frame >= last_frame) {
 							if (frames->get_animation_loop(animation)) {
@@ -229,13 +229,7 @@ void AnimatedSprite2D::_notification(int p_what) {
 		} break;
 
 		case NOTIFICATION_DRAW: {
-			if (frames.is_null()) {
-				return;
-			}
-			if (frame < 0) {
-				return;
-			}
-			if (!frames->has_animation(animation)) {
+			if (frames.is_null() || !frames->has_animation(animation)) {
 				return;
 			}
 
@@ -327,9 +321,14 @@ int AnimatedSprite2D::get_frame() const {
 }
 
 void AnimatedSprite2D::set_speed_scale(double p_speed_scale) {
+	if (speed_scale == p_speed_scale) {
+		return;
+	}
+
 	double elapsed = _get_frame_duration() - timeout;
 
-	speed_scale = MAX(p_speed_scale, 0.0f);
+	speed_scale = p_speed_scale;
+	playing_backwards = signbit(speed_scale) != backwards;
 
 	// We adapt the timeout so that the animation speed adapts as soon as the speed scale is changed.
 	_reset_timeout();
@@ -398,12 +397,13 @@ bool AnimatedSprite2D::is_playing() const {
 	return playing;
 }
 
-void AnimatedSprite2D::play(const StringName &p_animation, const bool p_backwards) {
+void AnimatedSprite2D::play(const StringName &p_animation, bool p_backwards) {
 	backwards = p_backwards;
+	playing_backwards = signbit(speed_scale) != backwards;
 
 	if (p_animation) {
 		set_animation(p_animation);
-		if (frames.is_valid() && backwards && get_frame() == 0) {
+		if (frames.is_valid() && playing_backwards && get_frame() == 0) {
 			set_frame(frames->get_frame_count(p_animation) - 1);
 		}
 	}
@@ -418,7 +418,7 @@ void AnimatedSprite2D::stop() {
 
 double AnimatedSprite2D::_get_frame_duration() {
 	if (frames.is_valid() && frames->has_animation(animation)) {
-		double speed = frames->get_animation_speed(animation) * speed_scale;
+		double speed = frames->get_animation_speed(animation) * Math::abs(speed_scale);
 		if (speed > 0) {
 			return 1.0 / speed;
 		}
diff --git a/scene/2d/animated_sprite_2d.h b/scene/2d/animated_sprite_2d.h
index 0a19e250d8..be1cc5353e 100644
--- a/scene/2d/animated_sprite_2d.h
+++ b/scene/2d/animated_sprite_2d.h
@@ -39,6 +39,7 @@ class AnimatedSprite2D : public Node2D {
 
 	Ref<SpriteFrames> frames;
 	bool playing = false;
+	bool playing_backwards = false;
 	bool backwards = false;
 	StringName animation = "default";
 	int frame = 0;
@@ -81,7 +82,7 @@ public:
 	void set_sprite_frames(const Ref<SpriteFrames> &p_frames);
 	Ref<SpriteFrames> get_sprite_frames() const;
 
-	void play(const StringName &p_animation = StringName(), const bool p_backwards = false);
+	void play(const StringName &p_animation = StringName(), bool p_backwards = false);
 	void stop();
 
 	void set_playing(bool p_playing);
diff --git a/scene/3d/sprite_3d.cpp b/scene/3d/sprite_3d.cpp
index 7a89bf81bb..380172f396 100644
--- a/scene/3d/sprite_3d.cpp
+++ b/scene/3d/sprite_3d.cpp
@@ -447,7 +447,7 @@ void Sprite3D::_draw() {
 	if (get_base() != get_mesh()) {
 		set_base(get_mesh());
 	}
-	if (!texture.is_valid()) {
+	if (texture.is_null()) {
 		set_base(RID());
 		return;
 	}
@@ -807,15 +807,7 @@ void AnimatedSprite3D::_draw() {
 		set_base(get_mesh());
 	}
 
-	if (frames.is_null()) {
-		return;
-	}
-
-	if (frame < 0) {
-		return;
-	}
-
-	if (!frames->has_animation(animation)) {
+	if (frames.is_null() || !frames->has_animation(animation)) {
 		return;
 	}
 
@@ -1050,29 +1042,22 @@ void AnimatedSprite3D::_validate_property(PropertyInfo &p_property) const {
 void AnimatedSprite3D::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_INTERNAL_PROCESS: {
-			if (frames.is_null()) {
-				return;
-			}
-			if (!frames->has_animation(animation)) {
+			if (frames.is_null() || !frames->has_animation(animation)) {
 				return;
 			}
-			if (frame < 0) {
-				return;
+
+			double speed = frames->get_animation_speed(animation) * Math::abs(speed_scale);
+			if (speed == 0) {
+				return; // Do nothing.
 			}
+			int last_frame = frames->get_frame_count(animation) - 1;
 
 			double remaining = get_process_delta_time();
-
 			while (remaining) {
-				double speed = frames->get_animation_speed(animation) * speed_scale;
-				if (speed == 0) {
-					return; // Do nothing.
-				}
-
 				if (timeout <= 0) {
 					timeout = _get_frame_duration();
 
-					int last_frame = frames->get_frame_count(animation) - 1;
-					if (!backwards) {
+					if (!playing_backwards) {
 						// Forward.
 						if (frame >= last_frame) {
 							if (frames->get_animation_loop(animation)) {
@@ -1177,9 +1162,14 @@ int AnimatedSprite3D::get_frame() const {
 }
 
 void AnimatedSprite3D::set_speed_scale(double p_speed_scale) {
+	if (speed_scale == p_speed_scale) {
+		return;
+	}
+
 	double elapsed = _get_frame_duration() - timeout;
 
-	speed_scale = MAX(p_speed_scale, 0.0f);
+	speed_scale = p_speed_scale;
+	playing_backwards = signbit(speed_scale) != backwards;
 
 	// We adapt the timeout so that the animation speed adapts as soon as the speed scale is changed.
 	_reset_timeout();
@@ -1191,7 +1181,10 @@ double AnimatedSprite3D::get_speed_scale() const {
 }
 
 Rect2 AnimatedSprite3D::get_item_rect() const {
-	if (!frames.is_valid() || !frames->has_animation(animation) || frame < 0 || frame >= frames->get_frame_count(animation)) {
+	if (frames.is_null() || !frames->has_animation(animation)) {
+		return Rect2(0, 0, 1, 1);
+	}
+	if (frame < 0 || frame >= frames->get_frame_count(animation)) {
 		return Rect2(0, 0, 1, 1);
 	}
 
@@ -1236,12 +1229,13 @@ bool AnimatedSprite3D::is_playing() const {
 	return playing;
 }
 
-void AnimatedSprite3D::play(const StringName &p_animation, const bool p_backwards) {
+void AnimatedSprite3D::play(const StringName &p_animation, bool p_backwards) {
 	backwards = p_backwards;
+	playing_backwards = signbit(speed_scale) != backwards;
 
 	if (p_animation) {
 		set_animation(p_animation);
-		if (frames.is_valid() && backwards && get_frame() == 0) {
+		if (frames.is_valid() && playing_backwards && get_frame() == 0) {
 			set_frame(frames->get_frame_count(p_animation) - 1);
 		}
 	}
@@ -1256,7 +1250,7 @@ void AnimatedSprite3D::stop() {
 
 double AnimatedSprite3D::_get_frame_duration() {
 	if (frames.is_valid() && frames->has_animation(animation)) {
-		double speed = frames->get_animation_speed(animation) * speed_scale;
+		double speed = frames->get_animation_speed(animation) * Math::abs(speed_scale);
 		if (speed > 0) {
 			return 1.0 / speed;
 		}
diff --git a/scene/3d/sprite_3d.h b/scene/3d/sprite_3d.h
index e6a546a76d..f6ad1bbdb8 100644
--- a/scene/3d/sprite_3d.h
+++ b/scene/3d/sprite_3d.h
@@ -209,6 +209,7 @@ class AnimatedSprite3D : public SpriteBase3D {
 
 	Ref<SpriteFrames> frames;
 	bool playing = false;
+	bool playing_backwards = false;
 	bool backwards = false;
 	StringName animation = "default";
 	int frame = 0;
@@ -237,7 +238,7 @@ public:
 	void set_sprite_frames(const Ref<SpriteFrames> &p_frames);
 	Ref<SpriteFrames> get_sprite_frames() const;
 
-	void play(const StringName &p_animation = StringName(), const bool p_backwards = false);
+	void play(const StringName &p_animation = StringName(), bool p_backwards = false);
 	void stop();
 
 	void set_playing(bool p_playing);