From d64a1dc8883fb348da4b62572543a8e4b3d27945 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 17 Oct 2026 13:18:02 +0200 Subject: [PATCH 1/2] fix: stack CustomFishing fish and keep food stacking consistent Whole fish were built on a clone of the caught CustomFishing item, so each kept CustomFishing's own exact size, and Cooking's catch size also kept equal fish (and their filets) apart. Catch size never changes the yield. - Build whole fish on a fresh stack with only the catch's name and model, marked cooking:clean_catch; the item scan clears CustomFishing data from fish built before (not while held). - Catch size and the marker no longer keep equal foods apart. - Opening a double chest merges aged foods like a single chest. - Adding food to a stack keeps the older copy's age. - Double-click gathers food that aged apart, in vanilla's slot order. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- .../cooking/fishing/LegacyFishScan.java | 30 ++- .../cooking/fishing/SeafoodWholeItems.java | 53 ++++- .../cooking/manager/ConversionManager.java | 68 +++++- .../cooking/manager/TagManager.java | 2 +- .../cooking/utils/InventoryAdder.java | 3 +- .../net/tfminecraft/cooking/utils/Keys.java | 4 + .../fishing/FishingRuntimeCoverageTest.java | 11 +- .../fishing/SeafoodWholeStackingTest.java | 219 ++++++++++++++++++ .../cooking/manager/ChestOpenAgingTest.java | 11 + .../manager/FoodInventoryClickTest.java | 114 +++++++++ .../cooking/manager/FoodStackMergeTest.java | 18 +- .../utils/RuntimeUtilitiesCoverageTest.java | 7 +- 13 files changed, 526 insertions(+), 16 deletions(-) create mode 100644 src/test/java/net/tfminecraft/cooking/fishing/SeafoodWholeStackingTest.java diff --git a/README.md b/README.md index 38963d7..6c97d3d 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Cooking makes food an interactive part of everyday life. Players grow and gather - **Serving and sharing** — plate dishes, ladle soup into bowls, and carve foods into portions for the table. - **Quality and freshness** — ingredient quality and composition influence finished dishes, while aging and food tags carry preparation history through the kitchen. - **Farming and husbandry** — crop fertility and harvest quality sit alongside animal care, breeding, genetics, and produce collection. -- **Fish and seafood** — turn supported catches into cooking ingredients while preserving catch size. +- **Fish and seafood** — turn supported catches into cooking ingredients that keep their catch size; fish of one kind and quality stack whatever their size. - **Character nutrition** — meals feed a character's food reserve and influence diet quality, with variety rewarding a broader selection of ingredients. - **MMOItems crafting stations** — the `cooking{item="seafood(type=seafood_whole;origin=Salmon)",amount=64,display="Salmon"}` ingredient takes Cooking food, plus the plain items Cooking converts on pickup (a fresh catch counts as the salmon it becomes). `item` is a `c.` path without the prefix and must name a food `type`; quality and tags filters work too. Cooking claims those items before MMOItems' `vanilla{}` type, so use `cooking{}` for salmon, wheat, sugar, beef and the other converted foods. Cancelling a queued craft refunds the least the line accepts: its lowest quality, aged out unless the line pins that tag. diff --git a/src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java b/src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java index 2c933c1..b5efd01 100644 --- a/src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java +++ b/src/main/java/net/tfminecraft/cooking/fishing/LegacyFishScan.java @@ -1,20 +1,27 @@ package net.tfminecraft.cooking.fishing; import org.bukkit.Material; +import org.bukkit.Tag; 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 net.tfminecraft.cooking.item.FoodItem; +import net.tfminecraft.cooking.manager.TagManager; +import net.tfminecraft.cooking.utils.Keys; import net.tfminecraft.tlibs.itemscan.ItemScanHandler; +/** Converts leftover CustomFishing fish, and clears CustomFishing data that older versions left on whole fish. */ public final class LegacyFishScan implements ItemScanHandler { @Override public boolean matches(ItemStack stack) { - return stack != null - && stack.getType() == Material.COD - && FoodItem.fromItem(stack) == null; + if (stack == null || !Tag.ITEMS_FISHES.isTagged(stack.getType())) { + return false; + } + PersistentDataContainer data = stack.getItemMeta().getPersistentDataContainer(); + return isFood(data) ? SeafoodWholeItems.carriesCatchData(data) : stack.getType() == Material.COD; } @Override @@ -22,9 +29,22 @@ public void update(Player player, Inventory inventory, int slot, ItemStack stack if (inventory == null || slot < 0) { return; } - ItemStack built = LegacyFishConversion.convert(player, stack); + ItemStack built; + if (!isFood(stack.getItemMeta().getPersistentDataContainer())) { + built = LegacyFishConversion.convert(player, stack); + } else if (TagManager.isHeldSlot(player, inventory, slot)) { + // Like TagManager, leave a held fish alone so eating or using it isn't interrupted. + return; + } else { + // Earlier handlers in this scan (TagManager) may have rewritten the slot, so clean what is there now. + built = SeafoodWholeItems.withoutCatchData(inventory.getItem(slot)); + } if (built != null) { inventory.setItem(slot, built); } } + + private static boolean isFood(PersistentDataContainer 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 861b9cb..a67b534 100644 --- a/src/main/java/net/tfminecraft/cooking/fishing/SeafoodWholeItems.java +++ b/src/main/java/net/tfminecraft/cooking/fishing/SeafoodWholeItems.java @@ -1,11 +1,15 @@ package net.tfminecraft.cooking.fishing; import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.persistence.PersistentDataContainer; +import org.bukkit.persistence.PersistentDataType; import net.tfminecraft.cooking.item.FoodItem; import net.tfminecraft.cooking.item.IngredientLineage; import net.tfminecraft.cooking.loader.FoodLoader; import net.tfminecraft.cooking.utils.ItemBuilder; +import net.tfminecraft.cooking.utils.Keys; public final class SeafoodWholeItems { private SeafoodWholeItems() {} @@ -46,11 +50,58 @@ public static ItemStack build(ItemStack caught, VanillaFish fish, int quality) { 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) { + 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); + } + + /** The whole fish with its Cooking data and look, but nothing else left over from the catch. */ + static ItemStack withoutCatchData(ItemStack whole) { + // Lore as components: ItemRef.mergeMeta's legacy text round trip can lose formatting. + ItemStack clean = lookOf(whole); + ItemMeta from = whole.getItemMeta(); + ItemMeta to = clean.getItemMeta(); + to.lore(from.lore()); + from.getPersistentDataContainer().copyTo(to.getPersistentDataContainer(), true); + to.getPersistentDataContainer().set(Keys.CLEAN_CATCH, PersistentDataType.BYTE, (byte) 1); + clean.setItemMeta(to); + return clean; + } + + /** + * The caught item's name and model on a fresh stack. CustomFishing tags every catch with its own + * exact size, so a whole fish built on the caught item itself never stacks with another. + */ + static ItemStack lookOf(ItemStack caught) { + ItemStack look = new ItemStack(caught.getType(), caught.getAmount()); + if (caught.hasItemMeta()) look.setItemMeta(copyLook(caught.getItemMeta(), look.getItemMeta())); + return look; + } + + static ItemMeta copyLook(ItemMeta from, ItemMeta to) { + if (from.hasDisplayName()) to.displayName(from.displayName()); + if (from.hasItemName()) to.itemName(from.itemName()); + if (from.hasCustomModelDataComponent()) to.setCustomModelDataComponent(from.getCustomModelDataComponent()); + if (from.hasItemModel()) to.setItemModel(from.getItemModel()); + return to; + } + private static FoodItem templateOrNull() { return FoodLoader.getByString(CustomFishingCatalog.WHOLE_TYPE); } private static ItemStack build(ItemStack caught, FoodItem item, int quality) { - return ItemBuilder.buildSingleWithQuality(item, caught, quality); + ItemStack whole = ItemBuilder.buildSingleWithQuality(item, lookOf(caught), quality); + if (whole != null) { + ItemMeta meta = whole.getItemMeta(); + meta.getPersistentDataContainer().set(Keys.CLEAN_CATCH, PersistentDataType.BYTE, (byte) 1); + whole.setItemMeta(meta); + } + return whole; } } diff --git a/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java b/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java index 68b4d44..5d86762 100644 --- a/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java +++ b/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java @@ -55,6 +55,10 @@ public void onClick(InventoryClickEvent event) { shiftFood(event, top, clicked); return; } + if (event.getAction() == InventoryAction.COLLECT_TO_CURSOR) { + collectFood(event, top); + return; + } if (event.getClick() != ClickType.LEFT && event.getClick() != ClickType.RIGHT) return; ItemStack cursor = event.getCursor(); ItemStack current = event.getCurrentItem(); @@ -127,6 +131,58 @@ private static void shiftFood(InventoryClickEvent event, Inventory top, Inventor event.setCurrentItem(remainder(item, remaining)); } + private record Slot(Inventory inventory, int index) { + ItemStack item() { + return inventory.getItem(index); + } + } + + /** + * Double-click gathers only exact copies, so food that aged apart would stay behind. When some + * did, gather it the way vanilla does: the container or crafting grid first, then storage, the + * hotbar and the off-hand, part stacks before full ones. Otherwise vanilla gathers as usual. + */ + private static void collectFood(InventoryClickEvent event, Inventory top) { + ItemStack cursor = event.getCursor(); + if (FoodItem.fromItem(cursor) == null) return; + Inventory bottom = event.getView().getBottomInventory(); + boolean crafting = top.getType() == InventoryType.CRAFTING; + List order = new ArrayList<>(); + // The crafting result (slot 0) is never gathered. + for (int i = crafting ? 1 : 0; i < top.getSize(); i++) order.add(new Slot(top, i)); + // The player view then lists the armour, head to feet. + if (crafting) for (int i = 39; i >= 36; i--) order.add(new Slot(bottom, i)); + for (int i = 9; i < 36; i++) order.add(new Slot(bottom, i)); + for (int i = 0; i < 9; i++) order.add(new Slot(bottom, i)); + if (crafting) order.add(new Slot(bottom, 40)); + + List food = new ArrayList<>(); + boolean agedApart = false; + for (Slot slot : order) { + ItemStack item = slot.item(); + if (!stackableFood(cursor, item)) continue; + food.add(slot); + agedApart |= !item.isSimilar(cursor); + } + if (!agedApart) return; + event.setCancelled(true); + + ItemStack gathered = cursor; + int limit = Math.min(cursor.getMaxStackSize(), bottom.getMaxStackSize()); + for (boolean fullStacks : new boolean[] {false, true}) { + for (Slot slot : food) { + ItemStack item = slot.item(); + int room = limit - gathered.getAmount(); + // A slot emptied by the first pass is null in the second. + if (room <= 0 || item == null || (item.getAmount() >= item.getMaxStackSize()) != fullStacks) continue; + int amount = Math.min(room, item.getAmount()); + gathered = mergedFood(gathered, item, amount); + slot.inventory().setItem(slot.index(), remainder(item, item.getAmount() - amount)); + } + } + event.getView().setCursor(gathered); + } + private static boolean stackableFood(ItemStack a, ItemStack b) { if (a == null || b == null || a.isEmpty() || b.isEmpty() || a.getType() != b.getType()) return false; FoodItem foodA = FoodItem.fromItem(a); @@ -137,7 +193,8 @@ private static boolean stackableFood(ItemStack a, ItemStack b) { return InventoryAdder.equalsFood(foodA, foodB) && sameApartFromAging(a, b); } - private static ItemStack mergedFood(ItemStack current, ItemStack incoming, int amount) { + /** current with amount more of incoming, keeping the more aged copy's data. */ + public static ItemStack mergedFood(ItemStack current, ItemStack incoming, int amount) { ItemStack merged = BundleFoodStacker.moreAged(current, incoming).clone(); merged.setAmount(current.getAmount() + amount); return merged; @@ -278,6 +335,7 @@ public void onOpen(InventoryOpenEvent e) { InventoryHolder holder = top.getHolder(); if (holder != null && !(holder instanceof org.bukkit.block.BlockState) && + !(holder instanceof org.bukkit.block.DoubleChest) && !(holder instanceof org.bukkit.entity.Entity) && !(holder instanceof Player)) { return; @@ -323,7 +381,8 @@ public void onOpen(InventoryOpenEvent e) { /** * True when two food stacks differ only in how far they have aged, so one can be copied over * the other to let them stack. equalsFood matches by kind and tag step alone, so without this - * a carved roast, a sausage chain or a big catch would be replaced by a fresh copy of another. + * a carved roast or a sausage chain would be replaced by a fresh copy of another. Catch size + * changes nothing once a fish is caught, so fish of one kind and quality stack whatever their size. */ public static boolean sameApartFromAging(ItemStack a, ItemStack b) { return withoutAging(a).isSimilar(withoutAging(b)); @@ -335,12 +394,15 @@ private static ItemStack withoutAging(ItemStack item) { ItemMeta meta = copy.getItemMeta(); if (meta == null) return copy; var pdc = meta.getPersistentDataContainer(); - // The clock, the progress within each tag step, and the lore written from them. Food lore + // The clock, the progress within each tag step, catch size, the clean-catch marker (fish + // built from plain vanilla fish never got it), and the lore written from them. Food lore // is always rebuilt whole from the item's data (ItemBuilder.stamp), which is compared. pdc.remove(Keys.LAST_UPDATE); pdc.remove(Keys.AGE_REMAINDER); pdc.remove(Keys.TAGS); pdc.remove(Keys.LORE_INDEX_MAP); + pdc.remove(Keys.CATCH_SIZE_CM); + pdc.remove(Keys.CLEAN_CATCH); meta.lore(null); copy.setItemMeta(meta); return copy; diff --git a/src/main/java/net/tfminecraft/cooking/manager/TagManager.java b/src/main/java/net/tfminecraft/cooking/manager/TagManager.java index 8f2248d..5010078 100644 --- a/src/main/java/net/tfminecraft/cooking/manager/TagManager.java +++ b/src/main/java/net/tfminecraft/cooking/manager/TagManager.java @@ -49,7 +49,7 @@ public void update(Player player, Inventory inventory, int slot, ItemStack stack } } - static boolean isHeldSlot(Player player, Inventory inventory, int slot) { + public static boolean isHeldSlot(Player player, Inventory inventory, int slot) { if (player == null || !(inventory instanceof PlayerInventory playerInventory)) { return false; } diff --git a/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java b/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java index 8bbec76..3a956f3 100644 --- a/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java +++ b/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java @@ -26,7 +26,8 @@ public static ItemStack addItem(Player p, ItemStack in) { if (space <= 0) continue; int add = Math.min(space, amount); - cur.setAmount(cur.getAmount() + add); + // Keep the older copy's data, so adding food never makes the stack fresher. + inv.setItem(slot, ConversionManager.mergedFood(cur, in, add)); amount -= add; if (amount <= 0) return null; diff --git a/src/main/java/net/tfminecraft/cooking/utils/Keys.java b/src/main/java/net/tfminecraft/cooking/utils/Keys.java index ca5fc93..538b11b 100644 --- a/src/main/java/net/tfminecraft/cooking/utils/Keys.java +++ b/src/main/java/net/tfminecraft/cooking/utils/Keys.java @@ -76,4 +76,8 @@ public class Keys { public static final NamespacedKey CUSTOM_FISHING_ID = new NamespacedKey(Cooking.plugin, "custom_fishing_id"); + + /** On whole fish built without the caught item's CustomFishing data. */ + public static final NamespacedKey CLEAN_CATCH = + new NamespacedKey(Cooking.plugin, "clean_catch"); } diff --git a/src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java b/src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java index cea37ad..d6aa5e9 100644 --- a/src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java +++ b/src/test/java/net/tfminecraft/cooking/fishing/FishingRuntimeCoverageTest.java @@ -43,6 +43,7 @@ public class FishingRuntimeCoverageTest { PluginManager plugins; Plugin customFishing; MockedStatic bukkit; + MockedStatic wholeItems; boolean oldRegistered; CatchMapping tuna = new CatchMapping("tuna", "Tuna", "fish"); @@ -55,6 +56,9 @@ public class FishingRuntimeCoverageTest { Server server=mock(Server.class); when(server.getPluginManager()).thenReturn(plugins); when(Cooking.plugin.getServer()).thenReturn(server); when(Cooking.plugin.getLogger()).thenReturn(Logger.getLogger("fishing-test")); bukkit=mockStatic(Bukkit.class,CALLS_REAL_METHODS); bukkit.when(Bukkit::getPluginManager).thenReturn(plugins); + // MockBukkit has no custom model data or item model components, so keep the fresh look as it is. + wholeItems=mockStatic(SeafoodWholeItems.class,CALLS_REAL_METHODS); + wholeItems.when(()->SeafoodWholeItems.copyLook(any(),any())).thenAnswer(inv->inv.getArgument(1)); CustomFishingCatalog.replace(Map.of("tuna",tuna),Map.of("rod",new RodBand(3,3)),new RodBand(2,2),Map.of(),Map.of(),Map.of(17,"tuna"),45); QualityConfig.apply(1,5,null,null); PermissionEffectsConfig.apply(List.of(),List.of()); env.originQuality.when(()->net.tfminecraft.cooking.quality.OriginQualityResolver.applyPickupPermissions(any(),anyInt())) @@ -64,7 +68,7 @@ public class FishingRuntimeCoverageTest { .thenAnswer(inv->env.stack(inv.getArgument(0),Material.COD,1)); } @AfterEach void cleanup() throws Exception { - bukkit.close(); env.close(); + wholeItems.close(); bukkit.close(); env.close(); var field=CustomFishingBridge.class.getDeclaredField("registered"); field.setAccessible(true); field.setBoolean(null,oldRegistered); CustomFishingCatalog.load(new File("src/main/resources/custom-fishing.yml")); } @@ -144,6 +148,11 @@ public class FishingRuntimeCoverageTest { ItemStack converted=LegacyFishConversion.convert(env.player,external); assertEquals(7,converted.getAmount()); assertEquals(45,env.resolve(converted).getCatchSizeCm()); LegacyFishScan scan=new LegacyFishScan(); assertFalse(scan.matches(null)); assertFalse(scan.matches(new ItemStack(Material.SALMON))); + // The workshop resolves food by its own key; the scan reads the real food id. + ItemMeta convertedMeta=converted.getItemMeta(); + convertedMeta.getPersistentDataContainer().set(net.tfminecraft.cooking.utils.Keys.FOOD_ID, + org.bukkit.persistence.PersistentDataType.STRING,CustomFishingCatalog.WHOLE_TYPE); + converted.setItemMeta(convertedMeta); assertTrue(scan.matches(external)); assertFalse(scan.matches(converted)); scan.update(env.player,null,0,external); scan.update(env.player,env.player.getInventory(),-1,external); scan.update(env.player,env.player.getInventory(),4,external); assertEquals(7,env.player.getInventory().getItem(4).getAmount()); diff --git a/src/test/java/net/tfminecraft/cooking/fishing/SeafoodWholeStackingTest.java b/src/test/java/net/tfminecraft/cooking/fishing/SeafoodWholeStackingTest.java new file mode 100644 index 0000000..3836931 --- /dev/null +++ b/src/test/java/net/tfminecraft/cooking/fishing/SeafoodWholeStackingTest.java @@ -0,0 +1,219 @@ +package net.tfminecraft.cooking.fishing; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.*; + +import java.util.ArrayList; +import java.util.List; + +import net.kyori.adventure.text.Component; +import org.bukkit.Material; +import org.bukkit.configuration.MemoryConfiguration; +import org.bukkit.NamespacedKey; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.inventory.meta.components.CustomModelDataComponent; +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.mockbukkit.mockbukkit.ServerMock; +import org.mockbukkit.mockbukkit.entity.PlayerMock; + +import net.tfminecraft.cooking.Cooking; +import net.tfminecraft.cooking.item.FoodItem; +import net.tfminecraft.cooking.loader.FoodLoader; +import net.tfminecraft.cooking.utils.ItemBuilder; +import net.tfminecraft.cooking.utils.Keys; + +/** Whole fish carry no per-catch CustomFishing data, so equal catches can stack. */ +class SeafoodWholeStackingTest { + private Cooking previous; + private List foods; + private ServerMock server; + + @BeforeEach + void setUp() { + server = MockBukkit.mock(); + previous = Cooking.plugin; + Cooking.plugin = mock(Cooking.class); + when(Cooking.plugin.namespace()).thenReturn("cooking"); + when(Cooking.plugin.getName()).thenReturn("Cooking"); + foods = new ArrayList<>(FoodLoader.oList); + FoodLoader.oList.clear(); + MemoryConfiguration config = new MemoryConfiguration(); + config.set("name", "{inherit}"); + FoodLoader.oList.add(new FoodItem(CustomFishingCatalog.WHOLE_TYPE, config)); + } + + @AfterEach + void tearDown() { + FoodLoader.oList.clear(); + FoodLoader.oList.addAll(foods); + Cooking.plugin = previous; + MockBukkit.unmock(); + } + + /** A whole tuna as older versions built it: a CustomFishing id but no clean-catch marker. */ + private ItemStack whole() { + return whole(Material.COD); + } + + private ItemStack whole(Material material) { + FoodItem food = new FoodItem(FoodLoader.getByString(CustomFishingCatalog.WHOLE_TYPE)); + food.setCatchSizeCm(80); + food.setCustomFishingId("tfmc_tuna_fish"); + return ItemBuilder.stamp(new ItemStack(material), food, "Tuna"); + } + + private static boolean clean(ItemStack stack) { + return stack.getItemMeta().getPersistentDataContainer().has(Keys.CLEAN_CATCH, PersistentDataType.BYTE); + } + + @Test + void aPlainCatchKeepsItsTypeAndAmount() { + ItemStack caught = mock(ItemStack.class); + when(caught.getType()).thenReturn(Material.COD); + when(caught.getAmount()).thenReturn(3); + + ItemStack look = SeafoodWholeItems.lookOf(caught); + + assertEquals(Material.COD, look.getType()); + assertEquals(3, look.getAmount()); + } + + @Test + void aCatchWithDataOnlyHasItsLookCopied() { + ItemStack caught = mock(ItemStack.class); + ItemMeta from = mock(ItemMeta.class); + when(caught.getType()).thenReturn(Material.COD); + when(caught.getAmount()).thenReturn(1); + when(caught.hasItemMeta()).thenReturn(true); + when(caught.getItemMeta()).thenReturn(from); + + ItemStack look = SeafoodWholeItems.lookOf(caught); + + assertEquals(Material.COD, look.getType()); + verify(from).hasDisplayName(); + verify(from, never()).displayName(); + verify(from, never()).getPersistentDataContainer(); + } + + @Test + void theLookIsTheNameItemNameModelDataAndItemModel() { + ItemMeta from = mock(ItemMeta.class); + ItemMeta to = mock(ItemMeta.class); + Component name = Component.text("Tuna Fish"); + Component itemName = Component.text("Tuna"); + CustomModelDataComponent model = mock(CustomModelDataComponent.class); + NamespacedKey itemModel = NamespacedKey.minecraft("cod"); + when(from.hasDisplayName()).thenReturn(true); + when(from.displayName()).thenReturn(name); + when(from.hasItemName()).thenReturn(true); + when(from.itemName()).thenReturn(itemName); + when(from.hasCustomModelDataComponent()).thenReturn(true); + when(from.getCustomModelDataComponent()).thenReturn(model); + when(from.hasItemModel()).thenReturn(true); + when(from.getItemModel()).thenReturn(itemModel); + + assertSame(to, SeafoodWholeItems.copyLook(from, to)); + + verify(to).displayName(name); + verify(to).itemName(itemName); + verify(to).setCustomModelDataComponent(model); + verify(to).setItemModel(itemModel); + } + + @Test + void cleaningKeepsTheCookingDataAndLore() { + ItemStack source = whole(); + // MockBukkit has no custom model data or item model components; copyLook covers those. + ItemMeta meta = spy(source.getItemMeta()); + doReturn(false).when(meta).hasCustomModelDataComponent(); + doReturn(false).when(meta).hasItemModel(); + ItemStack fish = mock(ItemStack.class); + when(fish.getType()).thenReturn(Material.COD); + when(fish.getAmount()).thenReturn(2); + when(fish.hasItemMeta()).thenReturn(true); + when(fish.getItemMeta()).thenReturn(meta); + + ItemStack clean = SeafoodWholeItems.withoutCatchData(fish); + + assertEquals(2, clean.getAmount()); + assertEquals(meta.displayName(), clean.getItemMeta().displayName()); + assertEquals(meta.lore(), clean.getItemMeta().lore()); + var pdc = clean.getItemMeta().getPersistentDataContainer(); + assertEquals(CustomFishingCatalog.WHOLE_TYPE, pdc.get(Keys.FOOD_ID, PersistentDataType.STRING)); + assertEquals(80, pdc.get(Keys.CATCH_SIZE_CM, PersistentDataType.INTEGER)); + } + + @Test + void onlyWholeCustomFishingFishBuiltOnTheCatchNeedCleaning() { + ItemStack filet = new ItemStack(Material.COD); + ItemMeta meta = filet.getItemMeta(); + meta.getPersistentDataContainer().set(Keys.FOOD_ID, PersistentDataType.STRING, "seafood_fish_filet"); + filet.setItemMeta(meta); + FoodItem vanilla = new FoodItem(FoodLoader.getByString(CustomFishingCatalog.WHOLE_TYPE)); + ItemStack cod = ItemBuilder.stamp(new ItemStack(Material.COD), vanilla, "Cod"); + + assertFalse(SeafoodWholeItems.carriesCatchData(filet.getItemMeta().getPersistentDataContainer())); + assertFalse(SeafoodWholeItems.carriesCatchData(cod.getItemMeta().getPersistentDataContainer())); + assertTrue(SeafoodWholeItems.carriesCatchData(whole().getItemMeta().getPersistentDataContainer())); + } + + @Test + void newWholeFishAreMarkedClean() { + ItemStack built = whole(); + try (var builder = mockStatic(ItemBuilder.class)) { + builder.when(() -> ItemBuilder.buildSingleWithQuality(any(FoodItem.class), any(ItemStack.class), anyInt())) + .thenReturn(built, (ItemStack) null); + ItemStack caught = mock(ItemStack.class); + when(caught.getType()).thenReturn(Material.COD); + when(caught.getAmount()).thenReturn(1); + VanillaFish cod = new VanillaFish("COD", "Cod", "fish", 45); + + assertSame(built, SeafoodWholeItems.build(caught, cod, 2)); + assertNull(SeafoodWholeItems.build(caught, cod, 2)); + } + assertTrue(clean(built)); + assertFalse(SeafoodWholeItems.carriesCatchData(built.getItemMeta().getPersistentDataContainer())); + } + + @Test + void theScanCleansOldWholeFishOfAnyFishMaterial() { + PlayerMock player = server.addPlayer(); + ItemStack fish = whole(Material.SALMON); + fish.setAmount(3); + LegacyFishScan scan = new LegacyFishScan(); + assertFalse(scan.matches(new ItemStack(Material.STONE))); + assertTrue(scan.matches(fish)); + + try (var items = mockStatic(SeafoodWholeItems.class, CALLS_REAL_METHODS)) { + items.when(() -> SeafoodWholeItems.copyLook(any(), any())).thenAnswer(call -> call.getArgument(1)); + player.getInventory().setItem(4, fish); + scan.update(player, player.getInventory(), 4, fish); + } + + ItemStack cleaned = player.getInventory().getItem(4); + assertEquals(Material.SALMON, cleaned.getType()); + assertEquals(3, cleaned.getAmount()); + assertEquals(80, FoodItem.fromItem(cleaned).getCatchSizeCm()); + assertTrue(clean(cleaned)); + assertFalse(scan.matches(cleaned)); + } + + @Test + void aHeldFishIsCleanedOnlyOnceItIsPutAway() { + PlayerMock player = server.addPlayer(); + ItemStack fish = whole(); + int held = player.getInventory().getHeldItemSlot(); + player.getInventory().setItem(held, fish); + + new LegacyFishScan().update(player, player.getInventory(), held, fish); + + assertFalse(clean(player.getInventory().getItem(held))); + } +} diff --git a/src/test/java/net/tfminecraft/cooking/manager/ChestOpenAgingTest.java b/src/test/java/net/tfminecraft/cooking/manager/ChestOpenAgingTest.java index a7da53c..e74c8c5 100644 --- a/src/test/java/net/tfminecraft/cooking/manager/ChestOpenAgingTest.java +++ b/src/test/java/net/tfminecraft/cooking/manager/ChestOpenAgingTest.java @@ -133,4 +133,15 @@ void foodAtAnotherFreshnessStepKeepsItsOwnData() { assertEquals(0, FoodItem.fromItem(chest.getItem(0)).getTagTrack("freshness").getValue()); assertEquals(Material.STONE, chest.getItem(2).getType()); } + @Test + void aDoubleChestMergesLikeASingleOne() { + chest = server.createInventory(mock(org.bukkit.block.DoubleChest.class), 18); + chest.setItem(0, beef(2, 0, 0)); + chest.setItem(17, beef(1, 0, 100)); + + open(); + + assertEquals(now - 100_000L, clock(chest.getItem(0))); + assertEquals(now - 100_000L, clock(chest.getItem(17))); + } } diff --git a/src/test/java/net/tfminecraft/cooking/manager/FoodInventoryClickTest.java b/src/test/java/net/tfminecraft/cooking/manager/FoodInventoryClickTest.java index 9b7f811..2c13734 100644 --- a/src/test/java/net/tfminecraft/cooking/manager/FoodInventoryClickTest.java +++ b/src/test/java/net/tfminecraft/cooking/manager/FoodInventoryClickTest.java @@ -31,6 +31,7 @@ import net.tfminecraft.cooking.item.tag.TagTrack; import net.tfminecraft.cooking.loader.FoodLoader; import net.tfminecraft.cooking.loader.TrackLoader; +import net.tfminecraft.cooking.utils.InventoryAdder; import net.tfminecraft.cooking.utils.ItemBuilder; import net.tfminecraft.cooking.utils.Keys; @@ -303,4 +304,117 @@ void restrictedSlotsAreLeftToTheirInventory(InventoryType.SlotType type) { assertEquals(older, chest.getItem(0)); assertEquals(newer, player.getItemOnCursor()); } + @Test + void doubleClickGathersFoodThatAgedApartAndKeepsTheOlderAge() { + ItemStack older = beef(2, 100); + chest.setItem(0, older); + chest.setItem(1, new ItemStack(Material.STONE)); + chest.setItem(2, beef(2, 1_000_100)); + player.getInventory().setItem(9, beef(3, 10)); + player.getInventory().setItem(0, beef(1, 50)); + player.setItemOnCursor(beef(1, 5)); + + assertTrue(click(3, ClickType.DOUBLE_CLICK, InventoryAction.COLLECT_TO_CURSOR).isCancelled()); + + assertEquals(7, player.getItemOnCursor().getAmount()); + assertTrue(player.getItemOnCursor().isSimilar(older)); + assertNull(chest.getItem(0)); + assertNull(player.getInventory().getItem(9)); + assertNull(player.getInventory().getItem(0)); + assertEquals(Material.STONE, chest.getItem(1).getType()); + assertEquals(2, chest.getItem(2).getAmount()); + } + + @Test + void doubleClickTakesPartStacksBeforeFullOnesAndStopsAtAFullCursor() { + chest.setItem(0, beef(64, 100)); + player.getInventory().setItem(9, beef(10, 10)); + player.setItemOnCursor(beef(1, 5)); + + click(3, ClickType.DOUBLE_CLICK, InventoryAction.COLLECT_TO_CURSOR); + + assertEquals(64, player.getItemOnCursor().getAmount()); + assertNull(player.getInventory().getItem(9)); + assertEquals(11, chest.getItem(0).getAmount()); + } + + @Test + void doubleClickInThePlayerInventoryAlsoGathersArmourAndTheOffHand() { + player.closeInventory(); + player.getInventory().setItem(9, beef(2, 10)); + ItemStack oldest = beef(1, 200); + player.getInventory().setItemInOffHand(oldest); + player.getInventory().setHelmet(beef(1, 150)); + player.setItemOnCursor(beef(3, 100)); + + assertTrue(click(10, ClickType.DOUBLE_CLICK, InventoryAction.COLLECT_TO_CURSOR).isCancelled()); + + assertEquals(7, player.getItemOnCursor().getAmount()); + assertNull(player.getInventory().getHelmet()); + assertTrue(player.getItemOnCursor().isSimilar(oldest)); + assertNull(player.getInventory().getItem(9)); + assertTrue(player.getInventory().getItemInOffHand().isEmpty()); + } + + @Test + void doubleClickInThePlayerInventoryGathersTheCraftingGridButNotTheResult() { + player.closeInventory(); + Inventory grid = mock(Inventory.class); + when(grid.getSize()).thenReturn(5); + when(grid.getType()).thenReturn(InventoryType.CRAFTING); + when(grid.getHolder()).thenReturn(player); + ItemStack oldest = beef(2, 300); + when(grid.getItem(2)).thenReturn(oldest); + player.getInventory().setItem(9, beef(2, 10)); + player.setItemOnCursor(beef(1, 100)); + InventoryView view = new InventoryViewMock(player, grid, player.getInventory(), InventoryType.CRAFTING) {}; + InventoryClickEvent event = new InventoryClickEvent(view, + InventoryType.SlotType.CONTAINER, 10, ClickType.DOUBLE_CLICK, InventoryAction.COLLECT_TO_CURSOR); + + server.getPluginManager().callEvent(event); + + assertTrue(event.isCancelled()); + assertEquals(5, player.getItemOnCursor().getAmount()); + assertTrue(player.getItemOnCursor().isSimilar(oldest)); + verify(grid).setItem(2, null); + verify(grid, never()).getItem(0); + } + + @Test + void doubleClickOverExactCopiesIsLeftToVanilla() { + ItemStack cursor = beef(1, 10); + chest.setItem(0, cursor.clone()); + player.setItemOnCursor(cursor); + + assertFalse(click(3, ClickType.DOUBLE_CLICK, InventoryAction.COLLECT_TO_CURSOR).isCancelled()); + assertEquals(1, chest.getItem(0).getAmount()); + } + + @Test + void doubleClickWithoutFoodIsLeftToVanilla() { + player.setItemOnCursor(new ItemStack(Material.STONE)); + assertFalse(click(3, ClickType.DOUBLE_CLICK, InventoryAction.COLLECT_TO_CURSOR).isCancelled()); + } + + @Test + void addingOlderFoodToAFresherStackKeepsTheOlderAge() { + player.getInventory().setItem(0, beef(2, 10)); + ItemStack older = beef(3, 100); + + assertNull(InventoryAdder.addItem(player, older)); + + assertEquals(5, player.getInventory().getItem(0).getAmount()); + assertTrue(player.getInventory().getItem(0).isSimilar(older)); + } + + @Test + void addingFresherFoodKeepsTheStacksOlderAge() { + ItemStack older = beef(2, 100); + player.getInventory().setItem(0, older); + + assertNull(InventoryAdder.addItem(player, beef(3, 10))); + + assertEquals(5, player.getInventory().getItem(0).getAmount()); + assertTrue(player.getInventory().getItem(0).isSimilar(older)); + } } diff --git a/src/test/java/net/tfminecraft/cooking/manager/FoodStackMergeTest.java b/src/test/java/net/tfminecraft/cooking/manager/FoodStackMergeTest.java index d2aae9b..214227e 100644 --- a/src/test/java/net/tfminecraft/cooking/manager/FoodStackMergeTest.java +++ b/src/test/java/net/tfminecraft/cooking/manager/FoodStackMergeTest.java @@ -89,7 +89,23 @@ void anyOtherDifferenceKeepsTheItemsApart() { assertFalse(ConversionManager.sameApartFromAging(plain, with(plain, m -> m.getPersistentDataContainer() .set(Keys.BASE_FOOD, PersistentDataType.DOUBLE, 12.0)))); assertFalse(ConversionManager.sameApartFromAging(plain, with(plain, m -> m.getPersistentDataContainer() - .set(Keys.CATCH_SIZE_CM, PersistentDataType.INTEGER, 90)))); + .set(Keys.SEAFOOD_CUT_TYPE, PersistentDataType.STRING, "fish")))); assertFalse(ConversionManager.sameApartFromAging(plain, new ItemStack(Material.COOKED_PORKCHOP))); } + @Test + void catchSizeDoesNotKeepEqualFishApart() { + ItemStack plain = roast(1, 1_000L, "cooked.3", "Fresh"); + + assertTrue(ConversionManager.sameApartFromAging( + with(plain, m -> m.getPersistentDataContainer().set(Keys.CATCH_SIZE_CM, PersistentDataType.INTEGER, 45)), + with(plain, m -> m.getPersistentDataContainer().set(Keys.CATCH_SIZE_CM, PersistentDataType.INTEGER, 90)))); + } + + @Test + void theCleanCatchMarkerDoesNotKeepOlderFishApart() { + ItemStack plain = roast(1, 1_000L, "cooked.3", "Fresh"); + + assertTrue(ConversionManager.sameApartFromAging(plain, + with(plain, m -> m.getPersistentDataContainer().set(Keys.CLEAN_CATCH, PersistentDataType.BYTE, (byte) 1)))); + } } diff --git a/src/test/java/net/tfminecraft/cooking/utils/RuntimeUtilitiesCoverageTest.java b/src/test/java/net/tfminecraft/cooking/utils/RuntimeUtilitiesCoverageTest.java index 6effcb7..41c718d 100644 --- a/src/test/java/net/tfminecraft/cooking/utils/RuntimeUtilitiesCoverageTest.java +++ b/src/test/java/net/tfminecraft/cooking/utils/RuntimeUtilitiesCoverageTest.java @@ -55,8 +55,11 @@ class RuntimeUtilitiesCoverageTest { when(inventory.getItem(2)).thenReturn(stack(food("other"),Material.BREAD,1)); when(inventory.getItem(4)).thenReturn(partial); ItemStack input=stack(bread,Material.BREAD,4); when(inventory.addItem(any(ItemStack.class))).thenAnswer(call->{ ItemStack remaining=call.getArgument(0); assertEquals(2,remaining.getAmount()); return new HashMap<>(Map.of(0,remaining)); }); - assertEquals(2,InventoryAdder.addItem(player,input).getAmount()); assertEquals(64,partial.getAmount()); assertEquals(4,input.getAmount()); - partial.setAmount(60); assertNull(InventoryAdder.addItem(player,input)); assertEquals(64,partial.getAmount()); + // The merged stack is written back to its slot. + assertEquals(2,InventoryAdder.addItem(player,input).getAmount()); assertEquals(4,input.getAmount()); + verify(inventory).setItem(eq(4),argThat(item->item.getAmount()==64)); + partial.setAmount(60); assertNull(InventoryAdder.addItem(player,input)); + verify(inventory,times(2)).setItem(eq(4),argThat(item->item.getAmount()==64)); when(inventory.addItem(any(ItemStack.class))).thenReturn(new HashMap<>()); assertNull(InventoryAdder.addItem(player,input)); ItemStack ordinary=new ItemStack(Material.STONE); assertNull(InventoryAdder.addItem(player,ordinary)); From a6663f8839b82e77516298efc7dc1eaaa07a5bb4 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:28:53 +0200 Subject: [PATCH 2/2] fix: gather aged-apart food on crafting-grid double-clicks Co-Authored-By: Claude Opus 5.5 --- .../tfminecraft/cooking/manager/ConversionManager.java | 8 ++++++-- .../cooking/manager/FoodInventoryClickTest.java | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java b/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java index 5d86762..9fe4443 100644 --- a/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java +++ b/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java @@ -46,8 +46,12 @@ public void onClick(InventoryClickEvent event) { || PlayerActionContext.capture(player) == null) return; Inventory top = event.getView().getTopInventory(); if (!stackingInventory(top)) return; - if (event.getSlotType() != InventoryType.SlotType.CONTAINER - && event.getSlotType() != InventoryType.SlotType.QUICKBAR) return; + InventoryType.SlotType slotType = event.getSlotType(); + // A double-click may start in the crafting grid; collectFood never takes the result. + boolean gridCollect = slotType == InventoryType.SlotType.CRAFTING + && event.getAction() == InventoryAction.COLLECT_TO_CURSOR; + if (slotType != InventoryType.SlotType.CONTAINER + && slotType != InventoryType.SlotType.QUICKBAR && !gridCollect) return; Inventory clicked = event.getClickedInventory(); if (clicked == null) return; diff --git a/src/test/java/net/tfminecraft/cooking/manager/FoodInventoryClickTest.java b/src/test/java/net/tfminecraft/cooking/manager/FoodInventoryClickTest.java index 2c13734..de4f997 100644 --- a/src/test/java/net/tfminecraft/cooking/manager/FoodInventoryClickTest.java +++ b/src/test/java/net/tfminecraft/cooking/manager/FoodInventoryClickTest.java @@ -368,8 +368,9 @@ void doubleClickInThePlayerInventoryGathersTheCraftingGridButNotTheResult() { player.getInventory().setItem(9, beef(2, 10)); player.setItemOnCursor(beef(1, 100)); InventoryView view = new InventoryViewMock(player, grid, player.getInventory(), InventoryType.CRAFTING) {}; + // Double-clicked in the grid itself (raw slot 1). InventoryClickEvent event = new InventoryClickEvent(view, - InventoryType.SlotType.CONTAINER, 10, ClickType.DOUBLE_CLICK, InventoryAction.COLLECT_TO_CURSOR); + InventoryType.SlotType.CRAFTING, 1, ClickType.DOUBLE_CLICK, InventoryAction.COLLECT_TO_CURSOR); server.getPluginManager().callEvent(event);