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.