PaperMC/nms-patches/PlayerChunk.patch
Spottedleaf 7f61a2526e #640: Fix chunk load/unload callbacks for chunk load cancellations
When a chunk goes from a ticket level where it is loading a
full chunk to an inactive state (i.e ticket level 33 to
ticket level 45) the full status future will be completed
with a "Right" Either (indicating unloaded). However, this
will also schedule the unload callback immediately.

However, the callback is not immediately executed. This means
the next unload/load callback that needs to be scheduled will
fail. The fix applied is to not schedule a callback if the
chunk is not loaded - if the Either is "right."

Even better, due to how completablefuture works, exceptions
are not printed by default. So the exception thrown by the
callback executor was not printed and the failure
hidden from console. This explains why no-one has tracked this issue.
Now the exception is printed so future failures with the
callback system (if any) can be tracked easier.
2020-03-06 14:34:13 +11:00

115 lines
6.3 KiB
Diff

--- a/net/minecraft/server/PlayerChunk.java
+++ b/net/minecraft/server/PlayerChunk.java
@@ -43,7 +43,7 @@
this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE;
this.tickingFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE;
this.entityTickingFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE;
- this.chunkSave = CompletableFuture.completedFuture((Object) null);
+ this.chunkSave = CompletableFuture.completedFuture(null); // CraftBukkit - decompile error
this.dirtyBlocks = new short[64];
this.location = chunkcoordintpair;
this.lightEngine = lightengine;
@@ -55,6 +55,15 @@
this.a(i);
}
+ // CraftBukkit start
+ public Chunk getFullChunk() {
+ if (!getChunkState(this.oldTicketLevel).isAtLeast(PlayerChunk.State.BORDER)) return null; // note: using oldTicketLevel for isLoaded checks
+ CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> statusFuture = this.getStatusFutureUnchecked(ChunkStatus.FULL);
+ Either<IChunkAccess, PlayerChunk.Failure> either = (Either<IChunkAccess, PlayerChunk.Failure>) statusFuture.getNow(null);
+ return either == null ? null : (Chunk) either.left().orElse(null);
+ }
+ // CraftBukkit end
+
public CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> getStatusFutureUnchecked(ChunkStatus chunkstatus) {
CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> completablefuture = (CompletableFuture) this.statusFutures.get(chunkstatus.c());
@@ -80,9 +89,9 @@
@Nullable
public Chunk getChunk() {
CompletableFuture<Either<Chunk, PlayerChunk.Failure>> completablefuture = this.a();
- Either<Chunk, PlayerChunk.Failure> either = (Either) completablefuture.getNow((Object) null);
+ Either<Chunk, PlayerChunk.Failure> either = (Either) completablefuture.getNow(null); // CraftBukkit - decompile error
- return either == null ? null : (Chunk) either.left().orElse((Object) null);
+ return either == null ? null : (Chunk) either.left().orElse(null); // CraftBukkit - decompile error
}
@Nullable
@@ -223,7 +232,7 @@
CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> completablefuture = (CompletableFuture) this.statusFutures.get(i);
if (completablefuture != null) {
- Either<IChunkAccess, PlayerChunk.Failure> either = (Either) completablefuture.getNow((Object) null);
+ Either<IChunkAccess, PlayerChunk.Failure> either = (Either) completablefuture.getNow(null); // CraftBukkit - decompile error
if (either == null || either.left().isPresent()) {
return completablefuture;
@@ -278,6 +287,30 @@
boolean flag1 = this.ticketLevel <= PlayerChunkMap.GOLDEN_TICKET;
PlayerChunk.State playerchunk_state = getChunkState(this.oldTicketLevel);
PlayerChunk.State playerchunk_state1 = getChunkState(this.ticketLevel);
+ // CraftBukkit start
+ // ChunkUnloadEvent: Called before the chunk is unloaded: isChunkLoaded is still true and chunk can still be modified by plugins.
+ if (playerchunk_state.isAtLeast(PlayerChunk.State.BORDER) && !playerchunk_state1.isAtLeast(PlayerChunk.State.BORDER)) {
+ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAccept((either) -> {
+ Chunk chunk = (Chunk)either.left().orElse(null);
+ if (chunk != null) {
+ playerchunkmap.callbackExecutor.execute(() -> {
+ // Minecraft will apply the chunks tick lists to the world once the chunk got loaded, and then store the tick
+ // lists again inside the chunk once the chunk becomes inaccessible and set the chunk's needsSaving flag.
+ // These actions may however happen deferred, so we manually set the needsSaving flag already here.
+ chunk.setNeedsSaving(true);
+ chunk.unloadCallback();
+ });
+ }
+ }).exceptionally((throwable) -> {
+ // ensure exceptions are printed, by default this is not the case
+ MinecraftServer.LOGGER.fatal("Failed to schedule unload callback for chunk " + PlayerChunk.this.location, throwable);
+ return null;
+ });
+
+ // Run callback right away if the future was already done
+ playerchunkmap.callbackExecutor.run();
+ }
+ // CraftBukkit end
CompletableFuture completablefuture;
if (flag) {
@@ -309,7 +342,7 @@
if (flag2 && !flag3) {
completablefuture = this.fullChunkFuture;
this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE;
- this.a(completablefuture.thenApply((either1) -> {
+ this.a(((CompletableFuture<Either<Chunk, PlayerChunk.Failure>>) completablefuture).thenApply((either1) -> { // CraftBukkit - decompile error
playerchunkmap.getClass();
return either1.ifLeft(playerchunkmap::a);
}));
@@ -347,6 +380,26 @@
this.w.a(this.location, this::k, this.ticketLevel, this::d);
this.oldTicketLevel = this.ticketLevel;
+ // CraftBukkit start
+ // ChunkLoadEvent: Called after the chunk is loaded: isChunkLoaded returns true and chunk is ready to be modified by plugins.
+ if (!playerchunk_state.isAtLeast(PlayerChunk.State.BORDER) && playerchunk_state1.isAtLeast(PlayerChunk.State.BORDER)) {
+ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAccept((either) -> {
+ Chunk chunk = (Chunk)either.left().orElse(null);
+ if (chunk != null) {
+ playerchunkmap.callbackExecutor.execute(() -> {
+ chunk.loadCallback();
+ });
+ }
+ }).exceptionally((throwable) -> {
+ // ensure exceptions are printed, by default this is not the case
+ MinecraftServer.LOGGER.fatal("Failed to schedule load callback for chunk " + PlayerChunk.this.location, throwable);
+ return null;
+ });
+
+ // Run callback right away if the future was already done
+ playerchunkmap.callbackExecutor.run();
+ }
+ // CraftBukkit end
}
public static ChunkStatus getChunkStatus(int i) {