From 13275eb5346641f3222873f8648f55386ec45495 Mon Sep 17 00:00:00 2001 From: Felix Cravic Date: Mon, 10 Aug 2020 08:55:01 +0200 Subject: [PATCH] Fixed synchronization with unloaded chunks --- .../net/minestom/server/UpdateManager.java | 2 +- .../server/collision/CollisionUtils.java | 11 ++- .../server/entity/EntityCreature.java | 6 ++ .../net/minestom/server/instance/Chunk.java | 2 +- .../minestom/server/instance/Instance.java | 6 +- .../server/instance/InstanceContainer.java | 80 ++++++++++++------- .../server/thread/PerGroupChunkProvider.java | 14 +++- .../server/utils/chunk/ChunkUtils.java | 4 + 8 files changed, 90 insertions(+), 35 deletions(-) diff --git a/src/main/java/net/minestom/server/UpdateManager.java b/src/main/java/net/minestom/server/UpdateManager.java index 46d823f14..b23f9e48c 100644 --- a/src/main/java/net/minestom/server/UpdateManager.java +++ b/src/main/java/net/minestom/server/UpdateManager.java @@ -98,7 +98,7 @@ public final class UpdateManager { * @param threadProvider the new thread provider * @throws NullPointerException if {@param threadProvider} is null */ - public void setThreadProvider(ThreadProvider threadProvider) { + public synchronized void setThreadProvider(ThreadProvider threadProvider) { Check.notNull(threadProvider, "The thread provider cannot be null"); this.threadProvider = threadProvider; } diff --git a/src/main/java/net/minestom/server/collision/CollisionUtils.java b/src/main/java/net/minestom/server/collision/CollisionUtils.java index c0fb5be66..84a64dd99 100644 --- a/src/main/java/net/minestom/server/collision/CollisionUtils.java +++ b/src/main/java/net/minestom/server/collision/CollisionUtils.java @@ -1,11 +1,13 @@ package net.minestom.server.collision; import net.minestom.server.entity.Entity; +import net.minestom.server.instance.Chunk; import net.minestom.server.instance.Instance; import net.minestom.server.instance.block.Block; import net.minestom.server.utils.BlockPosition; import net.minestom.server.utils.Position; import net.minestom.server.utils.Vector; +import net.minestom.server.utils.chunk.ChunkUtils; public class CollisionUtils { @@ -138,7 +140,14 @@ public class CollisionUtils { blockPos.setX((int) Math.floor(corner.getX())); blockPos.setY((int) Math.floor(corner.getY())); blockPos.setZ((int) Math.floor(corner.getZ())); - final short blockStateId = instance.getBlockStateId(blockPos); + + final Chunk chunk = instance.getChunkAt(blockPos); + if (ChunkUtils.isChunkUnloaded(chunk)) { + // Collision at chunk border + return false; + } + + final short blockStateId = chunk.getBlockStateId(blockPos.getX(), blockPos.getY(), blockPos.getZ()); final Block block = Block.fromStateId(blockStateId); // TODO: block collision boxes diff --git a/src/main/java/net/minestom/server/entity/EntityCreature.java b/src/main/java/net/minestom/server/entity/EntityCreature.java index 1e14aa8d9..f3b01c69b 100644 --- a/src/main/java/net/minestom/server/entity/EntityCreature.java +++ b/src/main/java/net/minestom/server/entity/EntityCreature.java @@ -346,7 +346,13 @@ public abstract class EntityCreature extends LivingEntity { return false; } + if (pathFinder == null) { + // Unexpected error + return false; + } + this.pathLock.lock(); + this.pathFinder.reset(); if (position == null) { this.pathLock.unlock(); diff --git a/src/main/java/net/minestom/server/instance/Chunk.java b/src/main/java/net/minestom/server/instance/Chunk.java index f9f8bbf4f..ee368b5cb 100644 --- a/src/main/java/net/minestom/server/instance/Chunk.java +++ b/src/main/java/net/minestom/server/instance/Chunk.java @@ -70,7 +70,7 @@ public final class Chunk implements Viewable { private PFColumnarSpace columnarSpace; // Cache - private boolean loaded = true; + private volatile boolean loaded = true; private Set viewers = new CopyOnWriteArraySet<>(); private ByteBuf fullDataPacket; diff --git a/src/main/java/net/minestom/server/instance/Instance.java b/src/main/java/net/minestom/server/instance/Instance.java index e1466e3b8..5b39297ba 100644 --- a/src/main/java/net/minestom/server/instance/Instance.java +++ b/src/main/java/net/minestom/server/instance/Instance.java @@ -134,9 +134,11 @@ public abstract class Instance implements BlockModifier, EventHandler, DataConta public abstract void loadOptionalChunk(int chunkX, int chunkZ, Consumer callback); /** - * Unload a chunk + * Schedule the removal of a chunk, this method does not promise when it will be done *

- * WARNING: all entities other than {@link Player} will be removed + * WARNING: during unloading, all entities other than {@link Player} will be removed + *

+ * For {@link InstanceContainer} it is done during {@link InstanceContainer#tick(long)} * * @param chunk the chunk to unload */ diff --git a/src/main/java/net/minestom/server/instance/InstanceContainer.java b/src/main/java/net/minestom/server/instance/InstanceContainer.java index 446d03ba9..1ade1c48b 100644 --- a/src/main/java/net/minestom/server/instance/InstanceContainer.java +++ b/src/main/java/net/minestom/server/instance/InstanceContainer.java @@ -53,6 +53,7 @@ public class InstanceContainer extends Instance { private ChunkGenerator chunkGenerator; private Map chunks = new ConcurrentHashMap<>(); + private Set scheduledChunksToRemove = new HashSet<>(); private ReadWriteLock changingBlockLock = new ReentrantReadWriteLock(); private Map currentlyChangingBlocks = new HashMap<>(); @@ -315,35 +316,13 @@ public class InstanceContainer extends Instance { @Override public void unloadChunk(Chunk chunk) { - final int chunkX = chunk.getChunkX(); - final int chunkZ = chunk.getChunkZ(); - - final long index = ChunkUtils.getChunkIndex(chunkX, chunkZ); - - UnloadChunkPacket unloadChunkPacket = new UnloadChunkPacket(); - unloadChunkPacket.chunkX = chunkX; - unloadChunkPacket.chunkZ = chunkZ; - chunk.sendPacketToViewers(unloadChunkPacket); - - for (Player viewer : chunk.getViewers()) { - chunk.removeViewer(viewer); + if (ChunkUtils.isChunkUnloaded(chunk)) { + return; + } + // Schedule the chunk removal + synchronized (this.scheduledChunksToRemove) { + this.scheduledChunksToRemove.add(chunk); } - - callChunkUnloadEvent(chunkX, chunkZ); - - // Remove all entities in chunk - getChunkEntities(chunk).forEach(entity -> { - if (!(entity instanceof Player)) - entity.remove(); - }); - - // Clear cache - this.chunks.remove(index); - this.chunkEntities.remove(index); - - chunk.unload(); - - UPDATE_MANAGER.signalChunkUnload(this, chunkX, chunkZ); } @Override @@ -607,6 +586,9 @@ public class InstanceContainer extends Instance { @Override public void tick(long time) { + // Unload all waiting chunks + UNSAFE_unloadChunks(); + super.tick(time); Lock wrlock = changingBlockLock.writeLock(); wrlock.lock(); @@ -614,6 +596,48 @@ public class InstanceContainer extends Instance { wrlock.unlock(); } + /** + * Unload all waiting chunks + *

+ * Unsafe because it has to be done on the same thread as the instance/chunks tick update + */ + private void UNSAFE_unloadChunks() { + synchronized (this.scheduledChunksToRemove) { + for (Chunk chunk : scheduledChunksToRemove) { + final int chunkX = chunk.getChunkX(); + final int chunkZ = chunk.getChunkZ(); + + final long index = ChunkUtils.getChunkIndex(chunkX, chunkZ); + + UnloadChunkPacket unloadChunkPacket = new UnloadChunkPacket(); + unloadChunkPacket.chunkX = chunkX; + unloadChunkPacket.chunkZ = chunkZ; + chunk.sendPacketToViewers(unloadChunkPacket); + + for (Player viewer : chunk.getViewers()) { + chunk.removeViewer(viewer); + } + + callChunkUnloadEvent(chunkX, chunkZ); + + // Remove all entities in chunk + getChunkEntities(chunk).forEach(entity -> { + if (!(entity instanceof Player)) + entity.remove(); + }); + + // Clear cache + this.chunks.remove(index); + this.chunkEntities.remove(index); + + chunk.unload(); + + UPDATE_MANAGER.signalChunkUnload(this, chunkX, chunkZ); + } + this.scheduledChunksToRemove.clear(); + } + } + private void callChunkLoadEvent(int chunkX, int chunkZ) { InstanceChunkLoadEvent chunkLoadEvent = new InstanceChunkLoadEvent(this, chunkX, chunkZ); callEvent(InstanceChunkLoadEvent.class, chunkLoadEvent); diff --git a/src/main/java/net/minestom/server/thread/PerGroupChunkProvider.java b/src/main/java/net/minestom/server/thread/PerGroupChunkProvider.java index 5c446d503..9c5608b8a 100644 --- a/src/main/java/net/minestom/server/thread/PerGroupChunkProvider.java +++ b/src/main/java/net/minestom/server/thread/PerGroupChunkProvider.java @@ -7,6 +7,7 @@ import net.minestom.server.utils.chunk.ChunkUtils; import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.atomic.AtomicBoolean; /** * Separate chunks into group of linked chunks @@ -106,15 +107,24 @@ public class PerGroupChunkProvider extends ThreadProvider { final Instance instance = entry.getKey(); final Map, Instance> instanceMap = entry.getValue(); + // True if the instance ended its tick call + AtomicBoolean instanceUpdated = new AtomicBoolean(false); + // Update all the chunks + instances for (Map.Entry, Instance> ent : instanceMap.entrySet()) { final Set chunks = ent.getKey(); - final boolean updateInstance = updatedInstance.add(instance); + final boolean shouldUpdateInstance = updatedInstance.add(instance); pool.execute(() -> { // Used to check if the instance has already been updated this tick - if (updateInstance) { + if (shouldUpdateInstance) { updateInstance(instance, time); + instanceUpdated.set(true); + } + + // Wait for the instance to be updated + // Needed because the instance tick is used to unload waiting chunks + while (!instanceUpdated.get()) { } for (ChunkCoordinate chunkCoordinate : chunks) { diff --git a/src/main/java/net/minestom/server/utils/chunk/ChunkUtils.java b/src/main/java/net/minestom/server/utils/chunk/ChunkUtils.java index 9e4f4c02e..b0306ceab 100644 --- a/src/main/java/net/minestom/server/utils/chunk/ChunkUtils.java +++ b/src/main/java/net/minestom/server/utils/chunk/ChunkUtils.java @@ -13,6 +13,8 @@ public final class ChunkUtils { } /** + * Get if a chunk is unloaded + * * @param chunk the chunk to check * @return true if the chunk is unloaded, false otherwise */ @@ -21,6 +23,8 @@ public final class ChunkUtils { } /** + * Get if a chunk is unloaded + * * @param instance the instance to check * @param x instance X coordinate * @param z instance Z coordinate