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 infinite loop state check in AnimationStateMachine #79141

Merged
merged 1 commit into from
Jul 7, 2023

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Jul 7, 2023

Fixes #79012.

If only the first pass is stored and compared, the loop detection will fail if an infinite loop starts in the middle of the path. So, all transition paths must be stored and compared. Also, remove is_state_changed = false as it was actually wrong (state change occurs just before the try infinite loop).

@TokageItLab TokageItLab requested a review from a team as a code owner July 7, 2023 05:36
@TokageItLab TokageItLab added bug high priority crash cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Jul 7, 2023
@TokageItLab TokageItLab added this to the 4.2 milestone Jul 7, 2023
@TokageItLab TokageItLab force-pushed the fix-infinity-state-loop branch from 036b144 to 79656d1 Compare July 7, 2023 05:47
@akien-mga
Copy link
Member

How is the issue communicated to the user when such a potential infinite loop is detected? I see it breaks but doesn't raise an error (here at least, it maybe does further up the stack). Will users understand that their setup is non functional due to recursion?

@TokageItLab TokageItLab force-pushed the fix-infinity-state-loop branch from 79656d1 to d197a77 Compare July 7, 2023 06:35
@TokageItLab
Copy link
Member Author

TokageItLab commented Jul 7, 2023

Perhaps the state continues to change every frame, but that is a normal transition and not an error.

Since Nodes in StateMachine cannot have a path to itself, if this check works, there is no longer an infinite loop in the true sense of the word.

@TokageItLab TokageItLab force-pushed the fix-infinity-state-loop branch from d197a77 to 9948ba0 Compare July 7, 2023 06:49
@TokageItLab
Copy link
Member Author

TokageItLab commented Jul 7, 2023

@akien-mga I've added a warning in case you're concerned.

image

@TokageItLab TokageItLab force-pushed the fix-infinity-state-loop branch 3 times, most recently from 72b4c34 to 7b66897 Compare July 7, 2023 06:53
@TokageItLab TokageItLab force-pushed the fix-infinity-state-loop branch from 7b66897 to fc40ba2 Compare July 7, 2023 07:12
@YuriSizov YuriSizov changed the title Fix infinity loop state can't break in AnimationStateMachine Fix infinite loop state check in AnimationStateMachine Jul 7, 2023
@akien-mga akien-mga merged commit dcbbde5 into godotengine:master Jul 7, 2023
@akien-mga
Copy link
Member

Thanks!

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.1.

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jul 10, 2023
@TokageItLab TokageItLab deleted the fix-infinity-state-loop branch February 14, 2024 05:29
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.

Crash when setting AnimationTree active with circular dependency (infinite loop)
4 participants