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

AnimationGraph: Transition with the same NodeIndex doesn't work #12604

Closed
theredfish opened this issue Mar 20, 2024 · 7 comments
Closed

AnimationGraph: Transition with the same NodeIndex doesn't work #12604

theredfish opened this issue Mar 20, 2024 · 7 comments
Labels
A-Animation Make things move and change over time C-Bug An unexpected or incorrect behavior D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@theredfish
Copy link
Contributor

theredfish commented Mar 20, 2024

Bevy version

What you did

During the version 0.13 I implemented animations for a 3D character where I can play the same animation multiple times smoothly. I tried to do the same with the new animation graph implementation and observed some issues. I use AnimationTransitions to play a new NodeIndex. It works between two different indexes and can play them smoothly back and forth. However I can't play the same animation (NodeIndex) two times in a row, it's like one animation is skipped when there is a transition.

What went wrong

Transition to the same NodeIndex isn't played. I can observe some regressions with the new implementation. I think we didn't consider cases where we want to halt the current animation to play exactly the same one but by interpolating it. It's the case when you want to play an attack animation - then you can check the seek_time to avoid a spam attack. That's what I did with version 0.13 and it worked well.

Additional information

How to reproduce

Use the animated_fox example, I commented some parts and forced to only play the same animation by pressing enter. You will see that the fox animations stops, then if you hit again it will replay it from the beginning (pretty much like replay/rewind) without interpolating.

Check here

cargo run --features wayland --example animated_fox

Gif of the issue + debug logs

You can see the logs on the left coming from my modified version of the bevy_animation crate to debug the events. We can see that playing the "slashing" animation two times push the old one to the transitions vector and then the weight is decreased. However the main animation doesn't seem to be played

animation-graph-same-anim-issue

Other behavior observed

If I launch two different animations by giving them some time, it runs smoothly (first part of the gif), but as soon as I spam the inputs to run them quickly (withdrawing / sheathing), it seems like it breaks the transition before it finishes and the character is stuck in a unfinished position (last part of the gif). I still use the play function from AnimationTransitions, I do not directly manipulate the AnimationPlayer. It seems similar to me and might be correlated to the same issue.

bevy-anim-graph-spam-two-nodes

Contributing

If you need help for this issue I would be happy to contribute if someone can provide me some guidance on where to look. So far my investigations have led me to the transition issue. I suppose the node being the same, there is an update issue for playing the decreasing animation. But I didn't make more progress and thought it would be better to ask for help.

@theredfish theredfish added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 20, 2024
@SolarLiner SolarLiner added A-Animation Make things move and change over time and removed S-Needs-Triage This issue needs to be labelled labels Mar 21, 2024
@james7132 james7132 added this to the 0.14 milestone Mar 29, 2024
@mweatherley
Copy link
Contributor

I'm looking into this right now. I'll post here when I have further updates on the matter.

@mweatherley
Copy link
Contributor

Okay, here's my understanding of why this happens on a basic level.

When AnimationTransitions::play is called, it queues up an AnimationTransition to fade out the previously active animation before starting the new_animation on the associated AnimationPlayer (player). When the two happen to be the same (meaning they have the same AnimationNodeIndex; let's call this index), this is what happens (in order):

  • The fade-out is created for index.
  • The player starts the animation at index, which does nothing since it is already playing.
  • The fade-out decreases the weight of the playing animation, rapidly causing it to halt (you will notice there is a brief delay before it stops completely). The reason that it appears paused is not because the active animation is actually paused, but because there is just no playing animation to drive the model's movement.
  • Once the fade-out has decreased the weight to 0, the animation for index is removed from the player. This means that now it can actually be restarted by calling play again on index.

This is a pretty thorny issue to solve in a satisfactory way, because it is indirectly a consequence of the fact that the AnimationPlayer tracks a single ActiveAnimation for each AnimationNodeIndex. This means that a full solution (i.e. allowing blending between the previously playing instance of the same animation and the restarted version) will require significant changes to the internals of the animation architecture.

However, in the meantime, we could consider implementing some kind of half-measure that prevents the worst of this from happening; for example, we could make it so that the animation just fully restarts (without blending) when AnimationTransitions::play is called with the same index as the current active animation.

I'll spend some time meditating on what a complete solution to this might actually look like as well, since it seems potentially important.

@theredfish
Copy link
Contributor Author

theredfish commented May 6, 2024

Thanks for digging into this! Now it makes sense to me why it was "paused" and it correlates with my logs.

which does nothing since it is already playing

This is a pretty thorny issue to solve in a satisfactory way, because it is indirectly a consequence of the fact that the AnimationPlayer tracks a single ActiveAnimation for each AnimationNodeIndex.

This behaviour is handled by the function start_with_transition in the current 0.13 version. It allows to replay the animation, even if it's the same, and blend to the first keyframe. Maybe we could bring back some logic from this function and adapt it for the animation graph?

However, in the meantime, we could consider implementing some kind of half-measure that prevents the worst of this from happening

If we can avoid the regression before 0.14 it would be great, but I can also understand that we need to move forward with the architecture. I will also try to find some time to explore a potential solution now that I have a better understanding of the issue.

@alice-i-cecile alice-i-cecile added D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished labels May 16, 2024
@pcwalton
Copy link
Contributor

pcwalton commented May 16, 2024

@mweatherley So as far as I'm aware the plan was originally to remove AnimationTransition as part of the switch to animation graphs. I kept it in in limited form to ease migration, but it had to be totally rearchitected and I'm not surprised things broke.

The right solution here is going to be some sort of Unity-like AnimationController system with full-blown node graphs and transitions, because AnimationTransition isn't sufficient for the needs of anything but the simplest scenarios. But the proper system will take time to design and implement. In the meantime, though, I'm happy to make quick fixes to the current system to try to ease migration.

In general, serious users of animation will need to make their own bespoke animation controllers that manipulate graphs and not use AnimationTransition.

@mweatherley
Copy link
Contributor

@pcwalton That makes a lot of sense; thanks for the context!

@theredfish
Copy link
Contributor Author

theredfish commented May 16, 2024

Thank you for the extra explanations and the vision about the crate. As mentioned on Discord, I can wait for a new version of the crate until it meets my needs. There are external solutions that I can use for the current 0.13 version.

I have "advanced" needs for animations so might be better for me to wait until the crate offers the functionalities I need. If at some point my knowledge and experience are sufficient to provide help I will. Or if someone has enough time for mentoring on some implementations and not the bandwidth to do it I could give it a try.

So we can close this issue.

@pcwalton
Copy link
Contributor

Feel free to leave this open. I think the time is definitely right for out-of-tree experiments with animation controllers. We have egui-based node graph libraries so you could even make artist-friendly editors for them. When one of them gets mature enough I'd be in favor of merging it into the Bevy tree.

@alice-i-cecile alice-i-cecile removed this from the 0.14 milestone May 26, 2024
@theredfish theredfish closed this as not planned Won't fix, can't repro, duplicate, stale May 26, 2024
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-Bug An unexpected or incorrect behavior D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

No branches or pull requests

6 participants