godot: AnimationPlayer play() does not play anymore when the same animation is playing.

Godot version: 3.1.2 stable

OS/device including version: Win7

Issue description: The AnimationPlayer won’t interrupt itself anymore to start playing the same animation anew on play(). When another animation than the current one is asked to be played, interrupting works as usual.

This is highly problematic as it breaks a lot of code in action games or anything that needs immediate user feedback. This completely breaks my main project. play_not_interrupting

Steps to reproduce: Make an animation that lasts for a few seconds to easily see the effect, trigger the animation in your process or physics_process through input, run and try to make the animation play again while it is still playing.

This used to work, but does not anymore:

func _physics_process(_delta):
	if Input.is_action_just_pressed("LMB"):
		$AnimationPlayer.play("default")

Minimal Reproduction Project: AnimationPlayer_cannot_interrupt.zip

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 1
  • Comments: 29 (22 by maintainers)

Commits related to this issue

Most upvoted comments

From my experience, you more often have to check if animation isn’t played yet than want to restart it.

Play() worked perfectly fine before 3.1

If you call play(), it plays

If you run play in either _process() or _physics_process() you immediately learned what either of those functions do.

Now, you learn nothing and play() is behaving differently from any other function you would call in either of those processes. Now you have to apparently stop() the AnimationPlayer before playing. What does .stop(false) even mean? Logically stop(false) means “play”. But in 3.1 it’s telling the AnimationPlayer to “pause” while stop(true) mean to actually stop!? Why would stop(false) then not be named .pause() then? Is the goal here to confuse people? Why would we need to tell something to stop anyway if we want to play it?? It’s completely redundant! It’s not like it teaches you nothing anymore, it teaches the wrong things now.

Besides where is the discussion for this? This is so elemental to the AnimaitonPlayer behavior and it seems like it has been implemented without any prior discussion.

This should be reverted.

It may work but it’s confusing as hell what does a a $AnimationPlayer.stop(false) even mean to any sane person it basically means opposite of stop which is play(). It’s not clear at first glance of what is expected behaviour of this function.

@Feniks-Gaming Hence:

For 4.0, we can discuss (in a proposal) changing the API again to make it more explicit. I think it would be worth having start(), pause(), resume(), stop() as explicit methods instead of the multi-behavioral play() and stop(). play() should maybe be removed altogether as it’s ambiguous.

It makes sense but if that is a case play() should be renamed to something else. When call play() on AudioStreamPlayer it doesn’t act the same way than calling play() on AnimationPlayer either every node that has play() function should behave consistently or if one of the nodes doesn’t then it’s function should be renamed.

@Feniks-Gaming You misunderstand the issue. play() is called each frame if you call it in _process or _physics_process. It’s just that it doesn’t do what you think it does. What it does is what the documentation now states, i.e. start the animation from where it was stopped, which can be:

  • The first animation frame if it was never played, or finished playing, or was reset manually
  • The last animation frame if it was never played and you use from_end = true (or play_backwards)
  • The previous animation frame if it had been paused

If it’s already playing, then it keeps playing. That doesn’t mean that play() does not run, but its logic is that “the show must go on”. What you want is restart(), and that’s not what it is, as discussed and documented above. Currently there’s no builtin restart behavior, so you have to stop or seek(0) if you want to restart from the first frame on play().

Listen @akien-mga and @KoBeWi I think you are focusing on a wrong problem here without seeing a bigger picture.

I agree with @golddotasksquestions the issue is not how often play() is used but how it defines the logic of the engine. If engine calls every function every frame inside the physics process. It should call every function every frame inside physic process INCLUDING play() function. It is vital that engine behaves in a predictable way that user can understand without spending couple of hours on discord/reddit and docs to find out that something is yet another exception to how you would expect engine to behave.

The issue isn’t the fact that you want to use play() one way or the other the issue is the fact that docs lie to you when they say “process function calls each function every frame”. The more exceptions we have the harder to intuitively use. The moment docs mislead you like this is the moment you stop trusting documentation and this is the moment when you may as well simply not have it.

The primary issue is not what is easier to use but what should be expected from the engine. If something is slightly less practical but promotes consistency and intuitive use it should stay that way. Logic of how the process function works demands that play() is called every frame just like any other function is.

The compatibility breakage was done on purpose in #25776 for 3.1, and while I agree that it could have used more discussion, it doesn’t make sense to backtrack and break compatibility again for 3.1 users.

The current behavior is consistent and #34128 documents it well. play and stop are used for both pause/resume and stop/restart actions. The previous 3.0.x behavior did not allow resuming an animation, so the current API provides a lot more flexibility. You can now even pause an animation half-way through, and resume playing in the reverse direction.

As mentioned, the previous behavior can be obtained by simply calling stop() (or seek(0)) before restarting with play().

For 4.0, we can discuss (in a proposal) changing the API again to make it more explicit. I think it would be worth having start(), pause(), resume(), stop() as explicit methods instead of the multi-behavioral play() and stop(). play() should maybe be removed altogether as it’s ambiguous.

