Implement AnimationPlayerBase class to make the blending process shared between AnimationPlayer and AnimationTree

[TokageItLab]

Describe the project you are working on

Stabilization of Godot animation feature

Describe the problem or limitation you are having in your project

Godot's AnimationPlayer blending is out dated and remains old duplicated

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Fixes:

• AnimationPlayer blending and AnimationTree blending are inconsistent godot#70207

Supersedes:

• Remove blending feature Default Blend Time from AnimationPlayer #5952
• Remove blending features from AnimationPlayer godot#70241

Most of the bugs in AnimationTree have recently been removed, but the AnimationPlayer blending is still having old AnimationTree code, making it difficult to keep them in sync.

AnimationPlayer blending currently has the following bugs:

• Breaking AnimationPlayer playback process due to possibility of blending the same cache
• Incorrect blend weights when blending more than 3 animations
• Inconsistent blend results

After discussion with @reduz in reference to the poll, we agreed to make a compromise by implementing the AnimationPlayerBase. But please understand that until then, fixing bugs in AnimationPlayer's blending will have a much lower priority.

If possible, we would like to eliminate the extra AnimationPlayer from the scene by having the AnimationTree get the animation list from the AnimationBase instead of getting it from the AnimationPlayer, as mentioned in comment godotengine/godot#28311 (comment), but that is a future plan.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Implement AnimationPlayerBase as the base class. It is implemented based on the new AnimationTree blending process. Then, AnimationPlayer and AnimationTree are reimplemented by extending AnimationPlayerBase.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Never

Is there a reason why this should be core and not an add-on in the asset library?

Animation system is core

[jitspoe]

Just to clarify, what you're proposing is:
AnimationPlayerBase: This handles just the animations, no blending.
AnimationPlayer: Inherits AnimationPlayerBase and adds blending. Effectively acting the way it does now.
AnimationTree: Uses animations from AnimationPlayerBase to avoid blend conflicts and handles its own blending internally.

If so, my first question is:
What's placed in the scene when importing a model with animation? Is AnimationPlayerBase used directly, or does it use AnimationPlayer? Would we need to manually add an AnimationPlayer to the scene?

Second question:
If the problem is keeping the blending logic in sync because there are 2 different classes to maintain, does this even address that issue? Seems it just adds another class to maintain.

Finally:
Have you tested with PR godotengine/godot#37001 ? That fixes a bug with a blend time of 0, so maybe we can always use a blend time of 0 with AnimationTree and save a lot of work not implementing new classes?

[TokageItLab]

AnimationPlayerBase will be implemented to cache each track, process their blends, and apply the blended values to each object. Currently, AnimationPlayer has duplicated code that is out of sync with AnimationTree for blending, so the main purpose is to get rid of it.

AnimationPlayer and AnimationTree calculate the weights and pass them to AnimationPlayerBase. Perhaps the playback process will be had AnimationPlayer and AnimationTree separately, but I don't know yet.

I can't guarantee that godotengine/godot#37001 will work well with this AnimationPlayerBase implementation, but it does not conflict with this proposal for now. At least the current AnimationPlayer is miscalculating weight for more than 3 animation blends, so if godotengine/godot#37001 fixes that it is fine, but if AnimationPlayerBase does not work with it then we will have to fix it again.

---

What's placed in the scene when importing a model with animation? Is AnimationPlayerBase used directly, or does it use AnimationPlayer? Would we need to manually add an AnimationPlayer to the scene?

It is still the AnimationPlayer. AnimationPlayerBase will not be exposed to editors.

However, if the following is accomplished, we can choose AnimationPlayer or AnimationTree to be used in the importer.

If possible, we would like to eliminate the extra AnimationPlayer from the scene by having the AnimationTree get the animation list from the AnimationBase instead of getting it from the AnimationPlayer, as mentioned in comment godotengine/godot#28311 (comment), but that is a future plan.

[Mikeysax]

Given this is being done in part for maintainability sake, would it also make sense to split the blending logic out into a AnimationBlend class and/or sub classes for different types of blending and let the Player and Tree worry about how to implement said intent?

The above might make it easier if they share similar blending logic but also make it easier to add, update, remove blending types from core

[TokageItLab]

I think it impossible to keep the old blending logic. This is because the blending results depend on an iterative order which the user has no control over. For example, the old blending logic produced different blend results between AB * 0.5 + C * 0.5 and C * 0.5 + AB * 0.5 (more detail is described in godotengine/godot#34134), it's so weird.

Also, the new blending process correctly handles weights above 1.0 and negative weights, so it can correctly handle AddBlend, etc., whereas the old blending process does not. But I don't know if it makes sense to add features such as Xfade-AddBlend to AnimationPlayer (although technically possible).

Indeed there is a bit of a concern about compatibility. The main issue is that for blending of tracks that do not exist in one animation, we need to create a reset track as an initial value if we want to avoid blending with zero values godotengine/godot#59961. But I expect possibly that we could provide an option like an "override reset track" on the AnimationPlayer side. But well, it is just a plan and we cannot guarantee that.

Edited:
Finally, the Deterministic option was added to allow blending without RESET animation in godotengine/godot#80813. It is an essential refinement of the old blend algorithm, but they are not exactly the same. It is more consistent than older blends in the case of chained AnimationNode or BlendSpace2D, etc. However Add2 is normalized and the result is nearly identical to Blend2. The detail is described in the Deterministic property of the class reference.

[akien-mga]

Implemented by godotengine/godot#80813.