Fix AnimatedSprite2D/3D::play using wrong end_frame when playing backwards
#93548
Conversation
AThousandShips
added
bug
topic:animation
cherrypick:4.2
|
Just seeing this also applies to the AnimatedSprite3D btw. |
|
The static check fails because there may be extra tab characters on the new line that you need to strip.
Indeed, you should duplicate this fix on AnimatedSprite3D as well. Mentioning @dalexeev because they have seen animated sprite's code extensively in the past. |
|
should i add the 3D version right here or do another pull request? If so I would rename the pr, commit the change and you have to change the pr tags |
|
Do it in this and make a single commit with an appropriate title |
AThousandShips
changed the title
AnimationSprite2D/3D::play using wrong end_frame
|
Meant to make a single comment of the entire PR please squash your commits into one, see here And please make the name match the name of the PR |
AThousandShips
changed the title
AnimationSprite2D/3D::play using wrong end_frameAnimatedSprite2D/3D::play using wrong end_frame
|
like this? Sorry I'm still a bit inexperienced with git |
|
That's just right |
scene/2d/animated_sprite_2d.cpp
Outdated
| @@ -476,6 +476,8 @@ void AnimatedSprite2D::play(const StringName &p_name, float p_custom_scale, bool | |||
| int end_frame = MAX(0, frames->get_frame_count(animation) - 1); | |||
There was a problem hiding this comment.
The fix is correct as is, but I think it makes sense to move this line to the else block below (so MAX(0, frames->get_frame_count(animation) - 1) would not be done twice when name != animation).
(same for 3d)
There was a problem hiding this comment.
Yes but you would still need one for the else branch, but you're right in that it would not be called twice
There was a problem hiding this comment.
That's what was suggested, to move the one that's outside the if to the else
There was a problem hiding this comment.
Yeah, I'm suggesting to have separate variables within both if/else blocks (with single assignment each).
On playing a different animation to the current backwards will start the new animation on the last animations last frame not the new ones
|
Thanks! And congrats for your first merged Godot contribution 🎉 |
akien-mga
changed the title
AnimatedSprite2D/3D::play using wrong end_frameAnimatedSprite2D/3D::play using wrong end_frame
akien-mga
changed the title
AnimatedSprite2D/3D::play using wrong end_frameAnimatedSprite2D/3D::play using wrong end_frame when playing backwards
On playing a different animation to the current backwards, will start the new animation on the last animations last frame not the new ones.
For example I have an
idleanimation with 2 frames. When I now play myactivationanimation backwards, which has 17 frames, it will start on frame 1 and then go down to 0 (because it's backwards), because theidleanimation has only 2 frames. I got it working, if i set the animation beforehand but this feels not right: