Skip to content

Commit

Permalink
Fix race in Contraption simplifiedEntityColliders update
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EliteTK committed Sep 10, 2024
1 parent 11787d8 commit 4b59528
Showing 1 changed file with 3 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,9 @@ public void invalidateColliders() {

private void gatherBBsOffThread() {
getContraptionWorld();
if (simplifiedEntityColliderProvider != null) {
simplifiedEntityColliderProvider.cancel(false);
}
simplifiedEntityColliderProvider = CompletableFuture.supplyAsync(() -> {
VoxelShape combinedShape = Shapes.empty();
for (Entry<BlockPos, StructureBlockInfo> entry : blocks.entrySet()) {
Expand All @@ -1402,7 +1405,6 @@ private void gatherBBsOffThread() {
})
.thenAccept(r -> {
simplifiedEntityColliders = Optional.of(r);
simplifiedEntityColliderProvider = null;
});
}

Expand Down

0 comments on commit 4b59528

Please sign in to comment.