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 another collision shape editor crash #76798

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented May 7, 2023

The collision shape editor checks in NOTIFICATION_PROCESS if the shape has not changed. In theory it should ensure that the shape is valid before trying to draw it. But turns out, for whatever reason, and contrary to the "Clear" context menu option, the revert arrow will remove the shape somewhere between NOTIFICATION_PROCESS and drawing function, which results in redraw with old data.

The PR makes the draw method use the currently cached shape, which means that in the aforementioned corner-case, the editor will draw old shape for a single frame.
Of course an alternative is adding a null check for node's shape, but the current_shape is updated reliably, so why not use it.

Fixes #76771

@KoBeWi KoBeWi added this to the 4.1 milestone May 7, 2023
@KoBeWi KoBeWi requested a review from a team as a code owner May 7, 2023 00:37
@Rindbee
Copy link
Contributor

Rindbee commented May 7, 2023

The main reason for #76771 is that shape is reset to null, but shape_type is not reset to -1. The data in CollisionShape2DEditor has not been updated yet.

CollisionShape2D does not emit a signal when its shape is reset to another shape. This caused CollisionShape2DEditor to fail to update the data in time.

@akien-mga akien-mga merged commit 41f1ec1 into godotengine:master May 8, 2023
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the race_condition_except_there_is_no_thread branch May 8, 2023 10:26
@akien-mga
Copy link
Member

Cherry-picked for 4.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants