fix: stop food and pot duplication

pom.xml

@@ -18,7 +18,32 @@
         <tlibs.version>2.1.0</tlibs.version>
     </properties>
 
+    <dependencyManagement>
+        <dependencies>
+            <!-- Keep MockBukkit's JUnit in line with the one these tests use. -->
+            <dependency>
+                <groupId>org.junit</groupId>
+                <artifactId>junit-bom</artifactId>
+                <version>5.10.2</version>
+                <type>pom</type>
+                <scope>import</scope>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+
     <dependencies>
+        <dependency>
+            <groupId>org.mockbukkit.mockbukkit</groupId>
+            <artifactId>mockbukkit-v1.21</artifactId>
+            <version>4.95.0</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <version>5.20.0</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>local</groupId>
             <artifactId>gson</artifactId>

src/main/java/net/tfminecraft/cooking/cooking/FryingReference.java

@@ -106,7 +106,8 @@ public void interact(FurnitureInteractEvent e) {
         ItemStack converted = IngredientConverter.convertIfNeeded(p, item);
         if (converted != item) {
             p.getInventory().setItemInMainHand(converted);
-            item = converted;
+            // setItemInMainHand stores a copy, so take from the hand itself or nothing is used up.
+            item = p.getInventory().getItemInMainHand();
         }
 
         FoodItem butterFi = FoodItem.fromItem(item);

src/main/java/net/tfminecraft/cooking/cup/DrinkConsumeListener.java

@@ -1,5 +1,7 @@
 package net.tfminecraft.cooking.cup;
 
+import java.util.function.Supplier;
+
 import org.bukkit.Bukkit;
 import org.bukkit.GameMode;
 import org.bukkit.Material;
@@ -66,24 +68,35 @@ public void onDrink(PlayerItemConsumeEvent event) {
         }
 
         int amountBefore = consumed.getAmount();
-        Bukkit.getScheduler().runTask(Cooking.plugin, () -> replaceWithEmptyCup(player, amountBefore));
+        EquipmentSlot hand = event.getHand() == EquipmentSlot.OFF_HAND ? EquipmentSlot.OFF_HAND : EquipmentSlot.HAND;
+        int slot = hand == EquipmentSlot.OFF_HAND ? -1 : player.getInventory().getHeldItemSlot();
+        Bukkit.getScheduler().runTask(Cooking.plugin, () -> replaceWithEmptyCup(player, amountBefore, slot));
+    }
+
+    /**
+     * Swaps the glass bottle vanilla leaves in the slot the cup was drunk from for an empty cup.
+     * Only that slot, and only if it still holds the bottle: switching slots within the tick
+     * must neither leave the bottle and add a cup, nor overwrite whatever is held instead. Cups
+     * are potions, so an empty slot means the bottle was moved away, or nothing was used up.
+     */
+    static void replaceWithEmptyCup(Player player, int amountBefore, int slot) {
+        replaceWithEmptyCup(player, amountBefore, slot, CupItems::emptyCup);
     }
 
