Objective

Fixes #5848.

Solution

• Adding function to get animation_clip from AnimationPlayer while documenting that it could be the default handle.
• Adding a function which tells if the animation player has finished playing the animation_clip.

---

The language used to explain what the default handle implies could be more clear. My problem is that the default handle could actually refer to an actual animation if an AnimationClip is set for the default handle, which leads me to ask, "Should animation_clip should be an Option?"

I changed it to an option which was a really simple change.

I have a change stashed locally that does this and a bit more, but it's not finished yet because it's actually a bit more complicated than that.

I don't think this PR handles animations being played backward, repeating animations or setting the elapsed manually

So for playing backward. It looks like the animation will repeat even if repeat is false. But I didn't change that. I could fix it in this PR.

In this PR, repeating animations never finish. The correctness of this is up to debate.

Setting elapsed manually has a one frame delay before finished is updated. This is unavoidable unless the AnimationPlayer keeps track of the AnimationClip's duration. It would take a frame to get this information whenever the animation player starts playing an animation. It could also be temporarily incorrect if the AnimationClip in Assets has been changed.

Which of these would you like me to work on?

Which of these would you like me to work on?

I actually haven't found yet a fix that makes me happy, which is why I didn't open the PR. Try some things, let's see how it works out 😄

Another thing about repeat that I don't like if you have ideas on how to fix: setting repeat to false on a currently playing and repeating animation should let the current loop finish before stopping. This is currently true if done during the first repeat, not during others

So for playing backward. It looks like the animation will repeat even if repeat is false. But I didn't change that. I could fix it in this PR.

In this PR, repeating animations never finish. The correctness of this is up to debate.

After a deeper look this is actually incorrect. I will set up so test cases and iterate over some solutions.

What should the behavior of a non-repeating animation player with a negative speed at time 0?

Should it stay on the first frame or should it play the entire animation backwards. What should happen if speed is negative with any positive time? Should you have to set the time to the duration of the animation to play it once backwards? What if you want to play it forward and then backward?

With the current implementation, it seems like a non-repeating animation with negative speed set to the duration of the animation will play the animation backward twice.

Also I find it odd that the elapsed time goes up (or down if negative) forever. This is also weird because a non-repeating backward animation with an elapsed time greater than the duration of the animation will be on the last frame for a period of time before playing the animation backward (twice).

What should the behavior of a non-repeating animation player with a negative speed at time 0? Should it stay on the first frame or should it play the entire animation backwards. What should happen if speed is negative with any positive time? Should you have to set the time to the duration of the animation to play it once backwards? What if you want to play it forward and then backward?

IMO, we should treat non-repeating behavior as a clamp on the time modulus the length of the clip. If it's not repeating and the speed is negative, we should play to the beginning then mark it finished. This is essentially playing the clip backwards, and allows users to ping-pong the clip if they so desire. It's the least surprising interaction between these two features.

@asafigan can you resolve the merge conflicts? I'd like to get this moving again and they're more complex than I want to tackle in the web UI :)

@DevinLeamy your review here would be welcome, since you're building on it.

@DevinLeamy your review here would be welcome, since you're building on it.

I'm on board with the changes here! IMO it probably would be easiest to patch them into #9002. @alice-i-cecile Any opposition to doing that?

No objections; just credit the original author at the top of your PR description :)

No objections; just credit the original author at the top of your PR description :)

Of course! It's been updated.

Updated and incorporated into #9002. Thanks a ton for your work, and if you get a chance your review there would be welcome!