SPIGOT-7608: Allow empty lists to morph to any PDT list

The minecraft serialisation logic for ListTag updates the type byte of
the list tag during writing to match either the first element in the
list or 0, if said list tag is empty.
As such, list content type information cannot be carried through a
write/read process, e.g. chunk load/unload or a creative client updating
the item.

The recently introduced persistent data type collections for lists hence
can also not enforce a specific list content type if the found list is
empty, which it currently attempts to do.
As such, a call to PersistentDataContainer#has would also yield false
for any empty list as the lists type byte would be 0.

The faulty behaviour has been fixed by considering an empty list in the
persistent data container to match any list type.
This change, while technically breaking the #has check, is needed and
reasonable as the #has check for this is currently broken in the first
place as described above.

By: Bjarne Koll <lynxplay101@gmail.com>

paper-server/src/main/java/org/bukkit/craftbukkit/persistence/CraftPersistentDataTypeRegistry.java

@@ -372,7 +372,7 @@ public final class CraftPersistentDataTypeRegistry {
             values.add(this.wrap(listPersistentDataType.elementType(), primitiveValue));
         }
 
-        return new NBTTagList(values, elementAdapter.nmsTypeByte());
+        return new NBTTagList(values, values.isEmpty() ? NBTTagList.TAG_END : elementAdapter.nmsTypeByte());
     }
 
     /**
@@ -405,6 +405,11 @@ public final class CraftPersistentDataTypeRegistry {
      * Computes if the passed {@link NBTBase} is a {@link NBTTagList} and it,
      * including its elements, can be read/written via the passed
      * {@link PersistentDataType}.
+     * <p>
+     * As empty lists do not explicitly store their type, an empty nbt list can be matched to any
+     * ListPersistentDataType.
+     * As the persistent data container API does a full copy, this is a non-issue as callers to
+     * PDC#get(key, LIST.strings()) and PDC#get(key, LIST.ints()) will receive individual copies, avoiding interference.
      *
      * @param type the persistent data type for which to check if the tag
      * matches.
@@ -422,6 +427,6 @@ public final class CraftPersistentDataTypeRegistry {
         final byte elementType = listTag.getElementType();
         final TagAdapter elementAdapter = this.getOrCreateAdapter(listPersistentDataType.elementType());
 
-        return elementAdapter.nmsTypeByte() == elementType;
+        return elementAdapter.nmsTypeByte() == elementType || elementType == NBTTagList.TAG_END;
     }
 }

paper-server/src/test/java/org/bukkit/craftbukkit/inventory/PersistentDataContainerTest.java

@@ -1,6 +1,8 @@
 package org.bukkit.craftbukkit.inventory;
 
 import static org.junit.jupiter.api.Assertions.*;
+import com.google.common.io.ByteArrayDataOutput;
+import com.google.common.io.ByteStreams;
 import java.io.IOException;
 import java.io.StringReader;
 import java.lang.reflect.Array;
@@ -11,6 +13,7 @@ import java.util.Map;
 import java.util.UUID;
 import java.util.function.BiConsumer;
 import java.util.stream.Stream;
+import net.minecraft.nbt.NBTCompressedStreamTools;
 import net.minecraft.nbt.NBTTagCompound;
 import org.bukkit.Bukkit;
 import org.bukkit.Material;
@@ -461,14 +464,33 @@ public class PersistentDataContainerTest extends AbstractTestingBase {
     }
 
     @Test
-    public void testEmptyListDataMaintainType() {
-        final ItemMeta meta = createNewItemMeta();
-        final PersistentDataContainer container = meta.getPersistentDataContainer();
+    public void testEmptyListApplicationToAnyType() throws IOException {
+        final CraftMetaItem craftItem = new CraftMetaItem(new NBTTagCompound());
+        final PersistentDataContainer container = craftItem.getPersistentDataContainer();
 
         container.set(requestKey("list"), PersistentDataType.LIST.strings(), List.of());
-
         assertTrue(container.has(requestKey("list"), PersistentDataType.LIST.strings()));
-        assertFalse(container.has(requestKey("list"), PersistentDataType.LIST.bytes()));
+        assertTrue(container.has(requestKey("list"), PersistentDataType.LIST.bytes()));
+        assertFalse(container.has(requestKey("list"), PersistentDataType.STRING));
+        assertEquals(List.of(), container.get(requestKey("list"), PersistentDataType.LIST.strings()));
+
+        // Write and read the entire container to NBT
+        final NBTTagCompound storage = new NBTTagCompound();
+        craftItem.applyToItem(storage);
+
+        final ByteArrayDataOutput writer = ByteStreams.newDataOutput();
+        NBTCompressedStreamTools.write(storage, writer);
+
+        final NBTTagCompound readStorage = NBTCompressedStreamTools.read(
+            ByteStreams.newDataInput(writer.toByteArray())
+        );
+        final CraftMetaItem readItem = new CraftMetaItem(readStorage);
+        final PersistentDataContainer readContainer = readItem.getPersistentDataContainer();
+
+        assertTrue(readContainer.has(requestKey("list"), PersistentDataType.LIST.strings()));
+        assertTrue(readContainer.has(requestKey("list"), PersistentDataType.LIST.bytes()));
+        assertFalse(readContainer.has(requestKey("list"), PersistentDataType.STRING));
+        assertEquals(List.of(), readContainer.get(requestKey("list"), PersistentDataType.LIST.strings()));
     }
 
     // This is a horrific marriage of tag container array "primitive" types the API offered and the new list types.