Fix #5417 #6929
Merged
Fix #5417 #6929
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This issue is most obvious in elevators with multiple doors. When the collider gets invalidated multiple times in quick succession, multiple futures are created. If a future which was started before all door state updates were finalized completed after all other futures, the final simplifiedEntityColliders would be outdated and invalid (containing ghost doors, or in the case of an elevator in motion, missing door collisions). This commit addresses this issue in two ways: First by cancelling any existing future to ensure that any future which completes was started after the most recent invalidation. Second by removing the null assignment of simplifiedEntityColliderProvider from the future. This prevents the future from becoming null after a null check and before a cancellation (the only time where the null value matters). Cancelling a future twice is not an issue so there's no need to track if the future is null other than to avoid a null dereference.
IThundxr
added
pr type: fix
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I don't do Minecraft mods, but I believe based on my understanding of the code and Java that this should address a race which would cause #5417. I have tested this in 1.18.2, and 1.20.1 on both this Forge version and the Fabric port and the issue which I could reproduce around once every 2 tries could no longer be reproduced in about 40 total tries (I'm pretty confident that logically this is the correct code but I am happy to discuss it if someone disagrees).