-    private static void replaceWithEmptyCup(Player player, int amountBefore) {
+    static void replaceWithEmptyCup(Player player, int amountBefore, int slot, Supplier<ItemStack> emptyCup) {
         if (amountBefore > 1) {
             return;
         }
-
-        ItemStack empty = CupItems.emptyCup();
-        ItemStack main = player.getInventory().getItemInMainHand();
-        ItemStack off = player.getInventory().getItemInOffHand();
-
-        if (main.getType() == Material.GLASS_BOTTLE || main.getType() == Material.AIR) {
-            player.getInventory().setItemInMainHand(empty);
-        } else if (off.getType() == Material.GLASS_BOTTLE) {
-            player.getInventory().setItemInOffHand(empty);
-        } else if (!ItemCache.isCupOfWater(main) && !ItemCache.isCupOfMilk(main)) {
-            player.getInventory().setItemInMainHand(empty);
+        PlayerInventory inv = player.getInventory();
+        ItemStack left = slot < 0 ? inv.getItemInOffHand() : inv.getItem(slot);
+        if (left == null || left.getType() != Material.GLASS_BOTTLE) {
+            return;
+        }
+        ItemStack empty = emptyCup.get();
+        if (slot < 0) {
+            inv.setItemInOffHand(empty);
+        } else {
+            inv.setItem(slot, empty);
         }
     }
 

src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java

@@ -23,6 +23,7 @@
 import net.tfminecraft.cooking.utils.FoodParser;
 import net.tfminecraft.cooking.utils.InventoryAdder;
 import net.tfminecraft.cooking.utils.ItemBuilder;
+import net.tfminecraft.cooking.utils.Keys;
 
 public class ConversionManager implements Listener {
 
@@ -132,7 +133,7 @@ public void onOpen(InventoryOpenEvent e) {
                 FoodItem fb = FoodItem.fromItem(b);
                 if (fb == null) continue;
 
-                if (InventoryAdder.equalsFood(fa, fb)) {
+                if (InventoryAdder.equalsFood(fa, fb) && sameApartFromAging(a, b)) {
                     ItemStack clone = b.clone();
                     clone.setAmount(a.getAmount());
                     setSlot(i, clone, top, bottom, topSize);
@@ -142,6 +143,32 @@ 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.
+     */
+    public static boolean sameApartFromAging(ItemStack a, ItemStack b) {
+        return withoutAging(a).isSimilar(withoutAging(b));
+    }
+
+    private static ItemStack withoutAging(ItemStack item) {
+        ItemStack copy = item.clone();
+        copy.setAmount(1);
+        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
+        // 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);
+        meta.lore(null);
+        copy.setItemMeta(meta);
+        return copy;
+    }
+
     private ItemStack getSlot(int index, Inventory top, Inventory bottom, int topSize) {
         return index < topSize ? top.getItem(index) : bottom.getItem(index - topSize);
     }

src/main/java/net/tfminecraft/cooking/manager/CookingManager.java

@@ -13,6 +13,7 @@
 import org.bukkit.event.block.Action;
 import org.bukkit.event.player.PlayerInteractEvent;
 import org.bukkit.event.world.ChunkLoadEvent;
+import org.bukkit.event.world.ChunkUnloadEvent;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.scheduler.BukkitRunnable;
 
@@ -27,6 +28,7 @@
 import net.tfminecraft.cooking.cooking.SauceReference;
 import net.tfminecraft.cooking.enums.Method;
 import net.tfminecraft.cooking.item.FoodItem;
+import net.tfminecraft.cooking.utils.StationChunks;
 import net.tfminecraft.interactiblefurniture.events.FurnitureBreakEvent;
 import net.tfminecraft.interactiblefurniture.events.FurnitureInteractEvent;
 import net.tfminecraft.interactiblefurniture.events.FurnitureSlotItemAddEvent;
@@ -76,6 +78,13 @@ private void resumeStationsInChunk(Chunk chunk) {
         }
     }
 
+    /** Drops references to furniture in an unloading chunk; loading it again builds fresh ones. */
+    @EventHandler
+    public void onChunkUnload(ChunkUnloadEvent event) {
+        Chunk chunk = event.getChunk();
+        stations.values().removeIf(ref -> StationChunks.isIn(ref.getFurniture(), chunk));
+    }
+
     @EventHandler
     public void onChunkLoad(ChunkLoadEvent event) {
         Chunk chunk = event.getChunk();
@@ -97,7 +106,9 @@ private CookingReference createReference(Furniture furniture, Method method) {
 
     private CookingReference getOrCreateReference(Furniture furniture) {
         CookingReference existing = stations.get(furniture.getEntityId());
-        if (existing != null) {
+        // A chunk reload replaces the furniture object under the same UUID. A reference still
+        // holding the old one would hand out items the live furniture also keeps, so rebuild it.
+        if (existing != null && existing.getFurniture() == furniture) {
             return existing;
         }
         Method method = FurnitureCache.getByFurniture(furniture);

src/main/java/net/tfminecraft/cooking/manager/CraftingManager.java

@@ -16,6 +16,7 @@
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.Listener;
 import org.bukkit.event.world.ChunkLoadEvent;
+import org.bukkit.event.world.ChunkUnloadEvent;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.scheduler.BukkitRunnable;
 import org.bukkit.util.Transformation;
@@ -34,6 +35,7 @@
 import net.tfminecraft.cooking.item.FoodItem;
 import net.tfminecraft.cooking.item.data.CookData;
 import net.tfminecraft.cooking.utils.ItemUpdater;
+import net.tfminecraft.cooking.utils.StationChunks;
 import net.tfminecraft.cooking.loader.CraftingStationLoader;
 import net.tfminecraft.interactiblefurniture.events.FurnitureBreakEvent;
 import net.tfminecraft.interactiblefurniture.events.FurnitureInteractEvent;
@@ -92,7 +94,8 @@ private boolean isCraftingStationFurniture(Furniture f) {
     public CraftingStation getOrCreateStation(Furniture f) {
         if (f.getType() == null) return null;
         CraftingStation existing = stations.get(f.getEntityId());
-        if (existing != null) return existing;
+        // A chunk reload replaces the furniture object under the same UUID; rebuild from the live one.
+        if (existing != null && existing.getFurniture() == f) return existing;
 
         for (CraftingStation template : CraftingStationLoader.get()) {
             if (template.getBlockId().equalsIgnoreCase(f.getType().getId())) {
@@ -105,6 +108,13 @@ public CraftingStation getOrCreateStation(Furniture f) {
         return null;
     }
 
+    /** Drops stations in an unloading chunk; loading it again builds fresh ones. */
+    @EventHandler
+    public void onChunkUnload(ChunkUnloadEvent event) {
+        Chunk chunk = event.getChunk();
+        stations.values().removeIf(station -> StationChunks.isIn(station.getFurniture(), chunk));
+    }
+
     @EventHandler
     public void onChunkLoad(ChunkLoadEvent event) {
         Chunk chunk = event.getChunk();

src/main/java/net/tfminecraft/cooking/mixing/MixingBowlHandler.java

@@ -319,7 +319,8 @@ private void handleMixinAdd(Furniture furniture, Player player, FurnitureInterac
         ItemStack converted = IngredientConverter.convertIfNeeded(player, hand);
         if (converted != hand) {
             player.getInventory().setItemInMainHand(converted);
-            hand = converted;
+            // setItemInMainHand stores a copy, so take from the hand itself or nothing is used up.
+            hand = player.getInventory().getItemInMainHand();
         }
 
         FoodItem foodItem = FoodItem.fromItem(hand);

src/main/java/net/tfminecraft/cooking/sausagemaker/SausageMakerHandler.java

@@ -136,10 +136,18 @@ private void startCrank(Furniture furniture, Player player) {
     }
 
     private void finishCrank(Furniture furniture, Player player) {
+        if (!player.isOnline()) {
+            return;
+        }
         List<FoodItem> meats = collectMeats(furniture);
         if (meats.size() < MEAT_SLOTS.length) {
             return;
         }
+        // The paper was checked when the crank started; it could have been put away since.
+        if (!hasPaper(player)) {
+            player.sendMessage("§cHold paper to casing.");
+            return;
+        }
 
         ItemStack chain = SausageItems.fromMeats(player, meats);
         if (chain == null) {

src/main/java/net/tfminecraft/cooking/utils/StationChunks.java

@@ -0,0 +1,27 @@
+package net.tfminecraft.cooking.utils;
+
+import org.bukkit.Chunk;
+import org.bukkit.Location;
+
+import net.tfminecraft.interactiblefurniture.furniture.Furniture;
+
+/** Chunk checks for cached stations that never load a chunk to answer. */
+public final class StationChunks {
+
+    private StationChunks() {
+    }
+
+    /** True when the furniture sits in this chunk. A carried piece counts where it is now. */
+    public static boolean isIn(Furniture furniture, Chunk chunk) {
+        if (furniture == null || chunk == null) return false;
+        Location loc = furniture.getLoc();
+        return isIn(loc, chunk);
+    }
+
+    static boolean isIn(Location loc, Chunk chunk) {
+        if (loc == null || loc.getWorld() == null || chunk.getWorld() == null) return false;
+        return loc.getWorld().getUID().equals(chunk.getWorld().getUID())
+                && (loc.getBlockX() >> 4) == chunk.getX()
+                && (loc.getBlockZ() >> 4) == chunk.getZ();
+    }
+}

src/test/java/net/tfminecraft/cooking/cup/EmptyCupReturnTest.java

@@ -0,0 +1,80 @@
+package net.tfminecraft.cooking.cup;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.bukkit.Material;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.PlayerInventory;
+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.entity.PlayerMock;
+
+/** A drunk cup leaves a glass bottle in its slot, which becomes an empty cup there and nowhere else. */
+class EmptyCupReturnTest {
+	private PlayerMock player;
+	private PlayerInventory inventory;
+
+	@BeforeEach
+	void setUp() {
+		player = MockBukkit.mock().addPlayer();
+		inventory = player.getInventory();
+	}
+
+	@AfterEach
+	void tearDown() {
+		MockBukkit.unmock();
+	}
+
+	private static ItemStack cup() {
+		return new ItemStack(Material.BOWL);
+	}
+
+	@Test
+	void theBottleInTheSlotDrunkFromBecomesACup() {
+		inventory.setItem(3, new ItemStack(Material.GLASS_BOTTLE));
+
+		DrinkConsumeListener.replaceWithEmptyCup(player, 1, 3, EmptyCupReturnTest::cup);
+
+		assertEquals(Material.BOWL, inventory.getItem(3).getType());
+	}
+
+	@Test
+	void switchingSlotsDoesNotOverwriteTheNewItem() {
+		inventory.setItem(3, new ItemStack(Material.GLASS_BOTTLE));
+		inventory.setItem(5, new ItemStack(Material.DIAMOND_SWORD));
+		inventory.setHeldItemSlot(5);
+
+		DrinkConsumeListener.replaceWithEmptyCup(player, 1, 3, EmptyCupReturnTest::cup);
+
+		assertEquals(Material.BOWL, inventory.getItem(3).getType());
+		assertEquals(Material.DIAMOND_SWORD, inventory.getItem(5).getType());
+	}
+
+	@Test
+	void aBottleMovedAwayGetsNoCupAsWell() {
+		inventory.setItem(3, new ItemStack(Material.BREAD));
+
+		DrinkConsumeListener.replaceWithEmptyCup(player, 1, 3, () -> fail("no cup should be made"));
+
+		assertEquals(Material.BREAD, inventory.getItem(3).getType());
+	}
+
+	@Test
+	void aSlotEmptiedOfItsBottleGetsNoCup() {
+		DrinkConsumeListener.replaceWithEmptyCup(player, 1, 3, () -> fail("no cup should be made"));
+
+		assertEquals(Material.AIR, inventory.getItem(3) == null ? Material.AIR : inventory.getItem(3).getType());
+	}
+
+	@Test
+	void theOffHandIsUsedWhenTheCupWasDrunkFromIt() {
+		inventory.setItemInOffHand(new ItemStack(Material.GLASS_BOTTLE));
+
+		DrinkConsumeListener.replaceWithEmptyCup(player, 1, -1, EmptyCupReturnTest::cup);
+
+		assertEquals(Material.BOWL, inventory.getItemInOffHand().getType());
+	}
+}