Fix chunk unloading

[electron93]

Don't reset Chunk.unloadQueued to false

net.minecraft.world.chunk.ChunkProviderServer.getLoadedChunk(int x, int z)

```java @Nullable public Chunk getLoadedChunk(int x, int z) { long i = ChunkPos.asLong(x, z); Chunk chunk = (Chunk)this.loadedChunks.get(i); if (chunk != null) { chunk.unloadQueued = false; } return chunk; } ```

Visualization of loaded chunks with Dynmap (screenshots)

   

Without Mirage, chunks were correctly unloaded. This PR makes this possible with Mirage installed. Maybe not 100%, but clearly better than it was.

[Yeregorix]

Thanks for highlighting the issue. I will test your PR tomorrow and merge if it's ok. 🙂

[electron93]

Thanks @Yeregorix 😸 !

I wrote a tool to diagnose such things.

This is what I managed to catch from the server console (at v1.3.15)

``` ... [15:56:14] [Server thread/WARN] [tool]: chunk = [619, -645]:galactech, unloadQueued -> false, call stack: -> java.lang.Thread.getStackTrace() : 1559 -> net.minecraft.world.gen.ChunkProviderServer.handler$bbl000$impl$methodWatcher_getLoadedChunk() : 2558 -> net.minecraft.world.gen.ChunkProviderServer.getLoadedChunk() : 80 -> net.minecraft.world.World.getChunk() : 8588 -> net.smoofyuniverse.mirage.impl.network.NetworkWorld.getChunk() : 554 -> net.smoofyuniverse.mirage.impl.network.NetworkWorld.isChunkLoaded() : 549 -> net.smoofyuniverse.mirage.impl.network.NetworkChunk.areNeighborsLoaded() : 568 -> net.smoofyuniverse.mirage.modifier.BedrockModifier.isReady() : 83 -> net.smoofyuniverse.mirage.impl.network.NetworkChunk.obfuscate() : 238 -> net.smoofyuniverse.mirage.Mirage.lambda$onServerStarted$0() : 169 -> org.spongepowered.api.scheduler.Task$Builder.lambda$execute$0() : 139 -> org.spongepowered.common.scheduler.SchedulerBase.lambda$startTask$0() : 197 -> org.spongepowered.common.scheduler.SyncScheduler.executeTaskRunnable() : 74 -> org.spongepowered.common.scheduler.SchedulerBase.startTask() : 188 -> org.spongepowered.common.scheduler.SchedulerBase.processTask() : 174 -> java.util.concurrent.ConcurrentHashMap$ValuesView.forEach() : 4707 -> org.spongepowered.common.scheduler.SchedulerBase.runTick() : 112 -> org.spongepowered.common.scheduler.SyncScheduler.tick() : 47 -> org.spongepowered.common.scheduler.SpongeScheduler.tickSyncScheduler() : 189 -> org.spongepowered.mod.SpongeMod.onTick() : 441 -> net.minecraftforge.fml.common.eventhandler.ASMEventHandler_29_SpongeMod_onTick_ServerTickEvent.invoke() : -1 -> net.minecraftforge.fml.common.eventhandler.ASMEventHandler.invoke() : 90 -> net.minecraftforge.fml.common.eventhandler.EventBus.forgeBridge$post() : 1256 -> net.minecraftforge.fml.common.eventhandler.EventBus.post() : 1206 -> net.minecraftforge.fml.common.FMLCommonHandler.onPreServerTick() : 279 -> net.minecraft.server.MinecraftServer.tick() : 657 -> net.minecraft.server.MinecraftServer.run() : 526 -> java.lang.Thread.run() : 748 [15:56:14] [Server thread/WARN] [tool]: chunk = [614, -646]:galactech, unloadQueued -> false, call stack: -> java.lang.Thread.getStackTrace() : 1559 -> net.minecraft.world.gen.ChunkProviderServer.handler$bbl000$impl$methodWatcher_getLoadedChunk() : 2558 -> net.minecraft.world.gen.ChunkProviderServer.getLoadedChunk() : 80 -> net.minecraft.world.World.getChunk() : 8588 -> net.smoofyuniverse.mirage.impl.network.NetworkWorld.getChunk() : 554 -> net.smoofyuniverse.mirage.impl.network.NetworkWorld.isChunkLoaded() : 549 -> net.smoofyuniverse.mirage.impl.network.NetworkChunk.areNeighborsLoaded() : 568 -> net.smoofyuniverse.mirage.modifier.BedrockModifier.isReady() : 83 -> net.smoofyuniverse.mirage.impl.network.NetworkChunk.obfuscate() : 238 -> net.smoofyuniverse.mirage.Mirage.lambda$onServerStarted$0() : 169 -> org.spongepowered.api.scheduler.Task$Builder.lambda$execute$0() : 139 -> org.spongepowered.common.scheduler.SchedulerBase.lambda$startTask$0() : 197 -> org.spongepowered.common.scheduler.SyncScheduler.executeTaskRunnable() : 74 -> org.spongepowered.common.scheduler.SchedulerBase.startTask() : 188 -> org.spongepowered.common.scheduler.SchedulerBase.processTask() : 174 -> java.util.concurrent.ConcurrentHashMap$ValuesView.forEach() : 4707 -> org.spongepowered.common.scheduler.SchedulerBase.runTick() : 112 -> org.spongepowered.common.scheduler.SyncScheduler.tick() : 47 -> org.spongepowered.common.scheduler.SpongeScheduler.tickSyncScheduler() : 189 -> org.spongepowered.mod.SpongeMod.onTick() : 441 -> net.minecraftforge.fml.common.eventhandler.ASMEventHandler_29_SpongeMod_onTick_ServerTickEvent.invoke() : -1 -> net.minecraftforge.fml.common.eventhandler.ASMEventHandler.invoke() : 90 -> net.minecraftforge.fml.common.eventhandler.EventBus.forgeBridge$post() : 1256 -> net.minecraftforge.fml.common.eventhandler.EventBus.post() : 1206 -> net.minecraftforge.fml.common.FMLCommonHandler.onPreServerTick() : 279 -> net.minecraft.server.MinecraftServer.tick() : 657 -> net.minecraft.server.MinecraftServer.run() : 526 -> java.lang.Thread.run() : 748 ... ```

Image


---

But even after these changes, some chunks still remain loaded. Maybe these are my local problems that are not related to Mirage.

[Yeregorix]

Your PR makes NetworkChunk#areNeighborsLoaded avoid to mark chunks as active and it fixes most cases where chunks are prevented to be unloaded. However, now that I'm aware of this problem I see many methods that are also affected but not optimised yet. WorldStorage/View#isExposed, WorldView#getBlock, WorldStorage/View#isChunkLoaded, and many more. I also need to decide whether WorldStorage#getChunkStorage and WorldView#getChunkView should mark a chunk as active or not. There are also few small problems with your changes:

• NetworkWorld#isChunkLoaded no longer returns false when InternalChunk#isViewAvailable returns false.
• InternalWorld#isChunkLoaded now returns false when the chunk is going to be unloaded (unloadQueued == true). However the chunk is loaded so it should returns true.

The approach is good but I'm closing this PR because the problem is way larger than I thought and I would like to handle it myself if you don't mind.

[Yeregorix]

I finally handled things slightly differently in commit e079275. The fix now also affects the API's isChunkLoaded. I let other methods unchanged for now because I think it's good to mark chunk active even when only reading information. That's what vanilla does and that will be necessary if someday I implement async obfuscation. It would be a pain to have neighbor chunks unloading in the middle of the process.