Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set StartGamePacket->blockNetworkIdsAreHashes to true #6526

Open
AkmalFairuz opened this issue Nov 20, 2024 · 5 comments
Open

Set StartGamePacket->blockNetworkIdsAreHashes to true #6526

AkmalFairuz opened this issue Nov 20, 2024 · 5 comments
Labels
Category: Protocol Related to (changes in) the Minecraft network protocol Priority: Low Type: Enhancement Contributes features or other improvements to PocketMine-MP

Comments

@AkmalFairuz
Copy link
Contributor

Problem description

Enabling blockNetworkIdsAreHashes will make it easier to maintain the multi-version plugin and we won't need canonical_block_states.nbt anymore to make protocol updates. For now, only PowerNukkitX did this.

@dktapps
Copy link
Member

dktapps commented Nov 21, 2024

We purposely chose not to do this because it reduces bandwidth efficiency while providing no meaningful benefit for multi version support on account of the frequent blockstate changes.

canonical_block_states.nbt is generated by a tool that also generates a bunch of other needed data, so it isn't a problem to update it anyway. We also need it for stuff like archival, testing, and generating blockstate upgrade schemas, so it doesn't change anything on that front anyway.

The only real benefit is to custom blocks on server networks using proxies, since adding custom blocks doesn't cause the IDs of existing blocks to change.

@dktapps dktapps added Priority: Low Category: Protocol Related to (changes in) the Minecraft network protocol Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Nov 21, 2024
@AkmalFairuz AkmalFairuz closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2024
@dktapps dktapps reopened this Nov 23, 2024
@Superomarking
Copy link

Why was this issue re-opened?

@dktapps
Copy link
Member

dktapps commented Nov 23, 2024

Because I didn't want it closed?

I never said we shouldn't do this, only explained why it hasn't been done already, because none of the advantages mentioned in the original post are valid.

However, there are clear advantages for custom blocks. It's just not clear whether these benefits are worth the bandwidth penalty - block network IDs on the wire grow from 1-2 bytes to 5 bytes each. Mathematically this looks bad, however the real world impact of this hasn't been quantified since no one has actually tried to implement it.

In any case, it's not likely to get worked on any time soon. Worth keeping this as a tracking issue in case someone on the team wants to look into it, but there's no active plans to do so.

@GameParrot
Copy link
Contributor

It's implemented in https://github.com/Swim-Services/PocketMine-MP/tree/1.21.50-pre, used for 1.21.50 support without the block states files

@dktapps
Copy link
Member

dktapps commented Nov 23, 2024

Like I said, we still need the blockstate file regardless of whether this is implemented, so the net difference is 0. PM depending on it or not affects nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Protocol Related to (changes in) the Minecraft network protocol Priority: Low Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

No branches or pull requests

4 participants