Rework AnimationNode process for retrieving the semantic time info

[TokageItLab]

• Closes Make each AnimationNode have time information to retrieve time semantically and make it can be passed bidirectional godot-proposals#8083

Transfer detailed time information bidirectional between AnimationNodes

For the sake of explanation, assume that "Output is the most upstream" and "AnimationNodeAnimation is the most downstream", and imagine that information such as delta and seeked is flowing from Output to reach AnimationNodeAnimation.

This PR ensures that process() returns the animation length and position, not the remaining time. It means that the informations are passed from downstream to upstream to calculate the exact remaining animation time.

Also, upstream to downstream, the time information was a mixture of delta and current position like seek ? current_pos : delta, but now the exact delta and current position are passed separately. By allowing the exchange of semantic time information in both directions solves some of the time-related problems.

Also, this PR adds a break_loop_at_end option to some AnimationNodes to solve transition with looping animation issues.

• Fixes AnimationNodeStateMachine don't play synced animations #67953
• Fixes Animation travel not working when an animation is looped #79208
• Fixes StateMachine transition Sync not working when using looped animations #79505
• Fixes Looped animations can't transition automatically in state machine #81715
• Fixes get_current_play_position() always returns 0 for all looping animations #82581
• Fixes Blendspace2D BLEND_MODE_DISCRETE_CARRY doesn't carry and resets for each animation #86984
• Fixes OneShot ignores fade out time when combined with TimeScale #87009
• Supersedes Fix display of progress bars on looped animations. #82089
• Supersedes Fix StateMachine transition Sync not working on looped animations #84586

Add custom timeline

This allows some time-related modification (length, timescale, offset, loop) on NodeAnimation with referring to the Animation without modifying the Animation resource.

For example, if you want to blend a walking motion with a running motion, simply set the same length and align period of the different length animations in NodeAnimation, then simply place a TimeScale upstream and the synchronization / adjust speed will work.

• Supersedes Added animation synchronization #62424
• Supersedes Implement LoopModeOverride to AnimationNodeAnimation to properly manage the loop state for each AnimationTree #79400

Other changes

The inconsistency between NodeBlendSpace and NodeBlend as to which time information was given priority in blending has been unified to give priority to the highest blend amount. As noted in godotengine/godot-proposals#8083, it would be ideal to allow users to make choices about obtaining preferred time, but this was abandoned because it would require more work. I have reduced the number of cases where the amount of time remaining is compared, which should reduce the number of cases where this is a problem.

The remaining time can be calculated from the length and current time, but calculating the length and current time from the remaining time is irreversible, so there is breaking compatibility as the changing return type of the GDVIRTUAL _process() as double remain to Dictionary node_time_info. As noted in the documentation, the time information must be stored and returned in a dictionary type. As a result of discussions at the animation team meeting, the Dictionary type has significant performance issues so _process() will be deprecated temporary. We plan to release a new API as soon as struct is implemented in the GDScript.

Production edit: closes godotengine/godot-roadmap#63

[jcostello]

Supersedes #62424

Now animations would sync properly?

[TokageItLab]

Supersedes #62424

Now animations would sync properly?

Although not exactly the same, this PR adds the ability to align the length of those animations, so it should work as an alternative.

The problem with #62424 is that if the animation time is modified upstream by sync, the correctness cannot be guaranteed if there are branches or nests in the downstream. However, this PR should be safe because such time modifications are limited to the most downstream as NodeAnimation.

[SaracenOne]

I think on principle, this is fine and pretty close to the solutions we discussed to solving the underlying semantic time calculations. While quite a few options have been added, I think hiding them behind the contextually relevant use_custom_timeline toggle goes a fair ways to avoiding my concerns about frontloading complexity on the user experimenting with this system.

I know we discussed the possibility of alternative modes for automatically deriving semantic time in situations where it might be ambigious, but honestly, I think just letting the user set an explicit loop time in those situations will be okay.

There are a few issues I want to point out though. I'm not sure if the custom timeline ping-pong is working correctly; it seems to get locked at the end when setting in. I'll keep poking around at it though before writing a formal review.

[fire]

Animation team meeting notes:

1. Lyuma: Needs more testing.
2. Fire: where is Tokage test scene?
3. Saracen will do a formal review.
4. Saracen: update and create a general animation test suite.
5. Tokage: Rework for nested AnimationNodeStateMachine #75759 As for StateMachine
   Tokage says:There is a demo project out there dedicated to testing the behavior of nested StateMachine travel and start
6. Lyuma: Does semantic time pr affect nested test machines? Do it impact all cases? What is semantic time fixing?
7. Saracen: If your animation is looping you don't know where the loop point is. You don't know the time of the animation.
8. Lyuma: Nested state machines are also ambiguous and is added to this pr.
9. Tokage: https://github.com/godotengine/godot/files/13993286/state_machine_improvement_demo.zip

[fire]

Animation team meeting notes:

1. Lyuma: Flattened state machines. Want to use nested state machines for using.
2. Saracen: Start point and end point. Group button wasn't merged. Proposal to add: "Collapse a bunch of state machines to a group."
3. Saracen: Nervous and want everyone to test more. Not only Saracen. More more rigious testing for big tests.
4. Tokage: Testing projects: https://github.com/godotengine/godot/files/13993496/state_machine_improvement_demo_2.zip
5. Lyuma: Need to be able to redistribute the test cases.

[fire]

Animation tests from Tokage:

I think we can test this by preparing several nested StateMachines with the same connection and different Types, and checking the value after each transition.

The same goes for NodeTransition and Oneshot.

[adamscott]

Comments from the production team:

• @TokageItLab Could you confirm what you did or what you didn't have the time to do from the list of the animation team.
• Animation team (@fire, @lyuma and @SaracenOne) + @TokageItLab, is it possible to have an explanation on how the tests work? To have instructions on how to run the test and a clear "before"/"after" state.
• In general, to know if it's mergeable for 4.3, we need a reassurance from the team that you agree on the design and that it's safe to merge for users upgrading from 4.2.

[David-Ochoa]

Add custom timeline

This allows some time-related modification (length, timescale, offset, loop) on NodeAnimation with referring to the Animation without modifying the Animation resource.

Just received a notification about this. I can tell you this will not solve all the issues, I made something like this manually changing the animation length for walk and run, but couldn't do it for the idle animation (it's bigger compared to the other two) , and after a few cycles the blending broke again.
I fixed it by changing all the animations based on the current position in the blendspace, I made a video (sorry it's in spanish) to explain the solution:
https://youtu.be/GPcai3hZFGc?si=tBoqSqValg7mUXlO

[TokageItLab]

@David-Ochoa Unfortunately, beta1 has broken sync sorry, so I recommend trying beta2 or later. And in my opinion, the workflow for adjusting the period and length, which was particularly tired, should be reduced as follows:

If there is walking in 4 directions and running in 4 directions

• The animation cycle had to be modified in AnimationPlayer or Blender, but can now be modified in AnimationNode
• By unifying the length in the AnimationNode, eight timescale nodes is no longer needed and now only one node is needed to adjust the whole speed of the movement with foot step

[David-Ochoa]

@TokageItLab
I'm trying to understand how the custom timeline works in my setup:
I have 4 animations
Idle - 10s
Walk - 1.066667s
Run - 0.7333333s
Sprint - 0.4s
I choose WALK as my base animation, then in the BlendSpace1D I set all the animations to use a Custom Timeline with the timeline length set to the WALK animation's length (1.066667s).
If I do that setup the IDLE animation plays too fast, so I change the timeline length to a factor of the WALK animation (WALK * 6 = 9.6s) and then the animation plays correctly.
My SPRINT animation is playin too slow, I change the timeline length to 0.5333335s (WALK / 2), but it blends incorrectly.
I made a video with the steps I mentioned:
Custom Timeline
Am I doing something wrong or how is supposed to use this feature?

[David-Ochoa]

And this are the same animations but changing tha length based on the range of the Move Position slider:
Change animations length

[TokageItLab]

@David-Ochoa So, do you think what we need is CustomTimeScale, as we should make StretchTimeScale becomee a preset and let the user enter a numerical value optionally?

My question is about the cycle of the steps. For example, if a walk animation has one left and one right footstep, and a run animation has two left and two right footsteps in one animation, even if the lengths are the same, this means that the run animation will loop for 2 cycles during 1 cycle of the walk animation.

Considering the solution to this case, the recommended solution is to extend the other animations to the animation with the largest number of cycles.

Then, align all animations to 1 second. Since the run animation has 2 cycles here, we want to adjust the timescale of the walk animation so that the number of cycles of the walk animation is aligned, but the lack of a custom timescale prevents us from doing so, is that your problem?

[David-Ochoa]

@TokageItLab No, the solution is to choose the correct frame to blend based on the current animation weight and the animation's length, I'm scaling the animations because I don't have acces to the internals of the AnimationTree and this way I feed the correct frames to the actual system (scaling the animations is a hack).
I don't know how to explain how this works now and how to change it in the internal to work this way, Right now I'm not at my Godot machine, maybe later I'll make more tests.

[TokageItLab]

@David-Ochoa CustomTimeline is mainly intended for synchronization assuming that all cycles are matched. If you are looking for something more than CyclicSync #34179, it is probably something beyond the role of CustomTimeline and requires a different proposal.

FYI, I expect the following to be the general setup for synchronization of footsteps by CyclicBlending. If you want to gradually change animation with increasing moving speed, you need to blend synchronized walking, running, and sprinting in the same cycle, then adjust the speed of the cycle according to the master timescale.

If there is root motion, that would also need to be taken into account. In the past, I have experimented and written a paper https://tokage.info/lab/?id=3 (Japanese), so I basically believe that we can normalize based on the maximum moving value, but it is a bit complicated because of the calculation and the possibility of needing to use a straight line with inflection points for the blend value.

[David-Ochoa]

@David-Ochoa CustomTimeline is mainly intended for synchronization assuming that all cycles are matched. If you are looking for something more than CyclicSync #34179, it is probably something beyond the role of CustomTimeline and requires a different proposal.

Yes, it's more complicated than what this solves.

FYI, I expect the following to be the general setup for synchronization of footsteps by CyclicBlending. If you want to gradually change animation with increasing moving speed, you need to blend synchronized walking, running, and sprinting in the same cycle, then adjust the speed of the cycle according to the master timescale.

Yes, I'm achiving this with the setup I have. I just wanted this to be incorporated in the AnimationTree because it's too complicated with a BlendSpace2D

If there is root motion, that would also need to be taken into account. In the past, I have experimented and written a paper https://tokage.info/lab/?id=3 (Japanese), so I basically believe that we can normalize based on the maximum moving value, but it is a bit complicated because of the calculation and the possibility of needing to use a straight line with inflection points for the blend value.

I'm using root motion, but as I mentioned it's in a BlendSpace1D, what you mention in the paper would apply for a BlendSpace2D, and at the moment I won't touch it.

I'll change my setup to scale the Custom Timeline instead of adding a Timescale and let you know how it went.

[TokageItLab]

1. Blend the walk, run, and sprint animations with the same cycle (e.g. 1cycle), length (e.g. 1s), and timing of the same footstep (e.g. 0.0s: stamping a left foot, 0.5s: stamping a right foot) by CustomTimeline.
2. Add one NodeTimeScale on the result of those blends.
3. Link the blend amount and the Scale of the NodeTimeScale on the gdscript. Use remap(), Curve or Gradient as needed. Or it might work to have the animation track have a Timescale value.

If they are always synchronized, there should not occur any discrepancy between each of their cycles in 4.3.beta2 or later.

[David-Ochoa]

@TokageItLab I made an example using the method I implemented with the time scale now using the Custom Timeline, if you move the slider of Move Position (not the blend position itself) the Timeline Length is updated, and the animations blend correctly.
My point is: a script should't be necesary, all the calculations could be done in the BlendSpace, it has all the information to get the correct frame to blend.
TestCustomTimeline.zip

[David-Ochoa]

I belive it needs a little more work, I'm not 100% sure the time I'm setting is the correct one

[TokageItLab]

I looked at the project and it looks like the problem is that the imported animation is not trimmed correctly. For some reason all animations are imported with the same length as the idle. Is it your intended import? The StretchTimeScale unifies the length of the animation to the specified length. So this will work if you trim the part of the imported Animation where the key does not exist in the resource.

In other words, the ability to trim an animation by CustomTimeline and StretchTimeScale are exclusive. This is almost identical to the case of the cycle problem I described above. As for the trimming, it may be an issue with the fbx importer. You could send a separate issue.

[lyuma]

the imported animation is not trimmed correctly ... all animations are imported with the same length as the idle

@David-Ochoa Indeed, I confirmed what Tokage said with another FBX application to verify that this is the case:

You can see that each animation is just over 10 seconds long with a lot of blank space at the end.

As for the trimming, it may be an issue with the fbx importer

Trimming in The new ufbx importer is implemented is implemented differently than trimming in glTF (or the old FBX / FBX2glTF import): In FBX, it uses the start and stop times in the header.

Using FBX2glTF, I do see idle is 10.033 seconds; run is 0.6667; sprint is 0.5333; walk is 1.0667, so that importer is trimming it based on keyframes, which could be incorrect in other ways.

You are able to use the advanced importer to manually trim each clip. To do this, note the end frame numbers of each animation in the AnimationPlayer. Then, in the advanced importer, use the "Slices" option and choose 1 for the amount. and use the start and stop frame numbers you noted. (NOTE: The import dialog uses frames for slice start and stop as per convention. To convert a time to a frame number, multiply by the FPS (30))

I do think parts of this pipeline could use some usability work. For example, the skeleton is currently invisible in the advanced import dialog: there's an open PR but it was not ready in time for the 4.3 release cutoff.

Once you have verified that you have the animation start and stop times correct in the .FBX, please create a new issue with the fixed reproduction project. Comments on an old PR get easily lost and so it will be harder to fix without an issue number to reference.

[David-Ochoa]

@TokageItLab , @lyuma I don't have the problem of the trimming, I'm using the FBX2glTF. I will make more tests and then open a new issue.

[TokageItLab]

@David-Ochoa Then I think you just don't understand the functionality and recommended workflow.

I insist that you just make the length time of each animation the same, and this can be accomplished by having the length of the CustomTimeline be the same and StretchTimeScale enabled. Then simply adjust the speed of each animation with TimeScale for the blended animation.

The following project should work fine with no code. (Made animations unique to eliminate import problems and add ValueTrack of TimeScale.)

TestCustomTimelineFixed.zip

If you want to reproduce the original animation time exactly when the blend amount is 1, simply calculate a TimeScale value by CustomTimelineLength / OriginalAnimationLength.

BTW, when you assign an animation to an AnimationNode in the editor, the playback start time may shift, causing the animation to be out of sync (turn on/off the active option to fix it for now), but this is a preview issue in the editor and should not be a problem unless the assignment is done at runtime.

It may be technically feasible to automate this TimeScale adjustment by incorporating it into BlendSpace, but I don't agree.

• The animation lengths of nested AnimationNodes can change anytime by blending, transitoning, changing statemachine's state and etc., so make user to blind for that informations, there is no transparency
• Also, in cases where you do not want the TimeScale to reflect the length of a particular animation in the BlendSpace, state management is required, which complicates the code in the core
• Moreover, it is necessary to scale the overall length of the actual imported animation in order to adjust the velocity. This would be more tedious than adjusting the value of the TimeScale key

So, as mentioned in the #87171 (comment), CustomTimeline, the most downstream time alignment implementation, should be the simplest architecturally possible solution to the synchronization problem.

If some automation is needed there, the user could implement an add-on that inserts a TimeScale track by referencing the length of the CustomTimeScale and the length of the original animation.

[David-Ochoa]

@David-Ochoa Then I think you just don't understand the functionality and recommended workflow.

I do, but I wanted it to be included in the Blendspace itself, but as you mentioned in the post you want it to be kept simple.

Why I wanted it to be included? Because this example is too simple, I have state machines inside the BlendSpace and getting the times based on substate machines gets complicated (not to mention Blendspace2d could be a nightmare)

So for the time being I'll implement it adding a timescale in front of the blendspace, the code is practically the same for the two options.

Thanks for your time, I know it's complicated to acomodate all the use cases and I know the Godot phylosophy is to provide the basic functionality and specific use cases should be implemented by the users.

This is a video implementing the recomended workflow vs what I was trying to acomplish scaling the timescale.

2024-06-26.14-45-06.mp4

[TokageItLab]

@David-Ochoa Have you checked out the project I attached above?

TestCustomTimelineFixed.zip

It can provide the same thing as your video without any code, you just use CustomTimeline to match the length with enabling StretchTimeScale and add ValueTrack of the TimeScale to the animations. Try moving the blend positions in BlendSpace1D.

bs.mp4

The same thing can apply for BlendSpace2D, and StateMachine in BlendSpace, just add a ValueTrack of TimeScale to each animation. And given that StateMachine cannot detect the correct length, now I was convinced that the ValueTrack of TimeScale method seemed to make sense.

So I don't see any major case that it doesn't cover at the moment except for the issues that are trimming and scaling exclusivity.

[David-Ochoa]

@TokageItLab Yes, I saw the project, but modifying the animation to reference the TimeScale in the statemachine is not practical because I'm using the animations as a library and they will be included in different statemachines that will not have the same structure. But looking at the example gave me an idea to simplify the TimeScale calculations.

[TokageItLab]

@David-Ochoa In my opinion, the use case that would cause the problem you are concerned about is where there are multiple StateMachines with TimeScale nodes upstream and animations of the same resource are placed within each StateMachine, but I believe that is a niche case:

WalkA - StateMachineA - TimeScaleA
　　　　　　　　　　　　　　　|
　　　　　　　　　　　　　　Blend2 - Output
　　　　　　　　　　　　　　　|
WalkA - StateMachineB - TimeScaleB

In such cases, it is common practice to make the resource unique, but it is also optional to change the value of the dummy property without directly changing the TimeScale value, or to manage the TimeScale by relying on the state yourself, without relying on the animation resource. As long as the blended animation lengths are aligned by CustomTimeline and StretchTimeScale, synchronization should not cause any discrepancies (although I remember some problems with StateMachine's Sync feature, but CustomTimeline has nothing to do with it. If you want accurate Sync, consider using NodeTransition instead).

Or you should consider reducing the number of TimeScale nodes as follows:

WalkA - StateMachineA
　　　　　　　|
　　　　　　Blend2 - TimeScale - Output
　　　　　　　|
WalkA - StateMachineB

After all, complexity depends on the number of TimeScale nodes that need to be managed, and reducing them should solve most of the problems. At least, this configuration (only one TimeScale of the movement by footsteps) should be applicable to any 4-way or 8-way movement in TPS/FPS, which are the most used cases.