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

Better handling of duplicate CompoundTag keys #101

Open
dktapps opened this issue Nov 27, 2021 · 1 comment
Open

Better handling of duplicate CompoundTag keys #101

dktapps opened this issue Nov 27, 2021 · 1 comment

Comments

@dktapps
Copy link
Member

dktapps commented Nov 27, 2021

Currently, a duplicate key appearing anywhere in some NBT data tree will cause the entire tree to be undecodable using this library. This is severely problematic for legacy PM worlds, because the entire chunk is stored as one big NBT blob in MCRegion, Anvil and PMAnvil worlds. This means that any appearance of a duplicated key will cause the entire chunk to be discarded as corrupted.

This issue has caused a large number of worlds created with PM older than ~2017 to lose any chunks containing furnace due to a duplicate BurnTime key (this was originally fixed by pmmp/PocketMine-MP@20b86bd). However, any worlds created before this commit which were not loaded prior to the fix of #54 will now experience full data loss if this happened in any of their chunks, which is not cool.

Possible solutions

  • Go back to the old way, and don't throw errors on duplicate tags - simply allow the last tag with the same name to overwrite the earlier ones - not perfect; someone else like me may consider this a bug in the future and then add a new check and cause the same problem.
  • Keep a list of tags under each name in a compound. This would avoid any loss of data, but would increase implementation complexity for minimal benefit.
  • Accept a user-provided callback to decide what to do.
  • Accept flags to decide what should be done (e.g. NbtSerializer::IGNORE_DUPLICATE_COMPOUND_KEYS).
@dktapps
Copy link
Member Author

dktapps commented Dec 16, 2021

For now I've resorted to just ignoring the duplicates, since this is a major issue that is actively causing data loss for older worlds. 3e0d9ef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant