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

Add root motion accumulator to fix broken RootMotionView #72931

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Feb 8, 2023

Fixes #72930.

rootmotionfix.mp4

Now we can get the actual blended rotation of the rotation with get_root_motion_rotation_accumulator(), not just the delta value.

The following would allow the root motion position to be correctly applied while taking the object's orientation into account, without keeping the current rotation at the start of the animation. Now we can close #58061 completely.

func _process(delta):
	if Input.is_action_just_pressed("animate"):
		state_machine.travel("Animate")
	set_quaternion(get_quaternion() * animation_tree.get_root_motion_rotation())
	var velocity: Vector3 = (animation_tree.get_root_motion_rotation_accumulator().inverse() * get_quaternion()) * animation_tree.get_root_motion_position() / delta
	set_velocity(velocity)
	move_and_slide()

I added a explanation of this code to the documentation. This should work even if the animation is Xfade or interrupted.

Also, using get_root_motion_rotation_accumulator() should be able to solve issue #67125. Closes #67125.


We should discuss whether this is the good naming for the methods.

I think get_root_motion_rotation_delta() and get_root_motion_rotation() would be better, but it would be breaks compat; This means to rename the current get_root_motion_rotation() to get_root_motion_rotation_delta() and to rename this PR's get_root_motion_rotation_accumulator() to get_root_motion_rotation().

Probably it is a better naming like get_root_motion_rotation_base() or get_root_motion_rotation_source()?

What do you think? @reduz @fire @lyuma @SaracenOne

@TokageItLab TokageItLab added this to the 4.0 milestone Feb 8, 2023
@TokageItLab TokageItLab requested review from a team as code owners February 8, 2023 23:49
@TokageItLab TokageItLab force-pushed the improve-root-motion-for-rot-and-pos branch 3 times, most recently from 730e7f5 to be2fb17 Compare February 9, 2023 12:34
@TokageItLab
Copy link
Member Author

The explanation in the document was incorrect and has been corrected.

@TokageItLab TokageItLab force-pushed the improve-root-motion-for-rot-and-pos branch 2 times, most recently from a263bb2 to 2d2284c Compare February 9, 2023 16:18
@TokageItLab TokageItLab force-pushed the improve-root-motion-for-rot-and-pos branch from 2d2284c to 7b18ad7 Compare February 9, 2023 16:22
Copy link
Member

@SaracenOne SaracenOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense to me 👍

@akien-mga akien-mga merged commit 929ee61 into godotengine:master Feb 9, 2023
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment