-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
Add AnimationTree Advance Expressions #54327
Conversation
6a8d5c6
to
c87065f
Compare
Out of curiosity, is there a property hint available in GDScript for exposing an expression hint in the inspector? This may be useful for people writing dialogue or level design-oriented add-ons. |
@Calinou I think we auto parse those now, so doing |
* Allows specifying an expression as a condition for state machine transitions. * For this to work safely (user not call queue_free or something in the expression), a const call mode was added to Object and Variant (and optionally Script). * This mode ensures only const functions can be called, making it safe to use from the editor. * Bonus: Fixed an animation import bug in Collada. This gives much greater flexibility for creating complex state machines. By directly interfacing with the script code, it is possible to create complex animation advance condition for switching between states.
c87065f
to
a6e72a9
Compare
This is awesome and will help to solve some issues. |
This design purposely avoids the blackboard system by using the base node's variables. Which I think covers a, b and c. a) Blackboard variables (a single list of all the variables referenced in the child nodes) |
That's great news. I'll download the dev build to give it a try. |
In the tests I did I cannot achive any of this points (read a StateMachine's variable, stay in the same node without duplicating it and update a variable in the StateMachine) |
Godot4Test.zip |
@David-Ochoa I think you are misunderstanding what this does. The idea is precisely to not use variables in the animation tree but variables from your game script directly. |
I took time to make more tests and exporting a variable in a script works as a blackboard if a single script has all the variables and the script is assigned to the AnimationTree (using another node doesn't work and would make harder to test the AnimationTree): Both other suggestions I made could be implemented with workarounds (changing the variable when animation finishes with a function call track and self connecting transitions duplicating the node). As I mentioned, using other node than the AnimationTree for advance expressions will make difficult to test the AnimationTree in the editor because selecting the node to change the variable hides the AnimationTree. |
Seems like it's on the right track! It already feels like it allows a significantly degree of cleaner code organisation for complex animation state machines. There's some usability issues with input of the actual expression field, and I'm kind of wondering if this would benefit for multi-edit, but that can be saved for another PR. I do think we need an equivilent system for blend trees though since it currently exists in a kind of half-way situation where different parts of the animation tree are driven in vastly different ways. So far I've found two bugs with the system. The first I've already made a fix for which is that the button to self base expression node would be relative the current scene root rather than the animation tree V-Sekai@1b109ac The second is that the expression editor seems to send its carat back to the first character when attempting to add spaces. |
Any news on this? |
Is this a script attached to the |
You need to attach a script to the Animation Tree and export some variables, in my case the variables are IsArmed and Is Attacking |
Being able to have expression in condition helps a lot! I think this makes state machine much more flexible. I was directed to here because I have a similar pr/proposal at #59872 / godotengine/godot-proposals#4352 . Do you think if it may still be worth allowing multiple transitions with the same from and to nodes with condition expression in? Since i think allowing multiple transitions addresses the "at end" part of condition @David-Ochoa mentioned above. |
There was a sentiment that not everyone agreed on the approach. If there is agreement, we can salvage it. Does anyone know what was the disagreement? |
From me it was more about mistaking the intended functionality, now it's clear to me. I don't know if somebody else has a disagreement. |
For me at least, I don't think there was a disagreement with this approach, it always seemed fine to me, just needing some extra polish with some UI issues surrounding it. My main concern is that while this approach for state machine transition seems fine, I was hoping we could decide on a similar approach for blend tree parameters, but perhaps its not worth holding up the implementation of this feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will be fine to merge once conflicts are resolved. I'm happy to take over this PR if it would help. I think the one UX issue I have with this though is the defintion of the expression base node, specifically that using anything other than the AnimationTree itself as the expression base feels for unintuitive and bug prone given you have to manually. Something that might help with that is adding an expression base on the AnimationTree node itself which can be used as default for nodes where no expression base is defined.
@reduz I've updated this branch to be rebase it onto master, but also add a few things which should address some of the issues I noticed with it, mainly UX stuff: fixing the selection of the expression base node from the AnimationTreeEditor, fixing edge stripping causing problems when typing spaces at the beginning or end of a new expression, and adding the ability to set an expression base node on the AnimationTree node itself for when one isn't explictly specified by the transition nodes. |
@SaracenOne feel free if you want to take it over to do a PR and co-author me |
Yeah, happy to take this over since I realise I was one of the people who initially advocated for it, but generally, what's the process involved in doing so? |
@SaracenOne Correct me if I'm wrong, but I think you just make a new PR and call it a day. |
Superseded by: #61196 |
This gives much greater flexibility for creating complex state machines. By directly interfacing with the script code, it is possible to create complex animation advance condition for switching between states.
How it looks:
Tree it would apply to: