feat!: proper heterogeneous lists

[Vonr]

this change is API breaking.
fixes #45

[Vonr]

serialization is broken right now

[Fisch37]

To be honest, I'm not happy with how this solution unnecessarily packages the list content.

1. Since NbtTag is just a wrapper for different tag types and an actual nbt list must never hold tags of different types this forces us to build a complete wrapper around Vec (any mutable reference would allow users to heterogenise lists and thus break the invariants this new NbtList tries to enforce).
2. This version of NbtList feels like a departure from a design that was previously very close to how actual NBT works. Serialising now potentially requires a rehomogenisation step which abstracts away nbt and means that we lose reproducibility across the save-load boundary (a saved and loaded NbtList may load differently from the way it was saved). This is very unintuitive, I feel, and will cause pitfalls for users.

What would be nicer, if we can figure out a good way to type it, would be a simple Vec and some way so that, at the same time, NbtTag::NbtList wouldn't require a generic type parameter.

[Vonr]

i'm not too sure what you mean by requiring re-homonegisation, considering there is no public API to remove or get a mutable reference to the inner elements.
the current way to do something like that would be to use into_inner, perform your magic, then use either the From<Vec> or FromIterator implementation.
i don't think there is a problem with reproducibility, but itd be great if you could give an example.

there is also the option of only wrapping things during ser/de.

[Fisch37]

That is more or less my point. With this implementation we can never get a mutable reference out of an NbtList (without unpacking the internal vec and enforcing the invariant yourself [that invariant being every element having the same type]).

I've come up with an alternative design that structurally enforces homogeneity. I'll post it in #45

[Vonr]

removed the type, now it just happens at ser/de time

[SzczurekYT]

I have to give this a more thorough look (when I will have a bit of time) before giving the approve stamp, but generally I agree with Fisch37 and I think I like this much more then the previous version.

[Fisch37]

As I've been asked to give an example that explains what I mean by reproducibility across save-load boundaries, I'll do that with the new behaviour and, after that, provide an outline for what I think would be the best fix for heterogeneous lists.

Suppose a program like this:

let mut known_players: Vec<NbtTag> = Vec::new();
// Players with a known name are stored with said name
known_players.push(NbtTag::String("Steve".to_string()));
known_players.push(NbtTag::String("Alex".to_string()));
// Players without a known name are stored as their UUID
known_players.push(NbtTag::IntArray(vec![1234,5678, 910, 1112])

// Everything is fine so far
println!("{}", known_players[0]); // "Steve"

// Serializing the data into NBT.
let raw_nbt = NbtTag::List(known_players).serialize();
// ...
// some code around here, probably saves the NBT to the disc or something.

// ...
// some time later, probably having loaded the NBT from the disc. Perhaps this occurs on program start?
let mut known_players = NbtTag::deserialize(raw_nbt).unwrap()
    .get_list().unwrap();
// Now the data we've saved comes out differently than it came in.
println!("{}", known_players[0]); // {"":"Steve"}

// If the programmer has not considered this, they will do the same thing they did before
known_players.push(NbtTag::String("RandomUsername"));

// Which means that now known_players is heterogenous again.
println!("{known_players}"); // [{"": "Steve"}, {"": "Alex"}, {"": [I;1234, 5678, 910, 1112]}, "RandomUsername"]

// it gets even worse (though this is technically fixable)
let raw_nbt = NbtTag::List(known_players).serialize();
let known_players = NbtTag::deserialize(raw_nbt).unwrap()
    .get_list().unwrap();

// And now everything is wrapped twice!
println!("{known_players}") // [{"": {"": "Steve"}, {"": {"": "Alex"}}, {"": {"": [I;1234,5678,910,1112]}}, {"": "RandomUsername"}]

as you can see this approach exposes users to a very-difficult-to-trace bug that they could only have avoided by knowing exactly how this library serializes heterogeneous lists. Even with a documentation, I find this intolerable.

My counterproposal

I have come up with an alternative approach that,

• while more verbose in code
• completely enforces the homogeneity invariant in a way that can only be circumvented using unsafe memory manipulation,
• doesn't package the nbt values
• still allows full manipulation of the contained list (including mutable borrowing)

What I mean is enums, traits, and trait objects. See this mock implementation:

/// Container enum for all possible list types
enum NbtList {
    Byte(Vec<i8>),
    Short(Vec<i8>),
    // ...
    IntArray(Vec<Vec<i8>>),
    // ...
    List(Vec<NbtList>),
    Compound(Vec<NbtCompound>)
}
impl NbtList {
    pub fn get_byte_list(&self) -> Option<&Vec<i8>> {
      unimplemented!()
    }
    pub fn get_byte_list_mut(&mut self) -> Option<&mut Vec<i8>> {
      unimplemented!()
    }
}
impl Index<usize> for NbtList {
    type Output = dyn NbtCompatible;
    // ...
}
impl IndexMut<usize> for NbtList {
    // ...
}

/// This trait indicates that a type can be contained in an NbtTag or NbtList.
trait NbtCompatible {
    // Wraps this NbtCompatible in its corresponding NbtTag variant.
    fn wrap(self) -> NbtTag;
    fn get_type_id(&self) -> u8;
}
// implemented for any value that can be contained in NbtTag as well as `Vec<T> where T: NbtCompatible`
impl NbtCompatible for i8 {
    fn get_type_id(&self) -> u8 {
        unimplemented!()
    }
}

impl dyn NbtCompatible {
    fn get_byte(&self) -> Option<&i8> { }
    fn get_byte_mut(&mut self) -> Option<&mut i8> { }
    // etc etc
}

Pros

• completely and statically guarantees that any NbtList is sound NBT
• reduced memory overhead of nbt lists (because every element in the vector is only as big as it needs to be for the given type. Currently a list of bytes requires 40 times as much memory as it needs to be)
  • this means especially things like region files will be much smaller (5 times) in memory
• allows mutable borrowing of list contents
• no fallible serialization

Cons

• more coding effort

ToDos

• originally NbtCompatible was supposed to have a wrap(self) -> NbtTag method that would allow us to simply use .wrap().serialize() on a dyn NbtCompatible trait object, but that cannot work. If this is accepted, it might make sense to move serialization and deserialization into NbtCompatible as well.
• I think a lot of the effort in coding this could be automated using macros. Perhaps even the variants of NbtList can be automated by doing something similar to what typetag does.
• NbtCompatible::wrap is possible to implement as wrap_borrowed(&self) -> BorrowedTag and wrap_borrowed_mut(&mut self) -> BorrowedTagMut. Doing this will require two more enums though, which, if those features are not implemented using macros, would bring us to four enums with matching variants. Whether this is an issue, I will leave for someone else to decide.

I'll open up a PR on Vonr's fork once I have made a full implementation.

[Vonr]

the given example no longer happens with the new code, wrapper compounds are unwrapped during deserialization. the point about memory usage is fair though