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

AnimationMixer: Validate ObjectID before blend in case the object was freed #85461

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Nov 28, 2023

Works around #85365, but it's likely only a partial fix. The proper fix would be to remove the Object pointer from the TrackCache and always go back to the ObjectID before doing operations like this.

Thanks @bitsawer for the help debugging.

I'd say this doesn't full address #85365 as there are many other cases where t->object is dereferenced without being sure it's valid. But for 4.2 this might be a suitable workaround for at least one known crash, until this is refactored for 4.3.

I confirmed this fixes the crash in Liblast and in the MRPs of #85365.

… freed

Works around godotengine#85365, but it's likely only a partial fix.
The proper fix would be to remove the Object pointer from the TrackCache
and always go back to the ObjectID before doing operations like this.
@akien-mga akien-mga added this to the 4.2 milestone Nov 28, 2023
@akien-mga akien-mga requested a review from a team as a code owner November 28, 2023 11:29
Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Makes sense, these values appear to be in sync throughout the codebase (in fact, we always take object id from the already stored object).

@akien-mga akien-mga merged commit 83ae2b1 into godotengine:master Nov 28, 2023
15 checks passed
@akien-mga akien-mga deleted the AnimationMixer-validate-object-before-blend branch November 28, 2023 12:39
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.

2 participants