Additional chunk data with SRF 13

[davidmayr]

This PR implements SRF 13 which adds a bitmask to specify flags for additional world data. The current additional world data consists of block and fluid ticks (closes #5) and poi chunk data. The format was also designed with future proofing in mind, in case we want to add new additional world data in the future.

To implement the POI chunk data, a moonrise data loader was implemented. And to make POI loading consistent with entity loading, the entity loader has been converted to a moonrise data loader as well. This also has the advantage of less disk io (it is async, but it would have still consumed resources) and less patching required. Chunk loading and world saving is still implemented via the traditional slime world conversion and serialization APIs.

This also fixes another bug (by modifying forceNoSave in the PaperHooks) where moonrise saving was accidentally triggered sometimes and saved to the temporary directory.

TODO

• Test for performance regressions
• Testing in general
• Document new API

[Copilot]

Pull Request Overview

This PR implements SRF 13 to support additional chunk data by introducing a new bitmask for POI chunks and tick data (block and fluid ticks) while preparing the format for future extensions. Key changes include:

• Updating serialization and deserialization logic (in SlimeSerializer, SlimeChunkConverter, etc.) to write and read additional world data.
• Introducing new moonrise data loaders for POI and entity data to reduce disk IO and improve consistency.
• Upgrading API definitions and properties (including updating SLIME_VERSION and adding new SlimeProperties) to support the additional data.

Reviewed Changes

Copilot reviewed 27 out of 32 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
core/src/main/java/com/infernalsuite/asp/serialization/slime/reader/SlimeWorldReaderRegistry.java	Registers the new v13 world format for SRF 13 support.
core/src/main/java/com/infernalsuite/asp/serialization/slime/SlimeSerializer.java	Implements serialization of additional world data based on new properties.
core/src/main/java/com/infernalsuite/asp/serialization/anvil/AnvilWorldReader.java	Updates chunk reading to include new data fields (POI, block ticks, fluid ticks) with a TODO for further conversion.
core/src/main/java/com/infernalsuite/asp/SlimeLogger.java	Adds a warn method to the logger.
aspaper-server/src/main/java/com/infernalsuite/asp/level/moonrise/SlimePoiDataLoader.java	Introduces a new POI data loader for moonrise with custom save behavior.
aspaper-server/src/main/java/com/infernalsuite/asp/level/moonrise/SlimeEntityDataLoader.java	Introduces a new entity data loader for moonrise to align with the POI loader and reduce disk IO.
aspaper-server/src/main/java/com/infernalsuite/asp/level/SlimeLevelInstance.java	Updates unloading and initializes new data controllers for POI and entity data.
aspaper-server/src/main/java/com/infernalsuite/asp/level/SlimeInMemoryWorld.java	Adjusts in-memory chunk unloading and serialization to capture additional world data.
aspaper-server/src/main/java/com/infernalsuite/asp/level/SlimeChunkConverter.java	Adds conversion utilities for block and fluid ticks and methods to work with POI chunk data.
aspaper-server/src/main/java/com/infernalsuite/asp/level/SafeNmsChunkWrapper.java	Provides new accessor methods for additional data (ticks and POI sections).
aspaper-server/src/main/java/com/infernalsuite/asp/level/NMSSlimeChunk.java	Updates getters to supply additional tick and POI data.
aspaper-server/src/main/java/com/infernalsuite/asp/SimpleDataFixerConverter.java	Modifies data fixer conversion to include additional chunk data.
api/src/main/java/com/infernalsuite/asp/api/world/properties/SlimeProperties.java	Adds new properties (SAVE_POI, SAVE_BLOCK_TICKS, SAVE_FLUID_TICKS) for controlling additional data saving.
api/src/main/java/com/infernalsuite/asp/api/world/SlimeChunk.java	Extends the interface to expose new methods for tick and POI data retrieval.
api/src/main/java/com/infernalsuite/asp/api/utils/SlimeFormat.java	Upgrades the SLIME version to 13 to signal the new format changes.

Files not reviewed (5)

• SLIME_FORMAT: Language not supported
• aspaper-server/minecraft-patches/sources/ca/spottedleaf/moonrise/paper/PaperHooks.java.patch: Language not supported
• aspaper-server/minecraft-patches/sources/ca/spottedleaf/moonrise/patches/chunk_system/scheduling/NewChunkHolder.java.patch: Language not supported
• aspaper-server/minecraft-patches/sources/net/minecraft/world/level/chunk/storage/SerializableChunkData.java.patch: Language not supported
• build-data/aspaper.at: Language not supported

[Copilot]

The TODO comment indicates that the conversion logic for POI, block ticks, and fluid ticks has not been implemented yet. It is recommended to add the appropriate conversion logic and corresponding tests to ensure proper functionality.

Suggested change

	.map(x -> new SlimeChunkSkeleton(chunkX, chunkZ, sectionArray, heightMaps, tileEntities, entities, extraTag, null, null, null, null)) //TODO: Convert poi, block and fluid
	.orElse(null);
	.map(x -> {
	List<CompoundBinaryTag> poi = compound.getList("poi", BinaryTagTypes.COMPOUND).stream().map(t -> (CompoundBinaryTag) t).toList();
	List<CompoundBinaryTag> blockTicks = compound.getList("block_ticks", BinaryTagTypes.COMPOUND).stream().map(t -> (CompoundBinaryTag) t).toList();
	List<CompoundBinaryTag> fluidTicks = compound.getList("fluid_ticks", BinaryTagTypes.COMPOUND).stream().map(t -> (CompoundBinaryTag) t).toList();
	return new SlimeChunkSkeleton(chunkX, chunkZ, sectionArray, heightMaps, tileEntities, entities, extraTag, poi, blockTicks, fluidTicks, null);
	})

[kyngs]

@davidmayr what about this?

[GliczDev]

since PoiChunk, block ticks and fluid ticks are all optional compound tags, wouldn't it be better to simply store them in extra tag (like upgrade data)?

tbh SRF 13 feels like style over substance - storing optional data in extra tag (just like upgrade data) wouldn't require format change and would be much, much easier (and, I suppose, storing it in extra tag would take even less bytes?)

[davidmayr]

since PoiChunk, block ticks and fluid ticks are all optional compound tags, wouldn't it be better to simply store them in extra tag (like upgrade data)?

tbh SRF 13 feels like style over substance - storing optional data in extra tag (just like upgrade data) wouldn't require format change and would be much, much easier (and, I suppose, storing it in extra tag would take even less bytes?)

Thanks for your input on this. I was actually thinking the same things you did when I began working on this, but then decided for the current approach. Here is why:

I'm fully against storing it in extra data, strictly because of the reason that the extra data is user modifiable (Who knows what undefined behavior might happen if someone accidentally saves a tag with that name in extra data. And I guess block_ticks or POI is not that rare to save in the extra data? I named things similarly on my projects.). I'm already not a huge fan of the PDC being in the extra data, but I guess that makes sort of sense.

However, I was indeed thinking about storing the data in one large compound, and I was talking this through with @kyngs (one of the people (or the person, not sure) that designed the v12 format) and we both decided for storing this data on a separate tag for each property and make it disableable with a flag(like it is now). This is just how the format and API has always been designed. Block Data, Biome Data, Heightmaps, Entities, TileEntities. They each have their own serialized part in the format. So doing this differently for additional data just feels hacked and patched together.

Theoretically, we could save the whole chunk data in one NBT tag. Minecraft does this too, it's no problem. But this is just not how slime was designed by the people that primarily maintained this before I did. I didn't want to make too many changes to the preexisting format, and saving everything inside of one large nbt tag is sometimes actually slightly larger than saving it in separate tags.

I wanted to make sure this feels like a properly integrated feature and not like a patched together mess to quickly get block ticks running.

However, what I am definitely open about is merging block and fluid ticks into a single compound tag. Simply because they are lists and therefor need to be wrapped inside a compound tag anyway. Also, Minecraft serializes them as one thing anyway and I just split it out again

Since this is still a draft PR, I'm open to hear your voice on this and make major changes if necessary @GliczDev. However, I'm pretty happy with the way this turned out right now. I'll still need to do javadoc and importing of POI & ticks on anvil world import.

[davidmayr]

Tests concluded:

• No noticeable regression in world size with default configs
• No noticeable regression in server memory usage
• No change in serialization/deserialization/load speed

[kyngs]

Overall, this looks solid—nice work!
Please take a look at the comment from copilot; I think it could still be relevant.

One potential concern here is around backward compatibility when using SlimeProperties to specify which additional data should be saved. If new types of world data are added, a plugin might not be aware of what additional data the server actually supports. If it tries to save a world using a SlimeProperty that exists in a newer version of the API but not on the server itself, this could result in an error due to the missing field.

[kyngs]

@davidmayr what about this?

[davidmayr]

Overall, this looks solid—nice work! Please take a look at the comment from copilot; I think it could still be relevant.

That's just for importing vanilla worlds. Would be a nice to have (that's why there is the TODO), but I'm not sure if I want to invest time in that right now (Although it shouldn't be too hard). I just don't want to do the testing, but I can implement it if you want

One potential concern here is around backward compatibility when using SlimeProperties to specify which additional data should be saved. If new types of world data are added, a plugin might not be aware of what additional data the server actually supports. If it tries to save a world using a SlimeProperty that exists in a newer version of the API but not on the server itself, this could result in an error due to the missing field.

But how should we solve that? Every type of constant or enum would have the same effect. Only maybe a string constant could fix that (since the compiler writes string constants directly) but I feel like that's not a nice solution either.