Animation Tree fakes default keyframes for non-keyframed values

[jcarlosrc]

Godot version

4.0 alpha 5 and alpha 6

System information

Windows 10

Issue description

Tracks that are not keyframed should not be animated. However, Animation Tree is using default values (at least for position and color) as they were keyframed values, causing animation issues.

In the videos: First, The expected behavior (Godot 4.0 Alpha 4 and all previous version): Sphere does not move because it is not keyframed in the one shot animation. Second, Alpha 5 and 6: the sphere moves to its default (0,0,0) position, however there is not keyframed at all at that position.

This issue happens also with color. To see it, make the Color Rect node visible and play the game. Transparency goes back to 0 causing scene not to be visible. This can not be an on-purpose behavior, because each animation should contain a track for every single node and position to behave properly, which does not make sense.

A4Behavior.mp4

A6Behavior.mp4

Steps to reproduce

Play the game with alpha 4 and alpha 6 versions. Pressing "Enter" activates the one shot animation, showing the bug.

Minimal reproduction project

BugReportAnimTreeAlpha6.zip

[TokageItLab]

I say again, as written in #59474 (comment). At least the #59474 issue is a bug, but the missing key animation is now blended with the initial value, which is an ideal change to make the blending result always consistent. This change was necessary to solve the bigger problem #46038. I proposed an implementation that use ResetTrack as the default value, but that was rejected in discussion with @reduz.
However, I am concerned that the inability to use Rest as InitValue may be a problem for 2DSkeleton, which does not have a TRS track like 3D. We will discuss that later with reduz.

[jcarlosrc]

First, the color rect behavior is not an isolated bug, is the result of the same behavior.
Secondly. As I can see it, it does not make sense. If we have an animation with just one track and need to mix it with another animation having 100 tracks. We want it just to mix the one track. However, we will need to copy all the 100 tracks and its keyframes to have the desired effect? In the videos avobe, I will need to copy all the keyframes for the sphere position to make it still. In this case is only one position, but it could be much more complex.This was never an issue before and should not become one now.
Third: Keyframing values the user never intented to keyframe can not be an ideal solution at all. If we keyframe a value and only one value is because we want that behavior. Mixing with a default value is the same as keyframing a value that was never intended to be keyframed.

[TokageItLab]

As a result of the fix in #59490, the missing Color track is indeed blended with the transparent value, but it is not a bug. It is just blended with the init value (zero). The bug in #59474 is due to the fact that the default Alpha value for Color is actually 1 instead of 0. This breaks the alpha channel blend even when the Color track is not missing. You must first understand the difference.

If we don't want to blend with the 0 value, we need at least some intended default value similar to the ResetTrack solution.

@reduz Do you think AnimationLibrary will solve this?

[jcarlosrc]

@TokageItLab Please consider the following minimal situation: We have two properties p and q, let's say legs and arms We have animations A1 and A2 with keyframes for both legs and arms (idle an walk). And a third animation B with only keyframes for arms (say hello). We want to mix A1 or A2 with B, replacing only arms. We only want to replace arms, but let legs unaltered to move. How could we achieve this under the current behavior?

• If we mix A1 or A2 with B, the engine will create a default keyframe for legs in B, causing legs to change (as seen in the videos).
• If we "complete" animation B to include legs, we will need to decide how. If we copy A1, it will mess up animation A2 when we want to mix B with A2. The only solution is creating another B, to match with each A. This is not a solution, because we could have many A, and will need to create as many B, leading to an exponential number of copies just for a simple behavior.

The solution is just not mixing legs at all, which does not have keyframes in B. In this way, only keyframed tracks (arms) get mixed, which makes sense.

[jcarlosrc]

As a result of the fix in #59490, the missing Color track is indeed blended with the transparent value, but it is not a bug. It is just blended with the init value (zero). The bug in #59474 is due to the fact that the default Alpha value for Color is actually 1 instead of 0. This breaks the alpha channel blend even when the Color track is not missing. You must first understand the difference.

If we don't want to blend with the 0 value, we need at least some intended default value similar to the ResetTrack solution.

@reduz Do you think AnimationLibrary will solve this?

Yes, the color is a bug because it has not a missing track. My fault.

[TokageItLab]

Technical Explanation. The biggest problem with the blending process is that all possible exist tracks are cached and iterated. This means that if a track is missing from one of the animations being blended, it must still be blended with some value.

While 1 frame, The iterating for blending is done by the number of starting nodes in the AnimationTreeNode. Unfortunately, this iterating process cannot know which animations are missing tracks, so it cannot perform normalization of blend weight at the appropriate timing. The old blending algorithm attempted to force normalize the weights to 1.0 by blending the values of the previous iterating process with using lerp(), but this caused obvious wrong calculations for blends of three or more and in the case of NodeAdd. The problem has been fixed by #57675 and should not be reverted.

Also, the forced normalization makes a huge difference in blending at 0 and 0.01, moreover the same blend value can produce very different results depending on when the blending timing. I think it is not makes sense in terms of consistency.

