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

Rework for nested AnimationNodeStateMachine #75759

Merged

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Apr 6, 2023

Prerequisite #75111. Now it has been included in this.

This PR replaces/removes the previous broken nested StateMachine behavior. Previously, StateMachine stored pointers to AnimationNode within child StateMachines. However, storing pointers to other Resources within a Resource was unsafe because it could lead to duplicated processing within the animation blending process. It was causing problems similar to #22887.

This PR does not store pointers directly, but generates paths to PlaybackObject so that the child (or parent) state machine pointers are accessed only at the moment of the blending process.

Also, make the PlaybackObject readonly and it is not stored in tres. This prevents the PlaybackObject from having the same reference, since the PlaybackObject is created with a unique value each time.

Make the StateMachine to calculate the remain time at the transition to End state

Until now, the remain time was always treated as 1 second until the End state was reached. This fix will allow the fade-out to be handled correctly when using the StateMachine from the outside such as BlendTree.


Implement three types of StateMachine for different use cases

RootStateMachine

This is the default state machine. The most standard StateMachine, applicable to most use cases.

image

When a seek to the beginning is commanded, a transition is made to StartState.

Godot.2023.04.07.-.00.22.51.07.mp4

And remain time is calculated only when transitioning to End state.

NestedStateMachine

It is useful to list several animations, such as StateMnachine with NodeTransition. You can use this like AnyState. Seeking to the beginning is treated as seeking to the beginning of the animation in the current state.

image

The remain time is calculated not only at the transition to End state, but also when the animation is stopped due to a state with no transition.

GroupedStateMachine

This is useful for organizing the StateMachine so that it does not make it too messy.

With some restrictions, GroupedStateMachine allows direct travel between StateMachines.

Godot.2023.04.07.-.00.22.51.06.mp4

Restrictions

Users cannot directly request play/travel to a StateMachinePlayback of GroupedStateMachine, since GroupedStateMachine must be processed by the parent StateMachinePlayback.

This means that a GroupedStateMachine must have a Root/Nested StateMachine as a parent or ancestor.

For example, you can play/travel in the parent Root/Nested StateMachine as follows:

root_playback.travel("Combo/Combo1")

The Start/End transition of a GroupedStateMachine does not allow parameters to be set. This is because the transitions are referenced as shown in the following figure:

image

Then, the number of transitions to/from the GroupedStateMachine and the number of internal Start/End transitions must be matched.

It is also recommended that no more than one transition be made to a GroupedStateMachine. This is because there is currently only one input port to the StateMachine, so if there are two or more transitions, it is not possible to specify the reference for each.

However, this problem can be solved in the future by improving the StateMachine to have more than two input ports, or by increasing the number of StartNodes, etc.

Incidentally, to ensure that the reference to transition is correct, direct transitions to the Start or End State of a GroupedStateMachine is not allowed.

root_playback.travel("Combo/Start") # It's not alllowed.

However, for convenience, if a travel is specified to a GroupedStateMachine, it is implicitly replaced by a travel to the next State after Start if a Start transition exists.

root_playback.travel("Combo")
root_playback.travel("Combo/Combo1") # Same result with above.

And, of course, if the priority depend on the ASter algorithm, the equality of travel path is not guaranteed before and after they are grouped.

TODO in the future

  • Update document
  • Optimize to cache parent-child relationships for path retrieval as like Optimize Node children management #75627
  • Increase the number of input ports or StartNodes for grouped state machines
  • Allow grouping using the GUI but the priority of this is low because the equality of transitions before and after can't be guaranteed without doing above (increasing port) first

state_machine_improvement_demo.zip
state_machine_improvement_demo_2.zip

@fire
Copy link
Member

fire commented Apr 7, 2023

Lyuma is testing nested nested state machines. We are discussing off github.

@alfredbaudisch
Copy link
Contributor

Is this one also related #70338?

@TokageItLab
Copy link
Member Author

TokageItLab commented Apr 7, 2023

@alfredbaudisch No, #70338 is related godotengine/godot-proposals#5972. Well, it is the next major task we should work on after this PR.

This PR is proposal of godotengine/godot-proposals#6130.

Copy link
Member

@SaracenOne SaracenOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long, but yes, having looked through the code and experimented with it, it looks good to me! 👍

@YuriSizov
Copy link
Contributor

Commits need to be squashed. And could you please do a write-up of how the compatibility is broken in this PR and how users should handle it in their projects?

Breaking compatibility: If a StateMachineTransition is connected to a nested StateMachine prior to this, it is removed. Also, there was a feature to connect another StateMachine as the End of a StateMachine, which is also removed to avoid reference confusion. It was like a GoTo Statement, also further passing the same reference to the blending process, causing the blending calculation to break or causing some StateMachines to not work.
@TokageItLab TokageItLab force-pushed the reimplement-grouped-statemachine branch from 36d9fea to 991e6e9 Compare April 18, 2023 10:07
@TokageItLab
Copy link
Member Author

TokageItLab commented Apr 18, 2023

Squashed.


Breaking compatibility: Most old StateMachines will work as is, only some transitions may be removed.

  • If a StateMachineTransition is connected to a nested StateMachine prior to this, it is removed
    • [Fixing way]: Simply re-connect it
  • Also, there was a feature to connect another StateMachine as the End of a StateMachine, which is also removed to avoid reference confusion
    • [Fixing way]: This requires actually repositioning the StateMachine as a child of the StateMachine, not the StateMachine as its reference. That means placing a loaded StateMachine, such as saving the StateMachine to .tres, and then re-connecting to it
      • At this point, if you need to loop several StateMachines, you must have two same StateMachines that you want to loop and create a transition that will loop them at their parent (e.g. [A->B->C->A] to [A->B->C]<=>[A->B->C])

Since it was like a GoTo Statement, also further passing the same reference to the blending process, causing the blending calculation to break or causing some StateMachines to not work.

I believe that few people used them heavily because they were broken completely to begin with (like this comment #71562 (comment)). So, if we make converter for automating above would require a lot of work, then I think it is little worth doing it.

@akien-mga akien-mga merged commit 13544fb into godotengine:master Apr 24, 2023
@akien-mga
Copy link
Member

akien-mga commented Apr 24, 2023

Thanks! Amazing work 🎉

10 bugs closed 😮

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