Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oneshot AnimationTree node doesn't respect fade-out duration when unsetting active parameter #53947

Closed
Flavelius opened this issue Oct 18, 2021 · 10 comments · Fixed by #74190
Closed

Comments

@Flavelius
Copy link
Contributor

Flavelius commented Oct 18, 2021

Godot version

3.3.4.stable.mono.official

System information

Windows 10

Issue description

When setting a oneshot node to active and waiting for it to finish, the active parameter gets reset automatically and the animation will blend in and out according to the oneshot-node's settings.
But when unsetting the active parameter while its animation is still playing (to end it prematurely) the fade-out setting is not respected and the animation is reset instantly (snapping back to start).

Here, the first 2 triggers are untoggled manually (snapping) and the last one is left alone to fade back itself:
https://user-images.githubusercontent.com/8841352/137686625-a6f980a7-d3f9-4798-91ba-cd250f04184e.mp4

Steps to reproduce

Create an animationTree with a oneshot node, toggle it active and immediately toggle it back inactive, see the animation snap back instead of fading back.

Minimal reproduction project

No response

@Calinou
Copy link
Member

Calinou commented Oct 18, 2021

Related to #39496.

@Flavelius
Copy link
Contributor Author

Flavelius commented Oct 18, 2021

Although it's the same node, it's a different issue. This one is not about an exposed parameter, but a setting of the node itself not being honored internally.

@TokageItLab
Copy link
Member

NodeOneShot is not designed to be suspended when playing, so I think using NodeTransition is the preferred method in this case. However, if we only want to play the animation once, NodeTransition requires us to write code to revert to the original animation, so this feature seems to be acceptable as an Enhancement.

@Flavelius
Copy link
Contributor Author

But the transition node can have more than two entries, in which case it seems even less designed for this task, besides oneshot being exactly what's needed in this case - playing a clip just once and then reverting, which also works fine blending at the end. So everything points towards there being a bug, or in this case just something the node's designer hasn't thought of when creating it.

@TokageItLab
Copy link
Member

TokageItLab commented Nov 10, 2021

Perhaps, NodeOneShot's fade-out is a predictive behavior based on the end time of the animation, so it's not compatible with the suspend behavior in the first place. There is a difference there between "deactivation and fade out with N seconds" and "fade out N seconds before deactivation".
It's true that NodeTransition is different from NodeOneShot in some places, but it can replace NodeOneShot although it is tedious. It seems to be blowing it out of proportion that saying "So everything points towards there being a bug".

As a solution, I think it would be possible to add an bool option to fade out after deactivation with manual in NodeOneShot.

@Flavelius
Copy link
Contributor Author

Flavelius commented Nov 11, 2021

Maybe I'm not understanding what you mean in detail here but for my understanding there's not really a great difference in the way both fades apply apart from one calculating its duration from clipendtime-fadetime and the other from clipstoptime+fadetime, both are available. My guess is that the transition node may even do it in the way that's applicable here too (calculating the fade with the time passed after stop/trigger and then fading but only actually stopping the clip after that fade ended, at least it presents itself like so), so the predictive behavior you mention is given too, it just extends the clip (play)time by fade duration for that

@TokageItLab
Copy link
Member

TokageItLab commented Nov 11, 2021

So, what I'm saying is that NodeOneShot is currently designed to play during Activation, and if you want to fade it out after it's been Deactivated, it needs to be played even though it's not in Activation. This means that some process changes are required in NodeOneShot. However, it is not impossible, so I am interested in this as enhancement.

@TokageItLab
Copy link
Member

TokageItLab commented Nov 11, 2021

I've been thinking about this a bit. Is it acceptable for the NodeOneShot animation to play from the head of the clip if it is re-activated during the fade-out after deactivation? That would mean that the animation would seek suddenly and then it will be removed fade-out animation blending effects.

Due to the current structure of the AnimationTree, to blend the same animations always requires more than one NodeAnimation assigned to the same animation, so if this is unacceptable, a fundamental change in the blending algorithm is required.

@Flavelius
Copy link
Contributor Author

Flavelius commented Nov 11, 2021

I think this would currently (with all the other bugs around blending) even be desired, as stop canceling this way already presents a 'soft' early out intention. And restarting it at the same time then in my opinion likely almost always signals the desire to 'hard' cancel out for replaying.

@TokageItLab
Copy link
Member

TokageItLab commented Nov 26, 2021

Me:

As a solution, I think it would be possible to add an bool option to fade out after deactivation with manual in NodeOneShot.

I've been thinking about this a bit. Is it acceptable for the NodeOneShot animation to play from the head of the clip if it is re-activated during the fade-out after deactivation? That would mean that the animation would seek suddenly and then it will be removed fade-out animation blending effects.

Flavelius:

I think this would currently (with all the other bugs around blending) even be desired, as stop canceling this way already presents a 'soft' early out intention. And restarting it at the same time then in my opinion likely almost always signals the desire to 'hard' cancel out for replaying.

@reduz What do you think? If it's not a problem, I'm thinking of trying to implement it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants