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

Added animation synchronization #62424

Closed
wants to merge 1 commit into from

Conversation

and-rad
Copy link
Contributor

@and-rad and-rad commented Jun 26, 2022

Fixes #23414

This PR adds the ability to synchronize the playback speeds of animations that are blended together in an animation tree. It keeps animations in sync even when they have different lengths.

Without synchronization, animations used in BlendSpace1D/2D or connected to Blend2/3 become desynchronized when their lengths don't match:

before.mp4

With synchronization, they stay in sync, depending on the current lead animation:

after.mp4

Background

I was looking for a way to implement #23414 as a necessary feature for third-person character animation. My original goal was to make this feature work without any extra steps required from devs, but that proved impractical and in some situations even undesired. I eventually went with a system similar to how it works in Unreal Engine.

The current implementation uses the concept of Sync Groups to decide which animations belong together. AnimationNodeAnimations have a new Sync Group property that is set by developers to a StringName of their choosing. Based on the final blend weights of all the animations in a sync group, a leader is determined and the other animations in that group are timescaled to match the leader's playback length. Here are two videos demonstrating the feature in the editor and at runtime:

demo.mp4
runtime.mp4

Limitations & Improvements

The feature as-is works for the vast majority of cases where such synchronization is desired: locomotion state machines and blend spaces dealing with walk and aim cycles. There are ways to make it even better, but they come with more significant changes to animation nodes, which is why I didn't implement them in this first draft.

TimeScale Node

The first limitation has to do with the TimeScale node. The system works as expected as long as any TimeScale node in the graph affects all animations in a Sync Group equally. That is, the picture on the left works, the one on the right doesn't:

timescale

This has to do with the way the TimeScale node works internally. It's a limitation that probably won't be a very big problem in practice, but still might be nice to make it work. The thing is, the only way that I know how to do it would make the TimeScale node effectively obsolete. Instead, there would be a new Playback Speed property on every node to control a branch's playback speed directly on that node. This would allow us to accumulate the playback speed and pass it down to the leaf node in a way that the Sync Group system can handle it properly.

An advantage would be that it declutters the animation tree, but a disadvantage might be that the playback speed property would exist for every node even if it wouldn't really make sense for that particular node. We can work around that by exposing the variable as a property only for those nodes where it makes sense. For now, I added the playback speed property to the AnimationNodeAnimation class only. This allows us to scale an individual animation up or down without breaking synchronization.

playback

Blend Logic

Another thing that would improve the end result at the cost of slightly complexer blend logic is changing the way that the final animation length is calculated. Right now, all animations in a group are scaled to the length of the leader. This is good enough, but it can produce notable speed pops when the group's leader changes.

Given two animations of different lengths:

Concept1

This is what happens in Unreal Engine and what this PR is currently doing:

Concept2

What I have in mind instead is this:

Concept4

The animations are not merely scaled to match the leader, but all of them - including the leader - are scaled according to their weight. This would prevent any popping and would make leader switches buttery smooth.

Desynchronization

There are still instances when the animations desynchronize. Most notably, while assigning sync groups in the editor. Restarting the animation tree fixes that, so I don't know if this is something that needs to be addressed necessarily. Still, it might be desirable to add some resynchronization logic as a response to properties being changed.

Testing

I invite everyone to throw their animation trees at this to stake out where the current implementation breaks. Things I haven't tested yet include:

  • Root motion
  • BlendSpace2D (I am currently setting up animations for a proper test case)

@TokageItLab
Copy link
Member

TokageItLab commented Jun 26, 2022

I agree that the timescale needs to be adjusted for synchronization, but in the end that is not enough for synchronization.

For example, an animation with the right foot first will not blend well with an animation with the left foot first. The phase needs to be adjusted.

Also need to think about RootMotion. An animation that moves 2 meters per second to the right and an animation that moves 5 meters per second forward will move in different directions than the user input, even if they are synchronized. Additional TimeScale adjustments may be necessary depending on the maximum value of the movement, and in some cases the animation stride length may need to be changed with NodeAdd.

I think it is impossible to cover these various use cases with the AnimationTree capability alone.

I suggest that those TimeScale adjustment should be done like CharacterController with a user script having animation as an argument and distributed in AssetLibrary/GDExtention, etc.

In fact, CharacterController, which specializes in such automatic adjustment of animations, is sold as an Asset for Unity.

This PR seems to break a lot of things for one use case. The only thing I can say for sure that is currently missing in AnimationTree for that use case is Sync option in NodeBlendSpace2D and NodeTransition.

If you want to implement automatic timescale adjustment, it should be an Asset as I said above, or at the very least it should be a new node like the #34179 implementation or an option in NodeBlendSpace2D, so it should only be resolved within a specific node.

@and-rad
Copy link
Contributor Author

and-rad commented Jun 26, 2022

For example, an animation with the right foot first will not blend well with an animation with the left foot first.

That's true, but I would consider these two different use cases. This kind of animation blending would break in other engines too, which is why in Unreal there's also the notion of sync markers. I think there should be a different PR for that.

The synchronization is based on sync groups. The animation in the group
with the highest final blend weight becomes the group's leader. All
other animations in the group are time-scaled to match the leader's
playback length.
@jcostello
Copy link
Contributor

@and-rad can this be reworked into a diferent node? I find it very necesary to actually sync animations

@AThousandShips
Copy link
Member

@AThousandShips AThousandShips removed this from the 4.x milestone Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sync animation in BlendSpace1D/2D
4 participants