Add ServerChunkEvents.CHUNK_LEVEL_TYPE_CHANGE

[DennisOchulor]

Add a new event that fires when a chunk's level type changes. (Relates to #4507)

Changed the name from CHUNK_STATUS_CHANGE as to not confuse it with the ChunkStatus class which refers to chunk generation stages.

The event only provides a ChunkPos and not a WorldChunk. If really needed, a potentially null WorldChunk can be obtained via ChunkHolder#getLatest() but I do not think it's necessary.

The documentation of CHUNK_UNLOAD has also been updated to clarify server chunk unload timing.

[2No2Name]

Edit: I am probably wrong about the threads that the level changes take place, since I was confusing this with chunks actually becoming available to the world, including the difference of getChunk and getChunkNow.

I think implementing this without major changes is a bad idea due to the missing explanation of the meaning of the chunk level type changes.
I believe that if understood correctly, the chunk level type is NOT what you want to track, as the status that is being tracked here only describes whether the game aims to load a chunk, but not whether the chunk is actually being loaded or available to the general game logic.

In my mod, lithium, which tracks whether a chunk is accessible in the world, I had several issues tracking this status correctly. I managed to get all events fire on the server thread, to avoid threading issues. Furthermore, Lithium (almost correctly) tracks whether the chunk is accessible in the world, instead of whether the game tries to make it accessible (status set but chunk future not done yet). I believe that if the API just tracks the level/status, many authors will incorrectly assume that this information is sufficient to know that a chunk is available in the world.

Including the other load statuses, especially regarding entity ticking, adds another layer of complexity and unclear meanings, requiring even more clear documentation.

[2No2Name]

On which thread should this be called, and should this be ensured with a similar check as in
https://github.com/CaffeineMC/lithium/blob/ebcbdb134365ab1862461548f857cdecf96e4d31/common/src/main/java/net/caffeinemc/mods/lithium/common/world/chunk/ChunkStatusTracker.java#L48

[DennisOchulor]

I've done some testing and it does seem like the event is always consistently called on the main server thread.

[2No2Name]

Which meaning does changing the chunk level type have?
There might be many misconceptions about this, leading to lots of incorrect implementations. The code comment is very minimal and doesn't indicate any potential issues.

For example, if a mod author believes the chunk status indicates one of the following, there will likely be major issues:

Whether the chunk is loaded, whether entities are ticking in that chunk, whether entities are loaded in that chunk, whether the chunk is ticking blocks in that chunk, etc.

[ishland]

This method is only called once if a chunk changes from entity ticking to inaccessible in a single tick. This will make event users to only get a INACCESSIBLE when such transition happens. The event should be called for all intermediate states to avoid confusion.

[DennisOchulor]

Does this mean there should be a check to see if the old/new chunk levels are sequential, and if not artificially invoke the event for all the intermediate level types in order?

[ishland]

Does this mean there should be a check to see if the old/new chunk levels are sequential, and if not artificially invoke the event for all the intermediate level types in order?

Yes.

[DennisOchulor]

This is a side note but increaseLevel() and decreaseLevel() are named backwards.

increaseLevel() is called when the chunk level type changes from:

• INACCESSIBLE(34+) to FULL(33)
• FULL(33) to BLOCK_TICKING(32)
• BLOCK_TICKING(32) to ENTITY_TICKING(31 and below)

These are all level decreases, and decreaseLevel() does the reverse of this (but potentially skips intermediate level types).

I don't think this changes the actual functionality of the event but just pointing it out.

[2No2Name]

Rewording my concerns:
When the chunk loading isn't done yet by the worker thread, the chunk level is already set to something accessible but the chunk actually isn't accessible. I would really like the API to not just track the chunk holder's chunk level type but the actual behavior of the chunk, but that is very complex and can include threading issues.

[DennisOchulor]

The event is now called when the chunk future has actually completed, and is called on the main thread's executor. I added some rough logging tests to verify the chunk's behaviour aligns with the level type but I'm honestly not sure how to test such a thing correctly.

This has been implemented but there is a potential issue:

This method is only called once if a chunk changes from entity ticking to inaccessible in a single tick. This will make event users to only get a INACCESSIBLE when such transition happens. The event should be called for all intermediate states to avoid confusion.

If a chunk goes straight from ENTITY_TICKING to INACCESSIBLE, and all the intermediate level types are artificially called, then the calls for ENTITY_TICKING -> BLOCK_TICKING and BLOCK_TICKING -> FULL may make modders assume the chunk is still accessible when it really isn't.