Root motion translation does not consider current rotation of object which will be applied root motion

[HitalloExiled]

Godot version

v3.4.2.stable.mono.official

System information

Windows 10, GLES3, RTX 3060

Issue description

In the current state, root motion always returns a transform with the delta of rotation and translation.

This works fine for most scenarios.

However, when an animation has both rotation and translation, things get complicated.

It is necessary to save the orientation at the beginning of the animation, apply the translation relative to the saved orientation while rotating the object using the root motion delta rotation.

Example:

using Godot;

class Player : KinematicBody
{
    private AnimationTree animationTree;
    Transform orientation;
    string state;
    Vector3 inputVelocity;

    public override void _Process(float delta)
    {
        Vector3 velocity;
        var rootMotion = animationTree.GetRootMotionTransform();

        if (state != "Turning")
        {
            orientation = GlobalTransform;
            velocity = inputVelocity;
            LookAt(inputVelocity, Vector3.Up);
        }
        else
        {
            velocity = orientation.basis.Xform(rootMotion.origin) / delta;

            var rotation = rootMotion;
            rotation.origin = Vector3.Zero;            
            GlobalTransform *= rotation;
        }

        this.MoveAndSlide(velocity);
    }
}

This can work on animations where translations end up with no delta translation and small/zero xfade.
Otherwise, if the animation ends with some amount of delta and/or xfade, things will break.

Digging on the issues i found this PR #29458 that tries to fix that.

Which unfortunately appears to have been abandoned.

Steps to reproduce

Create a root motion animation only with translantion.
Create a root motion animation with translantion an rotation.

create one state machine that travels from one state to another.

applies thee root motion to the transform

GlobalTransform *= rootMotion;

Minimal reproduction project

Example.zip

Presse W to walk and A or D to turn.

[TokageItLab]

It is fixed in 4.0 but still broken in 3.x. I misunderstood. The regression mentioned in #60774 does not exist in 3.x. This issue is for about another problem.

[LasseSchnepel]

Hi, in Godot4 Beta6 this still shows the same broken behaviour. Cannot confirm the fix.

[TokageItLab]

@LasseSchnepel Can you send MRP or video?

FYI, there is a DemoProject that shows that the rotation itself is gotten correctly. #29458 (comment)

[LasseSchnepel]

Hi Tokage, my assumption is that when I play the animation in the player (without the tree or with the root motion), the movement looks good. In your example project, the figure rotates and moves constantly forward. When used with root motion, your figure walks along a circle. I would expect that the figure moves as it would with the normal animation. (but with by the mesh at 0/0 and moving the outer node). This is also how root motion looks in all other engines afaik.

The rotating while moving forward animation is nothing one could easily animate in e.g. blender. In blender I want to walk my figure on a circle and then have Godot replicate that movement with root motion.

I attached an example project of mine (even though it is Godot.NET, is uses no .NET code, so should be usable without .NET):
rootmotion.zip
Press arrows up and left to move forward / rotate left.

[TokageItLab]

@LasseSchnepel Thanks for the project, that is a good reference.

After looking into it:
The root motion rotation is definitely usable. I think the problem you are implying is similar with #67125¹ rather than this issue. Here is the project I fixed.

fixed_rootmotion.zip

And I found several problems unrelated to this issue:

