ItemStack serialized to YAML with "internal" tag breaks on 1.20.6 > 1.21.1 update

[kangarko]

Expected behavior

The "internal" tag will continue to work...

Observed/Actual behavior

The tag is lost upon loading and storing the ItemStack, as it now seems to be renamed to "custom" without automatic migration

Steps/models to reproduce

Given the below yaml output, load it as itemstack and save it again on 1.21.1

        spawns:
        - custom_egg:
            ==: org.bukkit.inventory.ItemStack
            v: 3700
            type: MAGMA_CUBE_SPAWN_EGG
            meta:
              ==: ItemMeta
              meta-type: SPAWN_EGG
              display-name: '{"extra":[{"italic":false,"color":"white","text":"Spawn LavaLurker"}],"text":""}'
              lore:
              - '{"extra":[{"bold":false,"italic":false,"underlined":false,"strikethrough":false,"obfuscated":false,"color":"gray","text":""}],"text":""}'
              - '{"extra":[{"bold":true,"italic":false,"color":"dark_green","text":"< "},{"bold":false,"italic":false,"color":"gray","text":"Left click for menu."}],"text":""}'
              - '{"extra":[{"bold":true,"italic":false,"color":"dark_green","text":"> "},{"bold":false,"italic":false,"color":"gray","text":"Right click to summon."}],"text":""}'
              enchants:
                DURABILITY: 1
              ItemFlags:
              - HIDE_ENCHANTS
              internal: H4sIAAAAAAAA/+NiYOBi4HbKLy6O90sqCUlM52BgB/PCTBi4fBLLEn1Ki7JTixgYADZ7h3UpAAAA

Plugin and Datapack List

N/A

Paper version

This server is running Paper version 1.21.1-85-master@c5a1066 (2024-09-19T14:47:47Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)

Other

I managed to "fix" this by manually replacing "internal" with "custom", however that should be dealt with automatically. CraftMetaItem seems to contain support for both, not sure why it no longer works.

Thanks!

[electronicboy]

https://github.com/PaperMC/Paper/issues/10979

This is likely unmitigable in a reasonable manner and reason 101 as to why I've been advising against using YAML for serialising ItemStacks

[kangarko]

Thanks for a swift response!

Please consider adding just this one key, it's heavily used among many plugins, including NBT-API. This broke a lot of plugins not just mine, I believe the purpose of Bukkit is to mitigate these even at the expense of manual/duplicated code, see a lot of enums are being manually kept up. Thanks for the consideration.

[electronicboy]

There is no structure here to run through DFU. We could maybe create a means for handling known keys, but for anything else, all we could maybe try to do is transpose them over to the custom data component. However, I'm not sure that there is much desire to invest much effort into this broken form of Item persistence.

https://github.com/PaperMC/Paper/pull/10609 - will likely be the solution here, but that will not deal with situations upstream has not handled on upgrades. ItemMeta is highly broken, especially when it comes to serialisation; this is why I've been warning against it for eons. The best you can probably do is release an update for older versions to migrate the format to something sane and then get people to upgrade.

[kangarko]

Gotcha. I got it fixed in my own library nice and dirty way, but I agree with what you said, unfortunately keeping up 1.8.8-1.21 support is not straightforward. I propose you add a dirty fix for this for now, and in the future move towards a hard paper fork, get rid of all the deprecations etc., essentially force plugin authors to develop against paper, which would solve a lot of issues.

Here is my code for the record, it actually implements a whole custom yaml parser inspired by bukkit:

private class ConstructCustomObject extends ConstructYamlMap {

        private final boolean atLeast1_21 = MinecraftVersion.atLeast(V.v1_21);

        @Override
        public Object construct(Node node) {
            if (node.isRecursive())
                throw new YamlEngineException("Unexpected referential mapping structure. Node: " + node);

            final Map raw = (Map) super.construct(node);

            if (raw.containsKey(ConfigurationSerialization.SERIALIZED_TYPE_KEY)) {
                final String classPath = (String) raw.get(ConfigurationSerialization.SERIALIZED_TYPE_KEY);

                if (classPath.equals("ItemMeta")) {

                    // https://github.com/PaperMC/Paper/issues/11423
                    if (raw.containsKey("internal") && this.atLeast1_21)
                        raw.put("custom", raw.get("internal"));
                }

                final Map<String, Object> typed = new LinkedHashMap<>(raw.size());

                for (final Object entryRaw : raw.entrySet()) {
                    final Map.Entry entry = (Map.Entry) entryRaw;

                    typed.put(entry.getKey().toString(), entry.getValue());
                }

                try {
                    return ConfigurationSerialization.deserializeObject(typed);

                } catch (final IllegalArgumentException ex) {
                    throw new YamlEngineException("Could not deserialize object", ex);
                }
            }

            return raw;
        }

        @Override
        public void constructRecursive(Node node, Object object) {
            throw new YamlEngineException("Unexpected referential mapping structure. Node: " + node);
        }
    }