Skip to content

Making so animation transition is able to handle multiple active animations! #18954

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

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

Sirmadeira
Copy link

@Sirmadeira Sirmadeira commented Apr 26, 2025

Objective - Finished

In this PR I make it so AnimationTransitions component, actually has a concept called flows! Which will ease up the introduction of StateMachines for Animations
I also believe he shouldnt be limited by the idea of one sole "main" animation specially with the advent of layer through AnimationMasks. As the current implementation does

Solution

-In this case I make it so instead, of having the idea of one sole active animation, transitions actually has the ability of having multiple flows. Each flow having a one-to-one "main" animation. This will work nicely, with @pcwalton additive blends, and masks, and should mantain all the previous features of AnimationTransitions.

Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@Sirmadeira Sirmadeira changed the title Making so animation transition is non reliant on animation players Making so animation transition is non reliant on animation players, and transitions animation graph instead Apr 26, 2025
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Animation Make things move and change over time D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 29, 2025
@Sirmadeira
Copy link
Author

@mweatherley @pcwalton Kindly review, would love this feature on 0.16.1.

@Sirmadeira Sirmadeira changed the title Making so animation transition is non reliant on animation players, and transitions animation graph instead Making so animation transition is able to handle multiple active animations! May 4, 2025
@Lunkentuss
Copy link

Looks good overall and I like this change, and this is something I need myself. I have some comments however:

  • I think it's unnecessary to introduce to much backwards incompatible changes for this. I would propose to keep the function signatures for the new and play as is, and introduce something like new_with_flow_amount and play_with_flow or something similarly. Not only would this be backwards compatible, but it would be a simpler API for consumers that doesn't care about flows - which I expect is and will be quite common.
  • Remove ending exclamation marks (!) from the doc-strings you have changed/introduced, since this is a consistent with other documentation e.g. Animation Transitioning logic goes here!.

@Sirmadeira
Copy link
Author

@Lunkentuss Makes sense

@Sirmadeira
Copy link
Author

@alice-i-cecile If possible can we perhaps add this to the next merge train? I would love to take the glue code from my project?

@alice-i-cecile
Copy link
Member

@alice-i-cecile If possible can we perhaps add this to the next merge train? I would love to take the glue code from my project?

This requires two approvals first; I'll ask around. I don't have the expertise with animation to review this myself. @Lunkentuss, if you're comfortable, we'd welcome a review from you however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants