Skip to content

Fix unloaded chunk reads during parallel ticking #1514

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.

Sign up for GitHub

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

Open
ColinVaughn wants to merge 1 commit into
base: main
Choose a base branch
from
+50 −12
Open

Fix unloaded chunk reads during parallel ticking #1514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/src/main/java/dev/ryanhcode/sable/sublevel/system/SubLevelPhysicsSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ public RigidBodyHandle getPhysicsHandle(@NotNull final ServerSubLevel subLevel)
* @param oldState the old block state
* @param newState the new block state
*/
public void handleBlockChange(final SectionPos sectionPos, final LevelChunkSection section, final int localX, final int localY, final int localZ, final BlockState oldState, final BlockState newState) {
public synchronized void handleBlockChange(final SectionPos sectionPos, final LevelChunkSection section, final int localX, final int localY, final int localZ, final BlockState oldState, final BlockState newState) {
final ChunkPos chunk = sectionPos.chunk();
final LevelPlot plot = ((SubLevelContainerHolder) this.level).sable$getPlotContainer().getPlot(chunk);
if (plot != null) {
Expand Down
25 changes: 15 additions & 10 deletions common/src/main/java/dev/ryanhcode/sable/util/LevelAccelerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import net.minecraft.world.level.chunk.LevelChunk;
import net.minecraft.world.level.chunk.LevelChunkSection;
import net.minecraft.world.level.material.FluidState;
import org.jetbrains.annotations.NotNull;
import net.minecraft.world.level.material.Fluids;
import org.jetbrains.annotations.Nullable;

/**
Expand All @@ -40,7 +40,7 @@ public LevelAccelerator(final Level level) {
this.minSection = level.getMinSection();
}

public void clearCache() {
public synchronized void clearCache() {
this.cachedLevelChunks.clear();
this.cachedChunkObj = null;
this.cachedChunkPos = 0L;
Expand All @@ -58,13 +58,14 @@ public void setBlockFast(final BlockPos blockPos, final BlockState blockState) {

@Override
public @Nullable BlockEntity getBlockEntity(final BlockPos blockPos) {
return this.level.getBlockEntity(blockPos);
final LevelChunk chunk = this.getChunkIfLoaded(blockPos.getX() >> 4, blockPos.getZ() >> 4);
return chunk == null ? null : chunk.getBlockEntity(blockPos);
}

@Override
public BlockState getBlockState(final BlockPos pos) {
final LevelChunk chunk = this.getChunk(pos);
return this.getBlockState(chunk, pos);
final LevelChunk chunk = this.getChunkIfLoaded(pos.getX() >> 4, pos.getZ() >> 4);
return chunk == null ? Blocks.AIR.defaultBlockState() : this.getBlockState(chunk, pos);
}

/**
Expand All @@ -85,16 +86,20 @@ public BlockState getBlockState(final LevelChunk chunk, final BlockPos pos) {

@Override
public FluidState getFluidState(final BlockPos pos) {
final LevelChunk chunk = this.getChunk(pos);

return chunk.getFluidState(pos);
final LevelChunk chunk = this.getChunkIfLoaded(pos.getX() >> 4, pos.getZ() >> 4);
return chunk == null ? Fluids.EMPTY.defaultFluidState() : chunk.getFluidState(pos);
}

public LevelChunk getChunk(final BlockPos pos) {
return this.getChunk(pos.getX() >> 4, pos.getZ() >> 4);
}

public LevelChunk getChunk(final int chunkX, final int chunkZ) {
final LevelChunk chunk = this.getChunkIfLoaded(chunkX, chunkZ);
return chunk == null ? this.level.getChunk(chunkX, chunkZ) : chunk;
}

private synchronized @Nullable LevelChunk getChunkIfLoaded(final int chunkX, final int chunkZ) {
final long pos = ChunkPos.asLong(chunkX, chunkZ);

if (pos == this.cachedChunkPos && this.cachedChunkObj != null) {
Expand All @@ -115,7 +120,7 @@ public LevelChunk getChunk(final int chunkX, final int chunkZ) {
return chunk;
}

private @NotNull LevelChunk grabChunkFast(final int chunkX, final int chunkZ, final long pos) {
private @Nullable LevelChunk grabChunkFast(final int chunkX, final int chunkZ, final long pos) {
if (this.level.isClientSide) {
return this.level.getChunk(chunkX, chunkZ);
}
Expand All @@ -129,7 +134,7 @@ public LevelChunk getChunk(final int chunkX, final int chunkZ) {
return res;
}

return this.level.getChunk(chunkX, chunkZ);
return null;
}

public boolean isOutsideBuildHeight(final Vec3i pos) {
Expand Down
33 changes: 33 additions & 0 deletions neoforge/src/main/java/dev/ryanhcode/sable/neoforge/gametest/PhysicsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import dev.ryanhcode.sable.physics.config.dimension_physics.DimensionPhysicsData;
import dev.ryanhcode.sable.sublevel.ServerSubLevel;
import dev.ryanhcode.sable.sublevel.system.SubLevelPhysicsSystem;
import dev.ryanhcode.sable.util.LevelAccelerator;
import net.minecraft.core.BlockPos;
import net.minecraft.gametest.framework.GameTest;
import net.minecraft.gametest.framework.GameTestAssertPosException;
Expand All @@ -17,11 +18,43 @@
import org.joml.Vector3d;
import org.joml.Vector3dc;

import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;

import static dev.ryanhcode.sable.neoforge.gametest.SableTestHelper.*;

@GameTestHolder(Sable.MOD_ID)
public final class PhysicsTest {

@GameTest(template = "continuouscollision", timeoutTicks = 100)
public static void testUnloadedChunkReadsOffThread(final GameTestHelper helper) {
final ServerLevel level = helper.getLevel();
final BlockPos unloaded = helper.absolutePos(new BlockPos(16_384, 0, 16_384));
if (level.hasChunkAt(unloaded)) {
helper.fail("Test chunk was already loaded");
return;
}

CompletableFuture.supplyAsync(() -> {
final LevelAccelerator accelerator = new LevelAccelerator(level);
return accelerator.getBlockState(unloaded).isAir()
&& accelerator.getFluidState(unloaded).isEmpty()
&& accelerator.getBlockEntity(unloaded) == null;
})
.orTimeout(2, TimeUnit.SECONDS)
.whenComplete((safe, failure) -> level.getServer().execute(() -> {
if (failure != null) {
helper.fail("Off-thread unloaded chunk read blocked or failed: " + failure);
} else if (level.hasChunkAt(unloaded)) {
helper.fail("Off-thread read loaded the chunk");
} else if (!safe) {
helper.fail("Unloaded chunk did not read as empty");
} else {
helper.succeed();
}
}));
}

@GameTest(template = "continuouscollision")
public static void testContinuousCollision(final GameTestHelper helper) {
final ServerLevel level = helper.getLevel();
Expand Down
2 changes: 1 addition & 1 deletion ...e_rapier/src/main/java/dev/ryanhcode/sable/physics/impl/rapier/RapierPhysicsPipeline.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ public void handleBlockChange(final SectionPos sectionPos, final LevelChunkSecti
for (final Direction dir : Direction.values()) {
final BlockPos pos = globalBlockPos.relative(dir);
final VoxelNeighborhoodState state = VoxelNeighborhoodState.getState(this.accelerator, pos, null);
final RapierVoxelColliderData colliderData = this.colliderBakery.getPhysicsDataForBlock(this.level.getBlockState(pos));
final RapierVoxelColliderData colliderData = this.colliderBakery.getPhysicsDataForBlock(this.accelerator.getBlockState(pos));

final int colliderValue = colliderData == null ? 0 : colliderData.handle() + 1;
Rapier3D.changeBlock(this.scene.handle(), pos.getX(), pos.getY(), pos.getZ(), packBlockState(state, colliderValue));
Expand Down