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

Animation travel not working when an animation is looped #79208

Closed
jordanlis opened this issue Jul 8, 2023 · 23 comments · Fixed by #87171
Closed

Animation travel not working when an animation is looped #79208

jordanlis opened this issue Jul 8, 2023 · 23 comments · Fixed by #87171

Comments

@jordanlis
Copy link

jordanlis commented Jul 8, 2023

Godot version

4.1

System information

Windows 10

Issue description

Hi,

I think there's an issue with looping animation and traveling in the animationtree. I think there weren't any issue in the 4.1 RC 2, or previous builds.

I tested it and it was working perfectly fine in the previous build 4.0.3, but not in the 4.1 RC2

There's somewhere an issue with loop animation and travel in animationTree which is incompatible now...

Steps to reproduce

  • you set up an animation an select the "loop" option
    image
    image

  • you make a traveling state machine with an animationtree and activate transitions
    image
    image

  • Make the travel transition to "At end"
    image

  • you launch the animation with the tree, then try in code with the state machine to travel to the next animation

Expected : the travel is done and the loop is over

Current : not working, the travel doesn't work

Again, was working perfectly fine with 4.0.3. Currently on 4.1 : seems to work with transition "Immediate". The bug is only for "at end" transitions.

Minimal reproduction project

don't have one at the moment, too complicated to create (because I need to make the code and so on)

@viksl
Copy link
Contributor

viksl commented Jul 8, 2023

Please make a Minimal Reproduction Project so someone can test this. :)

@AThousandShips
Copy link
Member

AThousandShips commented Jul 9, 2023

Agreed, please add a minimal reproduction project, as these steps are not trivial to follow, otherwise this can't be investigated and will likely be closed:

  • A small Godot project which reproduces the issue, with no unnecessary files included. Be sure to not include the .godot folder in the archive (but keep project.godot).
  • Drag and drop a ZIP archive to upload it. Do not select another field until the project is done uploading.
  • Note for C# users: If your issue is not Mono-specific, please upload a minimal reproduction project written in GDScript or VisualScript. This will make it easier for contributors to reproduce the issue locally as not everyone has a Mono setup available.

@jordanlis
Copy link
Author

Sorry, I did et have time to make a project. If I make a project, and that we test it inside the animation tree by clicking the button, is that okay. It doesn't work with the travel function in code but I think that's the same with the play button in the animation tree.

@uguu-org
Copy link

I have a similar problem with transitions in AnimationTree, where auto advance doesn't work if animation is looping. Attached is a project that demonstrates this behavior.

animation_test.zip

  1. Open project in Godot 4.0.3
  2. Click on AnimationTree in gomako/gomako.tscn
  3. Observe that "idle" state eventually transitions to "yawn" state.
godot_4_0_3.mp4
  1. Open the same project in Godot 4.1
  2. Click on AnimationTree in gomako/gomako.tscn
  3. Observe that we are stuck looping in "idle" state.
godot_4_1.mp4

Note that even in Godot 4.0.3, we will get stuck if Xfade Time is set to zero. It's possible to workaround this by disabling loop bit for the ones where we want the animation state to travel, but then those animations won't loop outside of AnimationTree.

@jordanlis
Copy link
Author

It seems to be the same issue, but I don't understand what's the transition view that you have.

On my side, it can be seen in the animation tree : you click on play and the transition / travel doesn't happen
Is that okay with this project or should I upload mine??

@viksl
Copy link
Contributor

viksl commented Jul 11, 2023

@jordanlis preferably prepare a simple test project stripped of everything but bare minimum. These can be same issues or different. Test projects are needed to test these cases. Check what @AThousandShips asked of you above for instructions.

@jordanlis
Copy link
Author

OK I'll spend some time preparing a demo it need to be fixed.

@uguu-org
Copy link

@jordanlis that's the "auto advance" bit. But I could also do it with a state machine, attached:

animation_test2.zip

godot_4_0_3.mp4
godot_4_1.mp4

@TokageItLab
Copy link
Member

TokageItLab commented Jul 11, 2023

I remember this is an intended change by #75759. The length of the looped animation is infinite in order to accurately calculate the length of the nested StateMachine.

You can use statemachine_playback.next() to break out of the loop.

@TokageItLab TokageItLab closed this as not planned Won't fix, can't repro, duplicate, stale Jul 11, 2023
@viksl
Copy link
Contributor

viksl commented Jul 11, 2023

@TokageItLab How do you break out of a looped animation at the end of that animation, next breaks immediately and looped animaitons don't trigger signals? That sounds like what they want to do.

