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

Fix broken root motion scale & Refactor API & Add sample codes in documentation #69199

Merged

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Nov 26, 2022

Fixes #58061

  1. Fix root motion scale
    The current root motion scale is completely broken due to incorrect initial values and application process. This PR fix the initial value and the application process, and make the returned root motion scale to be able to used by the accumulator1.

Before with transform.basis *= root_motion_transform.basis:

root1.mp4

After with accumulator:

root2.mp4

rootmotiondemo.zip

  1. Match motion in RootMotionView and AnimationPlayer
    The apply order of the root motion position and root motion rotation differs between AnimationPlayer and RootMotionView. Considering the order in which Bone Pose is applied, the root motion position shuold not affected by the root motion rotation, so the current motion in RootMotionView is incorrect, fixed.

  2. Add sample code to document for apply root motion
    In above case, the root motion rotation must respect the object's orientation at the start of the animation. So I added the sample code in the documentation. Also, the scale needs to respect the object's value and accumulator at the start of the animation as well. I added it in the documentation too.

  3. Change API get_root_motion_transform() to get_root_motion_position/rotation/scale()
    For these reasons, it makes more sense to separate them as TRS than to take the root motion out as Transform3D.

Footnotes

  1. For example, if you animate 4 times and want the Scale to transition like 1 -> 3 -> 5 -> 7, you do not need to use the accumulator, but If you want to do Scale transitions like 1 -> 3 -> 9 -> 27, you need accumulator.

@TokageItLab TokageItLab added this to the 4.0 milestone Nov 26, 2022
@TokageItLab TokageItLab requested review from a team as code owners November 26, 2022 08:10
@TokageItLab TokageItLab changed the title Fix root motion broken scale and refactor API Fix broken root motion scale and refactor API Nov 26, 2022
@TokageItLab TokageItLab force-pushed the fix-and-refactor-root-motion branch 2 times, most recently from ddc5e48 to 20a7a7e Compare November 26, 2022 08:23
@TokageItLab TokageItLab force-pushed the fix-and-refactor-root-motion branch from 20a7a7e to 09adf5f Compare November 26, 2022 08:26
@TokageItLab TokageItLab changed the title Fix broken root motion scale and refactor API Fix broken root motion scale and refactor API and add sample codes in documentation Nov 26, 2022
@TokageItLab TokageItLab changed the title Fix broken root motion scale and refactor API and add sample codes in documentation Fix broken root motion scale & Refactor API & Add sample codes in documentation Nov 26, 2022
@TokageItLab TokageItLab changed the title Fix broken root motion scale & Refactor API & Add sample codes in documentation Fix broken root motion scale & Refactor API & Add sample codes in documentation Nov 26, 2022
@akien-mga akien-mga merged commit bb9cd40 into godotengine:master Nov 28, 2022
@akien-mga
Copy link
Member

Thanks!

@TokageItLab TokageItLab deleted the fix-and-refactor-root-motion branch February 14, 2024 05:28
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.

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