From 221f354be10cd92236da875af18f17181105b4c2 Mon Sep 17 00:00:00 2001 From: tastybento Date: Sat, 25 Jul 2026 11:59:45 -0700 Subject: [PATCH] fix: keep item meta on chest items Chest files have been read with plain SnakeYAML since 1.26.0 so that an item unknown to the server version can be skipped individually instead of taking down the whole file. That left every nested value as a plain map, including an item's meta section, and ItemStack.deserialize silently ignores meta that is not already an ItemMeta. Enchanted books, potions, custom names and anything else meta-carrying arrived stripped: players in Plains were getting enchanted books with nothing stored on them. Chest item maps are now walked depth first and any map carrying a "==" type key is deserialized through ConfigurationSerialization before the item is built, so meta is a real ItemMeta by the time Bukkit sees it. A part that cannot be deserialized is left as a map and logged, so a bad meta section costs its meta rather than the whole item. Verified on a real Paper 26.2 server against the shipped phase files: 1.26.0 gave 0/3 Plains enchanted books with enchantments, this gives 3/3. The legacy enchantment names in the phase YAML (PROTECTION_FALL and friends) are still remapped by CraftBukkit, so no phase file changes are needed. Bumps version to 1.26.2. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SAQ3YQkcfRoCZEwa6SPJcp --- pom.xml | 2 +- .../aoneblock/oneblocks/OneBlocksManager.java | 57 ++++++++++- .../oneblocks/OneBlocksManagerTest3.java | 99 +++++++++++++++++++ 3 files changed, 153 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index 382fa72..3cd72d1 100644 --- a/pom.xml +++ b/pom.xml @@ -67,7 +67,7 @@ -LOCAL - 1.26.1 + 1.26.2 BentoBoxWorld_AOneBlock bentobox-world diff --git a/src/main/java/world/bentobox/aoneblock/oneblocks/OneBlocksManager.java b/src/main/java/world/bentobox/aoneblock/oneblocks/OneBlocksManager.java index b1bfbf5..b7c4d18 100644 --- a/src/main/java/world/bentobox/aoneblock/oneblocks/OneBlocksManager.java +++ b/src/main/java/world/bentobox/aoneblock/oneblocks/OneBlocksManager.java @@ -39,6 +39,7 @@ import org.bukkit.configuration.ConfigurationSection; import org.bukkit.configuration.InvalidConfigurationException; import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.configuration.serialization.ConfigurationSerialization; import org.bukkit.entity.EntityType; import org.bukkit.inventory.ItemStack; import org.eclipse.jdt.annotation.NonNull; @@ -848,16 +849,64 @@ private void addChestsFromRaw(OneBlockPhase obPhase, Map chests, String fi return null; } try { - Map map = new LinkedHashMap<>(); - raw.forEach((k, v) -> map.put(Objects.toString(k), v)); - map.remove("=="); - return ItemStack.deserialize(map); + Object deserialized = deserializeRaw(raw, fileName); + if (deserialized instanceof ItemStack stack) { + return stack; + } + // No type key, so the map is a bare item definition. Its meta, if any, is + // already a real ItemMeta after the walk above. + if (deserialized instanceof Map map) { + Map args = new LinkedHashMap<>(); + map.forEach((k, v) -> args.put(Objects.toString(k), v)); + return ItemStack.deserialize(args); + } + addon.log("Skipping item " + id + " in " + fileName + ": it is not an item definition."); + return null; } catch (Exception e) { addon.log("Skipping item " + id + " in " + fileName + ": " + e.getMessage()); return null; } } + /** + * Walks a value parsed by plain SnakeYAML and turns any map carrying a + * serialized-type key into the object it describes, depth first. + *

