First look at strings for block type IDs#6786
Draft
dktapps wants to merge 8 commits intomajor-nextfrom
Draft
Conversation
This solves the following issues: - Conflicts on BlockTypeIds every time someone makes a PR to add a new block - Type IDs not consistent on different threads We still have an auto assigned type number internally, which is used to compute state IDs for runtime chunk storage. However, this is not directly exposed to plugins, and is fully dynamic. FastChunkSerializer, instead of directly passing state IDs to the next thread, now deconstructs state IDs into their type number and state data componets, and writes a string type name + state data separately. This allows the blocks to be consistently decoded on the destination thread, particularly when custom blocks are involved, as custom blocks are expected to claim unique string type IDs Plugins implementing custom blocks **may** use their Minecraft save ID as their type ID, however this is not required. A type ID must, however, be globally unique and must not conflict with any other blocks. I haven't yet measured the performance impact to FastChunkSerializer from this change, however, I do expect some minor performance degradation, since individual calls on BinaryStream are slower than a bulk pack() / unpack() call.
dktapps
added
Category: API
![pmmp-admin-bot[bot]](https://avatars.githubusercontent.com/in/1059952?s=60&v=4){kind=small}
we can do this now that the IDs are deterministic without unstable diffs.
dktapps
added a commit
to pmmp/ext-chunkutils2
that referenced
this pull request
Oct 31, 2025
…w bytes, instead of an array" and related commits It turns out this is only useful if we want to preserve the bytes for direct reuse later. We don't really want to do this as discussed in pmmp/PocketMine-MP#6786. When the data must be decomposed into integers, transformed and repacked to binary, getting the data as binary first is actually worse for performance (assuming that ByteBufferReader is used). So it looks like we're stuck with arrays, for better or worse :( This reverts commit 8e83c8d. This reverts commit 5614724. This reverts commit a26bed6.
Member
Author
|
This is stalled pending improvements to FastChunkSerializer and/or the structure of blockstate IDs stored in subchunks. The process of converting a state ID into a string type ID + unmasked state data feels like a hack, and will almost definitely be slow. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This solves the following issues:
We still have an auto assigned type number internally, which is used to compute state IDs for runtime chunk storage. However, this is not directly exposed to plugins, and is fully dynamic.
FastChunkSerializer, instead of directly passing state IDs to the next thread, now deconstructs state IDs into their type number and state data componets, and writes a string type name + state data separately. This allows the blocks to be consistently decoded on the destination thread, particularly when custom blocks are involved, as custom blocks are expected to claim unique string type IDs
Plugins implementing custom blocks may use their Minecraft save ID as their type ID, however this is not required. A type ID must, however, be globally unique and must not conflict with any other blocks.
I haven't yet measured the performance impact to FastChunkSerializer from this change, however, I do expect some minor performance degradation, since individual calls on BinaryStream are slower than a bulk pack() / unpack() call.
Related issues & PRs
Changes
API changes
Block->getTypeId()now returns astringBlockTypeIdsconstants are now converted tostringBlockIdentifiernow acceptsstringinstead ofintBlockTypeIdsto facilitate internal functionalityBehavioural changes
Functionally everything should be basically the same.
Backwards compatibility
BC breaks to APIs
Block->getStateId()may now change without warning between versions. This was true previously too (since the definition of the state data could change), but the type numbers may also move around without warning now, which will affect the resulting state ID. The function is marked as@internal, but that never seems to stop anyone...Follow-up
Consider using incrementing IDs in chunk runtime storage, and using type ID + state data as a hash key to look up state ID, rather than using type number + state data as state ID as we do currently. This would allow getting rid of type numbers completely, make chunk block ID validation easier, and remove current restrictions from state data bits (although expanding state data bits would probably require rethinking
RuntimeBlockStateRegistry).Tests
Playtested locally and PHPUnit tested.