From 329d0508fcae166dc55ccf976c7c5741924278e6 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Wed, 7 Oct 2026 17:49:10 +0200 Subject: [PATCH 1/2] fix: stack custom crop food onto produce that only aged differently Custom crop drops become Cooking food before pickup, so the vanilla pickup compared their creation clock and started a new stack for every harvest. Food pickups and the custom crop inventory rewrite now go through InventoryAdder, which ignores aging like the vanilla crop paths already do. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../crops/CropCustomCropsListener.java | 13 ++++---- .../cooking/manager/ConversionManager.java | 21 +++++++++++- .../cooking/utils/InventoryAdder.java | 33 ++++++++++++++----- .../cooking/crops/CropsCoverageTest.java | 7 ++++ .../manager/MealManagersCoverageTest.java | 20 +++++++++++ 5 files changed, 77 insertions(+), 17 deletions(-) diff --git a/src/main/java/net/tfminecraft/cooking/crops/CropCustomCropsListener.java b/src/main/java/net/tfminecraft/cooking/crops/CropCustomCropsListener.java index a0070f6..d87addd 100644 --- a/src/main/java/net/tfminecraft/cooking/crops/CropCustomCropsListener.java +++ b/src/main/java/net/tfminecraft/cooking/crops/CropCustomCropsListener.java @@ -24,6 +24,7 @@ import net.momirealms.customcrops.api.event.CropPlantEvent; import net.tfminecraft.cooking.Cooking; +import net.tfminecraft.cooking.utils.InventoryAdder; public final class CropCustomCropsListener implements Listener { @@ -127,15 +128,13 @@ private void scheduleInventoryRewrite(Player player, CropDefinition crop, int qu } ItemStack rewritten = CropHarvestItems.rewriteCustomDrop(candidate, crop, quality); if (rewritten == null || rewritten == candidate || rewritten.getType().isAir()) continue; - if (grewExisting) { - inventory.setItem(i, before[i].clone()); - additions.add(rewritten); - } else { - inventory.setItem(i, rewritten); - } + // Take the harvest back out and add it as food, so it joins produce that only aged differently. + inventory.setItem(i, grewExisting ? before[i].clone() : null); + additions.add(rewritten); } for (ItemStack addition : additions) { - for (ItemStack leftover : inventory.addItem(addition).values()) { + ItemStack leftover = InventoryAdder.addItem(online, addition); + if (leftover != null) { online.getWorld().dropItemNaturally(online.getLocation(), leftover); } } diff --git a/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java b/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java index 263d4c5..6ec7c7e 100644 --- a/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java +++ b/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java @@ -30,10 +30,13 @@ public class ConversionManager implements Listener { @EventHandler(ignoreCancelled = true) public void pickup(EntityPickupItemEvent e) { ItemStack item = e.getItem().getItemStack(); - if (FoodItem.fromItem(item) != null) return; if (!(e.getEntity() instanceof Player)) return; Player p = (Player) e.getEntity(); + if (FoodItem.fromItem(item) != null) { + stackOntoAgedFood(e, p, item); + return; + } if (replaceLegacyFish(e, p, item)) { return; } @@ -54,6 +57,22 @@ public void pickup(EntityPickupItemEvent e) { } } + /** + * Food on the ground keeps the clock it was made with, so a vanilla pickup starts a new stack + * beside an equal food that only aged differently. Add it the way Cooking adds food instead. + */ + private void stackOntoAgedFood(EntityPickupItemEvent event, Player player, ItemStack item) { + if (!InventoryAdder.hasStackFor(player, item)) return; + event.setCancelled(true); + ItemStack leftover = InventoryAdder.addItem(player, item.clone()); + if (leftover == null) { + event.getItem().remove(); + } else { + event.getItem().setItemStack(leftover); + } + player.playSound(player.getLocation(), Sound.ENTITY_ITEM_PICKUP, 1f, 1f); + } + private boolean replaceLegacyFish(EntityPickupItemEvent event, Player player, ItemStack item) { ItemStack stack = LegacyFishConversion.convert(player, item); if (stack == null) { diff --git a/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java b/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java index 446ab58..1fdc76e 100644 --- a/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java +++ b/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java @@ -22,15 +22,7 @@ public static ItemStack addItem(Player p, ItemStack in) { for (int slot = 0; slot < inv.getSize(); slot++) { ItemStack cur = inv.getItem(slot); - if (cur == null) continue; - - FoodItem fCur = FoodItem.fromItem(cur); - if (fCur == null) continue; - - if (!equalsFood(fIn, fCur)) continue; - if (!ConversionManager.sameApartFromAging(in, cur)) continue; - - int space = Math.min(cur.getMaxStackSize(), inv.getMaxStackSize()) - cur.getAmount(); + int space = room(inv, fIn, in, cur); if (space <= 0) continue; int add = Math.min(space, amount); @@ -48,6 +40,29 @@ public static ItemStack addItem(Player p, ItemStack in) { return left.values().iterator().next(); } + /** True when the player holds an equal food, differing at most in aging, with room for more. */ + public static boolean hasStackFor(Player p, ItemStack in) { + FoodItem fIn = FoodItem.fromItem(in); + if (fIn == null) return false; + PlayerInventory inv = p.getInventory(); + for (int slot = 0; slot < inv.getSize(); slot++) { + if (room(inv, fIn, in, inv.getItem(slot)) > 0) return true; + } + return false; + } + + private static int room(PlayerInventory inv, FoodItem fIn, ItemStack in, ItemStack cur) { + if (cur == null) return 0; + + FoodItem fCur = FoodItem.fromItem(cur); + if (fCur == null) return 0; + + if (!equalsFood(fIn, fCur)) return 0; + if (!ConversionManager.sameApartFromAging(in, cur)) return 0; + + return Math.min(cur.getMaxStackSize(), inv.getMaxStackSize()) - cur.getAmount(); + } + public static boolean equalsFood(FoodItem a, FoodItem b) { if (!a.getCategory().equals(b.getCategory())) return false; if (!a.getId().equals(b.getId())) return false; diff --git a/src/test/java/net/tfminecraft/cooking/crops/CropsCoverageTest.java b/src/test/java/net/tfminecraft/cooking/crops/CropsCoverageTest.java index 892a66a..9c70f07 100644 --- a/src/test/java/net/tfminecraft/cooking/crops/CropsCoverageTest.java +++ b/src/test/java/net/tfminecraft/cooking/crops/CropsCoverageTest.java @@ -163,6 +163,13 @@ class CropsCoverageTest { assertEquals(3,f.inventoryCount(Material.CARROT),"Harvesting two carrots must not convert three carrots already held");assertEquals(2,f.inventoryCount(Material.BREAD)); } + @Test void inventoryRewriteStacksTheHarvestOntoTheSameFoodThatAgedDifferently(){ + configureCustom();f.food("carrot",Material.BREAD);ConversionLoader.conversions.put("v.carrot","vegetable(type=carrot;origin=Carrot)"); + ItemStack held=CropHarvestItems.rewriteCustomDrop(new ItemStack(Material.CARROT,3),null,4);var meta=held.getItemMeta();meta.getPersistentDataContainer().set(Keys.LAST_UPDATE,org.bukkit.persistence.PersistentDataType.LONG,1L);held.setItemMeta(meta);f.player.getInventory().setItem(5,held); + try(MockedStatic quality=mockStatic(CropHarvestQuality.class)){quality.when(()->CropHarvestQuality.roll(anyString(),any(),any())).thenReturn(4);listener.onBreak(breakEvent(config("carrot"),new Location(f.world,0,65,0),f.player));f.player.getInventory().setItem(0,new ItemStack(Material.CARROT,2));f.server.getScheduler().performOneTick();} + assertNull(f.player.getInventory().getItem(0));assertEquals(5,f.player.getInventory().getItem(5).getAmount());assertEquals(5,f.inventoryCount(Material.BREAD)); + } + @Test void inventoryRewriteDoesNotConvertAStackThatShrankDuringHarvest(){ configureCustom();f.food("carrot",Material.BREAD);ConversionLoader.conversions.put("v.carrot","vegetable(type=carrot;origin=Carrot)");f.player.getInventory().setItem(0,new ItemStack(Material.CARROT,5));listener.onBreak(breakEvent(config("carrot"),new Location(f.world,0,65,0),f.player));f.player.getInventory().setItem(0,new ItemStack(Material.CARROT,3));f.server.getScheduler().performOneTick();assertEquals(3,f.inventoryCount(Material.CARROT));assertEquals(0,f.inventoryCount(Material.BREAD)); } diff --git a/src/test/java/net/tfminecraft/cooking/manager/MealManagersCoverageTest.java b/src/test/java/net/tfminecraft/cooking/manager/MealManagersCoverageTest.java index 408f973..bb94b71 100644 --- a/src/test/java/net/tfminecraft/cooking/manager/MealManagersCoverageTest.java +++ b/src/test/java/net/tfminecraft/cooking/manager/MealManagersCoverageTest.java @@ -344,6 +344,26 @@ void unsupportedPickupsMissingParsersAndCustomFishModelsStayInTheWorld() { var empty = pickup(new ItemStack(Material.WHEAT)); conversions.pickup(empty); assertFalse(empty.isCancelled()); } + @Test + void pickedUpFoodStacksOntoTheSameFoodThatOnlyAgedDifferently() { + ItemStack held = age(pickupOutput, 100L, "freshness.1"); held.setAmount(3); env.player.getInventory().setItem(4, held); + ItemStack ground = age(pickupOutput, 900L, "freshness.4"); ground.setAmount(2); + var event = pickup(ground); conversions.pickup(event); + assertTrue(event.isCancelled()); verify(event.getItem()).remove(); + assertEquals(5, env.player.getInventory().getItem(4).getAmount()); assertNull(env.player.getInventory().getItem(0)); + } + + @Test + void pickedUpFoodThatDoesNotAllFitStaysOnTheGroundWithTheRest() { + ItemStack ground = age(pickupOutput, 900L, "freshness.4"); ground.setAmount(5); ItemStack rest = ground.clone(); rest.setAmount(2); + try (MockedStatic delivery = mockStatic(InventoryAdder.class)) { + delivery.when(() -> InventoryAdder.hasStackFor(any(), any())).thenReturn(true); + delivery.when(() -> InventoryAdder.addItem(any(), any())).thenReturn(rest); + var event = pickup(ground); conversions.pickup(event); + assertTrue(event.isCancelled()); verify(event.getItem()).setItemStack(rest); verify(event.getItem(), never()).remove(); + } + } + @ParameterizedTest @ValueSource(strings = {"null", "air", "legacy-air", "fish-air"}) void unusablePickupReplacementsKeepTheOriginalItemEntity(String failure) { From 9988ea3693af021d68fdb90215a6f392c2fe7855 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:39:58 +0200 Subject: [PATCH 2/2] fix: keep the rest of a partial food pickup and offer pickups vanilla skips Paper shrinks the ground item to what fits while the pickup event runs, so adding only that amount and removing the entity lost the rest. Use the remaining count too, also for raw conversions. When no slot fits the food as it is, Paper fires no pickup event; offer one from the attempt event when an equal food that aged differently has room. Leave items owned by another player to vanilla, which refuses them after the events. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../cooking/manager/ConversionManager.java | 39 ++++++++++++++++++- .../cooking/utils/InventoryAdder.java | 3 +- .../manager/MealManagersCoverageTest.java | 39 +++++++++++++++++++ 3 files changed, 78 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 6ec7c7e..41bbd66 100644 --- a/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java +++ b/src/main/java/net/tfminecraft/cooking/manager/ConversionManager.java @@ -2,11 +2,17 @@ import net.tfminecraft.cooking.util.LegacyModelData; +import java.util.UUID; + +import org.bukkit.Bukkit; import org.bukkit.Sound; +import org.bukkit.entity.Item; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; import org.bukkit.event.entity.EntityPickupItemEvent; +import org.bukkit.event.player.PlayerAttemptPickupItemEvent; import org.bukkit.event.inventory.InventoryOpenEvent; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.InventoryHolder; @@ -33,6 +39,8 @@ public void pickup(EntityPickupItemEvent e) { if (!(e.getEntity() instanceof Player)) return; Player p = (Player) e.getEntity(); + // Paper fires pickup events before it refuses an item that belongs to another player. + if (!mayTake(p, e.getItem())) return; if (FoodItem.fromItem(item) != null) { stackOntoAgedFood(e, p, item); return; @@ -57,6 +65,31 @@ public void pickup(EntityPickupItemEvent e) { } } + /** + * Paper only fires the pickup event when the item fits as it is. Food that only fits on an equal + * food that aged differently never gets one, so offer it, leaving other plugins their say. + */ + @EventHandler(priority = EventPriority.HIGH, ignoreCancelled = true) + public void attemptPickup(PlayerAttemptPickupItemEvent e) { + Item entity = e.getItem(); + ItemStack item = entity.getItemStack(); + if (e.getRemaining() < item.getAmount()) return; + Player player = e.getPlayer(); + if (!player.getCanPickupItems() || !mayTake(player, entity)) return; + if (!InventoryAdder.hasStackFor(player, item)) return; + Bukkit.getPluginManager().callEvent(new EntityPickupItemEvent(player, entity, 0)); + } + + private static boolean mayTake(Player player, Item entity) { + UUID owner = entity.getOwner(); + return owner == null || owner.equals(player.getUniqueId()); + } + + /** Paper shrinks the ground item to what fits while the event runs; the rest is in getRemaining. */ + private static int groundAmount(EntityPickupItemEvent event) { + return event.getItem().getItemStack().getAmount() + event.getRemaining(); + } + /** * Food on the ground keeps the clock it was made with, so a vanilla pickup starts a new stack * beside an equal food that only aged differently. Add it the way Cooking adds food instead. @@ -64,7 +97,9 @@ public void pickup(EntityPickupItemEvent e) { private void stackOntoAgedFood(EntityPickupItemEvent event, Player player, ItemStack item) { if (!InventoryAdder.hasStackFor(player, item)) return; event.setCancelled(true); - ItemStack leftover = InventoryAdder.addItem(player, item.clone()); + ItemStack ground = item.clone(); + ground.setAmount(groundAmount(event)); + ItemStack leftover = InventoryAdder.addItem(player, ground); if (leftover == null) { event.getItem().remove(); } else { @@ -108,7 +143,7 @@ private static boolean hasCustomModelData(ItemStack item) { private void giveConverted(EntityPickupItemEvent event, Player player, ItemStack stack) { if (stack == null || stack.getType().isAir()) return; - stack.setAmount(event.getItem().getItemStack().getAmount()); + stack.setAmount(groundAmount(event)); event.setCancelled(true); event.getItem().remove(); ItemStack leftover = InventoryAdder.addItem(player, stack); diff --git a/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java b/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java index 1fdc76e..8bbec76 100644 --- a/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java +++ b/src/main/java/net/tfminecraft/cooking/utils/InventoryAdder.java @@ -52,7 +52,8 @@ public static boolean hasStackFor(Player p, ItemStack in) { } private static int room(PlayerInventory inv, FoodItem fIn, ItemStack in, ItemStack cur) { - if (cur == null) return 0; + // Equal foods share a material; checking it first keeps per-tick pickup attempts cheap. + if (cur == null || cur.getType() != in.getType()) return 0; FoodItem fCur = FoodItem.fromItem(cur); if (fCur == null) return 0; diff --git a/src/test/java/net/tfminecraft/cooking/manager/MealManagersCoverageTest.java b/src/test/java/net/tfminecraft/cooking/manager/MealManagersCoverageTest.java index bb94b71..b44a7eb 100644 --- a/src/test/java/net/tfminecraft/cooking/manager/MealManagersCoverageTest.java +++ b/src/test/java/net/tfminecraft/cooking/manager/MealManagersCoverageTest.java @@ -12,6 +12,7 @@ import org.bukkit.Material; import org.bukkit.block.BlockState; import org.bukkit.entity.Item; +import org.bukkit.event.player.PlayerAttemptPickupItemEvent; import org.bukkit.entity.LivingEntity; import org.bukkit.entity.Player; import org.bukkit.event.entity.EntityPickupItemEvent; @@ -353,6 +354,44 @@ void pickedUpFoodStacksOntoTheSameFoodThatOnlyAgedDifferently() { assertEquals(5, env.player.getInventory().getItem(4).getAmount()); assertNull(env.player.getInventory().getItem(0)); } + @Test + void aPartialPickupAddsTheWholeGroundStackNotJustWhatPaperSaidFits() { + ItemStack held = age(pickupOutput, 100L, "freshness.1"); held.setAmount(3); env.player.getInventory().setItem(4, held); + ItemStack fits = age(pickupOutput, 900L, "freshness.4"); fits.setAmount(2); + Item item = mock(Item.class); when(item.getItemStack()).thenReturn(fits); + var event = new EntityPickupItemEvent(env.player, item, 4); conversions.pickup(event); + assertTrue(event.isCancelled()); verify(item).remove(); assertEquals(9, env.player.getInventory().getItem(4).getAmount()); + } + + @Test + void foodOwnedByAnotherPlayerIsLeftForVanillaToRefuse() { + ItemStack held = age(pickupOutput, 100L, "freshness.1"); env.player.getInventory().setItem(4, held); + var owned = pickup(age(pickupOutput, 900L, "freshness.4")); when(owned.getItem().getOwner()).thenReturn(UUID.randomUUID()); + conversions.pickup(owned); assertFalse(owned.isCancelled()); assertEquals(1, env.player.getInventory().getItem(4).getAmount()); + UUID me = UUID.randomUUID(); when(env.player.getUniqueId()).thenReturn(me); + var mine = pickup(age(pickupOutput, 900L, "freshness.4")); when(mine.getItem().getOwner()).thenReturn(me); + conversions.pickup(mine); assertTrue(mine.isCancelled()); assertEquals(2, env.player.getInventory().getItem(4).getAmount()); + } + + @Test + void attemptedPickupWithNoVanillaRoomOffersFoodToItsAgedStack() { + env.server.getPluginManager().registerEvents(conversions, org.mockbukkit.mockbukkit.MockBukkit.createMockPlugin("AttemptPickup")); + ItemStack held = age(pickupOutput, 100L, "freshness.1"); held.setAmount(3); env.player.getInventory().setItem(4, held); + ItemStack ground = age(pickupOutput, 900L, "freshness.4"); ground.setAmount(2); + Item item = mock(Item.class); when(item.getItemStack()).thenReturn(ground); + conversions.attemptPickup(new PlayerAttemptPickupItemEvent(env.player, item, 1)); + assertEquals(3, env.player.getInventory().getItem(4).getAmount(), "Paper fires its own pickup when some of it fits"); + Item other = mock(Item.class); when(other.getItemStack()).thenReturn(ground.clone()); when(other.getOwner()).thenReturn(UUID.randomUUID()); + when(env.player.getCanPickupItems()).thenReturn(true); conversions.attemptPickup(new PlayerAttemptPickupItemEvent(env.player, other, 2)); + Item stone = mock(Item.class); when(stone.getItemStack()).thenReturn(new ItemStack(Material.STONE)); + conversions.attemptPickup(new PlayerAttemptPickupItemEvent(env.player, stone, 1)); + when(env.player.getCanPickupItems()).thenReturn(false); + conversions.attemptPickup(new PlayerAttemptPickupItemEvent(env.player, item, 2)); + assertEquals(3, env.player.getInventory().getItem(4).getAmount(), "A player who cannot pick items up is left alone"); + when(env.player.getCanPickupItems()).thenReturn(true); conversions.attemptPickup(new PlayerAttemptPickupItemEvent(env.player, item, 2)); + assertEquals(5, env.player.getInventory().getItem(4).getAmount()); verify(item).remove(); verify(other, never()).remove(); + } + @Test void pickedUpFoodThatDoesNotAllFitStaysOnTheGroundWithTheRest() { ItemStack ground = age(pickupOutput, 900L, "freshness.4"); ground.setAmount(5); ItemStack rest = ground.clone(); rest.setAmount(2);