+ * Chest files are read with SnakeYAML rather than YamlConfiguration so that an + * item unknown to this server version can be skipped individually. The cost is + * that nothing is deserialized on the way in: an item's {@code meta} section + * arrives as a plain map, and {@link ItemStack#deserialize(Map)} silently + * ignores meta that is not already an {@link org.bukkit.inventory.meta.ItemMeta}. + * That is how stored enchantments, potion effects and custom names were being + * dropped. Deserializing here restores them while keeping per-item failures + * contained. A part that cannot be deserialized is left as a map and logged, so + * a bad meta section costs its meta rather than the whole item. + * + * @param value raw value from the YAML parse + * @param fileName file the value came from, for log messages + * @return the deserialized value, or the value itself if there is nothing to do + */ + private Object deserializeRaw(Object value, String fileName) { + if (value instanceof Map map) { + Map converted = new LinkedHashMap<>(); + map.forEach((k, v) -> converted.put(Objects.toString(k), deserializeRaw(v, fileName))); + if (converted.get(ConfigurationSerialization.SERIALIZED_TYPE_KEY) instanceof String type) { + try { + return ConfigurationSerialization.deserializeObject(converted); + } catch (Exception e) { + addon.log("Could not read " + type + " in " + fileName + ": " + e.getMessage()); + } + } + return converted; + } + if (value instanceof List list) { + List converted = new ArrayList<>(list.size()); + list.forEach(v -> converted.add(deserializeRaw(v, fileName))); + return converted; + } + return value; + } + private int phaseLength(PhaseIndexEntry entry) { if (entry.getLength() > 0) { return entry.getLength(); diff --git a/src/test/java/world/bentobox/aoneblock/oneblocks/OneBlocksManagerTest3.java b/src/test/java/world/bentobox/aoneblock/oneblocks/OneBlocksManagerTest3.java index 12e28b4..d82a253 100644 --- a/src/test/java/world/bentobox/aoneblock/oneblocks/OneBlocksManagerTest3.java +++ b/src/test/java/world/bentobox/aoneblock/oneblocks/OneBlocksManagerTest3.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -16,6 +17,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.util.List; +import java.util.Map; import java.util.NavigableMap; import java.util.concurrent.CompletableFuture; import java.util.jar.JarEntry; @@ -25,6 +27,11 @@ import org.bukkit.block.Biome; import org.bukkit.configuration.InvalidConfigurationException; import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.configuration.serialization.ConfigurationSerializable; +import org.bukkit.configuration.serialization.ConfigurationSerialization; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.EnchantmentStorageMeta; +import org.bukkit.inventory.meta.ItemMeta; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; import org.junit.jupiter.api.AfterAll; @@ -732,6 +739,98 @@ void testLoadPhasesChestWithUnknownItemSkipsItem() throws IOException { verify(plugin, never()).logError(anyString()); } + /** + * Item meta in a chest file survives loading. Chest files are parsed with plain + * SnakeYAML, so nested meta has to be deserialized before the item is built - + * ItemStack.deserialize ignores a meta section that is still a plain map, which + * is how enchanted books arrived with nothing stored on them. + *

+ * The server's own "ItemMeta" serialization alias does not exist under + * MockBukkit, so a stand-in is registered that records what it is given and + * returns a mock. The YAML is a copy of what the shipped phase files hold. + */ + @Test + void testLoadPhasesChestItemKeepsMeta() throws IOException { + PHASES_DIR.mkdirs(); + java.nio.file.Files.writeString(new File(PHASES_DIR, "alpha.yml").toPath(), """ + '0': + name: Alpha + biome: PLAINS + blocks: + GRASS_BLOCK: 100 + """); + java.nio.file.Files.writeString(new File(PHASES_DIR, "alpha_chests.yml").toPath(), """ + '0': + chests: + '1': + contents: + 0: + ==: org.bukkit.inventory.ItemStack + v: 2230 + type: ENCHANTED_BOOK + meta: + ==: ItemMeta + meta-type: ENCHANTED + stored-enchants: + PROTECTION_FALL: 1 + rarity: COMMON + """); + java.nio.file.Files.writeString(INDEX_FILE.toPath(), """ + phases: + - file: alpha + section: '0' + name: Alpha + length: 100 + """); + // The item factory has to accept the meta for ItemStack to keep it + when(itemFactory.isApplicable(any(ItemMeta.class), any(Material.class))).thenReturn(true); + when(itemFactory.asMetaFor(any(ItemMeta.class), any(Material.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + StandInItemMeta.lastArgs = null; + ConfigurationSerialization.registerClass(StandInItemMeta.class, "ItemMeta"); + try { + obm.loadPhases(); + } finally { + ConfigurationSerialization.unregisterClass("ItemMeta"); + } + OneBlockPhase phase = obm.getPhase(0); + assertNotNull(phase); + assertEquals(1, phase.getChests().size()); + ItemStack loaded = phase.getChests().iterator().next().getChest().get(0); + assertNotNull(loaded, "Chest should contain the book"); + // The meta section reached the meta deserializer intact, so a real ItemMeta - + // not a plain map - is what ItemStack.deserialize is given. Before the fix + // the deserializer was never called at all. Whether the stack then keeps the + // meta cannot be checked here: ItemStack delegates meta to a craftDelegate, + // which is a mock while Bukkit is statically mocked. + assertNotNull(StandInItemMeta.lastArgs, "Meta should have been deserialized"); + assertEquals("ENCHANTED", StandInItemMeta.lastArgs.get("meta-type")); + assertEquals(Map.of("PROTECTION_FALL", 1), StandInItemMeta.lastArgs.get("stored-enchants")); + verify(plugin, never()).logError(anyString()); + } + + /** + * Stands in for the server's ItemMeta deserializer. Records the map it is + * handed and returns a mock, so a test can check what reached it. + */ + public static class StandInItemMeta implements ConfigurationSerializable { + + static Map lastArgs; + + public static EnchantmentStorageMeta deserialize(Map args) { + lastArgs = args; + EnchantmentStorageMeta meta = Mockito.mock(EnchantmentStorageMeta.class); + // ItemStack hands back a clone of its meta + when(meta.clone()).thenReturn(meta); + return meta; + } + + @Override + public Map serialize() { + return Map.of(); + } + } + /** * Object-form block entries can carry a required Minecraft version; gated * entries are skipped with a log line, satisfied ones load with their weight.