Properly implement serialization / deserialization for NBT Arrays (#1213)

* fix: use int array instead of list for chunk entities

* just get the int array and don't fall back for the list

* update deserialization to use proper NbtTag

* dix deserialization of nbt arrays

---------

Co-authored-by: Alexander Medvedev <lilalexmed@proton.me>

pumpkin-nbt/src/deserializer.rs

@@ -1,3 +1,4 @@
+use std::cell::RefCell;
 use std::io::{Seek, SeekFrom};
 
 use crate::*;
@@ -7,6 +8,20 @@ use serde::{Deserialize, forward_to_deserialize_any};
 
 pub type Result<T> = std::result::Result<T, Error>;
 
+thread_local! {
+    pub static CURR_VISITOR_LIST_TYPE: RefCell<Option<u8>> = const { std::cell::RefCell::new(None) };
+}
+
+pub(super) fn take_curr_visitor_seq_list_id() -> Option<u8> {
+    CURR_VISITOR_LIST_TYPE.with(|cell| cell.take())
+}
+
+pub(super) fn set_curr_visitor_seq_list_id(tag: Option<u8>) {
+    CURR_VISITOR_LIST_TYPE.with(|cell| {
+        *cell.borrow_mut() = tag;
+    });
+}
+
 #[derive(Debug)]
 pub struct NbtReadHelper<R: Read + Seek> {
     reader: R,
@@ -135,6 +150,9 @@ impl<'de, R: Read + Seek> de::Deserializer<'de> for &mut Deserializer<R> {
                     return Err(Error::NegativeLength(remaining_values));
                 }
 
+                //TODO this is a bit hacky but I couldn't think of a better way
+                // This flag gets auto cleared in visit_seq
+                set_curr_visitor_seq_list_id(Some(list_type));
                 let result = visitor.visit_seq(ListAccess {
                     de: self,
                     list_type,

pumpkin-nbt/src/tag.rs

@@ -393,14 +393,7 @@ impl Serialize for NbtTag {
             NbtTag::Long(v) => serializer.serialize_i64(*v),
             NbtTag::Float(v) => serializer.serialize_f32(*v),
             NbtTag::Double(v) => serializer.serialize_f64(*v),
-            NbtTag::ByteArray(v) => {
-                use serde::ser::SerializeSeq;
-                let mut seq = serializer.serialize_seq(Some(v.len()))?;
-                for byte in v.iter() {
-                    seq.serialize_element(byte)?;
-                }
-                seq.end()
-            }
+            NbtTag::ByteArray(v) => nbt_byte_array(v, serializer),
             NbtTag::String(v) => serializer.serialize_str(v),
             NbtTag::List(v) => {
                 use serde::ser::SerializeSeq;
@@ -411,22 +404,8 @@ impl Serialize for NbtTag {
                 seq.end()
             }
             NbtTag::Compound(v) => v.serialize(serializer),
-            NbtTag::IntArray(v) => {
-                use serde::ser::SerializeSeq;
-                let mut seq = serializer.serialize_seq(Some(v.len()))?;
-                for int in v.iter() {
-                    seq.serialize_element(int)?;
-                }
-                seq.end()
-            }
-            NbtTag::LongArray(v) => {
-                use serde::ser::SerializeSeq;
-                let mut seq = serializer.serialize_seq(Some(v.len()))?;
-                for long in v.iter() {
-                    seq.serialize_element(long)?;
-                }
-                seq.end()
-            }
+            NbtTag::IntArray(v) => nbt_int_array(v, serializer),
+            NbtTag::LongArray(v) => nbt_long_array(v, serializer),
         }
     }
 }
@@ -478,11 +457,42 @@ impl<'de> Deserialize<'de> for NbtTag {
                 self,
                 mut seq: A,
             ) -> Result<Self::Value, A::Error> {
-                let mut vec = Vec::new();
-                while let Some(value) = seq.next_element()? {
-                    vec.push(value);
+                let curr = deserializer::take_curr_visitor_seq_list_id().unwrap_or(LIST_ID);
+
+                match curr {
+                    INT_ARRAY_ID => {
+                        let mut vec = Vec::new();
+                        while let Some(value) = seq.next_element()? {
+                            vec.push(value);
+                        }
+                        Ok(NbtTag::IntArray(vec))
+                    }
+                    LONG_ARRAY_ID => {
+                        let mut vec = Vec::new();
+                        while let Some(value) = seq.next_element()? {
+                            vec.push(value);
+                        }
+                        Ok(NbtTag::LongArray(vec))
+                    }
+                    BYTE_ARRAY_ID => {
+                        let mut vec = Vec::new();
+                        while let Some(value) = seq.next_element()? {
+                            vec.push(value);
+                        }
+                        Ok(NbtTag::ByteArray(vec.into_boxed_slice()))
+                    }
+                    LIST_ID => {
+                        let mut vec = Vec::new();
+                        while let Some(value) = seq.next_element()? {
+                            vec.push(value);
+                        }
+                        Ok(NbtTag::List(vec))
+                    }
+                    _ => Err(serde::de::Error::custom(format!(
+                        "Invalid list type id: {}",
+                        curr
+                    ))),
                 }
-                Ok(NbtTag::List(vec))
             }
 
             fn visit_map<A: serde::de::MapAccess<'de>>(

pumpkin-world/src/chunk/format/mod.rs

@@ -286,21 +286,22 @@ impl ChunkEntityData {
         }
         let mut map = HashMap::new();
         for entity_nbt in chunk_entity_data.entities {
-            // TODO: This is wrong, we should use an int array, but our NBT lib for some reason does not work with int arrays and
-            // Just gives me a list when putting in a int array
-            let uuid = match entity_nbt.get_list("UUID") {
-                Some(uuid) => uuid,
+            let uuid = match entity_nbt.get_int_array("UUID") {
+                Some(uuid) => Uuid::from_u128(
+                    (uuid[0] as u128) << 96
+                        | (uuid[1] as u128) << 64
+                        | (uuid[2] as u128) << 32
+                        | (uuid[3] as u128),
+                ),
                 None => {
-                    log::warn!("TODO: use int arrays for UUID");
+                    println!(
+                        "Entity in chunk {},{} is missing UUID: {:?}",
+                        position.x, position.y, entity_nbt
+                    );
                     continue;
                 }
             };
-            let uuid = Uuid::from_u128(
-                (uuid.first().unwrap().extract_int().unwrap() as u128) << 96
-                    | (uuid.get(1).unwrap().extract_int().unwrap() as u128) << 64
-                    | (uuid.get(2).unwrap().extract_int().unwrap() as u128) << 32
-                    | (uuid.get(3).unwrap().extract_int().unwrap() as u128),
-            );
+
             map.insert(uuid, entity_nbt);
         }