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 State Machine: allow multiple transitions with the same from and to nodes #59872

Closed

Conversation

WindyDarian
Copy link
Contributor

@WindyDarian WindyDarian commented Apr 4, 2022

I have a use case that is kind of hard to work around with the restriction of only one transition for each direction between two nodes. The simplified version is my character has attack and idle animation states, and I want to go back from attack to idle when either the animation is fully played, or the attack is interrupted for whatever reason. If I have only one transition with "At End", it seems I cant easily force interrupt the state to go to the next state.

So I hacked around a bit and allowed multiple transitions from and to the same nodes. Seems work for my case, but it's not the most performant code, and not thoroughly tested. I wonder if the use case is common enough that I should submit a pull request still :P
transitions

Update 5/4/2022:
Changed the code to work with #24402 . So now the duplicate transtions can exist in a multi-transtion. The code change is now removing restrictions that don't allow transitions with the same from and to nodes.

So it now looks like this - transitions within the same multi transition, with the same from and to node, but different conditions.

image
image

@WindyDarian WindyDarian requested review from a team as code owners April 4, 2022 12:54
@Calinou Calinou added this to the 4.0 milestone Apr 4, 2022
@Diddykonga
Copy link
Contributor

I support the idea, although It's generally always recommended to open an Proposal first and link it in the PR, even if the idea is one that you think is guaranteed to have wide-spread support or usage. 👍

@TokageItLab
Copy link
Member

TokageItLab commented Apr 5, 2022

Use cases exist, but they need to be properly designed with discussing with @reduz. It is clear that the current AnimationStateMachine lacks interruption capability, and a proposal already exists as godotengine/godot-proposals#2200 and godotengine/godot-proposals#2523.

@WindyDarian As for the implementation of the transition branch, a similar pr already exists by @reduz as #54327, so I suggest you send feedback to it on that thread.

@WindyDarian
Copy link
Contributor Author

WindyDarian commented Apr 5, 2022

Thank you all. I added a proposal here godotengine/godot-proposals#4352

I feel this is still nice to have even if #54327 gets merged in. Since it may be harder to express "at animation end" as part of the condition expression than a transition type like it currently is, and having multiple transitions can allow things like different crossfade duration in different conditions.

Allow multiple transitions in AnimationNodeStateMachine with the same from and to nodes, but may have different transition conditions.

(originally 140987e, but updated to be part of multi transition)
@WindyDarian WindyDarian force-pushed the state_machine_transition branch from 140987e to 208b03f Compare May 4, 2022 04:06
@WindyDarian
Copy link
Contributor Author

WindyDarian commented May 4, 2022

Changed the code to work with #24402 . So now the duplicate transtions can exist in a multi-transtion. The code change is now removing restrictions that don't allow transitions with the same from and to nodes. (no more need for hacky line offset :P)

Edited this PR post on top and added new screenshots.

@akien-mga akien-mga changed the title Animation State Machine: allow multilple transitions with the same from and to nodes Animation State Machine: allow multiple transitions with the same from and to nodes May 4, 2022
@reduz
Copy link
Member

reduz commented Jun 23, 2022

If I am not mistaken, I think this is better superseded by #61196, which is simpler and more flexible.

@reduz
Copy link
Member

reduz commented Jun 23, 2022

@TokageItLab @fire ?

@WindyDarian
Copy link
Contributor Author

i think #61196 covers most of the use cases (and the one i need), so I don't mind closing this.

Though with this PR, different transitions connecting the same nodes, under different conditions, can have different cross fade duration and priority, which might be useful.

@reduz
Copy link
Member

reduz commented Jul 27, 2022

I think we should probably close this, as I understand the gains are marginal and this is a more complex solution than what is there. Even if the use case would come to have different fade speeds, we could make it call a second function for this.

@reduz
Copy link
Member

reduz commented Jul 27, 2022

That said, this was an awesome piece of work and thanks a lot for putting together the time to submit it!

@WindyDarian
Copy link
Contributor Author

closing. thanks!

@MarkoSFG
Copy link

MarkoSFG commented Mar 6, 2024

I think we should probably close this, as I understand the gains are marginal and this is a more complex solution than what is there. Even if the use case would come to have different fade speeds, we could make it call a second function for this.

Ah man, I wish this wasn't scuttled. Multiple crossfade times between two states is pretty vital if you don't want the result to look super janky / floaty in 3d (0 crossfade at natural animation end, 0.25 crossfade for interrupt) and working around the issue is super messy.

@TokageItLab
Copy link
Member

TokageItLab commented Mar 6, 2024

