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..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; @@ -30,10 +36,15 @@ 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(); + // 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; + } if (replaceLegacyFish(e, p, item)) { return; } @@ -54,6 +65,49 @@ 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. + */ + private void stackOntoAgedFood(EntityPickupItemEvent event, Player player, ItemStack item) { + if (!InventoryAdder.hasStackFor(player, item)) return; + event.setCancelled(true); + ItemStack ground = item.clone(); + ground.setAmount(groundAmount(event)); + ItemStack leftover = InventoryAdder.addItem(player, ground); + 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) { @@ -89,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 446ab58..8bbec76 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,30 @@ 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) { + // 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; + + 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..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; @@ -344,6 +345,64 @@ 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 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); + 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) {