Without changing the track cache iterating process, I think there is an only way to minimize blend tracks is for the user to insert an intended default keyframe instead of zero value as ResetTrack, but that implementation was rejected once #57675 (comment), so we need to discuss with @reduz whether to re-implement this or implement an alternative.

[jcarlosrc]

The user cannot define a proper default keyframe value because it is not possible. Any default keyframe will mess up another animation. For instance, if we define idle leg pose as default, it will mess up a run animation. And if we define a run default, it will mess up the idle animation.
While some inconsistency is indeed a problem, the current implementation has created a bigger problem in the basics of animation. Now it is impossible to mix arms without messing up legs. This is a core animation feature. We can't even use track filtering because it does not work. In the minimal project try to fix the sphere position filtering the OneShot to allow only box position track mixing: It does not work. The sphere still moves to the default position. Right now animation tree is just broken.

[TokageItLab]

As I explained above, "do not blend" is incorrect. It is correct to say "treat the weight as 1.0 for the non-missing tracks".

I say again, this is problematic from a consistency and technical standpoint: it is unnatural to have no animation when the weight is 0 but to have animation with a weight of 1 when the weight is 0.01. Also, once the blend value becomes 0, the animation is interrupted and the value in the middle of the animation is retained, the fact that it is only done on certain tracks means that the end result of the blend will be different depending on the order in which the animations are played. Moreover, the structure and iterating process of AnimationTree's track cache needs to be fundamentally changed since the previous blend calculation was completely broken because the normalization method was also incorrect.

My idea is to record intended value (e.g. the position of the sphere in your project) as a default value in ResetTrack. For example, Skeleton3D::Bone uses Rest instead of zero as its default value. This means that ResetTrack will act as Rest for animations other than Skeleton3D::Bone. This will cause missing tracks to be blended as the Reset animation's track value. A Skeleton::Bone never have more than one Rest; any value that is not a Rest should be treated as a PoseAnimation. Likewise, any value away from the Reset track as the default value should be keyframed as an animation.

However, the current ResetTrack does not. This is because it was rejected once. Need to discuss this again with @reduz.

[jcarlosrc]

Let's say we record the position of the sphere as a Reset Track. This will solve that specific case. But what if we want to mix the same box animation with another sphere animation. Such Reset track will get blended. However we just want the the new sphere values to show and mix only box values. We will need to keyframe the new sphere values also in the box animation. Imagine this happening to a fourth sphere animation we want to blend with the same box animation. We will need to create another box animation just to store the new sphere values. And if we consider other tracks, let's say a triangle? We will need to create box animations with all the combinations of sphere and the other tracks.

[TokageItLab]

You just need to record ResetAnimation as the default value for the box as well. If you think that replacing a sphere or a box with each of its parts in Skeleton, it is the same as if each part had a Rest.

Also, if the problem is that the combination of sphere and box animations is a lot, if ResetTrack solution is implement and those default values are recorded in ResetTrack correctly, then NodeAdd should work like filter. Or another solution, just separate the AnimationPlayer and AnimationTree for that. Likewise, for example, there is no need to control two characters with the one AnimationPlayer and AnimationTree always.

[jcarlosrc]

@TokageItLab I am not sure we understand each other. Please open the minimal project in this message, there is also a video. The box has a walk and idle animations. The sphere has a Hello animation and there is also a Reset animation. I just want the sphere to say hello and let the box play idle-walk. The animation tree set up is straightfoward: a Transition to choose idle or walk for the box and a OneShot to mix the sphere movement. It is not working in Alpha 6.
Now, please try yourself to fix the project and get the desired behavior: let the box to blend idle and walk freely and mix the hello sphere animation without messing up the box. I will really apreciate it having the solution.

BugReportAnimTreeAlpha6.zip

G4A6BugReport.mp4

[TokageItLab]

@jcarlosrc I salvaged the ResetTrack solution as #60093. By the way your filter settings are inverted.

If it merge #60093 and set the filters correctly, it should work as you want.

2022-04-10.12.53.35-1.mov

Or, if you don't use filters, we can get the same behavior by setting the MixMode of Node::OneShot to Add, as I said above.

2022-04-10.13.07.30-1.mov

[jcarlosrc]

So Alpha 5 / 6 is indeed broken and needs fixing by merging the required commit.

Please consider that this was a minimal example that should run out of the box without hacks or workarounds. I wish the provided solution is scalable to complex scenes and gives us a practical and simple workflow just as we had until G4Alpha 4.

Thanks again.

[TokageItLab]

It is not "broken" but rather "hard to use".

In fact, as you said, it is possible to work around this by duplicating the animation track so that it is not missing, but I know it is inconvenient and we need to merge #60093 as an enhancement.

@jcarlosrc If you are able to build Godot, it would be appreciated if you could test #60093 and send me feedback/consensus, but if not, in the meantime, please answer if the video I sent you above works as you think it should.