[SPIGOT-4862] loadChunk(x, z, false) on 1.14 is triggering chunk generation Created: 04/May/19  Updated: 06/Oct/19

Status: Open
Project: Spigot
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Minor
Reporter: Mike Primm Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: 1.14
Environment:

Seen on Linux platform (also on OSX)

 

 


Version: This server is running CraftBukkit version git-Spigot-1eece4f-09a453a (MC: 1.14) (Implementing API version 1.14-R0.1-SNAPSHOT)
Plugin: Dynmap
Guidelines Read: Yes

 Description   

The loadChunk(x, z, false) API, which is intended to load chunks successfully only if they are fully generated, appears to be successfully loading partially generated chunks (making them fully generated, and causing adjacent chunks to become 'more generated' - that is, following the progression of ChunkStatus from EMPTY towards FULL).  Unfortunately, this kind of defeats the point of 'generate=false', particularly for plugins like dynmap (and probably WorldBorder) that use generate=false to find defined/generated chunks without causing unbounded world growth.  I've given the code a look, but haven't been able to run down the root cause yet - I believe the fact that the 'ProtoChunkExtension' check drives a FULL load (same as generated=true) is a likely factor, but I haven't been able to find a logic change to avoid the behavior (e.g. only doing the load FULL is the ChunkStatus of the proto chunk is FULL).  Will keep looking on my end, as time permits.

I've got a temporary workaround for Dynmap - not a good one, but it sidesteps at least the worst of the issue, by checking IsChunkGenerated() in a 4 chunk radius around a chunk and only doing the LoadChunk if that is true for all chunks around the chunk.

 



 Comments   
Comment by md_5 [ 06/Oct/19 ]