There should also be a cleanup around the current_animation and assigned_animation properties, the API could be overall much clearer if we redesign it from the ground up to properly cater to the different playback use cases (including solving the awkwardness of play_backwards()).

play’s behavior should be reverted

Pls no. The resetting could be made optional behavior if there’s demand for it (which brings Godot proposals here). Either as an argument for play() or a new method.

It defies logic. We take a lot of effort to teach people how _process() is called every frame and _physics_process() is called every physics frame and then … this??? If play() is the “command” to start the AnimationPlayer playing an animation, and I call it in a function that is called ever frame, it SHOULD be triggered every frame.

Having to add stop every time you want The Animationplayer to start playing is completely illogical on the other hand and adds completely unnecessary code.

Don’t get me started on stop (true). That makes about as much sense as the playback GUI https://github.com/godotengine/godot-proposals/issues/167 Absolutely none at all.

This is so bad. And here I thought I already saw rock bottom with the AnimationPlayer “improvements” in 3.1.

I’m calling things “BS” when I feel like they are an expression of a double standard. To me, having this breaking change of an elemental feature from 3.0.6 to 3.1 signed off without any prior discussion or even request and then asking us to file a proposal, have a discussion and then maaaaybe if someone kind enough finds time and energy to submit a PR, would fix things in a future version of Godot that who-knows-comes-when, is a double standard to me. Closing this issue here without consensus makes it worse imho.

If you @akien-mga would also see the change of play("animation") as a unintended mishap, it would be not problem to revert it in 3.2. No change to the added functionality of stop() necessary.

The 3.1 play("animation") behavior is a lot harder to explain. It makes the process and physics_process harder to explain because it seemingly adds an exception to their behavior for those who cannot or did not read the source code. Explaining the AnimationPlayerand process/physics_process is something I do on a daily basis in the Godot community channels.

3.0.6 had seek() as well. There was nothing stopping anyone from using it as “resume from where you left of”. It’s even in the name.

This just adds another nail to the coffin that is a number of failed “improvements” to the AnimaitonPlayer in 3.1. It makes me sad. Because the AnimationPlayer in 3.0.6 though not perfect, was a great tool, and it has become more and more downgraded. 😦

I think we agree on functionality we disagree on a time frame proposed. 3.0 ->3.1 was happy to break functionality why is 3.1->3.2 breaking functionality a problem?

I that would work spot on in my opinion. Why wait till 4.0 is it not something we could bring with 3.2.1 for example?

That would break compatibility, which should not happen in maintenance releases like 3.2.1.

Sorry @akien-mga , but that’s BS: This change also broke compatibility from 3.0.6 to 3.1. There was literally no discussion about this what so ever. Neither https://github.com/godotengine/godot/issues/25745 nor https://github.com/godotengine/godot/issues/17423 have a discussion about changing the play("animation") behavior to "only play when not playing". As far as I know there isn’t even a issue requesting this change. The only trace related to play("animation") not starting to play this animation from the beginning is this comment: https://github.com/godotengine/godot/issues/17423#issuecomment-462062899 which however had no reaction and also only talks about the empty play(), not play("animation").

Added functionality is always good. Breaking things, or forcing confusing workflow onto people for everyday-use basic things, is not!

I’m all for discussing start(), restart(), pause(), resume(), stop() in a proposal, but until this discussion happened, this breaking change on play("animation") should be immediately reverted.

The added functionality of the absurd stop(true), stop(false) would be unaffected by reverting play("animation").

I that would work spot on in my opinion. Why wait till 4.0 is it not something we could bring with 3.2.1 for example?

That would break compatibility, which should not happen in maintenance releases like 3.2.1. And there’s little justification for breaking compatibility in 3.2 itself with regard to the new behavior in 3.1, as the new behavior is working properly and used in production by many.

Is the issue created in proposal for this or do we need to make one?

I don’t think there is one yet. It should be opened in https://github.com/godotengine/godot-proposals, ideally with a proposal for what the new public API should be (i.e. the methods and properties that AnimationPlayer would have in 4.0, especially the ones that would be added/removed/renamed/modified). Edit: Having a full proposal is not a requirement for opening the issue though, I just mean that it’s what the issue should eventually lead to, so that we have something concrete and well designed to implement.

I that would work spot on in my opinion. Why wait till 4.0 is it not something we could bring with 3.2.1 for example?

Is the issue created in proposal for this or do we need to make one?

Yes, those give me the same results as well. It’s definitely working in 3.0.6:

AnimationPlayer_interrupt.zip

play_not_interrupting306

Damnyou Githup UI! Sorry for the accidental close, I was trying to close the comment I was writing, not the whole issue.

I tested 3.1, 3.1.1, 3.1.2 and the current master branch, they all give the same behavior described above.

If you want to reset the animation to restart it, you can use:

$AnimationPlayer.stop(true)  # restart = true is the default, so you can also omit it
$AnimationPlayer.play("default")