@TokageItLab
Copy link
Member

TokageItLab commented Jul 11, 2023

There are several ways to break out of the loop:

  1. Use next() with AutoAdvance disabled (equivalent to using next() with AtEnd)
  2. Use AdvanceCondition with Immediate mode
  3. Use method track to place a function like loop_ended() before the end of the loop animation (just before the length of the xfade) to manage the can_transition states on the GDScript side, for example:
func loop_ended():
	if (character.can_transition):
		statemachine_playback.next()

@viksl
Copy link
Contributor

viksl commented Jul 11, 2023

@TokageItLab I hope I'm not misunderstanding. I gave it a try and the 1/ did not wait until the end of the transition be it with or without AtEnd. Am I missing something, please?

@TokageItLab
Copy link
Member

TokageItLab commented Jul 11, 2023

I can't see what you are trying. Even if it is 1 and 2, the xfade time of the transition should be respected. Although, there is no wait for the animation to end, so the user must determine if the animation has looped one loop or not.

If you are doing next() during a transition, that would be the intended behavior. It aborts the current transition and start the next transition.

@TokageItLab
Copy link
Member

Use next() with AutoAdvance disabled (equivalent to using next() with AtEnd)

I was a bit wrong about this. Actually they are not equivalent.

next() does not go to the next state unless there is an explicit path there. So if you want to use next(), you need to create a path to the next state with travel() or have a path to the next state with AutoAdvance before you use next().

@jordanlis
Copy link
Author

jordanlis commented Jul 12, 2023

I don't understand where there was any need to prevent the system to allow a transition in the state machine. For me it's a bug --> In the interface, the transition are not happening, what's the need behind the change ?

Now, it's not possible to transition from one state to another when an animation is looped ? In code, we need to do that with "next" which is not even waiting for the end frame to happen ?
Please, could you help me by clarifying the solution to have a looped animation and transition to the next state ( and waiting for the animation first animation to end) ?

It feels like a gaz factory for nothing... Why did you even do that ? I checked in the mentioned issue and nowhere it is said that the travel function must not work when the animation is looped...

Don't want to be rude or anythting, I'm just expressing something here that doesn't feel good for me.

@TokageItLab
Copy link
Member

TokageItLab commented Jul 12, 2023

what's the need behind the change ?

This was a necessary change for nested StateMachine to xfade correctly.

In order to determine the end time of a StateMachine from outside the StateMachine, the StateMachine must have an infinite time length while it is looping. The looping animation should also have an infinite time length to be consistent with it.

Before the AnimationStateMachine rework, the loop was broken by AtEnd even though it was looping, but the looping process is redundantly implemented in the AnimationNodeStateMachine, which is separate from the AnimationNodeAnimation. In addition, it did not correctly handle backward loops and pingpong loops, so it was just breaking down.

Please, could you help me by clarifying the solution to have a looped animation and transition to the next state ( and waiting for the animation first animation to end) ?

The implementation of 3 I have described above should work most closely with what you are intending to do. However, it is a bit tedious, and I have heard that @SaracenOne is currently working on a new SwitchMode implementation that will perform transitions when a specified time has elapsed, so it may be better to use that in the future.

However, while they may solve the problem for AnimationNodeStateMachine, they may not solve the problem for AnimationNodeTransition.

Note that even in Godot 4.0.3, we will get stuck if Xfade Time is set to zero. It's possible to workaround this by disabling loop bit for the ones where we want the animation state to travel, but then those animations won't loop outside of AnimationTree.

To be honest, this is the cleanest way to go. However, it is not good way because Animation or AnimationTree as a Resource may be referenced by multiple Nodes.

So, in my opinion, it would make sense to implement LoopModeOverride as a parameter of AnimationNodeAnimation, since the AnimationNode parameter is stored in each AnimationTree as a node. This allows us to manage the loop state for each character, even if the same Animation or AnimationTree resource is used repeatedly. Also, this is also beneficial in AnimationNodeTransition. I will send a PR for this later.

@jordanlis
Copy link
Author

Okay, not really sure to understand your first sentence.

Just to make sure that I understand : does your pr will allow looped animation + at end transition to travel to the next animation with

  • next function?
  • travel function?
  • pushing the play button in the animation tree?

Sorry, I feel that I don't understand everything so it's important for me that you clarify if you don't mind

from what I understand about the current solution I need to use next instead of travel and test if the animation has ended. I'll try to make something like that...

