Uploaded image for project: 'Minecraft: Java Edition'
  1. Minecraft: Java Edition
  2. MC-197524

Border chunks do not enforce neighbors to be loaded, causing light updates to get stuck

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Resolution: Fixed
    • Affects Version/s: 1.16.1, 1.16.2 Release Candidate 1, 1.16.2 Release Candidate 2, 1.16.2
    • Fix Version/s: 20w45a
    • Labels:
      None
    • Confirmation Status:
      Community Consensus
    • Category:
      Lighting
    • Mojang Priority:
      Important

      Description

      Upon chunk loading, promotion of a chunk to the light stage does require neighbors to be loaded in the features stage (which is the stage at which the lighting engine is able to interact with a chunk), so that border chunks, i.e., the last accessible chunks, directly after they are loaded do have their neighbors loaded in the correct state for the lighting engine to interact with them. However, if those neighbors get unloaded later on, the chunk is not demoted from the light stage again, so you can have chunks in light (and full) stage without all their neighbors being loaded. In particular, this can happen for border chunks which can then lead to light updates getting stuck at the chunk boundary to the unloaded neighbor.
      This can be visualized using the following steps:

      • Create a redstone-ready world
      • Set the render distance to 2
      • /setworldspawn 1000 56 1000
      • Fly to chunk (-14 0) (This will unload chunk (1 0))
      • Fly to chunk (-3 0)
      • /setblock 14 56 8 minecraft:sea_lantern
      • Fly back to chunk (0 0) and observe that the light got stuck at the boundary to (1 0)

       

      Currently, promotion of a chunk to the BORDER state only requires the chunk itself to be loaded as full chunk. I propose to add here the requirement that also the neighbors should be loaded in the features stage. In contrast to the worldgen stages, e.g., light or full, the accessibility status, i.e., INACCESSIBLE, BORDER, TICKING and ENTITY_TICKING does get demoted if the chunk ticket level drops too low and hence the condition is reevaluated each time the chunk gets promoted again. Hence with this change promotion to the BORDER status would re-enforce neighbors to be loaded in the features stage.
      This simply requires to change

      net.minecraft.server.world.ThreadedAnvilChunkStorage.java
      public CompletableFuture<Either<WorldChunk, ChunkHolder.Unloaded>> makeChunkAccessible(ChunkHolder holder) {
        return holder.getChunkAt(ChunkStatus.FULL, this).thenApplyAsync((either) -> {
        ...
      }
      

      to

      public CompletableFuture<Either<WorldChunk, ChunkHolder.Unloaded>> makeChunkAccessible(ChunkHolder holder) {
        return this.getRegion(holder.getPos(), 1, ChunkStatus::byDistanceFromFull).thenApply(either -> either.mapLeft(list -> list.get(list.size() / 2))).thenApplyAsync((either) -> {
        ...
      }
      

       

      Also note that the light stage is currently special cased to enforce the neighbor-loading even if the chunk was already generated into the light stage before.

      net.minecraft.server.world.ThreadedAnvilChunkStorage.java
      public CompletableFuture<Either<Chunk, ChunkHolder.Unloaded>> getChunk(ChunkHolder holder, ChunkStatus requiredStatus) {
      ...
        if (chunk.getStatus().isAtLeast(requiredStatus)) {
          CompletableFuture completableFuture2;
          if (requiredStatus == ChunkStatus.LIGHT) {
             completableFuture2 = this.upgradeChunk(holder, requiredStatus);
          } else {
             completableFuture2 = requiredStatus.runLoadTask(this.world, this.structureManager, this.serverLightingProvider, (chunkx) -> {
                return this.convertToFullChunk(holder);
             }, chunk);
          }
      
          this.worldGenerationProgressListener.setChunkStatus(chunkPos, requiredStatus);
          return completableFuture2;
        } else {
          return this.upgradeChunk(holder, requiredStatus);
        }
      ...
      }
      

       

      With my proposed change this special handling is no longer needed. The light stage will still require neighbors in the features stage for the initial lighting, but once the chunk is generated into the light stage it should no longer enforce neighbors to be loaded and leave that to the BORDER status, which is exactly how all other worldgen stages work.

       

      On a further note, there seems to be a clash between the two concepts of worldgen stages (which are in the code represented as the ChunkStatus) and between the accessibility status. The World exposes a method Chunk getChunk(int chunkX, int chunkZ, ChunkStatus leastStatus, boolean create) to load a chunk into the specified worldgen stage, and in fact WorldChunk getChunk(int i, int j) just calls this with ChunkStatus.FULL. However, as this problem here shows, the ChunkStatus only contains the persistent information about the worldgen stage the chunk has passed and does not correspond to the ticket level that keeps the chunk loaded. In most cases this is not the desired information and it would be more appropriate to provide the required accessibility status or something similar instead.
      On the other hand, digging into the code shows that all instances where this actually matters call the method with the create flag set to true which then issues a chunk ticket with the correct level for a border chunk. So in all these cases the chunk is indeed implicitly loaded in the BORDER state as usually required. Nevertheless, the Future returned by the method only depends on the full stage, so the promotion to the BORDER status usually happens slightly after the future is resolved, which can again cause problems.

      In conclusion I would suggest to think about a more appropriate public interface for getChunk(...) that conveys the correct concept, which in my opinion the ChunkStatus does not.

       

      Best,
      PhiPro

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              mgatland [Mojang] Matthew Gatland
              Reporter:
              PhiPro Philipp Provenzano
              Votes:
              8 Vote for this issue
              Watchers:
              9 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                CHK: