From a7f03fb9828d99981b558f12cdef7dfb3120f4c7 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 29 Oct 2026 14:05:48 +0100 Subject: [PATCH] perf: cheaper item-scan key checks and kept ItemsAdder templates The TLibs item scan calls TagManager and LegacyFishScan matches() on every non-air slot ten times a second. Both copied the whole item meta (every component plus the custom data tag) just to test one key. They now read the key through ItemStack#getPersistentDataContainer, the read-only view, with the same keys and the same answers. ItemRef now keeps one built template per ItemsAdder ref and hands every caller its own clone, so food rebuilds stop asking ItemsAdder for a new copy each time. Only refs TLibs routes to ItemsAdder are kept: v. and modeled. are cheap, m. can roll random MMOItems stats, and prefixes with a TLibs path handler (c.) may differ per build. Failed builds (null, air, TLibs' dirt fallback) are never kept. The templates are only on while Cooking listens for ItemsAdderLoadDataEvent, and are cleared on every ItemsAdder load, on /cooking reload and when TLibs swaps its creator. needsLoreRebuild uses a precompiled colour-code pattern instead of compiling it per lore line; the output is identical. Co-Authored-By: Claude Opus 5.5 --- .../java/net/tfminecraft/cooking/Cooking.java | 5 + .../cooking/fishing/LegacyFishScan.java | 8 +- .../cooking/fishing/SeafoodWholeItems.java | 5 +- .../cooking/manager/TagManager.java | 6 +- .../tfminecraft/cooking/utils/ItemRef.java | 80 +++++- .../cooking/utils/ItemUpdater.java | 6 +- .../cooking/utils/ItemsAdderReloadHook.java | 42 ++++ .../api/Events/ItemsAdderLoadDataEvent.java | 18 ++ .../fishing/FishingRuntimeCoverageTest.java | 3 +- .../cooking/fishing/ScanMatchParityTest.java | 152 ++++++++++++ .../cooking/utils/ItemLoreRebuildTest.java | 58 +++++ .../utils/ItemRefTemplateCacheTest.java | 233 ++++++++++++++++++ .../utils/ItemsAdderReloadHookTest.java | 99 ++++++++ 13 files changed, 703 insertions(+), 12 deletions(-) create mode 100644 src/main/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHook.java create mode 100644 src/test/java/dev/lone/itemsadder/api/Events/ItemsAdderLoadDataEvent.java create mode 100644 src/test/java/net/tfminecraft/cooking/fishing/ScanMatchParityTest.java create mode 100644 src/test/java/net/tfminecraft/cooking/utils/ItemRefTemplateCacheTest.java create mode 100644 src/test/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHookTest.java diff --git a/src/main/java/net/tfminecraft/cooking/Cooking.java b/src/main/java/net/tfminecraft/cooking/Cooking.java index a597bf6..2f5d215 100644 --- a/src/main/java/net/tfminecraft/cooking/Cooking.java +++ b/src/main/java/net/tfminecraft/cooking/Cooking.java @@ -89,6 +89,8 @@ import net.tfminecraft.cooking.oven.OvenCavityManager; import net.tfminecraft.cooking.oven.OvenHandler; import net.tfminecraft.cooking.oven.OvenLifecycleHandler; +import net.tfminecraft.cooking.utils.ItemRef; +import net.tfminecraft.cooking.utils.ItemsAdderReloadHook; import net.tfminecraft.tlibs.itemscan.ItemScanService; import net.tfminecraft.tlibs.character.Characters; @@ -225,6 +227,7 @@ static void warnIfCharacterIdentityMissing(Logger logger) { @Override public void onDisable() { if (incompatibleCharacterApi) return; + ItemRef.setCaching(false); breweryBridge.close(); SaturationGuard.stop(); NutritionDrainTask.stop(); @@ -265,6 +268,7 @@ public void loadConfigs() { } public void reloadAll() { + ItemRef.clearCache(); loadConfigs(); HusbandryLifecycleListener.applyStatsRevision(); craftingManager.rebuildStations(); @@ -326,6 +330,7 @@ public void registerListeners() { CropCustomCropsBridge.tryRegister(this); getServer().getPluginManager().registerEvents(new CustomFishingBridge(), this); CustomFishingBridge.tryRegister(this); + ItemsAdderReloadHook.registerIfPresent(this); if (husbandryEnabled) { getServer().getPluginManager().registerEvents(new HusbandryLifecycleListener(), this); getServer().getPluginManager().registerEvents(new HusbandryCareListener(), this); diff --git a/src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java b/src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java index b5efd01..0eef5b8 100644 --- a/src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java +++ b/src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java @@ -5,9 +5,10 @@ import org.bukkit.entity.Player; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.ItemStack; -import org.bukkit.persistence.PersistentDataContainer; import org.bukkit.persistence.PersistentDataType; +import io.papermc.paper.persistence.PersistentDataContainerView; + import net.tfminecraft.cooking.manager.TagManager; import net.tfminecraft.cooking.utils.Keys; import net.tfminecraft.tlibs.itemscan.ItemScanHandler; @@ -20,7 +21,8 @@ public boolean matches(ItemStack stack) { if (stack == null || !Tag.ITEMS_FISHES.isTagged(stack.getType())) { return false; } - PersistentDataContainer data = stack.getItemMeta().getPersistentDataContainer(); + // Runs on every scanned fish: read through the stack's data view instead of copying its whole meta. + PersistentDataContainerView data = stack.getPersistentDataContainer(); return isFood(data) ? SeafoodWholeItems.carriesCatchData(data) : stack.getType() == Material.COD; } @@ -44,7 +46,7 @@ public void update(Player player, Inventory inventory, int slot, ItemStack stack } } - private static boolean isFood(PersistentDataContainer data) { + private static boolean isFood(PersistentDataContainerView data) { return data.has(Keys.FOOD_ID, PersistentDataType.STRING); } } diff --git a/src/main/java/net/tfminecraft/cooking/fishing/SeafoodWholeItems.java b/src/main/java/net/tfminecraft/cooking/fishing/SeafoodWholeItems.java index a67b534..43ba30d 100644 --- a/src/main/java/net/tfminecraft/cooking/fishing/SeafoodWholeItems.java +++ b/src/main/java/net/tfminecraft/cooking/fishing/SeafoodWholeItems.java @@ -2,9 +2,10 @@ import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; -import org.bukkit.persistence.PersistentDataContainer; import org.bukkit.persistence.PersistentDataType; +import io.papermc.paper.persistence.PersistentDataContainerView; + import net.tfminecraft.cooking.item.FoodItem; import net.tfminecraft.cooking.item.IngredientLineage; import net.tfminecraft.cooking.loader.FoodLoader; @@ -54,7 +55,7 @@ public static ItemStack build(ItemStack caught, VanillaFish fish, int quality) { * True for a whole fish built by an older version on the caught CustomFishing item itself, so * it still carries that item's per-catch data. */ - static boolean carriesCatchData(PersistentDataContainer data) { + static boolean carriesCatchData(PersistentDataContainerView data) { return CustomFishingCatalog.WHOLE_TYPE.equals(data.get(Keys.FOOD_ID, PersistentDataType.STRING)) && data.has(Keys.CUSTOM_FISHING_ID, PersistentDataType.STRING) && !data.has(Keys.CLEAN_CATCH, PersistentDataType.BYTE); diff --git a/src/main/java/net/tfminecraft/cooking/manager/TagManager.java b/src/main/java/net/tfminecraft/cooking/manager/TagManager.java index 5010078..2646104 100644 --- a/src/main/java/net/tfminecraft/cooking/manager/TagManager.java +++ b/src/main/java/net/tfminecraft/cooking/manager/TagManager.java @@ -17,13 +17,13 @@ public class TagManager implements ItemScanHandler { private static final int OFF_HAND_SLOT = 40; + // Runs on every scanned slot: read the key through the stack's data view instead of copying its whole meta. @Override public boolean matches(ItemStack stack) { - if (stack == null || stack.getType() == Material.AIR || !stack.hasItemMeta()) { + if (stack == null || stack.getType() == Material.AIR) { return false; } - ItemMeta meta = stack.getItemMeta(); - return meta != null && meta.getPersistentDataContainer().has(Keys.FOOD_ID, PersistentDataType.STRING); + return stack.getPersistentDataContainer().has(Keys.FOOD_ID, PersistentDataType.STRING); } // Keep the existing legacy text representation, formatting, and exact-string comparisons. diff --git a/src/main/java/net/tfminecraft/cooking/utils/ItemRef.java b/src/main/java/net/tfminecraft/cooking/utils/ItemRef.java index 61466d0..52db17d 100644 --- a/src/main/java/net/tfminecraft/cooking/utils/ItemRef.java +++ b/src/main/java/net/tfminecraft/cooking/utils/ItemRef.java @@ -1,25 +1,101 @@ package net.tfminecraft.cooking.utils; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import org.bukkit.Material; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; import org.bukkit.persistence.PersistentDataContainer; import net.tfminecraft.tlibs.TLibs; +import net.tfminecraft.tlibs.objects.api.ItemAPI; +import net.tfminecraft.tlibs.objects.api.subapi.ItemCreator; public class ItemRef { + /* + * Built ItemsAdder templates, one per ref, so the inventory scan's food rebuilds stop asking ItemsAdder + * for a fresh copy every time. Callers only ever get clones. Only refs that TLibs sends to ItemsAdder + * are kept: v. and modeled. builds are cheap, m. builds can roll random MMOItems stats, magic. has no + * creation path, and a prefix with a registered TLibs path handler (Cooking's own c., for one) may + * build something different each time. Failures (no item, air, or TLibs' dirt fallback when ItemsAdder + * is missing or has not loaded the item yet) are never kept, so they are retried like before. + * Only on while Cooking listens for ItemsAdder data loads (see ItemsAdderReloadHook). Cleared on + * /cooking reload, on every ItemsAdder data load, and when TLibs hands out a new creator. + */ + private static final Set UNCACHED_PREFIXES = Set.of("v", "m", "modeled", "magic"); + private static volatile boolean caching; + private static volatile Templates templates; + + private record Templates(ItemCreator creator, Map items) {} + public static ItemStack resolve(String ref) { - return TLibs.getItemAPI().getCreator().getItemFromPath(ref); + ItemCreator creator = TLibs.getItemAPI().getCreator(); + Templates cache = cacheFor(ref, creator); + ItemStack cached = cache == null ? null : cache.items().get(ref); + if (cached != null) return cached.clone(); + return build(ref, creator, cache); } public static ItemStack apply(String ref, ItemStack source) { - ItemStack template = resolve(ref); + ItemCreator creator = TLibs.getItemAPI().getCreator(); + Templates cache = cacheFor(ref, creator); + ItemStack template = cache == null ? null : cache.items().get(ref); + if (template == null) template = build(ref, creator, cache); if (template == null) return null; ItemStack out = template.clone(); mergeMeta(out, source); return out; } + /** Drops every kept template; the next use of each ref builds it again. */ + public static void clearCache() { + templates = null; + } + + /** Turns template keeping on or off; either way the kept templates are dropped. */ + public static void setCaching(boolean enabled) { + caching = enabled; + templates = null; + } + + private static ItemStack build(String ref, ItemCreator creator, Templates cache) { + ItemStack built = creator.getItemFromPath(ref); + if (cache != null && isTemplate(built)) { + cache.items().putIfAbsent(ref, built.clone()); + } + return built; + } + + private static boolean isTemplate(ItemStack built) { + return built != null && !built.getType().isAir() && built.getType() != Material.DIRT; + } + + /** The template cache for this ref, or null when the ref is not built from ItemsAdder. */ + private static Templates cacheFor(String ref, ItemCreator creator) { + if (!caching || creator == null || !isItemsAdderRef(ref)) return null; + Templates current = templates; + if (current == null || current.creator() != creator) { + current = new Templates(creator, new ConcurrentHashMap<>()); + templates = current; + } + return current; + } + + // Mirrors the prefix routing in TLibs ItemCreator.getItemFromPath: everything else goes to ItemsAdder. + private static boolean isItemsAdderRef(String ref) { + if (ref == null) return false; + int dot = ref.indexOf('.'); + if (dot <= 0 || dot == ref.length() - 1) return false; + String prefix = ref.substring(0, dot).toLowerCase(Locale.ROOT); + if (UNCACHED_PREFIXES.contains(prefix)) return false; + ItemAPI api = TLibs.getItemAPI(); + return api.getPathHandler(prefix) == null; + } + // Keep the existing legacy text representation, formatting, and exact-string comparisons. @SuppressWarnings("deprecation") public static void mergeMeta(ItemStack out, ItemStack source) { diff --git a/src/main/java/net/tfminecraft/cooking/utils/ItemUpdater.java b/src/main/java/net/tfminecraft/cooking/utils/ItemUpdater.java index 860c5ab..4c22fbd 100644 --- a/src/main/java/net/tfminecraft/cooking/utils/ItemUpdater.java +++ b/src/main/java/net/tfminecraft/cooking/utils/ItemUpdater.java @@ -12,9 +12,13 @@ import org.bukkit.persistence.PersistentDataType; import java.util.List; +import java.util.regex.Pattern; public class ItemUpdater { + // Same pattern String.replaceAll("§.", "") compiled on every lore line. + private static final Pattern COLOUR_CODE = Pattern.compile("§."); + /** Ages the FoodItem. Returns true if any TagStep changed (including nested sauce). */ public static boolean applyAging(FoodItem fi, long deltaSeconds) { if (fi == null || deltaSeconds <= 0) return false; @@ -160,7 +164,7 @@ public static boolean needsLoreRebuild(List lore, boolean hasLoreIndex) if (line == null) { continue; } - String stripped = line.replaceAll("§.", ""); + String stripped = COLOUR_CODE.matcher(line).replaceAll(""); if (stripped.contains("Nutrition")) { nutrition = true; } diff --git a/src/main/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHook.java b/src/main/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHook.java new file mode 100644 index 0000000..a6e21fd --- /dev/null +++ b/src/main/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHook.java @@ -0,0 +1,42 @@ +package net.tfminecraft.cooking.utils; + +import org.bukkit.event.Event; +import org.bukkit.event.EventPriority; +import org.bukkit.event.Listener; +import org.bukkit.plugin.Plugin; + +/** + * Turns on {@link ItemRef}'s ItemsAdder templates and clears them whenever ItemsAdder loads its items + * (startup and /iareload). Without ItemsAdder, or without that event, templates stay off. + * Cooking does not compile against ItemsAdder, so the event class is looked up from ItemsAdder's own loader. + */ +public final class ItemsAdderReloadHook implements Listener { + static final String LOAD_EVENT = "dev.lone.itemsadder.api.Events.ItemsAdderLoadDataEvent"; + + private ItemsAdderReloadHook() {} + + /** Returns true when ItemsAdder is installed and the hook was registered. */ + public static boolean registerIfPresent(Plugin plugin) { + return registerIfPresent(plugin, LOAD_EVENT); + } + + static boolean registerIfPresent(Plugin plugin, String eventClass) { + Plugin itemsAdder = plugin.getServer().getPluginManager().getPlugin("ItemsAdder"); + if (itemsAdder == null) return false; + try { + Class loadEvent = + Class.forName(eventClass, false, itemsAdder.getClass().getClassLoader()).asSubclass(Event.class); + plugin.getServer().getPluginManager().registerEvent(loadEvent, new ItemsAdderReloadHook(), + EventPriority.MONITOR, (listener, event) -> { + if (loadEvent.isInstance(event)) ItemRef.clearCache(); + }, plugin); + } catch (ClassNotFoundException | RuntimeException | LinkageError e) { + // Without the reload signal a kept template could outlive /iareload, so keep none. + plugin.getLogger().warning("Cannot listen for " + eventClass + "; Cooking builds ItemsAdder items" + + " fresh every time instead of keeping templates (" + e + ")"); + return false; + } + ItemRef.setCaching(true); + return true; + } +} diff --git a/src/test/java/dev/lone/itemsadder/api/Events/ItemsAdderLoadDataEvent.java b/src/test/java/dev/lone/itemsadder/api/Events/ItemsAdderLoadDataEvent.java new file mode 100644 index 0000000..491f34e --- /dev/null +++ b/src/test/java/dev/lone/itemsadder/api/Events/ItemsAdderLoadDataEvent.java @@ -0,0 +1,18 @@ +package dev.lone.itemsadder.api.Events; + +import org.bukkit.event.Event; +import org.bukkit.event.HandlerList; + +/** Test stand-in for ItemsAdder's event; Cooking does not compile against ItemsAdder. */ +public class ItemsAdderLoadDataEvent extends Event { + private static final HandlerList HANDLERS = new HandlerList(); + + @Override + public HandlerList getHandlers() { + return HANDLERS; + } + + public static HandlerList getHandlerList() { + return HANDLERS; + } +} diff --git a/src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java b/src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java index d6aa5e9..3c88513 100644 --- a/src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java +++ b/src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java @@ -141,7 +141,8 @@ public class FishingRuntimeCoverageTest { ItemStack external=mock(ItemStack.class); when(external.getType()).thenReturn(Material.COD); when(external.hasItemMeta()).thenReturn(true); assertNull(LegacyFishConversion.convert(null,external)); ItemMeta meta=mock(ItemMeta.class); when(external.getItemMeta()).thenReturn(meta); - when(meta.getPersistentDataContainer()).thenReturn(new ItemStack(Material.COD).getItemMeta().getPersistentDataContainer()); + var externalData=new ItemStack(Material.COD).getItemMeta().getPersistentDataContainer(); + when(meta.getPersistentDataContainer()).thenReturn(externalData); when(external.getPersistentDataContainer()).thenReturn(externalData); assertNull(LegacyFishConversion.convert(null,external)); models.when(()->LegacyModelData.has(meta)).thenReturn(true); models.when(()->LegacyModelData.get(meta)).thenReturn(17); when(external.getAmount()).thenReturn(7); diff --git a/src/test/java/net/tfminecraft/cooking/fishing/ScanMatchParityTest.java b/src/test/java/net/tfminecraft/cooking/fishing/ScanMatchParityTest.java new file mode 100644 index 0000000..07100e5 --- /dev/null +++ b/src/test/java/net/tfminecraft/cooking/fishing/ScanMatchParityTest.java @@ -0,0 +1,152 @@ +package net.tfminecraft.cooking.fishing; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.bukkit.Material; +import org.bukkit.NamespacedKey; +import org.bukkit.Tag; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.persistence.PersistentDataContainer; +import org.bukkit.persistence.PersistentDataType; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockbukkit.mockbukkit.MockBukkit; + +import net.tfminecraft.cooking.Cooking; +import net.tfminecraft.cooking.manager.TagManager; +import net.tfminecraft.cooking.utils.Keys; + +/** The scan's key checks read the stack's data view and answer exactly as the old full-meta reads did. */ +class ScanMatchParityTest { + private Cooking previous; + + @BeforeEach + void setUp() { + MockBukkit.mock(); + previous = Cooking.plugin; + Cooking.plugin = mock(Cooking.class); + when(Cooking.plugin.namespace()).thenReturn("cooking"); + when(Cooking.plugin.getName()).thenReturn("Cooking"); + } + + @AfterEach + void tearDown() { + Cooking.plugin = previous; + MockBukkit.unmock(); + } + + // origin/main TagManager.matches + private static boolean oldTagMatch(ItemStack stack) { + if (stack == null || stack.getType() == Material.AIR || !stack.hasItemMeta()) { + return false; + } + ItemMeta meta = stack.getItemMeta(); + return meta != null && meta.getPersistentDataContainer().has(Keys.FOOD_ID, PersistentDataType.STRING); + } + + // origin/main LegacyFishScan.matches, with the old carriesCatchData inlined + private static boolean oldFishMatch(ItemStack stack) { + if (stack == null || !Tag.ITEMS_FISHES.isTagged(stack.getType())) { + return false; + } + PersistentDataContainer data = stack.getItemMeta().getPersistentDataContainer(); + if (!data.has(Keys.FOOD_ID, PersistentDataType.STRING)) { + return stack.getType() == Material.COD; + } + return CustomFishingCatalog.WHOLE_TYPE.equals(data.get(Keys.FOOD_ID, PersistentDataType.STRING)) + && data.has(Keys.CUSTOM_FISHING_ID, PersistentDataType.STRING) + && !data.has(Keys.CLEAN_CATCH, PersistentDataType.BYTE); + } + + private interface Edit { + void apply(PersistentDataContainer data); + } + + private static ItemStack stack(Material material, Edit edit) { + ItemStack stack = new ItemStack(material); + ItemMeta meta = stack.getItemMeta(); + edit.apply(meta.getPersistentDataContainer()); + stack.setItemMeta(meta); + return stack; + } + + private static Map samples() { + Map samples = new LinkedHashMap<>(); + samples.put("null", null); + samples.put("air", new ItemStack(Material.AIR)); + for (Material material : List.of(Material.STONE, Material.BREAD, Material.COD, Material.SALMON, + Material.TROPICAL_FISH, Material.PUFFERFISH, Material.COOKED_COD)) { + String name = material.name().toLowerCase(); + samples.put(name + " without meta", new ItemStack(material)); + ItemStack renamed = new ItemStack(material); + ItemMeta meta = renamed.getItemMeta(); + meta.setUnbreakable(true); + renamed.setItemMeta(meta); + samples.put(name + " with meta but no data", renamed); + samples.put(name + " food", stack(material, d -> d.set(Keys.FOOD_ID, PersistentDataType.STRING, "bread"))); + samples.put(name + " food id stored as a number", + stack(material, d -> d.set(Keys.FOOD_ID, PersistentDataType.INTEGER, 3))); + samples.put(name + " other cooking key only", + stack(material, d -> d.set(Keys.ORIGIN, PersistentDataType.STRING, "Bread"))); + samples.put(name + " food id under another plugin", + stack(material, d -> d.set(new NamespacedKey("other", "food_id"), PersistentDataType.STRING, "x"))); + samples.put(name + " old whole fish", stack(material, d -> { + d.set(Keys.FOOD_ID, PersistentDataType.STRING, CustomFishingCatalog.WHOLE_TYPE); + d.set(Keys.CUSTOM_FISHING_ID, PersistentDataType.STRING, "tfmc_tuna_fish"); + })); + samples.put(name + " clean whole fish", stack(material, d -> { + d.set(Keys.FOOD_ID, PersistentDataType.STRING, CustomFishingCatalog.WHOLE_TYPE); + d.set(Keys.CUSTOM_FISHING_ID, PersistentDataType.STRING, "tfmc_tuna_fish"); + d.set(Keys.CLEAN_CATCH, PersistentDataType.BYTE, (byte) 1); + })); + samples.put(name + " whole fish without a catch", stack(material, + d -> d.set(Keys.FOOD_ID, PersistentDataType.STRING, CustomFishingCatalog.WHOLE_TYPE))); + samples.put(name + " catch id stored as a number", stack(material, d -> { + d.set(Keys.FOOD_ID, PersistentDataType.STRING, CustomFishingCatalog.WHOLE_TYPE); + d.set(Keys.CUSTOM_FISHING_ID, PersistentDataType.INTEGER, 4); + })); + } + return samples; + } + + @Test + void tagManagerMatchesExactlyWhatTheFullMetaReadMatched() { + TagManager tags = new TagManager(); + List matched = new ArrayList<>(); + samples().forEach((name, stack) -> { + assertEquals(oldTagMatch(stack), tags.matches(stack), name); + if (tags.matches(stack)) matched.add(name); + }); + // Every material carrying the food id as text, and nothing else. + assertEquals(7 * 5, matched.size(), matched.toString()); + assertTrue(matched.contains("bread food")); + assertFalse(matched.contains("bread food id stored as a number")); + } + + @Test + void legacyFishScanMatchesExactlyWhatTheFullMetaReadMatched() { + LegacyFishScan scan = new LegacyFishScan(); + List matched = new ArrayList<>(); + samples().forEach((name, stack) -> { + assertEquals(oldFishMatch(stack), scan.matches(stack), name); + if (scan.matches(stack)) matched.add(name); + }); + assertTrue(matched.contains("cod without meta")); + assertTrue(matched.contains("cod food id stored as a number")); + assertTrue(matched.contains("salmon old whole fish")); + assertFalse(matched.contains("salmon clean whole fish")); + assertFalse(matched.contains("salmon without meta")); + assertFalse(matched.contains("cooked_cod without meta")); + } +} diff --git a/src/test/java/net/tfminecraft/cooking/utils/ItemLoreRebuildTest.java b/src/test/java/net/tfminecraft/cooking/utils/ItemLoreRebuildTest.java index 043dd6b..0eb50e7 100644 --- a/src/test/java/net/tfminecraft/cooking/utils/ItemLoreRebuildTest.java +++ b/src/test/java/net/tfminecraft/cooking/utils/ItemLoreRebuildTest.java @@ -4,6 +4,8 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -70,6 +72,62 @@ void raisingFreshnessThresholdsKeepsFullLore() { assertFalse(ItemUpdater.needsLoreRebuild(lore, true)); } + // origin/main needsLoreRebuild, which compiled the colour-code regex for every lore line. + private static boolean oldNeedsLoreRebuild(List lore, boolean hasLoreIndex) { + if (lore == null || lore.isEmpty() || !hasLoreIndex) { + return true; + } + boolean nutrition = false; + boolean food = false; + for (String line : lore) { + if (line == null) { + continue; + } + String stripped = line.replaceAll("§.", ""); + if (stripped.contains("Nutrition")) { + nutrition = true; + } + if (stripped.contains("Food") && !stripped.contains("% Food") && !stripped.contains("+")) { + food = true; + } + } + return !nutrition || !food; + } + + @Test + void colourCodesAreStrippedExactlyAsBefore() { + String s = "§"; + List lines = Arrays.asList( + s + "aNutrition " + s + "f6.0", s + "7Food " + s + "f10.0", "Nu" + s + "ltrition 6", + "F" + s + "x" + s + "a" + s + "b" + s + "c" + s + "d" + s + "e" + s + "food 3", s + s + "Food", + "F" + s + s + "ood", "Food" + s, s, s + "\nFood", s + "\rNutrition", "Nutri" + s + "
tion", + "Fo" + s + "\u0085od", s + "😀Food", s + "\uD83DFood", "12% Food", + s + "712" + s + "7% Food", "+2 Food", "Food " + s + "+", "%" + s + "a Food", "Fo" + s + "aod +", + "", null, "plain"); + List> lores = new ArrayList<>(); + for (String nutrition : lines) { + for (String food : lines) { + lores.add(Arrays.asList(nutrition, food)); + } + } + lores.add(Arrays.asList((String) null)); + lores.add(lines); + int needed = 0; + for (List lore : lores) { + for (boolean index : new boolean[] {true, false}) { + boolean expected = oldNeedsLoreRebuild(lore, index); + assertEquals(expected, ItemUpdater.needsLoreRebuild(lore, index), String.valueOf(lore)); + if (expected) needed++; + } + } + // Both answers occur, so the comparison covers more than one outcome. + assertTrue(needed > 0); + assertTrue(needed < lores.size() * 2); + assertFalse(ItemUpdater.needsLoreRebuild(List.of(s + "aNutrition", s + "7Food"), true)); + // A colour sign before a line break is not a colour code, so the break stays. + assertTrue(ItemUpdater.needsLoreRebuild(List.of("Nutrition", "Fo" + s + "\nod"), true)); + } + @Test void heldSilentDoesNotWrite() { assertFalse(ItemUpdater.shouldWriteToSlot(true, false, true)); diff --git a/src/test/java/net/tfminecraft/cooking/utils/ItemRefTemplateCacheTest.java b/src/test/java/net/tfminecraft/cooking/utils/ItemRefTemplateCacheTest.java new file mode 100644 index 0000000..a162941 --- /dev/null +++ b/src/test/java/net/tfminecraft/cooking/utils/ItemRefTemplateCacheTest.java @@ -0,0 +1,233 @@ +package net.tfminecraft.cooking.utils; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; + +import java.util.List; + +import org.bukkit.Material; +import org.bukkit.NamespacedKey; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.persistence.PersistentDataType; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockbukkit.mockbukkit.MockBukkit; +import org.mockito.MockedStatic; + +import net.tfminecraft.tlibs.TLibs; +import net.tfminecraft.tlibs.objects.api.ItemAPI; +import net.tfminecraft.tlibs.objects.api.subapi.ItemCreator; +import net.tfminecraft.tlibs.objects.api.subapi.ItemPathHandler; + +/** ItemsAdder templates are built once per ref and every caller gets its own copy. */ +class ItemRefTemplateCacheTest { + private static final String SALT = "ia.tfmc_cooking:salt"; + private static final NamespacedKey MARK = new NamespacedKey("cookingtest", "mark"); + + private MockedStatic tlibs; + private ItemAPI api; + private ItemCreator creator; + + @BeforeEach + void setUp() { + MockBukkit.mock(); + api = mock(ItemAPI.class); + creator = mock(ItemCreator.class); + when(api.getCreator()).thenReturn(creator); + tlibs = mockStatic(TLibs.class); + tlibs.when(TLibs::getItemAPI).thenReturn(api); + when(creator.getItemFromPath(anyString())).thenAnswer(call -> named(Material.PAPER, call.getArgument(0))); + ItemRef.setCaching(true); + } + + @AfterEach + void tearDown() { + ItemRef.setCaching(false); + tlibs.close(); + MockBukkit.unmock(); + } + + @SuppressWarnings("deprecation") + private static ItemStack named(Material material, String name) { + ItemStack stack = new ItemStack(material); + ItemMeta meta = stack.getItemMeta(); + meta.setDisplayName(name); + stack.setItemMeta(meta); + return stack; + } + + @SuppressWarnings("deprecation") + private static String name(ItemStack stack) { + return stack.getItemMeta().getDisplayName(); + } + + @Test + void anItemsAdderRefIsBuiltOnceAndEveryResolveIsAnIndependentCopy() { + ItemStack first = ItemRef.resolve(SALT); + ItemStack second = ItemRef.resolve(SALT); + ItemStack third = ItemRef.resolve(SALT); + + verify(creator, times(1)).getItemFromPath(SALT); + assertEquals(first, second); + assertEquals(second, third); + assertNotSame(first, second); + assertNotSame(second, third); + + // Whatever a caller does to its copy, later copies come out as built. + for (ItemStack used : List.of(first, second)) { + used.setAmount(12); + ItemMeta meta = used.getItemMeta(); + meta.getPersistentDataContainer().set(MARK, PersistentDataType.STRING, "changed"); + used.setItemMeta(meta); + used.setType(Material.STONE); + } + ItemStack fresh = ItemRef.resolve(SALT); + assertEquals(Material.PAPER, fresh.getType()); + assertEquals(1, fresh.getAmount()); + assertEquals(SALT, name(fresh)); + assertFalse(fresh.getItemMeta().getPersistentDataContainer().has(MARK)); + assertEquals(third, fresh); + verify(creator, times(1)).getItemFromPath(SALT); + } + + @Test + @SuppressWarnings("deprecation") + void applySharesTheTemplateButMergesIntoItsOwnCopy() { + ItemStack source = named(Material.APPLE, "Salted Apple"); + source.setAmount(5); + ItemMeta meta = source.getItemMeta(); + meta.setLore(List.of("Nutrition 1", "Food 2")); + meta.getPersistentDataContainer().set(MARK, PersistentDataType.STRING, "food"); + source.setItemMeta(meta); + + ItemStack first = ItemRef.apply(SALT, source); + ItemStack second = ItemRef.apply(SALT, source); + ItemStack plain = ItemRef.resolve(SALT); + + verify(creator, times(1)).getItemFromPath(SALT); + assertNotSame(first, second); + for (ItemStack out : List.of(first, second)) { + assertEquals(Material.PAPER, out.getType()); + assertEquals(5, out.getAmount()); + assertEquals("Salted Apple", name(out)); + assertEquals(List.of("Nutrition 1", "Food 2"), out.getItemMeta().getLore()); + assertEquals("food", out.getItemMeta().getPersistentDataContainer().get(MARK, PersistentDataType.STRING)); + } + // The merge never reached the kept template. + assertEquals(1, plain.getAmount()); + assertEquals(SALT, name(plain)); + assertFalse(plain.getItemMeta().hasLore()); + assertFalse(plain.getItemMeta().getPersistentDataContainer().has(MARK)); + } + + @Test + void applyOfAnUnknownRefStillReturnsNothing() { + when(creator.getItemFromPath("ia.tfmc_cooking:missing")).thenReturn(null); + assertNull(ItemRef.apply("ia.tfmc_cooking:missing", new ItemStack(Material.APPLE))); + assertNull(ItemRef.apply("ia.tfmc_cooking:missing", null)); + verify(creator, times(2)).getItemFromPath("ia.tfmc_cooking:missing"); + } + + @Test + void failedBuildsAreRetriedEveryTimeAndReturnedAsTLibsMadeThem() { + ItemStack air = new ItemStack(Material.AIR); + ItemStack dirt = new ItemStack(Material.DIRT); + when(creator.getItemFromPath("ia.tfmc_cooking:none")).thenReturn(null); + when(creator.getItemFromPath("ia.tfmc_cooking:air")).thenReturn(air); + // TLibs hands out dirt when ItemsAdder is missing or has not loaded the item yet. + when(creator.getItemFromPath("ia.tfmc_cooking:unloaded")).thenReturn(dirt); + + for (int i = 0; i < 3; i++) { + assertNull(ItemRef.resolve("ia.tfmc_cooking:none")); + assertSame(air, ItemRef.resolve("ia.tfmc_cooking:air")); + assertSame(dirt, ItemRef.resolve("ia.tfmc_cooking:unloaded")); + } + verify(creator, times(3)).getItemFromPath("ia.tfmc_cooking:none"); + verify(creator, times(3)).getItemFromPath("ia.tfmc_cooking:air"); + verify(creator, times(3)).getItemFromPath("ia.tfmc_cooking:unloaded"); + + // Once ItemsAdder has the item, it is kept from then on. + ItemStack loaded = named(Material.PAPER, "Loaded"); + when(creator.getItemFromPath("ia.tfmc_cooking:unloaded")).thenReturn(loaded); + assertSame(loaded, ItemRef.resolve("ia.tfmc_cooking:unloaded")); + assertEquals(loaded, ItemRef.resolve("ia.tfmc_cooking:unloaded")); + verify(creator, times(4)).getItemFromPath("ia.tfmc_cooking:unloaded"); + } + + @Test + void refsThatAreNotPlainItemsAdderBuildsAreNeverKept() { + ItemPathHandler cooking = mock(ItemPathHandler.class); + when(api.getPathHandler("c")).thenReturn(cooking); + List refs = java.util.Arrays.asList("v.paper", "V.paper", "m.material.SALT", "M.material.SALT", + "modeled.(type=paper;model=5)", "magic.staff", "c.bread", "C.bread", "fill_1", "ia.", ".salt", null); + for (String ref : refs) { + ItemStack built = named(Material.PAPER, String.valueOf(ref)); + when(creator.getItemFromPath(ref)).thenReturn(built); + assertSame(built, ItemRef.resolve(ref), ref); + assertSame(built, ItemRef.resolve(ref), ref); + verify(creator, times(2)).getItemFromPath(ref); + } + } + + @Test + void aPrefixThatGainsAPathHandlerIsNoLongerServedFromTheCache() { + ItemStack kept = ItemRef.resolve("x.thing"); + assertEquals(kept, ItemRef.resolve("x.thing")); + verify(creator, times(1)).getItemFromPath("x.thing"); + + when(api.getPathHandler("x")).thenReturn(mock(ItemPathHandler.class)); + ItemRef.resolve("x.thing"); + verify(creator, times(2)).getItemFromPath("x.thing"); + } + + @Test + void clearingTheCacheRebuildsEveryRef() { + ItemRef.resolve(SALT); + ItemRef.resolve(SALT); + verify(creator, times(1)).getItemFromPath(SALT); + + ItemRef.clearCache(); + ItemRef.resolve(SALT); + ItemRef.resolve(SALT); + verify(creator, times(2)).getItemFromPath(SALT); + } + + @Test + void turningCachingOffBuildsFreshEveryTimeAndDropsWhatWasKept() { + ItemRef.resolve(SALT); + ItemRef.setCaching(false); + ItemStack built = named(Material.PAPER, "Fresh"); + when(creator.getItemFromPath(SALT)).thenReturn(built); + assertSame(built, ItemRef.resolve(SALT)); + assertSame(built, ItemRef.resolve(SALT)); + verify(creator, times(3)).getItemFromPath(SALT); + + ItemRef.setCaching(true); + assertSame(built, ItemRef.resolve(SALT)); + assertNotSame(built, ItemRef.resolve(SALT)); + verify(creator, times(4)).getItemFromPath(SALT); + } + + @Test + void aNewTLibsCreatorStartsAnEmptyCache() { + ItemRef.resolve(SALT); + ItemCreator replacement = mock(ItemCreator.class); + when(replacement.getItemFromPath(SALT)).thenReturn(named(Material.PAPER, "New")); + when(api.getCreator()).thenReturn(replacement); + + assertEquals("New", name(ItemRef.resolve(SALT))); + assertEquals("New", name(ItemRef.resolve(SALT))); + verify(replacement, times(1)).getItemFromPath(SALT); + verify(creator, times(1)).getItemFromPath(SALT); + } + + @Test + void aMissingCreatorFailsLikeBefore() { + when(api.getCreator()).thenReturn(null); + assertThrows(NullPointerException.class, () -> ItemRef.resolve(SALT)); + assertThrows(NullPointerException.class, () -> ItemRef.apply(SALT, null)); + } +} diff --git a/src/test/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHookTest.java b/src/test/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHookTest.java new file mode 100644 index 0000000..a6ca229 --- /dev/null +++ b/src/test/java/net/tfminecraft/cooking/utils/ItemsAdderReloadHookTest.java @@ -0,0 +1,99 @@ +package net.tfminecraft.cooking.utils; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; + +import org.bukkit.Material; +import org.bukkit.event.HandlerList; +import org.bukkit.inventory.ItemStack; +import org.bukkit.plugin.Plugin; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockbukkit.mockbukkit.MockBukkit; +import org.mockbukkit.mockbukkit.ServerMock; +import org.mockito.MockedStatic; + +import dev.lone.itemsadder.api.Events.ItemsAdderLoadDataEvent; +import net.tfminecraft.tlibs.TLibs; +import net.tfminecraft.tlibs.objects.api.ItemAPI; +import net.tfminecraft.tlibs.objects.api.subapi.ItemCreator; + +/** ItemsAdder (re)loading its items drops Cooking's kept ItemsAdder templates. */ +class ItemsAdderReloadHookTest { + private static final String SALT = "ia.tfmc_cooking:salt"; + + private ServerMock server; + private MockedStatic tlibs; + private ItemCreator creator; + private Plugin cooking; + + @BeforeEach + void setUp() { + server = MockBukkit.mock(); + ItemAPI api = mock(ItemAPI.class); + creator = mock(ItemCreator.class); + when(api.getCreator()).thenReturn(creator); + when(creator.getItemFromPath(anyString())).thenAnswer(call -> new ItemStack(Material.PAPER)); + tlibs = mockStatic(TLibs.class); + tlibs.when(TLibs::getItemAPI).thenReturn(api); + ItemRef.setCaching(false); + cooking = MockBukkit.createMockPlugin("Cooking"); + } + + @AfterEach + void tearDown() { + ItemRef.setCaching(false); + HandlerList.unregisterAll(cooking); + tlibs.close(); + MockBukkit.unmock(); + } + + /** An event Bukkit refuses to register: it has no static getHandlerList. */ + public static class NoHandlerList extends org.bukkit.event.Event { + @Override + public HandlerList getHandlers() { + return new HandlerList(); + } + } + + private void resolveTwice() { + ItemRef.resolve(SALT); + ItemRef.resolve(SALT); + } + + @Test + void withoutItemsAdderNothingIsRegisteredAndTemplatesStayOff() { + assertFalse(ItemsAdderReloadHook.registerIfPresent(cooking)); + resolveTwice(); + verify(creator, times(2)).getItemFromPath(SALT); + } + + @Test + void everyItemsAdderLoadDropsTheKeptTemplates() { + MockBukkit.createMockPlugin("ItemsAdder"); + assertTrue(ItemsAdderReloadHook.registerIfPresent(cooking)); + + resolveTwice(); + verify(creator, times(1)).getItemFromPath(SALT); + + server.getPluginManager().callEvent(new ItemsAdderLoadDataEvent()); + resolveTwice(); + verify(creator, times(2)).getItemFromPath(SALT); + + server.getPluginManager().callEvent(new ItemsAdderLoadDataEvent()); + resolveTwice(); + verify(creator, times(3)).getItemFromPath(SALT); + } + + @Test + void anItemsAdderWithoutTheLoadEventLeavesTemplatesOff() { + MockBukkit.createMockPlugin("ItemsAdder"); + assertFalse(ItemsAdderReloadHook.registerIfPresent(cooking, "dev.lone.itemsadder.api.Events.Missing")); + assertFalse(ItemsAdderReloadHook.registerIfPresent(cooking, String.class.getName())); + assertFalse(ItemsAdderReloadHook.registerIfPresent(cooking, NoHandlerList.class.getName())); + resolveTwice(); + verify(creator, times(2)).getItemFromPath(SALT); + } +}