Fresh 1.14 Spigot server with newly generated world.

No plugins except the testing plugin mentioned below.

Possible implementation (needs review): https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/pull-requests/549/

I believe theres three types of chunks for exactly this reason - theres the regular chunks, then there's the non ticking chunks, and then further out still there's the border chunks.

The API should only expose the first two.

Events will only be initiated by the first one, potentially implicating entities the second.

I don't think the true border chunks even have their entities added to the server.

Whatever the case may be I'd rather reduce the incidence of such calls to begin with even if they're not completely eliminated in obscure cases like you mention

I wonder if there are cases in which minecraft will access those border chunk entities, (eg. AI (targeting, etc.) of an entity in a ticking chunk searching for nearby entities further than 1 chunk away) and how craftbukkit would deal with this then regarding event calls.

I wouldn’t mind completely squashing all traces of border chunks from the API - I can’t think of a reason they need to be exposed, they’re really a technical detail. No events involving them should be being called, so this is basically option #1.

only difference re your last para is we should not mess with handling vanilla entities, simply just filter them in the Bukkit call.

After looking into this a bit more I agree that accessing these border chunks and treating them as regular loaded chunks (like before) is probably not safe for plugins.

2 possible solutions could be:

• Change the meaning of isChunkLoaded to 'chunk is loaded and active' (basically like it is currently), and throw ChunkLoad/UnloadEvents whenever a chunk becomes active / inactive (changes between ticking/border state). Chunks that are unloaded while in ticking state will also need to throw the unload event (if that is even possible). Actual chunk loading/unloading/caching would be hidden from plugins. Advantage: Plugins could keep working as is. Disadvantage: Plugins that use the chunk load/unload events for server analysis will no longer get reliable values for the actual loaded chunks (API could provide getCachedChunksCount or similar), and maybe there are other usecases for these events?, and entities would need to get manually registered/unregistered from the world by craftbukkit.
• Expose a new is-chunk-active state, together with ChunkActivate/DeactivateEvents. This could look something like this (WIP): https://hub.spigotmc.org/stash/users/blablubbabc/repos/bukkit/compare/diff?targetBranch=refs%2Fheads%2Fmaster&sourceBranch=refs%2Fheads%2Fchunk-is-loaded&targetRepoId=11#src/main/java/org/bukkit/World.java  and  https://hub.spigotmc.org/stash/users/blablubbabc/repos/craftbukkit/compare/diff?targetBranch=refs%2Fheads%2Fmaster&sourceBranch=refs%2Fheads%2Fis-chunk-loaded&targetRepoId=12#src/main/java/org/bukkit/craftbukkit/CraftWorld.java . Disadvantage: Plugins will have to update to use this instead and behave buggy otherwise; world#getLoadedChunks would get removed; double the number of event calls.

Personally I would probably prefer the first solution currently. That would mean calling the chunk load/unload events in place of the chunk activate/deactivate events. Left to do would then be un/registering of entities for deactivated chunks, and checking how the ChunkUnloadedEvent's is/setSaveChunk can be fitted for this (my current assumption is that minecraft will save chunks only if they have been activated before, is this correct? In that case the ChunkUnloadEvent could set the is-goin-to-get-saved-flag for the chunk).

>What exactly are the limitations of these border chunks that plugins might want to be aware of?

Accessing a border chunk will promote it to a real chunk and then load more border chunks.

This can cause lots of lag, eg https://www.spigotmc.org/threads/is-this-an-intensive-method.370339/

I really think we need to tighten the API so this can't happen.

Here is one first attempt: https://hub.spigotmc.org/stash/users/blablubbabc/repos/craftbukkit/commits/d9cefcf517db4c01c611f6199b6011aaff0860ec

This seems to restore the behavior of isChunkLoaded quite well to how it has previously been (returns true until the chunk has actually been unloaded).

However, This is probably not safe yet:

• Its not considering chunk unloads taking place during a chunk unload. I am not sure yet if this is possible though.
• I haven't fully checked yet all the code inside craftbukkit that references isChunkLoaded and might require the chunk to not only be loaded but also active. Only getHighestBlockYAt has been updated so far, but I haven't tested this yet and there are 6 other methods referencing isChunkLoaded that need to be checked.
• I have yet to validate whether plugins are still able to modify the chunk during the chunk unload event before it gets saved (as it has been before 1.14).
• World#getLoadedChunks wasn't updated yet.

As you can see on line 325 of CraftWorld, isChunkLoaded is checking simply if getChunk is not null without loading it.

You’re probably gonna be digging deep into NMS to change that

I moved the entity valid flag issue into a separate ticket: https://hub.spigotmc.org/jira/browse/SPIGOT-4774