I assume that the essence of the problem of awkward transitions, not only in 3D, is that StateMachine support only one crossfade at the same time, and this PR does not seem to have much to do with the problem.

Currently, AnimationStateMachinePlayback only handles two animations at a time, fading_from and current, so to correctly handle multiple transitions in such a case, it would be necessary to list them and handle multiple animations. Related #37001.

@AThousandShips AThousandShips removed this from the 4.0 milestone Mar 6, 2024
@MarkoSFG
Copy link

MarkoSFG commented Mar 6, 2024

It's not quite the same issue but yeah blending from a transition between two states into a third state is also definitely necessary.

@WindyDarian
Copy link
Contributor Author

I think there are 3 different needs:

  1. with nodes A and B, allow transition from A to B under more than one conditions. (Add AnimationTree Advance Expressions #61196 has this)
  2. with nodes A and B, when transitioning from A to B under different conditions, allow different fade times (this PR would have this)
  3. allow blending between more than two states during transition, or more than one fade happening at a time, for example, if going from A to B, before fade finishes go to C, should result in an animation blend of all A, B and C (Fix for short animation blend taking too long when played during a long animation blend. #37001 )

for 2, for now I still have to hack around. Though this PR is too outdated, maybe we can reopen godotengine/godot-proposals#4352 as a general request? For context, I know Unreal Engine and another proprietary engine allows more than 1 transitions between 2 nodes, and that is usually how I do animation interrupts in these engines.

@MarkoSFG
Copy link

MarkoSFG commented Mar 31, 2024

I assume that the essence of the problem of awkward transitions, not only in 3D, is that StateMachine support only one crossfade at the same time, and this PR does not seem to have much to do with the problem.

Currently, AnimationStateMachinePlayback only handles two animations at a time, fading_from and current, so to correctly handle multiple transitions in such a case, it would be necessary to list them and handle multiple animations. Related #37001.

After forking godot and adding (and fixing) various things (shared parameters, multiple transitions to / from same nodes) with the state machine code I am now also more familiar with what is missing. As you mentioned; blending is definitely required between A->B and also (A->B)->C and also ((A->B)->C)->D (it shouldn't matter how many stacked transitions are happening, it should be able to support it - keeping in mind that reasonably you'd expect no worse than (A->B)->C for brief moments at a time).

However that is just the tip of the state machine iceberg of problems / missing functionality. Along with the "teleport blending" mentioned above you also need a lot of currently missing UI / UX niceness; "AnyState" node that is purely UI to allow for ordered (important!) transitions with non-zero fade times to various state nodes. Grouped state machines; need to be able to go from parent's "AnyState" to any nodes within the grouped state machine, they also need to be able to exit to specific nodes in their parent state machine (very frequently going from any one shot back to locomotion).

I can see why so many people opt to use monstrous blend trees in place of state machines (but that is its own flavor of unusable). I hope that in the medium term (few years, etc) we can bring across at least the level of state machine usability that Unity3d has; you really do need it for non-trivial 3d projects.

@MarkoSFG
Copy link

MarkoSFG commented May 5, 2024

I've implemented "Any State", SubState state machine type (stores nodes for UI cleanliness but needs a parent state machine to actually playback its states) as well as support for multiple ongoing fades / transitions (using Any State "set_trigger") here;

https://github.com/MarkoSFG/godot

There are several other changes / additions as well; multiple transitions from one node to another, shared parameters for blend nodes (so you can just change the shared parameter and have it affect however many blend nodes used that shared parameter instead of having to explicitly change the value on every blend node), type folding / expanding for inspector (convenient when editing multiple objects of same type, while not introducing visual clutter of always expand all), bulk import settings (folder based settings, mostly for adding animations "Save to File" automatically as well as collating them in target animation library) and a handful of other misc small fixes / additions.

@TokageItLab at this stage I want to progress further with my project; and as a result put my changes through more testing and what not but maybe it'll be helpful as a reference for multiple ongoing transitions, etc? Down the line I'll look at bringing across some changes one by one but in the meantime if anyone wants to grab anything they can :)

Sorry for hijacking this thread @WindyDarian.

@TokageItLab
Copy link
Member

TokageItLab commented May 5, 2024

@MarkoSFG A quick impression of your repository changes:

I recommend that you look into the existing functions and proposals in more detail (but we apologize for any lack of documentation on the recent changes) before walking down your own way, and then organize the missing functions individually as a proposal or issue. Since we can discuss the appropriate approach before the actual coding.

@MarkoSFG
Copy link

MarkoSFG commented May 5, 2024

@TokageItLab thanks for taking a look; the main differences for SubState vs Grouped are;

-how blending and transitions interact (for example you can't set up a transition from within one grouped state machine directly into a node within another grouped state machine; and that's fair enough, it wasn't meant for that)
-SubState does not playback its states, it exists purely as a UI organizer for other state machines (and small bit of functionality in that its "Start" state just decides which node any transitions coming into the state machine itself should blend to; there can be multiple transitions from Start -> nodes in a SubState machine which logs errors in other state machines)

I could have cannibalized Grouped but that would break functionality for people using it as it is now as there are significant differences between the two. Essentially the goal with SubState was to clean up editor UI; it doesn't ultimately matter how that's achieved as long as a state machine can have a whole bunch of states and present it in a clean (and nested) manner - SubState just did that using the existing state machine systems but it's certainly not the only way to implement it under the hood.

I looked into the state machines and tried to set things up with what was there; but the transition code could not transition from one state to another unless there was an explicit transition between the two without some sort of Any State node / similar. To be explicit; nodes within one state machine need to be able to transition to each other (from any node to any other node within the same state machine) - not for a node to transition to a grouped set of nodes (Grouped state machine, etc). This results in complex state machines being massive spiderwebs that are hard to use (both to maintain and to set up in the first place). I tried to set up what I needed before embarking on making engine changes and it just wasn't feasible (this is a data setup that worked on 2 previously shipped titles at studios I've worked at in the past and it worked well). I have seen people typically use colossal BlendTrees in Godot instead to solve what Unity3d can do with its Animator component in a (in my opinion) cleaner manner. If you take a look at the Unity3d Animator component and compare it to the Godot workflow for larger / more complex state machines you'll see there are some very important differences.

That blackboard proposal seems interesting but is slightly different than the shared parameters I added; the shared parameters are purely for determining blend amount (Blend2, Blend3, 2d, 1d, etc) not for determining transitions, etc - it's a smaller scale solution that purely addresses easily updating blend amounts on certain nodes during runtime (without having to explicitly update every node in the state machine); but maybe blackboards can handle blend amount as well? An exercise for whoever undertakes blackboards for sure.

Mostly I just wanted to share the changes I've done for my own benefit if it will help with the Godot goals with its animation system; I realize that people will have their own requirements and desires for what the animation system should be and what it should be able to do; so if it's not all that useful; that's fine :) no big deal.

@TokageItLab
Copy link
Member

TokageItLab commented May 5, 2024

how blending and transitions interact (for example you can't set up a transition from within one grouped state machine directly into a node within another grouped state machine; and that's fair enough, it wasn't meant for that)

As mentioned above, this is definitely same feature which deprecated/removed in 4.1, and it was concluded that this should not be done from a software architecture viewpoint. It causes the pointer confusion problem and the same problem as with the GoTo statement.

To explain the root cause, the StateMachine (and all of other AnimationNode classes) entity is a resource reference as .tres, which may be referenced by multiple AnimationTrees and AnimationNodes, so when StateMachine resources are connected to each other with a pointer, they become messed up.

So StateMachine must always transition through a parent-child relationship.

This results in complex state machines being massive spiderwebs that are hard to use (both to maintain and to set up in the first place).

This should not be the case. I am often asked about this, but Godot's StateMachine can be teleported by start() without any transition, it is reason that a NestedStateMachine can behave like an AnyState. You can save a NestedStateMachine which contains all animation without transiton as a .tres, place it as instance (and you can duplicate it) in the parent RootStateMachine, and transition them between each other. I am in the process of writing a document to explain these StateMachine behaviors for blogpost since a large number of changes were made between 4.0 and 4.3.

Finally, as I have pointed out, many of the changes in the repository are not agreeable or require significant argument, but if you are interested in implementing only godotengine/godot-proposals#4352, I assume we can support that.

@MarkoSFG
Copy link

MarkoSFG commented May 5, 2024

Yup - I understand your point about StateMachine and how it's been structured and the way I've implemented it doesn't have to be the way it's done (where I used the SubState state machine as a data holder only); ultimately the end user (game dev) doesn't care how things are done under the hood and any solution that achieves UI cleanliness / usability / organization is just as good.

For what I was meaning about spider webs; teleporting without transition is certainly possible but practically unusable for any game that cares at all about animation quality / action RPGs that rely on animations (vs something like RTS games possibly where you're zoomed out / where animations are not the focus and it wouldn't be as much of an issue); so what I've seen people do to solve it with Godot is to make giant blend trees where they can achieve the same end result for players at the cost of it being harder to work with on their end.

Thanks for taking a look, maybe there's a chance what I've proposed will become useful in the future but no worries if not. I'll keep updating the same repo as I go.

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

Successfully merging this pull request may close these issues.

8 participants