• delta in PhysicsProcess seems to be broken currently 🤔
• root motion scale is broken (I will send a fixing PR soon)
• the Left animation in your project is not at 90 degrees at the last keyframe (I don't mean to nitpick, but please be aware that the player will shift a bit after the rotation)

In any case, thanks for a good reference project. I don't see the need for any new implementation for root motion rotation for now, but I feel that the documentation needs to be a bit more detailed on how root motion is applied.

Footnotes

1. AnimationTree blend - initial rotation ignored when using root motion and changing the blend value in code. #67125 is a problem where they want to use the first keyframe as the initial value, but in your case they want to use the current player's orientation as the initial value ↩

[LasseSchnepel]

Hi @TokageItLab ,

thank you for checking on my project. When I take your zip file and run it, I still see the same issue:

• Step 1: Left rotation: fine, player rotates as expected
• Step 2: Going Forward: broken, player moves into global forward instead of local forward direction, so it slides sideways instead of walking forward (global left).

If I add current_rotation = transform.basis.get_rotation_quaternion() to the place where I travel to forward it looks better. But this is broken when I press the keys while another animation is still running (as I'm not storing the current_rotation at the start of the animation but somewhere in between the rotation). I wouldn't know at which time I should store the current_rotation, especially with complex animation trees, with blending and auto-advancing state transitions.

Isn't the current_rotation something, the AnimationTree should/could handle?

[TokageItLab]

Step 2: Going Forward: broken, player moves into global forward instead of local forward direction, so it slides sideways instead of walking forward (global left).

I have not added current rotation there yet, so you can just add it.

I wouldn't know at which time I should store the current_rotation, especially with complex animation trees, with blending and auto-advancing state transitions.
Isn't the current_rotation something, the AnimationTree should/could handle?

I don't think so, the extracted value as the root motion is not yet affected by the current rotation. So the correct process is to blend the animations under the assumption that they are all facing the same direction (means be not affected by the current rotation), and apply the current rotation on the script side after.

The problem would be that the StateMachine never actually plays the next animation until the end of the animation. Yes, maybe retrieving should be done each time, but I don't think that is the role of the AnimationTree.

As explained in #29458 (comment), it is easier to get the object to which the root motion is applied on the script side, and since the object is not always only one, I do not think that AnimationTree should have that role. I think it would be sufficient to connect the retrieving method to the transition signal of the StateMachine.

[LasseSchnepel]

Hi @TokageItLab, if I understand you correctly, rootmotion will never work with the statemachine. I struggle to understand, how I should solve a standard character animation system without a statemachine and by blending. Can you make an example?
In my example project, there's animation for rotation to the left and moving forward, but in a final application, I'd have rotation to the right, several attacks (that might as well rotate & translate the player).
I tried several things, but never could get it to work, especially, when Transitions have xfade, exit animations immedietly etc.
Right now I'm helping myself by checking the current node in the statemachine and applying a baked motion on my character. This comes with many obvious drawbacks (xfade doesn't work, animation and code have to match, ...)
I would be really happy if Godot would support my use case, but I see no way how to achieve it. But of course, I'm happy to see any working examples.

[TokageItLab]

@LasseSchnepel It is natural that root motion including rotation will not work correctly if there is xfade in the transition. Previously, the default value of xfade for NodeOneshot has been changed 0.1 to 0 for that reason.

Interrupting animation is not yet implemented in StateMachine. For that reason I use NodeTransition and NodeOneShot a lot in my projects instead of StateMachine. (StateMachine could be improved in the future)

At least with NodeTransition/NodeOneShot and an xfade of 0, it is possible to fire the animation and retrieve current rotation at the same time, but I am not sure if it matches your use case. At least, I think that rotation by root motion is a special case, like the spinning sword attack animation does not change the actual direction of the player. So I think it is common in 3D Games to manage the rotation on the script side, without using only root motion.

[HitalloExiled]

@TokageItLab I had played around with the godot's source code a bit and was able to fix the root motion behavior just by storing the global skeleton basis at the start of the animation and using it to xfom the translation passed to the rootmotion object.

You can see in this commit.

With the fix this line was enough to mimic the behavior we have in Unity using your sample.

player.transform *= root_tr

However, I'm not confident enough in my fix to submit a PR. I imagine this would be a breaking change and should be enabled by a flag.

I'm also not familiar with Godot's contribution guidelines.

[TokageItLab]

@HitalloExiled The current_rotation needed to move the root motion is not always global_transform. For example, if the player is a child of a node in a rotating field, parent_rotation is required. Therefore, it still should be handled by the GDScript side and not by the AnimationTree side.

[TokageItLab]

@HitalloExiled Also, player.transform *= root_tr cannot be applied to Scale. As I wrote in the documentation, an accumlator is required.

What is needed in Godot core is not to default to code that can only be used for specific use cases, but make it easy for extracting values that are easy to use on the GDScript side.

[HitalloExiled]

The current_rotation needed to move the root motion is not always global_transform. For example, if the player is a child of a node in a rotating field, parent_rotation is required.

@TokageLab The transform used as a reference could be defined through a property.

Therefore, it still should be handled by the GDScript side and not by the AnimationTree side.

I would agree that this could be done via script if it weren't for the issue with xfade (that my fix yet doesn't solve it, I'm working on it.). It is currently not possible to work around this limitation.

And as stated in another comment, in Unity it just works. And it seems to me to be possible to implement the same behavior in Godot without compromising anything.

Also, player.transform *= root_tr cannot be applied to Scale. As I wrote in the documentation, an accumlator is required.

I'm ok with that. we just need the relative delta of translation.

[TokageItLab]

I'm ok with that.

It is not acceptable to add it to core just because you want to use the same code as Unity. Godot is not Unity, and all game engines don't need to handle root motion in the same way.

I would agree that this could be done via script if it weren't for the issue with xfade (that my fix yet doesn't solve it, I'm working on it.). It is currently not possible to work around this limitation.

So I say your suggested code is certainty not worthy to be included in core yet.

As I said above, what should be implemented in the core is to make it easy to get the values that can be used on the GDScript. Therefore, I agree with what you said in #29458 about getting the transition(xfade) position values from StateMachine/NodeTransition/NodeOneshot, and I recommend that you write that in proposal (https://github.com/godotengine/godot-proposals).

[HitalloExiled]

So I say your suggested code is certainty not worthy to be included in core yet.

I just want to show that it's possible and requires minimal changes.

As I said above, what should be implemented in the core is to make it easy to get the values that can be used on the GDScript. Therefore, I agree with what you said in #29458 about getting the transition(xfade) position values from StateMachine/NodeTransition/NodeOneshot, and I recommend that you write that in proposal (https://github.com/godotengine/godot-proposals).

I already created the proposal, but I see it only as an enabler to work around the current problem.

Having said that, I can create another proposal requesting the changes on the root motion, this should give an idea of the size of the need.

[TokageItLab]

I recently found this issue with RootMotionView, so I'm looking into it again.
As a result, I guess that what we need is not just a root motion delta, but an accumulator (the actual animation blend value).

So,

player.transform.basis *= tree->get_root_motion_rotation();
player.transform.origin += (tree->get_root_motion_rotation_accumulator().inverse() * player.transform.basis).xform(tree->get_root_motion_position());

I think it will work. (I will send a PR to add the accumulator later.)