fix! Remove ability to create heterogeneous lists - #51
Draft
Fisch37 wants to merge 3 commits into
Draft
Conversation
Contributor
Author
|
The trait(s) are actually not a required part of this PR, I realised. They help make some of the code neater, but all uses of them could be replaced by match-statements over NbtList. Perhaps they would be better served as a separate PR? |
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
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 is an implementation of the fix I have outlined in #46 (see this message) to address #45 namely the fact that we currently allow heterogeneous lists to be created (and serialize them), which is not standard NBT.
I will save us the time and not repeat my arguments for this implementation as you can find them in the message linked above.
There are still some features to reimplement for the new system and open questions remain. For this reason, this request is still in draft phase.
ToDos
Open Questions
Should TryAsRef and TryAsMut be unrolled into individual methods?
This PR includes two generic traits
TryAsRefandTryAsMut, which are fallible versions ofAsRefandAsMut, and provide the extraction methods for both NbtList and dyn NbtCompatible.While they are very helpful, keeping them as part of the API requires exposing them to users, which could be considered problematic as they do not align with the rest of the project's goals.
How to handle empty lists?
Empty lists can try_as_ref and try_as_mut into any supported Vec type. If NbtList was previously holding an allocated list, this will drop that allocation, which could be considered undesirable. However it is very unusual for a mutable reference to not be unused, thus this issue is very rare and need not be addressed.