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

Make AnimationMixer consider Discrete for RESET track #89389

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Mar 11, 2024

Deterministic sets the initial value if an animation are lacking some tracks from another animation, but this should not be done for Discrete tracks, so add some flagging when caching and initializing. It should behave as follows:

Deterministic RESET Track CallbackModeDiscrete Is init value appeared in animation with lacking some tracks
true Continuous Dominant Yes, init value is continuous clearly
true Continuous Recessive Yes, init value is continuous clearly
true Continuous Force Continuous Yes, init value is continuous clearly
true Discrete Dominant No, init value is discrete clearly
true Discrete Recessive No, init value is discrete clearly
true Discrete Force Continuous No, init value is discrete clearly
true None (but there is track in another animation) Dominant No, init value may be discrete
true None (but there is track in another animation) Recessive No, init value may be discrete
true None (but there is track in another animation) Force Continuous Yes, init value is discrete clearly by Force Continuous
false Continuous Dominant No, Un-Deterministic doesn't process init value
false Continuous Recessive No, Un-Deterministic doesn't process init value
false Continuous Force Continuous No, Un-Deterministic doesn't process init value
false Discrete Dominant No, Un-Deterministic doesn't process init value
false Discrete Recessive No, Un-Deterministic doesn't process init value
false Discrete Force Continuous No, Un-Deterministic doesn't process init value
false None (but there is track in another animation) Dominant No, Un-Deterministic doesn't process init value
false None (but there is track in another animation) Recessive No, Un-Deterministic doesn't process init value
false None (but there is track in another animation) Force Continuous No, Un-Deterministic doesn't process init value

@TokageItLab TokageItLab added this to the 4.3 milestone Mar 11, 2024
@TokageItLab TokageItLab requested a review from a team as a code owner March 11, 2024 16:44
@TokageItLab TokageItLab marked this pull request as draft March 11, 2024 16:45
@TokageItLab TokageItLab marked this pull request as ready for review March 11, 2024 17:17
Copy link
Contributor

@lyuma lyuma 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 I see the idea, that this code wishes to add an additional case to disallow continuous blending if there is no continuous RESET track but not in a force continuous blend.

When there is no RESET track for a particular value, I understand it seems to default to true, but I don't understand why it is forced to true when FORCE_CONTINUOUS is set

Anyway I'll mark it approved because I trust your judgement and I'm not sure anyone is familiar enough with the track cache to give a better review.

Basically, this is introducing additional state to the track cache, and there are more cases that set it to true than set it false, so I want to make sure it is correct.

scene/animation/animation_mixer.cpp Outdated Show resolved Hide resolved
scene/animation/animation_mixer.cpp Show resolved Hide resolved
scene/animation/animation_mixer.cpp Show resolved Hide resolved
scene/animation/animation_mixer.cpp Show resolved Hide resolved
@akien-mga akien-mga merged commit 6a4ff44 into godotengine:master Apr 6, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

Godot 4.3.dev4 bug - animation on AnimatedSprite2D not working if frame is present in RESET animation
3 participants