Item Meta Issue

[Protonull]

Yesterday someone mentioned in Discord that SAH's ItemMetaConverterHack is sometimes taking longer than expected, and since PaperMC has transitioned over to Kyori Adventure, I figured I'd take a look at it and give a report here. The problem that the hack exists to fix is a discrepancy between how text components like display names and lore are saved on items.

Prior to 1.13, display data looked like such:

{
    "Name": "Hello, World!"
}

1.13+, it looks like this:

{
    "Name": '{"text":"Hello, World!"}'
}

However, CraftBukkit, when using itemMeta.setDisplayName("Hello, World!") will set this:

{
    "Name": '{"text":"","extra":[{"text":"Hello, World!"}]}'
}

The implications of this issue are failing to match stuff in factories, shop chests, brewery, etc, issues that Devoted Hell have had, which is why we have ItemMetaConverterHack obsessively converting items to the CraftBukkit schema. Now, I've run some tests and we don't need to worry about pre-1.13 display data as Kyori will automatically component-ifies legacy display data so everything matches just fine. The difficulty is trying to match Kyori-set components with CraftBukkit-set components, like so:

final var formerItem = new ItemStack(Material.STICK);
formerItem.editMeta((meta) -> {
    meta.setDisplayName("Hello!");
});

final var latterItem = new ItemStack(Material.STICK);
latterItem.editMeta((meta) -> {
    meta.displayName(Component.text("Hello!"));
});

Assert.assertTrue(formerItem.isSimilar(latterItem)); // This will fail

The problem is though that even if we update every single civ plugin to use Kyori instead of CraftBukkit, we'd have to be vigilant with every single external plugin we use that could possibly set display data, and this doesn't even fix already existing items with the CraftBukkit schema, thus I think we're stuck with assuming that the CraftBukkit schema will stick around. We also have to contend that custom matching methods like ItemUtils.areItemsSimilar() are only bandaid fixes since NMS doesn't use them, so we need to continue converting item meta so that NMS can match items. And so I think we're stuck with just trying to find a more efficient way to convert display data.

[ProgrammerDan]

This is a great analysis and find, and also maddening. Thanks for the write-up.

[Protonull]

It's these kinds of things that has me thinking that we may want to look into mixins or having civ specific patches

[ProgrammerDan]

Makes onboarding devs just that much harder, but boy do I hard agree. Civcraft did maintain some "secret" mixins; the venerable Erocs managed that for years.