Fix display of progress bars on looped animations.

[SaracenOne]

This PR changes the internal behaviour of the AnimationTree nodes 'time' parameters. Rather than storing the time for an individual animation, if the animation is looping, it stores the total cumulative time for every loop. This shouldn't cause precision issues since its a 64bit value. This is intended to future-proof for a new transition switch based on explicit time windows. This does not solve every aspect, I think we may want to refactor more polling of information into the nodes themselves, but this may prove a useful stopgap.

Narrowed scope, this PR simply now just introduces a workaround to display the progress bars in the state machine editor even if the animation is looping which was previous not possible due to internal changes which make it so that AnimationNodes no longer return the remaining time for a looped animation. It does this by introducing an alternative path where it attempts to look at the node, animation, and internal 'time' parameter directly.

Closes #81715

[TokageItLab]

I tested this current PR and it seems that changing NodeAnimation causes loops random "despite Animation is not set looping" and a recurring bug that breaks the first frame when the State transitions.

incorrect looping state (click to detail)

Probably the fundamental cause of this random looping bug is that the state of is_looping information is not propagated to the AnimationStateMachine.

This was same before my refactoring. I remember that instead of being propagated the is_looping state, the looping was determined when the remaining time was decreasing (so the loop was broken when NodeAnimation playback mode is backward). After refactoring, looping is determined by HUGE_LENGTH (I remember doing this at the time of refactoring because there was no way to get the loop state semantically then).

Even if you make it return the correct RemainTime, it would be impossible to propagate the looping state unless you add an is_looping state to the argument.

We might come up with an implementation that adds an option to the StateMachine side to count loops independently of the AnimationNode, but it is impossible to count loops correctly since there is no semantic way to estimate the remaining time, so it should not be done.

Before:

Desktop.2023.09.22.-.17.35.14.09.mp4

After this PR:

Desktop.2023.09.22.-.17.35.14.10.mp4

Current AnimationTree API is also not semantic about estimating the remaining time, but it is stable by returning HUGE_LENGTH when information is missing to correctly estimate the time remaining.

I have written some of my opinions to RocketChat, so first, NodeAnimation should not be modified in the current stage. So first, we need to add a property to NodeSync to estimate what the remaining time is from for semantic transitions.

And I think the addition of this property should allow Nodes to have their own custom time, and allow Nodes downstream of NodeAnimation the possibility to override the upstream time remaining.

Then, finally we may need to do some parts of what this PR is trying to do ...or this PR could be a complete distraction. But for now, I warn you that it is very dangerous to rush to do this PR now.

If you want to solve #81715, what we can do at this stage is to implement loop_override, as I mentioned there.

And what could be improved in the GUI for now (because we don't have the way to estimate the semantic remain time of the animation that StateMachine's Node is processing), it would be to display HUGE_LENGTH (means remain time is unknown) to the user as an explicit symbol (but the PR for about that is not there yet).

[SaracenOne]

Thanks again for looking at this. I do agree we probably need to investigate this issue more deeply and be somewhat of a quick-fix for what does seem like a deeper architectural problem. That being said, I do think this would be worth keeping around since cumulative time would allow for a more flexible transition system, but I agree about not rushing a change like this, hence why I'm keeping it in draft form. Either way, I have addressed some of the issues raised including non-looping animation looping (that was an oversight on my behalf). Still need to investigate first-frame breaking issue.

[SaracenOne]

@TokageItLab I've looked more into the issues and I'm starting to understand them a bit better. While I think I fixed most of what was raised, I've decided that it makes sense to roll back the more risky architectural changes to the time value, and just limit it to the workaround which should re-enable the display of the progress bar on looping animations.

While I still think the change the time value makes sense for future architectural reasons, I've moved it to seperate branch and will revisit it with a more substantial proposal to back it up. I'm hoping this PR should be good enough for now. Modified the PR title to better reflect this and moved it out of draft status

[TokageItLab]

Indeed, it is possible to retrieve the length of a NodeAnimation if it is limited to NodeAnimation, and it may be able to display that correctly as a GUI.

However, searching for the path in every frame when playing NodeAnimation is a performance killer and quite annoying. Although it's just the editor, so it won't be a problem at runtime.

These codes should will be unnecessary in the future if remain time can be retrieved semantically, but I am still doubtful that this workaround is necessary at this stage, and I am negative about this implementation. Even if we merge this at worst, we should leave a comment that these codes need to be removed at some point.

[SaracenOne]

I agree this is a suboptimal approach, and figuring it out how to implement time tracking in a more holistic way would indeed be preferable. I consider this more of a backup plan just in case we can't decide on an optimal solution before release.

I'll look more into problem in the meantime.

[TokageItLab]

Alright, I wrote the proposal godotengine/godot-proposals#8083. It will supersedes this PR.

[akien-mga]

Superseded by #87171. Thanks for the contribution!