Root motion implementation

[Hilpogar]

Objective

Closes: #23355
Final step for the root motion implementation !

Solution

Add the RootMotion component that stores the informations about the root motion for the current frame:

pub struct RootMotion {
    /// Translation delta with the previous frame.
    pub translation_delta: Vec3,
    /// Rotation delta with the previous frame.
    pub rotation_delta: Quat,
}

Add the RootMotionMode to control how the RootMotion is extracted. There are 2 modes, one with only translation and one with translation and rotation :

pub enum RootMotionMode {
    /// Extract only translation from the root target.
    Translation,
    /// Extract both translation and rotation from the root target.
    #[default]
    TranslationAndRotation,
}

Add the field root_motion_target and root_motion_mode to the AnimationPlayer:

pub struct AnimationPlayer {
    <...>
    root_motion_target: Option<AnimationTargetId>,
    root_motion_mode: RootMotionMode,
}

Testing

The test_root_motion tests the extraction of the root motion in a configuration with 2 blended clips. The test is done once with the clips running forward and once backward.

---

Showcase

You can run the root_motion example to see it in action.

[mgi388]

Note for someone to check if this asset is allowed to be committed or if it needs to be a remote/web asset.

[Hilpogar]

Is this because of potential copyright or licensing issues with the model, or is it just to avoid overloading the repository? If it’s a licensing issue, please note that this is simply the existing fox model to which I’ve added root motion to the running animation.

[mgi388]

It’s to do with repo size / bloat. E.g., see #23567 (comment)

I don’t know the bytes threshold but AFAIK, I think bevy wants to avoid putting more assets in this repo if possible.

[ChristopherBiscardi]

We need a comment indicating what the changes to the file are here. Notably in case it ever needs to be modified or understood in the future. Otherwise it will be hard for future contributors to comprehend why this is a separate file.

Personally I think a FoxRootMotion.md that explains the steps taken to create the new file from the original would work.

[alice-i-cecile]

I agree with that. This is not so big that I think we have to move it out though IMO, but clear guidance here would have been nice.

[Hilpogar]

I've created a ForRootMotion.md containing the instructions to recreate the file. This is a very slight modification of the file, do you think the instructions are clear enough?

[Hilpogar]

I realized that removing the AnimationPlayer from an entity could "leak" the RootMotion component. So I added a hook to remove it with the AnimationPlayer. I've also added the cleaning behaviors in the tests.

[emberlightstudios]

Thank you for implementing this....

Other game engines that I have used define the root motion behavior on a per clip basis. But here we are doing the graph evaluation first to blend clips then extracting the delta at the end. I think this is a very clean design. Well done.

I don't really have much to add. Maybe in the example you should put the apply_root_motion in PostUpdate and run it after AnimationSystems, but that's about it.

[Hilpogar]

Good point, I change that 👍

[ChristopherBiscardi]

This actually looks good, and I'm approving with the exception that I feel it is important to write more documentation around what/why the FoxRootMotion.glb exists and how to recreate it in the future if necessary.

[ChristopherBiscardi]

We need a comment indicating what the changes to the file are here. Notably in case it ever needs to be modified or understood in the future. Otherwise it will be hard for future contributors to comprehend why this is a separate file.

Personally I think a FoxRootMotion.md that explains the steps taken to create the new file from the original would work.

[alice-i-cecile]

I don't understand why this needs to exist. Why can't we just keep the component around with None?

It feels like we should either be adding and removing the component, and store a raw Entity target, or keep the component around using Option with None as a valid state.

[Hilpogar]

I like the idea to have the RootMotion component only inserted on entities where the root motion is enabled so it's easier to iterate on them. But I agree that this solution isn't perfect.
I think it's better to remove the root motion configuration (target + mode) from the AnimationPlayer and put it directly inside the RootMotion component. If we want to remove the RootMotion we just have to remove the component.

[Hilpogar]

In the end just moved the root motion configuration in a separate component with a hook that removes RootMotion when it's removed. Do you agree with this approach ?