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

Fix AnimationTree state machine start() #54456

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

rafallus
Copy link
Contributor

Fixes #41475

@rafallus rafallus requested a review from a team as a code owner October 31, 2021 16:57
@rafallus
Copy link
Contributor Author

Another side effect of this PR is that the state of the State Machine is preserved even when it becomes inactive. Meaning on "restart" the initial state is not played (as the hint in the "Toggle autoplay..." button suggests). I think this behavior is more convenient, at least I wouldn't expect the State Machine to restart each time it becomes active. Please let me know what your thoughts are about this so I can make changes accordingly.

@Calinou Calinou added this to the 4.0 milestone Oct 31, 2021
@fire
Copy link
Member

fire commented Nov 3, 2021

@TokageItLab Can you review this?

@TokageItLab
Copy link
Member

TokageItLab commented Nov 4, 2021

@fire Since I am not very familiar with StateMachine, I believe that @SaracenOne or @reduz can provide a more appropriate review.

I don't use AnimationStateMachine in my projects because of its difficulty in managing transitions and lack of versatility such as godotengine/godot-proposals#2200 or godotengine/godot-proposals#2523. Also, since the StateMachine code is independent of the AnimationTree, so I've never cared about its performance or usability.

@SaracenOne
Copy link
Member

SaracenOne commented Aug 26, 2022

While the bug described in the linked issue still appears to be present, in what I assume is due to internal changes with how 'start' works in AnimationTrees now, this fix no-longer appears to work in the latest master branch (it may still be salvagable for fixing it in 3.x though). I will close this for now, but hope an updated fix can be found for this issue since I believe it severely limits the ability to serialize AnimationTrees for savegames.

@SaracenOne SaracenOne closed this Aug 26, 2022
@SaracenOne
Copy link
Member

Mistake on my behalf, a recent update in the build system caused my to accidentally test this with the wrong binary; I'm re-reviewing it now.

@SaracenOne SaracenOne reopened this Aug 26, 2022
@SaracenOne SaracenOne self-requested a review August 26, 2022 04:58
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.

Tested and seems good!

@akien-mga akien-mga merged commit 0123752 into godotengine:master Aug 26, 2022
@akien-mga
Copy link
Member

Thanks!

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.

AnimationPlayerStateMachinePlayback: start() does not work when current_node is ""
6 participants