From c5d9e2355e2053127341477d182e3101e4e5e5ed Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:29:49 +0100 Subject: [PATCH 1/3] fix: stack eggs and other aging food in hoppers Aging food carries its own clock, so hoppers, which only stack exact copies, gave every egg its own slot. When a hopper moves food into a storage block or picks it up off the ground, and the slot vanilla would fill first holds an equal food that only aged apart, both now take the more aged copy's data so vanilla stacks them. Food never gets fresher, droppers and containers with slot rules are left alone, and non-food hopper traffic only costs a food-id lookup. Co-Authored-By: Claude Opus 5.5 --- .../java/net/tfminecraft/cooking/Cooking.java | 2 + .../cooking/manager/HopperFoodStacker.java | 100 ++++++ .../manager/HopperFoodStackerTest.java | 306 ++++++++++++++++++ 3 files changed, 408 insertions(+) create mode 100644 src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java create mode 100644 src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java diff --git a/src/main/java/net/tfminecraft/cooking/Cooking.java b/src/main/java/net/tfminecraft/cooking/Cooking.java index d1cb93c..7dc7fa4 100644 --- a/src/main/java/net/tfminecraft/cooking/Cooking.java +++ b/src/main/java/net/tfminecraft/cooking/Cooking.java @@ -57,6 +57,7 @@ import net.tfminecraft.cooking.manager.ConversionManager; import net.tfminecraft.cooking.manager.CookingManager; import net.tfminecraft.cooking.manager.CraftingManager; +import net.tfminecraft.cooking.manager.HopperFoodStacker; import net.tfminecraft.cooking.manager.PlateManager; import net.tfminecraft.cooking.manager.TagManager; import net.tfminecraft.cooking.baking.BakingTrayAging; @@ -394,6 +395,7 @@ public void registerListeners() { } getServer().getPluginManager().registerEvents(new ConversionManager(), this); getServer().getPluginManager().registerEvents(new BundleFoodStacker(), this); + getServer().getPluginManager().registerEvents(new HopperFoodStacker(), this); getServer().getPluginManager().registerEvents(new PopcornMachineHandler(), this); getServer().getPluginManager().registerEvents(new GlazeHandler(), this); getServer().getPluginManager().registerEvents(new RegenBlocker(), this); diff --git a/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java b/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java new file mode 100644 index 0000000..d83393e --- /dev/null +++ b/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java @@ -0,0 +1,100 @@ +package net.tfminecraft.cooking.manager; + +import org.bukkit.entity.Item; +import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; +import org.bukkit.event.Listener; +import org.bukkit.event.inventory.InventoryMoveItemEvent; +import org.bukkit.event.inventory.InventoryPickupItemEvent; +import org.bukkit.event.inventory.InventoryType; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.ItemStack; +import org.bukkit.persistence.PersistentDataType; + +import net.tfminecraft.cooking.item.FoodItem; +import net.tfminecraft.cooking.utils.InventoryAdder; +import net.tfminecraft.cooking.utils.Keys; + +/** + * Aging foods carry their own clock, so hoppers, which only stack exact copies, give every piece + * of the same food its own slot: eggs laid an hour apart fill a hopper five slots at a time. When a + * hopper moves food into a storage block, or picks it up off the ground, and the slot vanilla would + * fill first holds an equal food that only aged differently, both take the more aged copy's data so + * vanilla stacks them there. Food never gets fresher this way, and vanilla still picks the slot. + */ +public class HopperFoodStacker implements Listener { + + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) + public void onMove(InventoryMoveItemEvent event) { + // Hoppers take the item out of the source before this event and add whatever it carries + // after, so a changed copy moves cleanly. A dropper keeps the source item when it changes. + if (event.getInitiator().getType() != InventoryType.HOPPER) return; + Inventory destination = event.getDestination(); + if (!storage(destination)) return; + ItemStack stackable = stackable(destination, event.getItem()); + if (stackable != null) event.setItem(stackable); + } + + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) + public void onPickup(InventoryPickupItemEvent event) { + Inventory destination = event.getInventory(); + if (!storage(destination)) return; + // The hopper copies the ground item after this event, so it takes the changed data. + Item entity = event.getItem(); + ItemStack stackable = stackable(destination, entity.getItemStack()); + if (stackable != null) entity.setItemStack(stackable); + } + + /** Containers that take an item in any slot from any side, in slot order. */ + private static boolean storage(Inventory inventory) { + return switch (inventory.getType()) { + case CHEST, BARREL, HOPPER, SHULKER_BOX, DISPENSER, DROPPER -> true; + default -> false; + }; + } + + /** + * The moved food as it should enter destination to stack, or null when it can stay as it is. + * Vanilla fills the first slot that is empty or holds an exact copy with room. When the first + * slot it could fill instead holds an equal food that aged differently, the fresher side takes + * the older one's data: the slot is rewritten in place, or a copy of it is returned to move. + * If another plugin cancels the move after this, a rewritten slot has only aged to the moved + * food's age within the same freshness step; nothing is gained or lost. + */ + static ItemStack stackable(Inventory destination, ItemStack moved) { + // Most hopper traffic is not food; read the food id without copying the item's meta. + if (moved == null || moved.isEmpty() || !moved.hasItemMeta() + || !moved.getPersistentDataContainer().has(Keys.FOOD_ID, PersistentDataType.STRING)) { + return null; + } + FoodItem food = null; + for (int slot = 0; slot < destination.getSize(); slot++) { + ItemStack current = destination.getItem(slot); + if (current == null || current.isEmpty()) return null; + if (current.getType() != moved.getType()) continue; + int limit = Math.min(current.getMaxStackSize(), destination.getMaxStackSize()); + if (current.getAmount() >= limit) continue; + if (current.isSimilar(moved)) return null; + + if (food == null) { + food = FoodItem.fromItem(moved); + if (food == null) return null; + } + FoodItem other = FoodItem.fromItem(current); + if (other == null || !InventoryAdder.equalsFood(food, other) + || !ConversionManager.sameApartFromAging(moved, current)) continue; + + // Equal age keeps the stored stack, so only the moving item changes. + if (BundleFoodStacker.moreAged(current, moved) == current) { + ItemStack out = current.clone(); + out.setAmount(moved.getAmount()); + return out; + } + ItemStack older = moved.clone(); + older.setAmount(current.getAmount()); + destination.setItem(slot, older); + return null; + } + return null; + } +} diff --git a/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java b/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java new file mode 100644 index 0000000..c5d9fd1 --- /dev/null +++ b/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java @@ -0,0 +1,306 @@ +package net.tfminecraft.cooking.manager; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +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.List; + +import org.bukkit.Material; +import org.bukkit.configuration.MemoryConfiguration; +import org.bukkit.entity.Item; +import org.bukkit.event.inventory.InventoryMoveItemEvent; +import org.bukkit.event.inventory.InventoryPickupItemEvent; +import org.bukkit.event.inventory.InventoryType; +import org.bukkit.inventory.Inventory; +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.mockbukkit.mockbukkit.ServerMock; +import org.mockbukkit.mockbukkit.world.WorldMock; + +import net.tfminecraft.cooking.Cooking; +import net.tfminecraft.cooking.item.FoodItem; +import net.tfminecraft.cooking.item.tag.TagStep; +import net.tfminecraft.cooking.item.tag.TagTrack; +import net.tfminecraft.cooking.loader.FoodLoader; +import net.tfminecraft.cooking.loader.TrackLoader; +import net.tfminecraft.cooking.utils.ItemBuilder; +import net.tfminecraft.cooking.utils.Keys; + +/** Hoppers stack equal foods that only aged apart, keeping the more aged copy's data. */ +class HopperFoodStackerTest { + private Cooking previous; + private List foods; + private List tracks; + private ServerMock server; + private Inventory hopper; + private Inventory chest; + private long now; + + @BeforeEach + void setUp() { + server = MockBukkit.mock(); + previous = Cooking.plugin; + Cooking.plugin = mock(Cooking.class); + when(Cooking.plugin.getName()).thenReturn("Cooking"); + when(Cooking.plugin.namespace()).thenReturn("cooking"); + foods = new ArrayList<>(FoodLoader.oList); + FoodLoader.oList.clear(); + tracks = new ArrayList<>(TrackLoader.oList); + TrackLoader.oList.clear(); + TrackLoader.oList.add(new TagTrack("freshness", true, List.of( + new TagStep("fresh", "Fresh", 0, 1, 1), new TagStep("stale", "Stale", 1_000_000, 1, 1)))); + MemoryConfiguration config = new MemoryConfiguration(); + config.set("name", "egg"); + FoodItem egg = new FoodItem("egg", config); + egg.addOrModifyTrack(TrackLoader.getByString("freshness")); + FoodLoader.oList.add(egg); + hopper = server.createInventory(null, InventoryType.HOPPER); + chest = server.createInventory(null, 27); + now = System.currentTimeMillis(); + } + + @AfterEach + void tearDown() { + FoodLoader.oList.clear(); + FoodLoader.oList.addAll(foods); + TrackLoader.oList.clear(); + TrackLoader.oList.addAll(tracks); + MockBukkit.unmock(); + Cooking.plugin = previous; + } + + /** An egg whose stored freshness is value as of secondsAgo seconds ago, at the given stars. */ + private ItemStack egg(int amount, int value, long secondsAgo, int quality) { + FoodItem food = new FoodItem(FoodLoader.getByString("egg")); + food.getTagTrack("freshness").setValue(value); + food.setLastUpdate(now - secondsAgo * 1000L); + food.setQualityRange(quality, quality); + return ItemBuilder.stamp(new ItemStack(Material.EGG, amount), food, "egg"); + } + + private ItemStack egg(int amount, long secondsAgo) { + return egg(amount, 0, secondsAgo, 1); + } + + private static long clock(ItemStack item) { + return item.getItemMeta().getPersistentDataContainer().get(Keys.LAST_UPDATE, PersistentDataType.LONG); + } + + /** A hopper pushing moved into chest, after Cooking's listener ran. */ + private InventoryMoveItemEvent push(ItemStack moved) { + InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper, moved, chest, true); + new HopperFoodStacker().onMove(event); + return event; + } + + @Test + void aFresherEggTakesTheStoredStacksDataSoItStacks() { + chest.setItem(0, egg(3, 600)); + ItemStack moved = egg(1, 0); + + InventoryMoveItemEvent event = push(moved); + + assertTrue(event.getItem().isSimilar(chest.getItem(0))); + assertEquals(1, event.getItem().getAmount()); + assertEquals(now - 600_000L, clock(chest.getItem(0))); + assertEquals(3, chest.getItem(0).getAmount()); + } + + @Test + void anOlderEggGivesTheStoredStackItsDataSoNothingGetsFresher() { + chest.setItem(0, egg(3, 0)); + ItemStack moved = egg(1, 600); + + InventoryMoveItemEvent event = push(moved); + + assertTrue(event.getItem().isSimilar(moved)); + assertTrue(chest.getItem(0).isSimilar(moved)); + assertEquals(3, chest.getItem(0).getAmount()); + } + + @Test + void equallyAgedEggsKeepTheStoredStacksData() { + // Ten points stored now and none stored ten seconds ago are the same age. + chest.setItem(0, egg(2, 0, 10, 1)); + ItemStack stored = chest.getItem(0).clone(); + + InventoryMoveItemEvent event = push(egg(1, 10, 0, 1)); + + assertTrue(event.getItem().isSimilar(stored)); + assertTrue(chest.getItem(0).isSimilar(stored)); + } + + @Test + void anEarlierEmptySlotIsLeftToVanilla() { + chest.setItem(1, egg(3, 600)); + ItemStack moved = egg(1, 0); + + InventoryMoveItemEvent event = push(moved); + + assertTrue(event.getItem().isSimilar(moved)); + assertEquals(now - 600_000L, clock(chest.getItem(1))); + } + + @Test + void anExactCopyWithRoomIsLeftToVanilla() { + ItemStack moved = egg(1, 0); + ItemStack copy = moved.clone(); + copy.setAmount(4); + chest.setItem(0, copy); + chest.setItem(1, egg(3, 600)); + + InventoryMoveItemEvent event = push(moved); + + assertTrue(event.getItem().isSimilar(moved)); + assertEquals(now - 600_000L, clock(chest.getItem(1))); + } + + @Test + void fullStacksOtherItemsAndOtherStarsAreSkipped() { + chest.setItem(0, egg(16, 600)); + chest.setItem(1, new ItemStack(Material.EGG, 2)); + chest.setItem(2, new ItemStack(Material.STONE)); + chest.setItem(3, egg(2, 0, 600, 3)); + chest.setItem(4, egg(2, 300)); + ItemStack moved = egg(1, 0); + + InventoryMoveItemEvent event = push(moved); + + assertTrue(event.getItem().isSimilar(chest.getItem(4))); + assertEquals(16, chest.getItem(0).getAmount()); + } + + @Test + void foodThatDiffersBeyondAgingIsSkipped() { + ItemStack named = egg(2, 600); + ItemMeta meta = named.getItemMeta(); + meta.setDisplayName("Golden egg"); + named.setItemMeta(meta); + chest.setItem(0, named); + ItemStack moved = egg(1, 0); + + InventoryMoveItemEvent event = push(moved); + + assertTrue(event.getItem().isSimilar(moved)); + assertTrue(chest.getItem(0).isSimilar(named)); + } + + @Test + void aFullChestChangesNothing() { + for (int slot = 0; slot < chest.getSize(); slot++) chest.setItem(slot, egg(16, 600)); + ItemStack moved = egg(1, 0); + + assertTrue(push(moved).getItem().isSimilar(moved)); + } + + @Test + void itemsThatAreNotFoodAreLeftAlone() { + chest.setItem(0, new ItemStack(Material.EGG, 2)); + ItemStack named = new ItemStack(Material.EGG); + ItemMeta meta = named.getItemMeta(); + meta.setDisplayName("Painted egg"); + named.setItemMeta(meta); + + assertTrue(push(named).getItem().isSimilar(named)); + assertTrue(push(new ItemStack(Material.EGG)).getItem().isSimilar(new ItemStack(Material.EGG))); + } + + @Test + void aHopperPullingIntoItselfStacksToo() { + Inventory above = server.createInventory(null, 27); + hopper.setItem(0, egg(2, 600)); + ItemStack moved = egg(1, 0); + InventoryMoveItemEvent event = new InventoryMoveItemEvent(above, moved, hopper, false); + + new HopperFoodStacker().onMove(event); + + assertTrue(event.getItem().isSimilar(hopper.getItem(0))); + } + + @Test + void droppersAreLeftAlone() { + Inventory dropper = server.createInventory(null, InventoryType.DROPPER); + chest.setItem(0, egg(3, 600)); + ItemStack moved = egg(1, 0); + InventoryMoveItemEvent event = new InventoryMoveItemEvent(dropper, moved, chest, true); + + new HopperFoodStacker().onMove(event); + + assertTrue(event.getItem().isSimilar(moved)); + } + + @Test + void containersWithSlotRulesAreLeftAlone() { + Inventory furnace = server.createInventory(null, InventoryType.FURNACE); + furnace.setItem(0, egg(3, 600)); + ItemStack moved = egg(1, 0); + InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper, moved, furnace, true); + + new HopperFoodStacker().onMove(event); + + assertTrue(event.getItem().isSimilar(moved)); + } + + @Test + void aHopperPickingUpAFresherEggGivesTheGroundItemTheStoredData() { + WorldMock world = server.addSimpleWorld("world"); + hopper.setItem(0, egg(2, 600)); + Item ground = world.dropItem(world.getSpawnLocation(), egg(3, 0)); + + new HopperFoodStacker().onPickup(new InventoryPickupItemEvent(hopper, ground)); + + assertTrue(ground.getItemStack().isSimilar(hopper.getItem(0))); + assertEquals(3, ground.getItemStack().getAmount()); + assertEquals(now - 600_000L, clock(ground.getItemStack())); + } + + @Test + void aHopperPickingUpAnOlderEggAgesTheStoredStack() { + WorldMock world = server.addSimpleWorld("world"); + hopper.setItem(0, egg(2, 0)); + ItemStack old = egg(1, 600); + Item ground = world.dropItem(world.getSpawnLocation(), old); + + new HopperFoodStacker().onPickup(new InventoryPickupItemEvent(hopper, ground)); + + assertTrue(ground.getItemStack().isSimilar(old)); + assertTrue(hopper.getItem(0).isSimilar(old)); + assertEquals(2, hopper.getItem(0).getAmount()); + } + + @Test + void pickupsIntoOtherInventoriesAreLeftAlone() { + WorldMock world = server.addSimpleWorld("world"); + Inventory furnace = server.createInventory(null, InventoryType.FURNACE); + furnace.setItem(0, egg(2, 600)); + ItemStack fresh = egg(1, 0); + Item ground = world.dropItem(world.getSpawnLocation(), fresh); + + new HopperFoodStacker().onPickup(new InventoryPickupItemEvent(furnace, ground)); + + assertTrue(ground.getItemStack().isSimilar(fresh)); + } + + @Test + void onlyAMoveThatNeedsNewDataGetsACopy() { + assertNull(HopperFoodStacker.stackable(chest, null)); + assertNull(HopperFoodStacker.stackable(chest, new ItemStack(Material.AIR))); + ItemStack moved = egg(1, 0); + assertNull(HopperFoodStacker.stackable(chest, moved)); + chest.setItem(0, egg(2, 600)); + ItemStack out = HopperFoodStacker.stackable(chest, moved); + assertNotNull(out); + assertSame(Material.EGG, out.getType()); + } +} From 23d99f1fb21923025baa406841d95aacae865cf8 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:34:17 +0100 Subject: [PATCH 2/3] fix: keep hopper food stacking safe when the event is cancelled Only change what a cancelled event leaves untouched: the moved item (discarded on cancel) or the ground item (restored on the next tick if it is still there). The incoming food takes the stored stack's data; when it was the older one, the stack takes its data on the next tick once the slot has really grown. Merges need room for the whole item, so nothing spills into later slots with younger data. The lifecycle test now checks that Cooking registers the listener. Co-Authored-By: Claude Opus 5.5 --- .../cooking/manager/HopperFoodStacker.java | 110 ++++++++---- .../cooking/CookingLifecycleTest.java | 6 + .../manager/HopperFoodStackerTest.java | 157 +++++++++++++----- 3 files changed, 197 insertions(+), 76 deletions(-) diff --git a/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java b/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java index d83393e..e11af84 100644 --- a/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java +++ b/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java @@ -1,5 +1,8 @@ package net.tfminecraft.cooking.manager; +import java.util.function.Consumer; + +import org.bukkit.Bukkit; import org.bukkit.entity.Item; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; @@ -11,6 +14,7 @@ import org.bukkit.inventory.ItemStack; import org.bukkit.persistence.PersistentDataType; +import net.tfminecraft.cooking.Cooking; import net.tfminecraft.cooking.item.FoodItem; import net.tfminecraft.cooking.utils.InventoryAdder; import net.tfminecraft.cooking.utils.Keys; @@ -19,11 +23,26 @@ * Aging foods carry their own clock, so hoppers, which only stack exact copies, give every piece * of the same food its own slot: eggs laid an hour apart fill a hopper five slots at a time. When a * hopper moves food into a storage block, or picks it up off the ground, and the slot vanilla would - * fill first holds an equal food that only aged differently, both take the more aged copy's data so - * vanilla stacks them there. Food never gets fresher this way, and vanilla still picks the slot. + * fill first holds an equal food that only aged differently, the incoming food takes the stored + * stack's data so vanilla stacks it there. Vanilla still picks the slot. + * + *

Other plugins may cancel the event after this listener, so it changes only what a cancelled + * event leaves untouched: the moved item, which a cancelled move discards, or the ground item, which + * is put back on the next tick if it is still there. When the incoming food was the older one, the + * stack takes its data on the next tick, once the slot has really grown, so food never gets fresher. */ public class HopperFoodStacker implements Listener { + private final Consumer nextTick; + + public HopperFoodStacker() { + this(task -> Bukkit.getScheduler().runTask(Cooking.plugin, task)); + } + + HopperFoodStacker(Consumer nextTick) { + this.nextTick = nextTick; + } + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) public void onMove(InventoryMoveItemEvent event) { // Hoppers take the item out of the source before this event and add whatever it carries @@ -31,8 +50,10 @@ public void onMove(InventoryMoveItemEvent event) { if (event.getInitiator().getType() != InventoryType.HOPPER) return; Inventory destination = event.getDestination(); if (!storage(destination)) return; - ItemStack stackable = stackable(destination, event.getItem()); - if (stackable != null) event.setItem(stackable); + Merge merge = merge(destination, event.getItem()); + if (merge == null) return; + event.setItem(merge.carried()); + if (merge.incomingIsOlder()) nextTick.accept(merge::ageStack); } @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) @@ -41,8 +62,18 @@ public void onPickup(InventoryPickupItemEvent event) { if (!storage(destination)) return; // The hopper copies the ground item after this event, so it takes the changed data. Item entity = event.getItem(); - ItemStack stackable = stackable(destination, entity.getItemStack()); - if (stackable != null) entity.setItemStack(stackable); + Merge merge = merge(destination, entity.getItemStack()); + if (merge == null) return; + entity.setItemStack(merge.carried()); + nextTick.accept(() -> { + // A merge only happens when the whole item fits, so an item still on the ground was + // not picked up: give it its own data back. + if (entity.isValid()) { + if (entity.getItemStack().isSimilar(merge.stored())) entity.setItemStack(merge.incoming()); + } else if (merge.incomingIsOlder()) { + merge.ageStack(); + } + }); } /** Containers that take an item in any slot from any side, in slot order. */ @@ -54,46 +85,61 @@ private static boolean storage(Inventory inventory) { } /** - * The moved food as it should enter destination to stack, or null when it can stay as it is. - * Vanilla fills the first slot that is empty or holds an exact copy with room. When the first - * slot it could fill instead holds an equal food that aged differently, the fresher side takes - * the older one's data: the slot is rewritten in place, or a copy of it is returned to move. - * If another plugin cancels the move after this, a rewritten slot has only aged to the moved - * food's age within the same freshness step; nothing is gained or lost. + * The incoming food meets an equal food that aged differently in the first slot vanilla would + * fill, with room for all of it. */ - static ItemStack stackable(Inventory destination, ItemStack moved) { + record Merge(Inventory destination, int slot, ItemStack stored, ItemStack incoming, boolean incomingIsOlder) { + + /** The incoming food carrying the stored stack's data, so vanilla stacks it there. */ + ItemStack carried() { + ItemStack out = stored.clone(); + out.setAmount(incoming.getAmount()); + return out; + } + + /** Once the incoming food has joined the stack, give the stack its older data. */ + void ageStack() { + ItemStack current = destination.getItem(slot); + if (current == null || !current.isSimilar(stored) + || current.getAmount() < stored.getAmount() + incoming.getAmount()) return; + ItemStack older = incoming.clone(); + older.setAmount(current.getAmount()); + destination.setItem(slot, older); + } + } + + /** + * Where the incoming food can stack after all, or null when vanilla handles it as it is. Vanilla + * fills the first slot that is empty or holds an exact copy with room; a merge is offered when + * that slot instead holds an equal food that aged differently and has room for all of it. + */ + static Merge merge(Inventory destination, ItemStack incoming) { // Most hopper traffic is not food; read the food id without copying the item's meta. - if (moved == null || moved.isEmpty() || !moved.hasItemMeta() - || !moved.getPersistentDataContainer().has(Keys.FOOD_ID, PersistentDataType.STRING)) { + if (incoming == null || incoming.isEmpty() || !incoming.hasItemMeta() + || !incoming.getPersistentDataContainer().has(Keys.FOOD_ID, PersistentDataType.STRING)) { return null; } FoodItem food = null; for (int slot = 0; slot < destination.getSize(); slot++) { ItemStack current = destination.getItem(slot); if (current == null || current.isEmpty()) return null; - if (current.getType() != moved.getType()) continue; - int limit = Math.min(current.getMaxStackSize(), destination.getMaxStackSize()); - if (current.getAmount() >= limit) continue; - if (current.isSimilar(moved)) return null; + if (current.getType() != incoming.getType()) continue; + int room = Math.min(current.getMaxStackSize(), destination.getMaxStackSize()) - current.getAmount(); + if (room <= 0) continue; + if (current.isSimilar(incoming)) return null; if (food == null) { - food = FoodItem.fromItem(moved); + food = FoodItem.fromItem(incoming); if (food == null) return null; } FoodItem other = FoodItem.fromItem(current); if (other == null || !InventoryAdder.equalsFood(food, other) - || !ConversionManager.sameApartFromAging(moved, current)) continue; - - // Equal age keeps the stored stack, so only the moving item changes. - if (BundleFoodStacker.moreAged(current, moved) == current) { - ItemStack out = current.clone(); - out.setAmount(moved.getAmount()); - return out; - } - ItemStack older = moved.clone(); - older.setAmount(current.getAmount()); - destination.setItem(slot, older); - return null; + || !ConversionManager.sameApartFromAging(incoming, current)) continue; + // Part of it would spill into later slots with the stored data; leave that to vanilla. + if (incoming.getAmount() > room) return null; + // Equal age keeps the stored stack's data. + boolean older = BundleFoodStacker.moreAged(current, incoming) != current; + return new Merge(destination, slot, current.clone(), incoming.clone(), older); } return null; } diff --git a/src/test/java/net/tfminecraft/cooking/CookingLifecycleTest.java b/src/test/java/net/tfminecraft/cooking/CookingLifecycleTest.java index 2a48aa7..dd75c32 100644 --- a/src/test/java/net/tfminecraft/cooking/CookingLifecycleTest.java +++ b/src/test/java/net/tfminecraft/cooking/CookingLifecycleTest.java @@ -165,6 +165,12 @@ class CookingLifecycleTest { assertTrue(plugin.getCommand("cooking").testPermissionSilent(caller)); assertTrue(plugin.getCommand("animals").getAliases().contains("livestock")); assertFalse(HandlerList.getRegisteredListeners(plugin).isEmpty()); + // Hoppers only stack aging food through this listener; nothing else would notice it missing. + for (HandlerList handlers : List.of(org.bukkit.event.inventory.InventoryMoveItemEvent.getHandlerList(), + org.bukkit.event.inventory.InventoryPickupItemEvent.getHandlerList())) { + assertTrue(java.util.Arrays.stream(handlers.getRegisteredListeners()).anyMatch(registered -> + registered.getPlugin() == plugin && registered.getListener() instanceof HopperFoodStacker)); + } verify(built(CookingManager.class)).start(); verify(built(PlateManager.class), never()).start(); } diff --git a/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java b/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java index c5d9fd1..687a05e 100644 --- a/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java +++ b/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java @@ -1,9 +1,9 @@ package net.tfminecraft.cooking.manager; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -46,6 +46,8 @@ class HopperFoodStackerTest { private Inventory hopper; private Inventory chest; private long now; + private final List ticks = new ArrayList<>(); + private final HopperFoodStacker stacker = new HopperFoodStacker(ticks::add); @BeforeEach void setUp() { @@ -100,32 +102,62 @@ private static long clock(ItemStack item) { /** A hopper pushing moved into chest, after Cooking's listener ran. */ private InventoryMoveItemEvent push(ItemStack moved) { InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper, moved, chest, true); - new HopperFoodStacker().onMove(event); + stacker.onMove(event); return event; } + /** What vanilla does with an item that matches slot 0 exactly. */ + private static void addTo(Inventory inventory, int slot, ItemStack item) { + ItemStack current = inventory.getItem(slot); + assertTrue(current.isSimilar(item)); + current.setAmount(current.getAmount() + item.getAmount()); + inventory.setItem(slot, current); + } + + private void tick() { + List due = new ArrayList<>(ticks); + ticks.clear(); + due.forEach(Runnable::run); + } + @Test void aFresherEggTakesTheStoredStacksDataSoItStacks() { chest.setItem(0, egg(3, 600)); - ItemStack moved = egg(1, 0); - InventoryMoveItemEvent event = push(moved); + InventoryMoveItemEvent event = push(egg(1, 0)); assertTrue(event.getItem().isSimilar(chest.getItem(0))); assertEquals(1, event.getItem().getAmount()); assertEquals(now - 600_000L, clock(chest.getItem(0))); - assertEquals(3, chest.getItem(0).getAmount()); + assertTrue(ticks.isEmpty()); } @Test - void anOlderEggGivesTheStoredStackItsDataSoNothingGetsFresher() { - chest.setItem(0, egg(3, 0)); + void anOlderEggStacksAndTheStackTakesItsDataOnTheNextTick() { + ItemStack stored = egg(3, 0); + chest.setItem(0, stored); ItemStack moved = egg(1, 600); InventoryMoveItemEvent event = push(moved); + // Nothing in the chest changes while the move can still be cancelled. + assertTrue(chest.getItem(0).isSimilar(stored)); + assertTrue(event.getItem().isSimilar(stored)); + addTo(chest, 0, event.getItem()); + tick(); - assertTrue(event.getItem().isSimilar(moved)); assertTrue(chest.getItem(0).isSimilar(moved)); + assertEquals(4, chest.getItem(0).getAmount()); + } + + @Test + void aCancelledMoveOfAnOlderEggLeavesTheStackAlone() { + ItemStack stored = egg(3, 0); + chest.setItem(0, stored); + + push(egg(1, 600)); + tick(); + + assertTrue(chest.getItem(0).isSimilar(stored)); assertEquals(3, chest.getItem(0).getAmount()); } @@ -138,7 +170,7 @@ void equallyAgedEggsKeepTheStoredStacksData() { InventoryMoveItemEvent event = push(egg(1, 10, 0, 1)); assertTrue(event.getItem().isSimilar(stored)); - assertTrue(chest.getItem(0).isSimilar(stored)); + assertTrue(ticks.isEmpty()); } @Test @@ -146,10 +178,7 @@ void anEarlierEmptySlotIsLeftToVanilla() { chest.setItem(1, egg(3, 600)); ItemStack moved = egg(1, 0); - InventoryMoveItemEvent event = push(moved); - - assertTrue(event.getItem().isSimilar(moved)); - assertEquals(now - 600_000L, clock(chest.getItem(1))); + assertTrue(push(moved).getItem().isSimilar(moved)); } @Test @@ -160,10 +189,7 @@ void anExactCopyWithRoomIsLeftToVanilla() { chest.setItem(0, copy); chest.setItem(1, egg(3, 600)); - InventoryMoveItemEvent event = push(moved); - - assertTrue(event.getItem().isSimilar(moved)); - assertEquals(now - 600_000L, clock(chest.getItem(1))); + assertTrue(push(moved).getItem().isSimilar(moved)); } @Test @@ -173,9 +199,8 @@ void fullStacksOtherItemsAndOtherStarsAreSkipped() { chest.setItem(2, new ItemStack(Material.STONE)); chest.setItem(3, egg(2, 0, 600, 3)); chest.setItem(4, egg(2, 300)); - ItemStack moved = egg(1, 0); - InventoryMoveItemEvent event = push(moved); + InventoryMoveItemEvent event = push(egg(1, 0)); assertTrue(event.getItem().isSimilar(chest.getItem(4))); assertEquals(16, chest.getItem(0).getAmount()); @@ -190,10 +215,7 @@ void foodThatDiffersBeyondAgingIsSkipped() { chest.setItem(0, named); ItemStack moved = egg(1, 0); - InventoryMoveItemEvent event = push(moved); - - assertTrue(event.getItem().isSimilar(moved)); - assertTrue(chest.getItem(0).isSimilar(named)); + assertTrue(push(moved).getItem().isSimilar(moved)); } @Test @@ -220,10 +242,9 @@ void itemsThatAreNotFoodAreLeftAlone() { void aHopperPullingIntoItselfStacksToo() { Inventory above = server.createInventory(null, 27); hopper.setItem(0, egg(2, 600)); - ItemStack moved = egg(1, 0); - InventoryMoveItemEvent event = new InventoryMoveItemEvent(above, moved, hopper, false); + InventoryMoveItemEvent event = new InventoryMoveItemEvent(above, egg(1, 0), hopper, false); - new HopperFoodStacker().onMove(event); + stacker.onMove(event); assertTrue(event.getItem().isSimilar(hopper.getItem(0))); } @@ -235,7 +256,7 @@ void droppersAreLeftAlone() { ItemStack moved = egg(1, 0); InventoryMoveItemEvent event = new InventoryMoveItemEvent(dropper, moved, chest, true); - new HopperFoodStacker().onMove(event); + stacker.onMove(event); assertTrue(event.getItem().isSimilar(moved)); } @@ -247,7 +268,7 @@ void containersWithSlotRulesAreLeftAlone() { ItemStack moved = egg(1, 0); InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper, moved, furnace, true); - new HopperFoodStacker().onMove(event); + stacker.onMove(event); assertTrue(event.getItem().isSimilar(moved)); } @@ -258,24 +279,49 @@ void aHopperPickingUpAFresherEggGivesTheGroundItemTheStoredData() { hopper.setItem(0, egg(2, 600)); Item ground = world.dropItem(world.getSpawnLocation(), egg(3, 0)); - new HopperFoodStacker().onPickup(new InventoryPickupItemEvent(hopper, ground)); + stacker.onPickup(new InventoryPickupItemEvent(hopper, ground)); + addTo(hopper, 0, ground.getItemStack()); + ground.remove(); + tick(); - assertTrue(ground.getItemStack().isSimilar(hopper.getItem(0))); - assertEquals(3, ground.getItemStack().getAmount()); - assertEquals(now - 600_000L, clock(ground.getItemStack())); + assertEquals(now - 600_000L, clock(hopper.getItem(0))); + assertEquals(5, hopper.getItem(0).getAmount()); } @Test - void aHopperPickingUpAnOlderEggAgesTheStoredStack() { + void aHopperPickingUpAnOlderEggAgesTheStackOnceItIsIn() { WorldMock world = server.addSimpleWorld("world"); - hopper.setItem(0, egg(2, 0)); + ItemStack stored = egg(2, 0); + hopper.setItem(0, stored); ItemStack old = egg(1, 600); Item ground = world.dropItem(world.getSpawnLocation(), old); - new HopperFoodStacker().onPickup(new InventoryPickupItemEvent(hopper, ground)); + stacker.onPickup(new InventoryPickupItemEvent(hopper, ground)); + assertTrue(hopper.getItem(0).isSimilar(stored)); + assertTrue(ground.getItemStack().isSimilar(stored)); + addTo(hopper, 0, ground.getItemStack()); + ground.remove(); + tick(); - assertTrue(ground.getItemStack().isSimilar(old)); assertTrue(hopper.getItem(0).isSimilar(old)); + assertEquals(3, hopper.getItem(0).getAmount()); + } + + @Test + void aCancelledPickupGetsItsOwnDataBack() { + WorldMock world = server.addSimpleWorld("world"); + ItemStack stored = egg(2, 600); + hopper.setItem(0, stored); + ItemStack fresh = egg(3, 0); + Item ground = world.dropItem(world.getSpawnLocation(), fresh); + + stacker.onPickup(new InventoryPickupItemEvent(hopper, ground)); + assertTrue(ground.getItemStack().isSimilar(stored)); + tick(); + + assertTrue(ground.getItemStack().isSimilar(fresh)); + assertEquals(3, ground.getItemStack().getAmount()); + assertTrue(hopper.getItem(0).isSimilar(stored)); assertEquals(2, hopper.getItem(0).getAmount()); } @@ -287,20 +333,43 @@ void pickupsIntoOtherInventoriesAreLeftAlone() { ItemStack fresh = egg(1, 0); Item ground = world.dropItem(world.getSpawnLocation(), fresh); - new HopperFoodStacker().onPickup(new InventoryPickupItemEvent(furnace, ground)); + stacker.onPickup(new InventoryPickupItemEvent(furnace, ground)); assertTrue(ground.getItemStack().isSimilar(fresh)); + assertTrue(ticks.isEmpty()); + } + + @Test + void foodThatWouldNotAllFitIsLeftToVanilla() { + hopper.setItem(0, egg(14, 0)); + assertNull(HopperFoodStacker.merge(hopper, egg(3, 600))); + assertNotNull(HopperFoodStacker.merge(hopper, egg(2, 600))); + } + + @Test + void theStackIsNotAgedWhenItDidNotGrow() { + ItemStack stored = egg(3, 0); + chest.setItem(0, stored); + HopperFoodStacker.Merge merge = HopperFoodStacker.merge(chest, egg(1, 600)); + assertTrue(merge.incomingIsOlder()); + + merge.ageStack(); + assertTrue(chest.getItem(0).isSimilar(stored)); + chest.setItem(0, null); + merge.ageStack(); + assertNull(chest.getItem(0)); } @Test - void onlyAMoveThatNeedsNewDataGetsACopy() { - assertNull(HopperFoodStacker.stackable(chest, null)); - assertNull(HopperFoodStacker.stackable(chest, new ItemStack(Material.AIR))); + void onlyFoodThatNeedsNewDataGetsAMerge() { + assertNull(HopperFoodStacker.merge(chest, null)); + assertNull(HopperFoodStacker.merge(chest, new ItemStack(Material.AIR))); ItemStack moved = egg(1, 0); - assertNull(HopperFoodStacker.stackable(chest, moved)); + assertNull(HopperFoodStacker.merge(chest, moved)); chest.setItem(0, egg(2, 600)); - ItemStack out = HopperFoodStacker.stackable(chest, moved); - assertNotNull(out); - assertSame(Material.EGG, out.getType()); + HopperFoodStacker.Merge merge = HopperFoodStacker.merge(chest, moved); + assertNotNull(merge); + assertFalse(merge.incomingIsOlder()); + assertEquals(0, merge.slot()); } } From bfac0cf2e390c64712beb433e73102759ae18a8a Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:56:32 +0100 Subject: [PATCH 3/3] fix: change hopper stacks only once nothing can cancel the event Drop the next-tick work. A fresher moved item takes the stored data through the event at HIGHEST, which a cancelled move throws away. The stored stack (older moved item) and the ground item (pickup) are only changed at MONITOR, which runs for events nothing cancelled, right before vanilla adds the item. A later exact copy with room is left to vanilla, as it fills that slot. Co-Authored-By: Claude Opus 5.5 --- .../cooking/manager/HopperFoodStacker.java | 102 ++++++++------- .../manager/HopperFoodStackerTest.java | 123 +++++++++--------- 2 files changed, 111 insertions(+), 114 deletions(-) diff --git a/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java b/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java index e11af84..58c9629 100644 --- a/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java +++ b/src/main/java/net/tfminecraft/cooking/manager/HopperFoodStacker.java @@ -1,8 +1,5 @@ package net.tfminecraft.cooking.manager; -import java.util.function.Consumer; - -import org.bukkit.Bukkit; import org.bukkit.entity.Item; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; @@ -14,7 +11,6 @@ import org.bukkit.inventory.ItemStack; import org.bukkit.persistence.PersistentDataType; -import net.tfminecraft.cooking.Cooking; import net.tfminecraft.cooking.item.FoodItem; import net.tfminecraft.cooking.utils.InventoryAdder; import net.tfminecraft.cooking.utils.Keys; @@ -23,57 +19,52 @@ * Aging foods carry their own clock, so hoppers, which only stack exact copies, give every piece * of the same food its own slot: eggs laid an hour apart fill a hopper five slots at a time. When a * hopper moves food into a storage block, or picks it up off the ground, and the slot vanilla would - * fill first holds an equal food that only aged differently, the incoming food takes the stored - * stack's data so vanilla stacks it there. Vanilla still picks the slot. + * fill first holds an equal food that only aged differently, the fresher side takes the older one's + * data so vanilla stacks them there. Food never gets fresher, and vanilla still does the moving. * - *

Other plugins may cancel the event after this listener, so it changes only what a cancelled - * event leaves untouched: the moved item, which a cancelled move discards, or the ground item, which - * is put back on the next tick if it is still there. When the incoming food was the older one, the - * stack takes its data on the next tick, once the slot has really grown, so food never gets fresher. + *

Other plugins may cancel these events, so nothing outside the event changes before they are + * done. A fresher moved item takes the stored data through the event at HIGHEST, which a cancelled + * move throws away. Changes to the stored stack or to a ground item wait for MONITOR, which only + * runs for an event nothing cancelled; vanilla adds the item straight after it. */ public class HopperFoodStacker implements Listener { - private final Consumer nextTick; - - public HopperFoodStacker() { - this(task -> Bukkit.getScheduler().runTask(Cooking.plugin, task)); - } - - HopperFoodStacker(Consumer nextTick) { - this.nextTick = nextTick; - } - @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) public void onMove(InventoryMoveItemEvent event) { - // Hoppers take the item out of the source before this event and add whatever it carries - // after, so a changed copy moves cleanly. A dropper keeps the source item when it changes. - if (event.getInitiator().getType() != InventoryType.HOPPER) return; - Inventory destination = event.getDestination(); - if (!storage(destination)) return; - Merge merge = merge(destination, event.getItem()); - if (merge == null) return; - event.setItem(merge.carried()); - if (merge.incomingIsOlder()) nextTick.accept(merge::ageStack); + if (!hopperIntoStorage(event)) return; + Merge merge = merge(event.getDestination(), event.getItem()); + if (merge != null && !merge.incomingIsOlder()) event.setItem(merge.carried()); } - @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) + @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) + public void afterMove(InventoryMoveItemEvent event) { + if (!hopperIntoStorage(event)) return; + // A fresher item already carries the stored data, so only an older one still merges here. + Merge merge = merge(event.getDestination(), event.getItem()); + if (merge != null && merge.incomingIsOlder()) merge.ageStoredStack(); + } + + @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) public void onPickup(InventoryPickupItemEvent event) { Inventory destination = event.getInventory(); if (!storage(destination)) return; - // The hopper copies the ground item after this event, so it takes the changed data. + // The hopper copies the ground item after this event, and all of it fits in the slot. Item entity = event.getItem(); Merge merge = merge(destination, entity.getItemStack()); if (merge == null) return; - entity.setItemStack(merge.carried()); - nextTick.accept(() -> { - // A merge only happens when the whole item fits, so an item still on the ground was - // not picked up: give it its own data back. - if (entity.isValid()) { - if (entity.getItemStack().isSimilar(merge.stored())) entity.setItemStack(merge.incoming()); - } else if (merge.incomingIsOlder()) { - merge.ageStack(); - } - }); + if (merge.incomingIsOlder()) { + merge.ageStoredStack(); + } else { + entity.setItemStack(merge.carried()); + } + } + + /** + * Hoppers take the item out of the source before the event and add whatever it carries after, + * so a changed copy moves cleanly. A dropper keeps the source item when it changes. + */ + private static boolean hopperIntoStorage(InventoryMoveItemEvent event) { + return event.getInitiator().getType() == InventoryType.HOPPER && storage(event.getDestination()); } /** Containers that take an item in any slot from any side, in slot order. */ @@ -97,13 +88,10 @@ ItemStack carried() { return out; } - /** Once the incoming food has joined the stack, give the stack its older data. */ - void ageStack() { - ItemStack current = destination.getItem(slot); - if (current == null || !current.isSimilar(stored) - || current.getAmount() < stored.getAmount() + incoming.getAmount()) return; + /** The stored stack carrying the older incoming food's data, so vanilla stacks it there. */ + void ageStoredStack() { ItemStack older = incoming.clone(); - older.setAmount(current.getAmount()); + older.setAmount(stored.getAmount()); destination.setItem(slot, older); } } @@ -111,7 +99,8 @@ void ageStack() { /** * Where the incoming food can stack after all, or null when vanilla handles it as it is. Vanilla * fills the first slot that is empty or holds an exact copy with room; a merge is offered when - * that slot instead holds an equal food that aged differently and has room for all of it. + * that slot instead holds an equal food that aged differently and has room for all of it, and no + * later slot holds an exact copy with room. */ static Merge merge(Inventory destination, ItemStack incoming) { // Most hopper traffic is not food; read the food id without copying the item's meta. @@ -124,7 +113,7 @@ static Merge merge(Inventory destination, ItemStack incoming) { ItemStack current = destination.getItem(slot); if (current == null || current.isEmpty()) return null; if (current.getType() != incoming.getType()) continue; - int room = Math.min(current.getMaxStackSize(), destination.getMaxStackSize()) - current.getAmount(); + int room = room(destination, current); if (room <= 0) continue; if (current.isSimilar(incoming)) return null; @@ -136,11 +125,24 @@ static Merge merge(Inventory destination, ItemStack incoming) { if (other == null || !InventoryAdder.equalsFood(food, other) || !ConversionManager.sameApartFromAging(incoming, current)) continue; // Part of it would spill into later slots with the stored data; leave that to vanilla. - if (incoming.getAmount() > room) return null; + if (incoming.getAmount() > room || laterExactCopy(destination, slot, incoming)) return null; // Equal age keeps the stored stack's data. boolean older = BundleFoodStacker.moreAged(current, incoming) != current; return new Merge(destination, slot, current.clone(), incoming.clone(), older); } return null; } + + private static int room(Inventory inventory, ItemStack stack) { + return Math.min(stack.getMaxStackSize(), inventory.getMaxStackSize()) - stack.getAmount(); + } + + /** True when a slot after this one already stacks the item exactly; vanilla puts it there. */ + private static boolean laterExactCopy(Inventory destination, int after, ItemStack incoming) { + for (int slot = after + 1; slot < destination.getSize(); slot++) { + ItemStack later = destination.getItem(slot); + if (later != null && later.isSimilar(incoming) && room(destination, later) > 0) return true; + } + return false; + } } diff --git a/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java b/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java index 687a05e..7ba5ae0 100644 --- a/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java +++ b/src/test/java/net/tfminecraft/cooking/manager/HopperFoodStackerTest.java @@ -46,8 +46,7 @@ class HopperFoodStackerTest { private Inventory hopper; private Inventory chest; private long now; - private final List ticks = new ArrayList<>(); - private final HopperFoodStacker stacker = new HopperFoodStacker(ticks::add); + private final HopperFoodStacker stacker = new HopperFoodStacker(); @BeforeEach void setUp() { @@ -99,10 +98,11 @@ private static long clock(ItemStack item) { return item.getItemMeta().getPersistentDataContainer().get(Keys.LAST_UPDATE, PersistentDataType.LONG); } - /** A hopper pushing moved into chest, after Cooking's listener ran. */ + /** A hopper pushing moved into chest, after Cooking's listeners ran and nothing cancelled it. */ private InventoryMoveItemEvent push(ItemStack moved) { InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper, moved, chest, true); stacker.onMove(event); + stacker.afterMove(event); return event; } @@ -114,51 +114,50 @@ private static void addTo(Inventory inventory, int slot, ItemStack item) { inventory.setItem(slot, current); } - private void tick() { - List due = new ArrayList<>(ticks); - ticks.clear(); - due.forEach(Runnable::run); - } - @Test void aFresherEggTakesTheStoredStacksDataSoItStacks() { - chest.setItem(0, egg(3, 600)); + ItemStack stored = egg(3, 600); + chest.setItem(0, stored); InventoryMoveItemEvent event = push(egg(1, 0)); - assertTrue(event.getItem().isSimilar(chest.getItem(0))); + assertTrue(event.getItem().isSimilar(stored)); assertEquals(1, event.getItem().getAmount()); - assertEquals(now - 600_000L, clock(chest.getItem(0))); - assertTrue(ticks.isEmpty()); + assertTrue(chest.getItem(0).isSimilar(stored)); + assertEquals(3, chest.getItem(0).getAmount()); } @Test - void anOlderEggStacksAndTheStackTakesItsDataOnTheNextTick() { + void anOlderEggGivesTheStackItsDataOnceNothingCancelledTheMove() { ItemStack stored = egg(3, 0); chest.setItem(0, stored); ItemStack moved = egg(1, 600); + InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper, moved, chest, true); - InventoryMoveItemEvent event = push(moved); + stacker.onMove(event); // Nothing in the chest changes while the move can still be cancelled. assertTrue(chest.getItem(0).isSimilar(stored)); - assertTrue(event.getItem().isSimilar(stored)); + stacker.afterMove(event); addTo(chest, 0, event.getItem()); - tick(); + assertTrue(event.getItem().isSimilar(moved)); assertTrue(chest.getItem(0).isSimilar(moved)); assertEquals(4, chest.getItem(0).getAmount()); } @Test - void aCancelledMoveOfAnOlderEggLeavesTheStackAlone() { + void aCancelledMoveChangesNothingOutsideTheEvent() { ItemStack stored = egg(3, 0); chest.setItem(0, stored); + ItemStack moved = egg(1, 600); + InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper, moved, chest, true); - push(egg(1, 600)); - tick(); + // MONITOR (afterMove) never runs for a cancelled event. + stacker.onMove(event); assertTrue(chest.getItem(0).isSimilar(stored)); assertEquals(3, chest.getItem(0).getAmount()); + assertTrue(event.getItem().isSimilar(moved)); } @Test @@ -170,7 +169,21 @@ void equallyAgedEggsKeepTheStoredStacksData() { InventoryMoveItemEvent event = push(egg(1, 10, 0, 1)); assertTrue(event.getItem().isSimilar(stored)); - assertTrue(ticks.isEmpty()); + assertTrue(chest.getItem(0).isSimilar(stored)); + } + + @Test + void stackChangesWaitUntilNothingCanCancelTheEvent() throws Exception { + var move = HopperFoodStacker.class.getMethod("onMove", InventoryMoveItemEvent.class) + .getAnnotation(org.bukkit.event.EventHandler.class); + var after = HopperFoodStacker.class.getMethod("afterMove", InventoryMoveItemEvent.class) + .getAnnotation(org.bukkit.event.EventHandler.class); + var pickup = HopperFoodStacker.class.getMethod("onPickup", InventoryPickupItemEvent.class) + .getAnnotation(org.bukkit.event.EventHandler.class); + assertEquals(org.bukkit.event.EventPriority.HIGHEST, move.priority()); + assertEquals(org.bukkit.event.EventPriority.MONITOR, after.priority()); + assertEquals(org.bukkit.event.EventPriority.MONITOR, pickup.priority()); + assertTrue(move.ignoreCancelled() && after.ignoreCancelled() && pickup.ignoreCancelled()); } @Test @@ -245,6 +258,7 @@ void aHopperPullingIntoItselfStacksToo() { InventoryMoveItemEvent event = new InventoryMoveItemEvent(above, egg(1, 0), hopper, false); stacker.onMove(event); + stacker.afterMove(event); assertTrue(event.getItem().isSimilar(hopper.getItem(0))); } @@ -257,6 +271,7 @@ void droppersAreLeftAlone() { InventoryMoveItemEvent event = new InventoryMoveItemEvent(dropper, moved, chest, true); stacker.onMove(event); + stacker.afterMove(event); assertTrue(event.getItem().isSimilar(moved)); } @@ -269,6 +284,7 @@ void containersWithSlotRulesAreLeftAlone() { InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper, moved, furnace, true); stacker.onMove(event); + stacker.afterMove(event); assertTrue(event.getItem().isSimilar(moved)); } @@ -276,52 +292,29 @@ void containersWithSlotRulesAreLeftAlone() { @Test void aHopperPickingUpAFresherEggGivesTheGroundItemTheStoredData() { WorldMock world = server.addSimpleWorld("world"); - hopper.setItem(0, egg(2, 600)); + ItemStack stored = egg(2, 600); + hopper.setItem(0, stored); Item ground = world.dropItem(world.getSpawnLocation(), egg(3, 0)); stacker.onPickup(new InventoryPickupItemEvent(hopper, ground)); - addTo(hopper, 0, ground.getItemStack()); - ground.remove(); - tick(); - assertEquals(now - 600_000L, clock(hopper.getItem(0))); - assertEquals(5, hopper.getItem(0).getAmount()); + assertTrue(ground.getItemStack().isSimilar(stored)); + assertEquals(3, ground.getItemStack().getAmount()); + assertTrue(hopper.getItem(0).isSimilar(stored)); + assertEquals(2, hopper.getItem(0).getAmount()); } @Test - void aHopperPickingUpAnOlderEggAgesTheStackOnceItIsIn() { + void aHopperPickingUpAnOlderEggGivesTheStackItsData() { WorldMock world = server.addSimpleWorld("world"); - ItemStack stored = egg(2, 0); - hopper.setItem(0, stored); + hopper.setItem(0, egg(2, 0)); ItemStack old = egg(1, 600); Item ground = world.dropItem(world.getSpawnLocation(), old); stacker.onPickup(new InventoryPickupItemEvent(hopper, ground)); - assertTrue(hopper.getItem(0).isSimilar(stored)); - assertTrue(ground.getItemStack().isSimilar(stored)); - addTo(hopper, 0, ground.getItemStack()); - ground.remove(); - tick(); + assertTrue(ground.getItemStack().isSimilar(old)); assertTrue(hopper.getItem(0).isSimilar(old)); - assertEquals(3, hopper.getItem(0).getAmount()); - } - - @Test - void aCancelledPickupGetsItsOwnDataBack() { - WorldMock world = server.addSimpleWorld("world"); - ItemStack stored = egg(2, 600); - hopper.setItem(0, stored); - ItemStack fresh = egg(3, 0); - Item ground = world.dropItem(world.getSpawnLocation(), fresh); - - stacker.onPickup(new InventoryPickupItemEvent(hopper, ground)); - assertTrue(ground.getItemStack().isSimilar(stored)); - tick(); - - assertTrue(ground.getItemStack().isSimilar(fresh)); - assertEquals(3, ground.getItemStack().getAmount()); - assertTrue(hopper.getItem(0).isSimilar(stored)); assertEquals(2, hopper.getItem(0).getAmount()); } @@ -336,7 +329,6 @@ void pickupsIntoOtherInventoriesAreLeftAlone() { stacker.onPickup(new InventoryPickupItemEvent(furnace, ground)); assertTrue(ground.getItemStack().isSimilar(fresh)); - assertTrue(ticks.isEmpty()); } @Test @@ -347,17 +339,20 @@ void foodThatWouldNotAllFitIsLeftToVanilla() { } @Test - void theStackIsNotAgedWhenItDidNotGrow() { - ItemStack stored = egg(3, 0); - chest.setItem(0, stored); - HopperFoodStacker.Merge merge = HopperFoodStacker.merge(chest, egg(1, 600)); - assertTrue(merge.incomingIsOlder()); + void aLaterExactCopyWithRoomIsLeftToVanilla() { + ItemStack moved = egg(1, 0); + ItemStack aged = egg(2, 600); + chest.setItem(0, aged); + ItemStack copy = moved.clone(); + copy.setAmount(3); + chest.setItem(1, copy); - merge.ageStack(); - assertTrue(chest.getItem(0).isSimilar(stored)); - chest.setItem(0, null); - merge.ageStack(); - assertNull(chest.getItem(0)); + assertTrue(push(moved).getItem().isSimilar(moved)); + assertTrue(chest.getItem(0).isSimilar(aged)); + + copy.setAmount(16); + chest.setItem(1, copy); + assertTrue(push(moved).getItem().isSimilar(aged)); } @Test