>For my purposes (or Dynmap), all I really need is for loadChunk(x,z,false) to work like spectator mode (that is, no chunk generation triggering - just load the chunk if it is 'ready to use' or else don't) - given that spectator mode actually works right (with the spectatorGenerateChunks=false gamerule).

Apparently it doesn't in 1.14: https://bugs.mojang.com/browse/MC-157812

Comment by Spottedleaf [ 14/Aug/19 ]

I've opened a PR https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/pull-requests/589/overview to resolve this issue, and it has been tested to work on my end

Comment by i509vcb [ 13/Jun/19 ]

I would probably look into that code in PlayerChunkMap as that may be the key to reimplementing regenerateChunks

Comment by blablubbabc [ 09/May/19 ]

> I am interested to hunt down the root code driving the selection of which chunks get 'pulled' up the generation landscape in response to a specific chunk being requested by a player's activity area

I don't have a full glimpse on all of this yet, but a starting point might be:

  • PlayerChunkMap#movePlayer is called each tick and checks if the player has changed his chunk.
  • ChunkMapDistance#a(SectionPosition, EntityPlayer) is called when the player has changed his chunk (section).
  • ChunkMapDistance.a(PlayerChunkMap) seems to do the updating of PlayerChunks for any chunk state changes that happened (calling PlayerChunk#a(PlayerChunkMap)), possibly also triggering the actual chunk state changes.
  • The actual code determining the new chunk states is buried deep inside the various ChunkMapDistance subclasses and LightEngineGraph..
  • PlayerChunk#a(PlayerChunkMap) is triggering chunk loading depending on the new chunk state.
  • PlayerChunkMap performs the chunk loading / generation. PlayerChunkMap#a(ChunkCoordIntPair, int, IntFunction) will trigger chunk loading in a certain radius around a requested chunk, depending on the requested chunk status (cascading).
Comment by Mike Primm [ 09/May/19 ]

For my purposes (or Dynmap), all I really need is for loadChunk(x,z,false) to work like spectator mode (that is, no chunk generation triggering - just load the chunk if it is 'ready to use' or else don't) - given that spectator mode actually works right (with the spectatorGenerateChunks=false gamerule).

The other behavior differences are more likely to be of issue to folks like WorldBorder, where doing things like 'filling' and 'trimming' of chunks to shape world generation are a lot more relevant.  I am interested to hunt down the root code driving the selection of which chunks get 'pulled' up the generation landscape in response to a specific chunk being requested by a player's activity area - have any of you guys had any luck localizing where in the Vanilla code this is being driven?

Comment by blablubbabc [ 05/May/19 ]

"For 'load without generate', there should never be a need to affect other chunks : if the chunk is fully generated/populated, the state of its neighborhood should be consistent with that and no further generation should be required to load it (given that it is already in final form - that is, ChunkStatus.FULL)."

Well, I have tested loading chunks and then outputting the status of neighboring chunks, and from my observations (https://prnt.sc/nko1rk) it looks like the cascading of the chunk generation states (N-1, N-2, ..) might not start at the center chunk one is loading via loadChunk, but at a distance of 3 chunks (all chunks withing a 2 chunk radius around the specified chunk get loaded+fully generated as well (end up in status FULL). So if a player is generating chunks around him, at some point the cascading starts (FULL, FULL-1, FULL-2, ..), but if you load that edge chunk which is in FULL state, it will promote the 2-chunk radius neighbors to FULL state as well (just like when using minecraft's forceLoad command).

I haven't been able to test the spectator chunk loading behavior yet due to some minecraft bug I run into when trying this (https://bugs.mojang.com/browse/MC-138053).

It's probably not that easy to change the behavior of load chunk to not load the neighbor chunks as well (this seems to be minecraft behavior, craftbukkit is only requesting the chunk to get and stay loaded). So the other option for retaining the old behavior would then be for isChunkGenerated and loadChunk(generated=false) to check the neighboring chunks, just like in your workaround solution, and only consider chunks as 'is generated' if all chunks in that area are generated.

But then there would be no way for plugins to determine whether a chunk is actually generated (bukkit's isChunkGenerated would no longer be consistent with minecraft's isChunkGenerated (is chunk in status FULL); not sure how much of an issue that would be though).

The other option/compromise would be for isChunkGenerated to return the actual generation status of the chunk, and loadChunk(generate=false) to check the surrounding area before loading the chunk (to ensure no generation of neighboring chunks). It would need to be clarified in the javadocs that loadChunk's is-chunk-generated check and isChunkGenerated are no longer the same / consistent.

Comment by Mike Primm [ 05/May/19 ]

Current (and previous) behavior of 'natural' chunk generation results in a radius affect: there are N stages of chunk generation/population before a chunk goes from non-existence to being fully loadable - this was only 2 for a long time (up until 1.13) - basically 'generated' (landscape blocks produced) and 'populated' (trees, structures, plants, etc added - things that often could extend over adjacent chunks.  1.13 made this more like 4 steps.  In any case, in order to get to step N (completely generated) all the adjacent chunks needed to be up to N-1 (or higher), and in order to get to N-1, all those neighbors need to be up to N-2 (or higher), etc.   For pre-1.13, things were simple since N=2 - a chunk could be fully generated+populated by its neighbors being generated (or generated+populated).  For 1.13, there were 3 intermediate steps, so essentially everyone within 3 needed to be at least the first step of generation, within two be second and within 1 be third before a given chunk could be 'completed'.  For 'load without generate', there should never be a need to affect other chunks : if the chunk is fully generated/populated, the state of its neighborhood should be consistent with that and no further generation should be required to load it (given that it is already in final form - that is, ChunkStatus.FULL).  Normal user activity drives load-with-generate to pull needed chunks up to FULL (cascading the need for the prerequisite states to be checked and driven on the chunks around the chunk to be loaded) - for the load-without-generate, the behavior should be 'load if in FULL state already, with no triggered dependency update' (which, logically, should be the behavior the vanilla server should be doing for users in 'spectator' mode when the gamerule spectatorsGenerateChunks is set to false).  1.13 had a problem with this - not that it didn't work, but that pre-1.13 chunks (fully generated ones) were not able to be spectator-loaded, as Mojang had combined the process of migrating existing chunks from pre-1.13 to 1.13 into the world-generation handling and not accounted for the impact on spectator mode: not sure if that was fixed, but that isn't the specific issue here.

 

Comment by blablubbabc [ 05/May/19 ]

Does this produce the expected behavior? https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/pull-requests/551/

One thing to note though: Even if a chunk has been fully generated, loading that chunk can still lead to this cascading of neighboring chunks getting generated as well (due to loadChunk loading+generating the surrounding chunks (2 chunk radius I think) as well).

Comment by md_5 [ 04/May/19 ]

>Because the chunk itself shows as generated, even though it is not in the FULL state,

 

Isn't that a bug also then?

Comment by Mike Primm [ 04/May/19 ]

Because the chunk itself shows as generated, even though it is not in the FULL state, which results in the loading of the chunk causing the chunk to be generated up to the FULL state, which cascades into pulling the chunks around it into the corresponding minimum states above EMPTY to allow it to be generated to the FULL state, which leads to those chunks being 'generated'.  The problem is that isGenerated is not the same as ChunkStatus = FULL, or logical equivalent - it looks to be closer to ChunkStatus > EMPTY.
The base issue is the generate=false isn't a passive action (from a world-gen point of view), as it was in the past - it's really supposed to be consistent with the chunk load behavior of spectators when their activity is set to not cause further world generation.

Comment by md_5 [ 04/May/19 ]

> by checking IsChunkGenerated() in a 4 chunk radius around a chunk and only doing the LoadChunk if that is true for all chunks around the chunk.

 

Why can't you just call isChunkGenerated on the chunk itself?

 

     * @param generate Whether or not to generate a chunk if it doesn't
     *     already exist 
Generated at Sat Dec 13 15:25:51 UTC 2025 using Jira 10.3.13#10030013-sha1:56dd970ae30ebfeda3a697d25be1f6388b68a422.