Regarding World#isChunkLoaded: I tested removing the chunk.loaded check in CraftWorld, which has no effect. This flags seems to only get set to false once the chunk is actually getting unloaded. So I assume the chunk returned by the ChunkProviderServer is null. I have yet to check in which state the chunk is in this situation (not unloaded but not loaded according to World#isChunkLoaded).

Edit: I just checked and the chunk is indeed in the 'border' state.

Would it be possible to revert the behavior of World#isChunkLoaded to be consistent again with the ChunkUn/LoadEvents?

Previously it would report that the chunk is loaded until after the chunk got actually unloaded. Now it reports false if the chunk is in this border state, and also if the chunk is currently getting unloaded (PlayerChunkMap#g), which can get aborted though.

Previously one could check the isChunkLoaded to avoid unnecesarry chunk loads, but still be sure to get a ChunkLoadEvent once the chunk gets eventually loaded and then perform any plugin setup necessary for the chunk (ie. loading, placing and activating objects in this chunk). I assume we don't want to add ChunkDe/ActivateEvents. Since a plugin can get loaded after the chunk has been loaded (but deactivated), a plugin would currently need to periodically poll to check whether a chunk has become 'active.

What exactly are the limitations of these border chunks that plugins might want to be aware of? Currently I can see that they might skip ticking, but (entity) ticking has always been dynamic with Spigot's activation range anyways.

If there is a benefit in this, it might make sense to expose a 'isChunkActive' to plugins (for example to avoid unnecessary chunk activations by API calls that require an active chunk (I don't have a concrete example for this currently), or shutdown plugin tasks for objects in these inactive tasks (instead of shutting these down on chunk unload)).

Offtopic: While looking though the code I noticed that the the mapping for nms.World#isInsideWorld(BlockPosition) should probably be 'isOutsideWorld' instead.

sorry.

ChunkUnloadEvent calling has been fixed in https://hub.spigotmc.org/jira/browse/SPIGOT-4768

One report per issue please

I ran into a similar issue, afaik ChunkUnloadEvents are never created, only ChunkLoadEvents are. Here is a test jar.

Remove the checks to chunk.loaded in CraftWorld.

Does that solve your issues?

The problem with exposing edge chunks is that they do not behave the same as normal chunks when manipulated.

I tried different coordinates and now I get a chunk unload event. My current guess would be that I might have been to close to the spawn area that the chunk in question was kept loaded. It seems that depending on the distance to the spawn, some chunks near the spawn are kept loaded and active, and some are kept loaded but in the inactive state (no chunk unloads take place but World#isChunkLoaded reports 'false').

The only issues remaining would then be:

• The entities still report to be alive and valid, even after the chunk unload.
• World#isChunkLoaded reports false for the inactive but loaded chunks (ie. near the spawn area).

Yes, I tested the above scenario again and tried moving further away, teleporting to far places and leaving the server. So far I did not find the chunk-unloaded-notice in the server log for the chunk in question.

I am currently looking at the code for PlayerChunkMap and trying to figure out:

• If there is some kind of large upper limit for this inactive-chunks cache.
• Whether chunks can get saved without a corresponding ChunkUnloadEvent notifying plugins.
• Whether ChunkUnloadEvent are actually always called, even if world saving is disabled (there seems to be a branching condition for this before the chunk unload event gets called).
• Whether the ChunkUnloadEvent gets called before or after the saving: From a quick look at looks to me like it might be called after the saving, breaking any plugins that might want to do cleanup before the chunk gets saved.

Did you actually verify my theory?

Okay. Is there a way to query whether a chunk is actually unloaded? The behavior of world#isChunkLoaded seemed to have changed, since it now indicates whether the chunk is 'active', not whether it has been unloaded.

Edit: Also, I assume that chunks still only get saved as part of the unload. Is that still correct? If not, an alternative to the ChunkUnloadEvent for indicating the chunk save might be useful. Or treating this no-ticking zone like chunk unloading to have similar behavior as before for plugins.

Edit: Okay I just tested this again, and it seems like the persistency flag does indeed work, because the non-persistent mobs are gone after restarting the server. I was however not yet able to observe the chunk unload for this particular chunk.

Edit: It might make sense to either treat this chunk-inactivity as unloading/loading for plugins, or add a new set of API/events for this chunk inactivity. Because plugins that previously used the chunk loading/unloading for performing cleanup (such as stopping running tasks for things located in/near that chunk) can then make use of this and perform cleanup whenever a chunk becomes 'inactive'.

Probably the chunks are not unloading, they are just being moved to the non ticking zone and therefore not exposed to bukkit.

If you move even farther away you will probably see them unload.

• Missing chunk load and unload events.
• Entities not getting marked as 'invalid' when they get removed due to the chunk unloading.
• Entities still being there after an unload-load cycle, regardless of the persist flag.

Can you try and explain what exactly the issue is more concisely