Root motion produces inaccurate transform

[rcorre]

Godot version:

3.2.2.stable.custom_build.0fe60f842

OS/device including version:

Linux 5.7.7-arch1-1

Issue description:

I have an animation that rotates my root bone by 90 degrees.
When animated directly by the AnimationPlayer it rotates the object exactly 90degrees.
When I add it to an AnimationTree and play the animation once via a OneShot or Transition node, the total rotation is slightly less than 90 degrees.
You can see this easily if you add a RootMotionView.

Steps to reproduce:

1. Open example project
2. Play the "Spin" animation from the AnimationPlayer
3. Observe the cube rotate exactly 90 degrees
4. Activate the OneShot in the AnimationTree
5. Observe the cube rotate less than 90 degrees

Note that the root motion view no longer lines up with the grid after one rotation:

Minimal reproduction project:

example.zip

[jiteshvm]

Hello,
I am interested in fixing this issue. Any pointers/direction will be helpful.

[rcorre]

Hi @jiteshvm, I'm hopeing someone more knowledgeable than me can give some advice, as the AnimationTree code is a bit of a mystery to me yet :)

If I were looking at this, I'd probably start with https://github.com/godotengine/godot/blob/master/scene/animation/animation_tree.cpp#L857-L858, and track how rot and rot_blend_accum change over the course of the animation.

[Gyrth]

I would like to add that translation is also not calculated correctly with root motion.
To show this happening I edited your example project. The new animation moves the cube exactly one unit/meter to the left.

But when I trigger the OneShot in-engine it falls short to the mark.

I got the character script from the TPS example project, so I assume this is correctly calculating the root motion to apply to the KinematicBody.

example.zip

[thimenesup]

I also have experienced the same issue about the root motion transform not being entirely accurate, happens in 3.1.2 too.

[TokageItLab]

This can be solved by adding a 0.1 second margin before and after the animation. However, since it seemed to be working properly outside of OneShotNode, I thought it was a bug in OneShotNode, so I checked and found that the fade_in and fade_out values were implicitly set to 0.1. This matches the margin value I mentioned in the beginning. Setting these two values to 0 will return the correctly root motion value. Does anyone know why these values were 0.1?

In for now I sent a PR for the fix in #43670.

[TokageItLab]

Since the root motion is calculated additively afterwards using delta time, the current OneShotNode and TransitionNode behavior is correct in a sense (assuming that the values of fade_in and fade_out are 0.1). However, the problem is that OneShotNode's fade_in and fade_out can be changed in the Inspector in the current version 3.2, but even if you set them to 0, there will return incorrect root motion.

After the fix of #43670, the full length of the root motion will be played back as long as these values are 0.

[thimenesup]

Alright, can confirm that the behaviour is due to the fade_in property in the one shot node and its not really a bug.

[TokageItLab]

@thimenesup Can you set truly the OneShotNode's fade_in and fade_out values to 0? Unless there is a #43670 fix, there would appear to be a gap and the gap is maybe a bug.

[thimenesup]

If you mean setting the property as is, it works. (3.1 with code)

[TokageItLab]

It seemed a bit odd, so I looked deeper and found that there seemed to be a problem with the timing of updating the initial value. The initial value can be kept at 0.1. But in that case we need to update fade_in/out value as follows

animation_blend_tree.cpp

float AnimationNodeOneShot::process(float p_time, bool p_seek) {
	bool active = get_parameter(this->active);
	bool prev_active = get_parameter(this->prev_active);
	float time = get_parameter(this->time);
	float remaining = get_parameter(this->remaining);
	float time_to_restart = get_parameter(this->time_to_restart);
        float fade_in = get_fadein_time();        // update
        float fade_out = get_fadeout_time();        // update

In NodeTransiton, the initial value of xfade is 0.0, so this is not a problem. If we want to align the behavior between NodeTransiton and NodeOneShot, I suggest setting the initial value of both to 0.0.

[TokageItLab]

Recently in Godot 4.0, I did some research, and it seems that the cause is that when the ProcessCallback of AnimationTree is Idle, it is not calculating the animation playback time correctly (or it is possible that only the root motion is calculated incorrectly), so it is not NodeOneShot and NodeTransition themselves that are at fault. So, for now, the problem can be solved by setting AnimationTree's ProcessCallback to Physics and setting fade-in/out value to 0, but the fundamental cause of the time (or root motion) calculation error with ProcessCallback = Idle should be fixed.

[TokageItLab]

4.0 bug (regression by #53819) will be fixed by #60774. The cause of this bug is that the delta subtraction is not correctly calculating the prev_time because the clamped current time is passed when calculating the root motion.

So, 3.x is fine now, but 0.1 sec fade by default might be occoring confusion.