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 a crash bug in LightmapGI::_assign_lightmaps triggered after reparenting #101853

Merged

Conversation

jamesmintram
Copy link
Contributor

@jamesmintram jamesmintram commented Jan 20, 2025

Fix a crash bug in LightmapGI::_assign_lightmaps triggered after reparenting

After reparenting the LightmapGI node, the following node would be initialized to null:

Node *node = get_node(light_data->get_user_path(i));

Which would then crash when de-referenced further down. This fix detects which the node has been reparented and clears its users.

@jamesmintram jamesmintram force-pushed the jamesrm/lightmapgi-crash-fix branch from 8b20344 to fa7ed3f Compare January 20, 2025 23:03
@Calinou Calinou added bug topic:rendering crash topic:3d cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Jan 21, 2025
@Calinou Calinou added this to the 4.4 milestone Jan 21, 2025
@AThousandShips AThousandShips changed the title [WIP] Fix a crash bug in LightmapGI::_assign_lightmaps triggered after repa… [WIP] Fix a crash bug in LightmapGI::_assign_lightmaps triggered after reparenting Jan 21, 2025
@jamesmintram jamesmintram marked this pull request as ready for review January 21, 2025 15:16
@jamesmintram jamesmintram requested a review from a team as a code owner January 21, 2025 15:17
@jamesmintram jamesmintram changed the title [WIP] Fix a crash bug in LightmapGI::_assign_lightmaps triggered after reparenting Fix a crash bug in LightmapGI::_assign_lightmaps triggered after reparenting Jan 21, 2025
@badsectoracula
Copy link
Contributor

I tried this PR and it does work as expected. I didn't get Godot to crash without it, though it did spam a lot of errors that went away with the PR.

@clayjohn clayjohn requested a review from BlueCube3310 February 5, 2025 04:39
Copy link
Contributor

@BlueCube3310 BlueCube3310 left a comment

Choose a reason for hiding this comment

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

Looks good to me

…renting

After reparenting the LightmapGI node, the following `node` would be
initialized to null:

Node *node = get_node(light_data->get_user_path(i));

Which would then crash when de-referenced further down. This fix detects
which the node has been reparented and clears its users.
@akien-mga akien-mga force-pushed the jamesrm/lightmapgi-crash-fix branch from fa7ed3f to 80096e9 Compare February 5, 2025 11:19
@Repiteo Repiteo merged commit 819874b into godotengine:master Feb 5, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Feb 5, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release crash topic:rendering topic:3d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants