Add CallbackModeDiscrete to AnimationMixer and improve the setting method for UpdateMode in AnimationTrackEditor

[TokageItLab]

Describe the project you are working on

Godot Animation features stabilization

Describe the problem or limitation you are having in your project

The blending method for Discrete values is proprietary and is not under user control.

In 3.x, the behavior of AnimationTree forced the Discrete value to be applied first and the Continuous value to be applied later when blending. Therefore, the Continuous value always took precedence in that case.

Since 4.0, when Discrete and Continuous values were mixed, the Discrete value was given priority, but since the problem godotengine/godot#80813 (comment) occurred, we have decided to force the conversion to the Continuous value when they are mixed so that they look good for many users from a usability point of view.

Forcing the conversion is done at the time of track cache creation. All animations in the animation list are iterated when creating the track cache, then animations that are not assigned (blended) will also affect the forced conversion, which may confuse the user with a warning if the user unintentionally changes the UpdateMode.

Related: godotengine/godot#83545

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

Make users to better understand and control Discrete mode

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

AnimationTrackEditor

Many users probably do not recognize the difference between Discrete mode and Continuous mode + Nearest.

This is because the default UpdateMode created when keying Int is set to Discrete mode, and most often the UpdateMode is not intentionally set to Discrete mode. Probably would be better to set the default to Continuous mode + Nearest and then insert an UpdateMode same with the Reset value if there is.

Also, it would be fine to have a batch conversion function for UpdateMode, but since AnimationTrackEditor lacks a track selection function, I feel that this improvement is needed first.

By the way, the same improvement may be needed for the angle interpolation mode, but this is much more difficult to coexist than Discrete and Continuous, so I feel we have to continue to issue warnings as we do now. There is a coexistence path of separating the applying process without blending, but not for angles.

AnimationMixer

Implement CallbackModeDiscrete in the similar way as the Method track.

• enum CallbackModeDiscrete
  • Dominant
    • When blending Continuous and Discrete values, the Discrete value takes precedence
  • Recessive
    • When blending Continuous and Discrete values, the Continuous value takes precedence
    • AnimationPlayer's Default
  • Force Continuous
    • Always treat Discrete values as Continuous + Nearest
    • AnimationTree's Default

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

No

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

It is core behavior

[LakshayaG73]

With this, for track 'A:b' say,

Will I be able to pick update mode 'Discreet' in animation A, and 'Continuous' in animation B, and have them function as intended?

[TokageItLab]

If blending occurs, the behavior will be as described in the Enum description.
If you will be set Dominant/Recessive mode to AnimationPlayer, they should work fine with respecting the behavior of the Continuous and Discrete modes for each (as the same behavior before 4.2), as long as you play them separately.

[LakshayaG73]

Since this is essentially bringing back old functionality (pre 4.2), I'd say pushing it to 4.3 is not a good idea. That's my opinion.

[AThousandShips]

We are not adding new features or fixes of this kind to 4.2 before release, we're already way past that point by some two months almost, it could be cherry picked for 4.2.1, there's no "pushing" of this to 4.3, features has not been open for 4.2 since September

[TokageItLab]

To clarify, this issue will be prioritized as much as possible, but since this is clearly an additional feature (enhancement), not a bugfix, it will most likely not be incorporated into 4.2.0. However, it will definitely be added to 4.3 and will likely be cherry picked for 4.2.1.

[LakshayaG73]

This is not a feature. This is a regression.

This 'feature' existed in 4.1. It was removed in 4.2.

Restoring it in 4.3 does not make it a 'feature'.

It's your decision. But please don't call it a feature.

I can understand that from the context of the new implementation it is a 'feature'. But from the point of view of the consumer, it is a regression.

[AThousandShips]

That's all a matter of discussion but it's not relevant to when to merge it, so let's stay on topic

Edit: Because it's not a critical regression fix, we only merge absolutely necessary and prepared fixes now, this isn't implemented yet and not a critical showstopper even if it was, like crashing, or fundamentally blocking use

Even if we consider it a regression and a bug, rather than an enhancement, it doesn't change this because we won't delay releasing for implementing and testing this

[Zireael07]

How is that not relevant to timing the merge? O_o

[AThousandShips]

@Giancarlonsito please express your support or lack of support with reactions, instead of bumping the issue with a single word comment 🙂 it notifies everyone subscribed to this issue and to the repository

[Giancarlonsito]

@Giancarlonsito please express your support or lack of support with reactions, instead of bumping the issue with a single word comment 🙂 it notifies everyone subscribed to this issue and to the repository

sorry pound, just testing, i'm newat GitHub ahaha