@TokageItLab
Copy link
Member

TokageItLab commented Jul 13, 2023

No, looping animation is never AutoAdvanced by AtEnd, nor should it be.

This problem is not something that should be solved by the StateMachine, but is fundamentally caused by the fact that AnimationTree lacks an API to manage the looping state of the Animation, so I will add a parameter to manage the looping state in #79400.

If you want to break the loop, simply override the loop setting from the inspector of the AnimationTree and disable the loop by the functionality added by that PR. And for example to do it in the script:

animation_tree["parameters/Animation/loop_mode_override"] = AnimationNodeAnimation.LOOP_NONE

@jordanlis
Copy link
Author

No, not autoadvance. Just manually in script or in the animationtree.you see what I mean?

@jordanlis
Copy link
Author

I still don't get your solution. Your pr will allow to disable the looped animation, so if I Want to go from the looped animation to another, I won't be able to do it until I disable it? And then I have to reactivate it if I need to play it again... And I will be able to wait the end of the animation thanks to the script you put in 3 apparently.

I really think that all of this should really stay simple.

Like for example a parameter of the next() function that allows to force the transition.
example : next(override) or some think like this that allows to break the loop and continue to the next animation but wait for the atEnd transition.

What do you think?

@TokageItLab
Copy link
Member

TokageItLab commented Jul 14, 2023

example : next(override) or some think like this that allows to break the loop and continue to the next animation but wait for the atEnd transition.

It is not possible. Since the looping animation now has infinite length, it is no longer possible to detect the end of that animation on the StateMachine side. Nor does it solve anything in NodeTransition.

The NodeAnimation returns the remaining time when iterating. AtEnd fires a transition when the remaining time falls below a certain value while taking xfade into account, but NodeAnimation now always returns a large constant HUGE_LENGTH, during the loop, so no transition is fired1.

This change was necessary mostly to fix the following problems:

Also, keep in mind that AnimationBlendTree and AnimationStateMachine are nestable. This means that looping and non-looping animations can blend together. Therefore, the loop must always be detected so that all animations are not interrupted unnaturally. For this reason, looping animations should return a HUGE_LENGTH.


As I have explained many times, you can break out of the loop in several ways:

  1. Using TransitionMode = AtEnd AutoAdvance = true, and playback.next() with MethodTrack. This is available in 4.1.
  2. By PR of Implement LoopModeOverride to AnimationNodeAnimation to properly manage the loop state for each AnimationTree #79400, you disable the loop by a loop override in NodeAnimation on AnimationTree on the intended timing when you want to break the loop. If you set the loop override mode to No Override after the transition, the next time you play the loop animation, the loop will be enabled again (this is never complicated but simple way) - This is different from changing the loop in AnimationPlayer's Animation, where no re-caching occurs and the loop state can be managed on a per-character (AnimationTree2) basis.
  3. Wait for future implementation of transition mode "TimeElapsed".

Footnotes

  1. You may think this looks like a hack, but passing the loop state as an argument separate from the time is probably a poorer way. As I explained earlier, it is necessary to redundantly write special cases for loops many times outside of AnimationNodeAnimation, and we should not implement those special cases in StateMachine or NodeTransition

  2. Strictly, even if there are multiple "same" animations in the AnimationTree, each loop can be managed individually

@jordanlis
Copy link
Author

jordanlis commented Jul 15, 2023

Ok, I just let go for now.

I don't understand how to transition at the end of the looped animation with your solution (I'll try to understand by trying with the code you gave, but I don't see what it does and where to put that)

Next function is just breaking the loop in my code, and the downside is that it triggers other animation when not needed (it seems to look for any available path, so weird things are happening now with every other transitions...)

For me, the travel should just work and break the loop. That's it. I need to rework totally my code appearently, and I don't know how to make my state machine work with this new godot 4.x.

Thanks for your time anyway.

Edit :
To anyone reading this, the only real solution is

  1. To put a boolean ("force_next" in this example) in all your looped animation with at end transition and set it at both the beginning AND the end of all these animations
    image

  2. Put somewhere in your code after all your "travel()" transitions a "next()" if the boolean is true
    image

Seems to work even if that's a gaz machine has I already told.
BUT, it should produce weird things if you have multiple path available... You need to play with the priority setting maybe ? Don't know.

@ratala321
Copy link

ratala321 commented Sep 30, 2023

I had the same problem, what I did to solve it in one line of code :
state_machine.travel("myState")
state_machine.next()

Make sure to add state_machine.next() to activate the travelling.

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