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

Unused parameters get reset to 0 when animated by an active AnimationNodeStateMachine AnimationTree despite nothing explicitly defining them. #80971

Closed
KnightNine opened this issue Aug 24, 2023 · 6 comments · Fixed by #80813

Comments

@KnightNine
Copy link

Godot version

4.1

System information

Windows 10

Issue description

I noticed that any parameters being modified across all of my AnimationPlayer's Animations get set to 0 (or the RESET track values) when not in use by the AnimationTree using that player.

Example:
I have two animations relevant to a ColorRect.

  • one that changes the RGBA values
  • another that changes the Rect's position

In my AnimationNodeStateMachine when I switch between the two animations, the parameters that are not in use by the active animation get reset.

This is with the RESET track values setting the parameters so that the animation reset can be seen:
animation reset

Without the RESET track the unused parameters between the animations get set to 0 and therefore you cannot see the Rect when the "move" animation is playing since Alpha is set to 0:
animation reset2

This is annoying because you are forced to use multiple AnimationTrees ( and AnimationPlayers considering: #80951 ) for each parameter you wish to modify if you don't want the values to reset themselves.

Deleting the RESET track should nullify this RESET behavior.

Note: I've mainly only used AnimationNodeStateMachines across my projects so I am unsure if this behavior is seen with other tree_roots.

Steps to reproduce

  • Create a ColorRect to animate, then create an AnimationPlayer and AnimationTree node in your scene.
  • Define a looping animation of the ColorRect changing color
  • Define a looping animation of the ColorRect moving around
  • Delete the RESET animation if you created it.
  • Set the AnimationTree tree_root to an AnimationNodeStateMachine and connect the anim_player to the AnimationPlayer in the scene.
  • In the AnimationNodeStateMachine connect the two animations via transitions with advance_mode set to enabled so the user can manually switch between the animations.
  • Activate the AnimationTree and try switching between the animations.

Minimal reproduction project

TestProj.zip

@KnightNine
Copy link
Author

KnightNine commented Aug 28, 2023

Note:
If you have an AnimationPlayer containing both an animation that uses bezier curve tracks and an animation that uses a property track for the same property,
you become unable to use the bezier curve track animation to animate that property at all within an animation tree using that animation player.

I also want to confirm that this issue is relevant to the AnimationNodeBlendTree tree_root as well.

@sketchyfun
Copy link
Contributor

sketchyfun commented Aug 30, 2023

I've just bumped into this issue myself (and wasted a few hours trying to figure out what was overriding my node's positions!)

It also seems to happen even if there's no nodes at all in the AnimationTree (an empty AnimationNodeStateMachine I mean), just the fact it's linked to an animation player and set to active is enough to set any used property animation tracks to 0

It also appeared not to affect 3D Position tracks, but upon further investigation, they actually crash the game/editor instead when an animation using them is played within the AnimationTree...

@KnightNine
Copy link
Author

KnightNine commented Aug 30, 2023

I think this should be pushed to 4.2 since there are probably a lot of things you simply cannot do without a bunch of code to manually override the animations.

@lyuma
Copy link
Contributor

lyuma commented Aug 31, 2023

containing both an animation that uses bezier curve tracks and an animation that uses a property track for the same property,

For blending value with bezier tracks, see issue: #49431

upon further investigation, they actually crash the game/editor instead when an animation using them is played within the AnimationTree...

@sketchyfun if you found a crash, please PLEASE file a new issue. If it is crashing, it is not a known bug. Include a minimal reproduction project and the version of the engine. Thanks!

just the fact it's linked to an animation player and set to active is enough to set any used property animation tracks to 0

Any track which enters the track cache will be output, either to the RESET animation value or to zero. Here is a PR #60093 which shows some of the code in question:

I could see it being expected behavior that tracks which you do not hit do not output. HOWEVER, note that attempting to fix the system in this way would create a different problem:

  • In Unity, they have some bizarre behavior. Basically, all tracks of all animations are output by an Animator every frame. Until an animation writes to that track, Unity reads in the current value of that output on the frame the animator was enabled, and uses that. Sometimes this causes unexpected behavior.
  • Another alternative approach could be to only add tracks to the track cache once they have been hit, but that could lead to a different set of unexpected behavior when an animation goes from state A (outputs value V) then to state B (outputs value V + W) and then back to state A (now also outputs V + W???)
  • Another possible fix for your issue might be to only add tracks from the set of Animations which are reachable from any blend tree / state machine nodes. I think this could work, but it also feels a little unexpected, that adding an unused state in the state machine affects outputs from other states. (This behavior exists in Unity, for example, and can be rather confusing)

So it's a little unclear to me what the expected behavior should be in your case. It ought to be possible to use a track filter to filter for the specific output tracks you want. This might require going through an AnimationNodeBlendTree at the root: I would be curious if this works: regardless of the blending behavior, it should be possible to filter tracks you want.

Finally, note that there is work in progress to overhaul AnimationTree and AnimationPlayer to share a common class rather than having wildly different code. This might make it easier to fix these types of bugs.

@sketchyfun
Copy link
Contributor

sketchyfun commented Aug 31, 2023

Forgive me, but I'm still having trouble understanding where this issue arises from? If one animation affects position and another affects rotation, why is it that playing (via an AnimationTree) the position animation resets the rotation, and the rotation animation resets the position?

I kind of understand that if equivalent tracks are missing in the destination animation then it would need some other values to blend towards (zero, or RESET values I guess?), but surely if there is a blend time of 0, aka no blending between the two animations, then this shouldn't occur?

I guess it blends regardless of blend time... Which is strange, as I was expecting the properties to be left alone if moving to a new animation that doesn't touch that particular property. As it is now, it actually makes it kind of useless

@TokageItLab
Copy link
Member

TokageItLab commented Aug 31, 2023

Missing tracks are not processed means that when changing the blend amount of multiple AnimationNodes, the results will not equal depending on the order in which the AnimationNodes are operated by you.

We are currently working on unifying AnimationPlayer's blending process with AnimationTree, which should improve this issue.

Setting the initial value as Reset works well in many cases in a blending-based system like AnimationTree, but is not ideal for queued playback like AnimationPlayer.

The StateMachine works similarly to a queue. However, it relies on the AnimationTree blending process, so internally there is no "fixed animation blending order" such as a "queue"; Simply said, it can't know which animation will be processed first, the current animation or the destination animation of the transition. In other words, there is no guarantee that the playback result will always be correct between the same states in the old blending algorithm.

My plan is to refactor the AnimationPlayer/Tree to allow it to not process tracks when initial values are missing as an option (but default value in AnimationPlayey is true and AnimationTree is false), however perhaps this option would come at the disadvantage of the possibility that the playback results would not always be equal for complex blending states especially for the AnimationTree.

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

Successfully merging a pull request may close this issue.

6 participants