new event McMMOModifyBlockDropItemEvent and mcMMO now modifies the drop list in BlockDropItemEvent instead of spawning items

Fixes #5214

Changelog.txt

@@ -1,4 +1,7 @@
 Version 2.2.042
+    mcMMO now listens to BlockDropItemEvent at LOW priority instead of HIGHEST
+    Bonus drops from mcMMO now simply modify quantity in BlockDropItemEvent
+    Added McMMOModifyBlockDropItemEvent event, this event is called when mcMMO modifies the quantity of an ItemStack during a BlockDropItemEvent, it is modifiable and cancellable.
     You can now define custom sounds to be played in sounds.yml (Thank you JeBobs, see notes)
     Added a cap to how much Blast Mining PVP damage can do to other players
 

src/main/java/com/gmail/nossr50/events/items/McMMOModifyBlockDropItemEvent.java

@@ -0,0 +1,226 @@
+package com.gmail.nossr50.events.items;
+
+import static java.util.Objects.requireNonNull;
+
+import org.bukkit.block.Block;
+import org.bukkit.block.BlockState;
+import org.bukkit.entity.Item;
+import org.bukkit.entity.Player;
+import org.bukkit.event.Cancellable;
+import org.bukkit.event.Event;
+import org.bukkit.event.HandlerList;
+import org.bukkit.event.block.BlockDropItemEvent;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Called when mcMMO is modifying the amount of bonus drops to add to an Item involved in a {@link BlockDropItemEvent}.
+ * <p>
+ * This event is called before mcMMO has modified the ItemStack quantity on the {@link Item} entity.
+ * <p>
+ * This event is called once per Item entity that is involved in the {@link BlockDropItemEvent}.
+ * <p>
+ * This event is called during mcMMO logic on the {@link BlockDropItemEvent}, and can be used to
+ * modify the quantity that mcMMO will add to the ItemStack.
+ * <p>
+ * This event is considered cancelled if it is either cancelled directly or if bonus drops are 0 or
+ * less.
+ */
+public class McMMOModifyBlockDropItemEvent extends Event implements Cancellable {
+    private final @NotNull BlockDropItemEvent blockDropItemEvent;
+    private final int originalBonusAmountToAdd;
+    private int modifiedItemStackQuantity;
+    private final @NotNull Item itemThatHasBonusDrops;
+    private boolean isCancelled = false;
+    private final int originalItemStackQuantity;
+
+    public McMMOModifyBlockDropItemEvent(@NotNull BlockDropItemEvent blockDropItemEvent,
+            @NotNull Item itemThatHasBonusDrops, int bonusDropsToAdd) {
+        super(false);
+        requireNonNull(blockDropItemEvent, "blockDropItemEvent cannot be null");
+        requireNonNull(itemThatHasBonusDrops, "itemThatHasBonusDrops cannot be null");
+        if (bonusDropsToAdd <= 0) {
+            throw new IllegalArgumentException("cannot instantiate a new"
+                    + " McMMOModifyBlockDropItemEvent with a bonusDropsToAdd that is <= 0");
+        }
+        this.blockDropItemEvent = blockDropItemEvent;
+        this.itemThatHasBonusDrops = itemThatHasBonusDrops;
+        this.originalItemStackQuantity = itemThatHasBonusDrops.getItemStack().getAmount();
+        this.originalBonusAmountToAdd = bonusDropsToAdd;
+        this.modifiedItemStackQuantity = itemThatHasBonusDrops.getItemStack().getAmount()
+                + bonusDropsToAdd;
+    }
+
+    @Override
+    public boolean isCancelled() {
+        return isCancelled;
+    }
+
+    @Override
+    public void setCancelled(boolean cancel) {
+        this.isCancelled = cancel;
+    }
+
+    /**
+     * The original BlockDropItemEvent which caused this event to be fired.
+     * @return the original BlockDropItemEvent
+     */
+    public @NotNull BlockDropItemEvent getBlockDropItemEvent() {
+        return blockDropItemEvent;
+    }
+
+    /**
+     * The original bonus mcMMO would have added before any modifications to this event from
+     * other plugins.
+     * @return the original bonus amount to add
+     */
+    public int getOriginalBonusAmountToAdd() {
+        return originalBonusAmountToAdd;
+    }
+
+    /**
+     * The Item entity that is being modified by this event.
+     * This item returned by this call should not be modified, it is provided as a convenience.
+     * @return the Item entity that is having bonus drops added to it.
+     */
+    public @NotNull Item getItem() {
+        return itemThatHasBonusDrops;
+    }
+
+
+    /**
+     * The modified ItemStack quantity that will be set on the Item entity if this event is not
+     * cancelled.
+     *
+     * @return the modified ItemStack quantity that will be set on the Item entity
+     */
+    public int getModifiedItemStackQuantity() {
+        return modifiedItemStackQuantity;
+    }
+
+    /**
+     * The original ItemStack quantity of the Item entity before any modifications from this event.
+     * This is a reflection of the state of the Item when mcMMO fired this event.
+     * It is possible it has modified since then, so do not rely on this value to be the current.
+     * @return the original ItemStack quantity of the Item entity before any modifications from this event
+     */
+    public int getOriginalItemStackQuantity() {
+        return originalItemStackQuantity;
+    }
+
+    /**
+     * The amount of bonus that will be added to the ItemStack quantity if this event is not
+     * cancelled.
+     * @return the amount of bonus that will be added to the ItemStack quantity
+     */
+    public int getBonusAmountToAdd() {
+        return Math.max(0, modifiedItemStackQuantity - originalItemStackQuantity);
+    }
+
+    /**
+     * Set the amount of bonus that will be added to the ItemStack quantity if this event is not
+     * cancelled.
+     * @param bonus the amount of bonus that will be added to the ItemStack quantity
+     * @throws IllegalArgumentException if bonus is less than 0
+     */
+    public void setBonusAmountToAdd(int bonus) {
+        if (bonus < 0) throw new IllegalArgumentException("bonus must be >= 0");
+        this.modifiedItemStackQuantity = originalItemStackQuantity + bonus;
+    }
+
+    /**
+     * Set the modified ItemStack quantity that will be set on the Item entity if this event is not
+     * cancelled. This CANNOT be lower than the original quantity of the ItemStack.
+     * @param modifiedItemStackQuantity the modified ItemStack quantity that will be set on the Item entity
+     * @throws IllegalArgumentException if modifiedItemStackQuantity is less than originalItemStackQuantity
+     */
+    public void setModifiedItemStackQuantity(int modifiedItemStackQuantity) {
+        if (modifiedItemStackQuantity < originalItemStackQuantity) {
+            throw new IllegalArgumentException(
+                    "modifiedItemStackQuantity cannot be less than the originalItemStackQuantity");
+        }
+        this.modifiedItemStackQuantity = modifiedItemStackQuantity;
+    }
+
+    public boolean isEffectivelyNoBonus() {
+        return modifiedItemStackQuantity == originalItemStackQuantity;
+    }
+
+    /**
+     * Delegate method for {@link BlockDropItemEvent}, gets the Player that is breaking the block
+     * involved in this event.
+     *
+     * @return The Player that is breaking the block involved in this event
+     */
+    public @NotNull Player getPlayer() {
+        return blockDropItemEvent.getPlayer();
+    }
+
+    /**
+     * Delegate method for {@link BlockDropItemEvent#getBlock()}.
+     * Gets the Block involved in this event.
+     *
+     * @return the Block involved in this event
+     */
+    public @NotNull Block getBlock() {
+        return blockDropItemEvent.getBlock();
+    }
+
+    /**
+     * Delegate method for {@link BlockDropItemEvent#getBlockState()}.
+     * Gets the BlockState of the block involved in this event.
+     *
+     * @return the BlockState of the block involved in this event
+     */
+    public @NotNull BlockState getBlockState() {
+        return blockDropItemEvent.getBlockState();
+    }
+
+    private static final @NotNull HandlerList handlers = new HandlerList();
+
+    @Override
+    public @NotNull HandlerList getHandlers() {
+        return handlers;
+    }
+
+    public static @NotNull HandlerList getHandlerList() {
+        return handlers;
+    }
+
+    @Override
+    public @NotNull String toString() {
+        return "McMMOModifyBlockDropItemEvent{" +
+                "blockDropItemEvent=" + blockDropItemEvent +
+                ", originalBonusAmountToAdd=" + originalBonusAmountToAdd +
+                ", modifiedItemStackQuantity=" + modifiedItemStackQuantity +
+                ", itemThatHasBonusDrops=" + itemThatHasBonusDrops +
+                ", isCancelled=" + isCancelled +
+                ", originalItemStackQuantity=" + originalItemStackQuantity +
+                '}';
+    }
+
+    @Override
+    public final boolean equals(Object o) {
+        if (!(o instanceof McMMOModifyBlockDropItemEvent that)) {
+            return false;
+        }
+
+        return originalBonusAmountToAdd == that.originalBonusAmountToAdd
+                && modifiedItemStackQuantity == that.modifiedItemStackQuantity
+                && isCancelled == that.isCancelled
+                && originalItemStackQuantity == that.originalItemStackQuantity
+                && blockDropItemEvent.equals(that.blockDropItemEvent)
+                && itemThatHasBonusDrops.equals(
+                that.itemThatHasBonusDrops);
+    }
+
+    @Override
+    public int hashCode() {
+        int result = blockDropItemEvent.hashCode();
+        result = 31 * result + originalBonusAmountToAdd;
+        result = 31 * result + modifiedItemStackQuantity;
+        result = 31 * result + itemThatHasBonusDrops.hashCode();
+        result = 31 * result + Boolean.hashCode(isCancelled);
+        result = 31 * result + originalItemStackQuantity;
+        return result;
+    }
+}

src/main/java/com/gmail/nossr50/listeners/BlockListener.java

@@ -1,5 +1,6 @@
 package com.gmail.nossr50.listeners;
 
+import static com.gmail.nossr50.util.MetadataConstants.METADATA_KEY_BONUS_DROPS;
 import static com.gmail.nossr50.util.Misc.getBlockCenter;
 
 import com.gmail.nossr50.api.ItemSpawnReason;
@@ -14,6 +15,7 @@ import com.gmail.nossr50.datatypes.skills.ToolType;
 import com.gmail.nossr50.events.fake.FakeBlockBreakEvent;
 import com.gmail.nossr50.events.fake.FakeBlockDamageEvent;
 import com.gmail.nossr50.events.fake.FakeEvent;
+import com.gmail.nossr50.events.items.McMMOModifyBlockDropItemEvent;
 import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.skills.alchemy.Alchemy;
 import com.gmail.nossr50.skills.excavation.ExcavationManager;
@@ -35,6 +37,8 @@ import com.gmail.nossr50.util.sounds.SoundType;
 import com.gmail.nossr50.worldguard.WorldGuardManager;
 import com.gmail.nossr50.worldguard.WorldGuardUtils;
 import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
 import org.bukkit.ChatColor;
 import org.bukkit.GameMode;
 import org.bukkit.Location;
@@ -62,6 +66,7 @@ import org.bukkit.event.block.BlockPistonRetractEvent;
 import org.bukkit.event.block.BlockPlaceEvent;
 import org.bukkit.event.block.EntityBlockFormEvent;
 import org.bukkit.inventory.ItemStack;
+import org.bukkit.metadata.MetadataValue;
 
 public class BlockListener implements Listener {
     private final mcMMO plugin;
@@ -70,88 +75,96 @@ public class BlockListener implements Listener {
         this.plugin = plugin;
     }
 
-    @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false)
+    @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = false)
     public void onBlockDropItemEvent(BlockDropItemEvent event) {
         //Make sure we clean up metadata on these blocks
+        final Block block = event.getBlock();
         if (event.isCancelled()) {
-            if (event.getBlock().hasMetadata(MetadataConstants.METADATA_KEY_BONUS_DROPS)) {
-                event.getBlock().removeMetadata(MetadataConstants.METADATA_KEY_BONUS_DROPS, plugin);
+            if (block.hasMetadata(METADATA_KEY_BONUS_DROPS)) {
+                block.removeMetadata(METADATA_KEY_BONUS_DROPS, plugin);
             }
             return;
         }
 
-        int tileEntityTolerance = 1;
+        try {
+            int tileEntityTolerance = 1;
 
-        // beetroot hotfix, potentially other plants may need this fix
-        if (event.getBlock().getType() == Material.BEETROOTS) {
-            tileEntityTolerance = 2;
-        }
+            // beetroot hotfix, potentially other plants may need this fix
+            final Material blockType = block.getType();
+            if (blockType == Material.BEETROOTS) {
+                tileEntityTolerance = 2;
+            }
 
-        //Track how many "things" are being dropped
-        HashSet<Material> uniqueMaterials = new HashSet<>();
-        boolean dontRewardTE = false; //If we suspect TEs are mixed in with other things don't reward bonus drops for anything that isn't a block
-        int blockCount = 0;
+            //Track how many "things" are being dropped
+            final Set<Material> uniqueMaterials = new HashSet<>();
+            boolean dontRewardTE = false; //If we suspect TEs are mixed in with other things don't reward bonus drops for anything that isn't a block
+            int blockCount = 0;
 
-        for (Item item : event.getItems()) {
-            //Track unique materials
-            uniqueMaterials.add(item.getItemStack().getType());
+            final List<Item> eventItems = event.getItems();
+            for (Item item : eventItems) {
+                //Track unique materials
+                uniqueMaterials.add(item.getItemStack().getType());
 
-            //Count blocks as a second failsafe
-            if (item.getItemStack().getType().isBlock()) {
-                blockCount++;
+                //Count blocks as a second failsafe
+                if (item.getItemStack().getType().isBlock()) {
+                    blockCount++;
+                }
             }
-        }
 
-        if (uniqueMaterials.size() > tileEntityTolerance) {
-            //Too many things are dropping, assume tile entities might be duped
-            //Technically this would also prevent something like coal from being bonus dropped if you placed a TE above a coal ore when mining it but that's pretty edge case and this is a good solution for now
-            dontRewardTE = true;
-        }
-
-        //If there are more than one block in the item list we can't really trust it and will back out of rewarding bonus drops
-        if (blockCount <= 1) {
-            for (Item item : event.getItems()) {
-                ItemStack is = new ItemStack(item.getItemStack());
+            if (uniqueMaterials.size() > tileEntityTolerance) {
+                // Too many things are dropping, assume tile entities might be duped
+                // Technically this would also prevent something like coal from being bonus dropped
+                // if you placed a TE above a coal ore when mining it but that's pretty edge case
+                // and this is a good solution for now
+                dontRewardTE = true;
+            }
 
-                if (is.getAmount() <= 0) {
-                    continue;
-                }
+            //If there are more than one block in the item list we can't really trust it
+            // and will back out of rewarding bonus drops
+            if (!block.getMetadata(METADATA_KEY_BONUS_DROPS).isEmpty()) {
+                final MetadataValue bonusDropMeta = block
+                        .getMetadata(METADATA_KEY_BONUS_DROPS).get(0);
+                if (blockCount <= 1) {
+                    for (final Item item : eventItems) {
+                        final ItemStack eventItemStack = item.getItemStack();
+                        int originalAmount = eventItemStack.getAmount();
+
+                        if (eventItemStack.getAmount() <= 0) {
+                            continue;
+                        }
 
-                //TODO: Ignore this abomination its rewritten in 2.2
-                if (!mcMMO.p.getGeneralConfig()
-                        .getDoubleDropsEnabled(PrimarySkillType.MINING, is.getType())
-                        && !mcMMO.p.getGeneralConfig()
-                        .getDoubleDropsEnabled(PrimarySkillType.HERBALISM, is.getType())
-                        && !mcMMO.p.getGeneralConfig()
-                        .getDoubleDropsEnabled(PrimarySkillType.WOODCUTTING, is.getType())) {
-                    continue;
-                }
+                        final Material itemType = eventItemStack.getType();
+                        if (!mcMMO.p.getGeneralConfig()
+                                .getDoubleDropsEnabled(PrimarySkillType.MINING, itemType)
+                                && !mcMMO.p.getGeneralConfig()
+                                .getDoubleDropsEnabled(PrimarySkillType.HERBALISM, itemType)
+                                && !mcMMO.p.getGeneralConfig()
+                                .getDoubleDropsEnabled(PrimarySkillType.WOODCUTTING, itemType)) {
+                            continue;
+                        }
 
-                //If we suspect TEs might be duped only reward block
-                if (dontRewardTE) {
-                    if (!is.getType().isBlock()) {
-                        continue;
-                    }
-                }
+                        //If we suspect TEs might be duped only reward block
+                        if (dontRewardTE) {
+                            if (!itemType.isBlock()) {
+                                continue;
+                            }
+                        }
 
-                if (event.getBlock().getMetadata(MetadataConstants.METADATA_KEY_BONUS_DROPS).size()
-                        > 0) {
-                    final BonusDropMeta bonusDropMeta =
-                            (BonusDropMeta) event.getBlock().getMetadata(
-                                    MetadataConstants.METADATA_KEY_BONUS_DROPS).get(0);
-                    int bonusCount = bonusDropMeta.asInt();
-                    final Location centeredLocation = getBlockCenter(event.getBlock());
-                    for (int i = 0; i < bonusCount; i++) {
-
-                        ItemUtils.spawnItemNaturally(event.getPlayer(),
-                                centeredLocation, is, ItemSpawnReason.BONUS_DROPS);
+                        int amountToAddFromBonus = bonusDropMeta.asInt();
+                        final McMMOModifyBlockDropItemEvent modifyBlockDropItemEvent
+                                = new McMMOModifyBlockDropItemEvent(event, item, amountToAddFromBonus);
+                        plugin.getServer().getPluginManager().callEvent(modifyBlockDropItemEvent);
+                        if (!modifyBlockDropItemEvent.isCancelled()
+                                && modifyBlockDropItemEvent.getModifiedItemStackQuantity() > originalAmount) {
+                            eventItemStack.setAmount(modifyBlockDropItemEvent.getModifiedItemStackQuantity());
+                        }
                     }
                 }
             }
-        }
-
-        if (event.getBlock().hasMetadata(MetadataConstants.METADATA_KEY_BONUS_DROPS)) {
-            event.getBlock().removeMetadata(MetadataConstants.METADATA_KEY_BONUS_DROPS, plugin);
+        } finally {
+            if (block.hasMetadata(METADATA_KEY_BONUS_DROPS)) {
+                block.removeMetadata(METADATA_KEY_BONUS_DROPS, plugin);
+            }
         }
     }
 

src/test/java/com/gmail/nossr50/events/items/McMMOModifyBlockDropItemEventTest.java

@@ -0,0 +1,252 @@
+package com.gmail.nossr50.events.items;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.bukkit.block.Block;
+import org.bukkit.block.BlockState;
+import org.bukkit.entity.Item;
+import org.bukkit.entity.Player;
+import org.bukkit.event.HandlerList;
+import org.bukkit.event.block.BlockDropItemEvent;
+import org.bukkit.inventory.ItemStack;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+class McMMOModifyBlockDropItemEventTest {
+    private BlockDropItemEvent blockDropItemEvent;
+    private Item itemEntity;
+    private ItemStack itemStack;
+    private Player player;
+    private Block block;
+    private BlockState blockState;
+
+    @BeforeEach
+    void setUp() {
+        // Mocks for delegate passthroughs
+        player = mock(Player.class, RETURNS_DEEP_STUBS);
+        block = mock(Block.class, RETURNS_DEEP_STUBS);
+        blockState = mock(BlockState.class, RETURNS_DEEP_STUBS);
+
+        // Primary Bukkit event mock
+        blockDropItemEvent = mock(BlockDropItemEvent.class, RETURNS_DEEP_STUBS);
+        when(blockDropItemEvent.getPlayer()).thenReturn(player);
+        when(blockDropItemEvent.getBlock()).thenReturn(block);
+        when(blockDropItemEvent.getBlockState()).thenReturn(blockState);
+
+        // Item + ItemStack mock
+        itemStack = mock(ItemStack.class);
+        when(itemStack.getAmount()).thenReturn(3); // original count
+        itemEntity = mock(Item.class);
+        when(itemEntity.getItemStack()).thenReturn(itemStack);
+    }
+
+    private McMMOModifyBlockDropItemEvent newEvent(int bonus) {
+        return new McMMOModifyBlockDropItemEvent(blockDropItemEvent, itemEntity, bonus);
+    }
+
+    @Nested
+    @DisplayName("Constructor & validation")
+    class ConstructorValidation {
+
+        @Test
+        void ctorNullEventThrows() {
+            assertThrows(NullPointerException.class,
+                    () -> new McMMOModifyBlockDropItemEvent(null, itemEntity, 1));
+        }
+
+        @Test
+        void ctorNullItemThrows() {
+            assertThrows(NullPointerException.class,
+                    () -> new McMMOModifyBlockDropItemEvent(blockDropItemEvent, null, 1));
+        }
+
+        @Test
+        void ctorZeroBonusThrows() {
+            assertThrows(IllegalArgumentException.class,
+                    () -> new McMMOModifyBlockDropItemEvent(blockDropItemEvent, itemEntity, 0));
+        }
+
+        @Test
+        void ctorNegativeBonusThrows() {
+            assertThrows(IllegalArgumentException.class,
+                    () -> new McMMOModifyBlockDropItemEvent(blockDropItemEvent, itemEntity, -5));
+        }
+
+        @Test
+        void ctorSetsOriginalsAndModifiedCorrectly() {
+            // original amount = 3, bonus = 2
+            McMMOModifyBlockDropItemEvent ev = newEvent(2);
+            assertEquals(3, ev.getOriginalItemStackQuantity());
+            assertEquals(2, ev.getOriginalBonusAmountToAdd());
+            assertEquals(5, ev.getModifiedItemStackQuantity());
+            assertFalse(ev.isCancelled());
+            assertFalse(ev.isEffectivelyNoBonus());
+        }
+    }
+
+    @Nested
+    @DisplayName("Cancellable contract")
+    class Cancellation {
+        @Test
+        void cancelAndUncancel() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1);
+            assertFalse(ev.isCancelled());
+            ev.setCancelled(true);
+            assertTrue(ev.isCancelled());
+            ev.setCancelled(false);
+            assertFalse(ev.isCancelled());
+        }
+    }
+
+    @Nested
+    @DisplayName("Delta & absolute quantity semantics")
+    class DeltaAndAbsolute {
+
+        @Test
+        void getBonusAmountToAddReflectsDifferenceFromOriginal() {
+            // original 3, bonus 4 => modified 7
+            McMMOModifyBlockDropItemEvent ev = newEvent(4);
+            assertEquals(4, ev.getBonusAmountToAdd());
+            assertEquals(7, ev.getModifiedItemStackQuantity());
+        }
+
+        @Test
+        void setBonusAmountToAddUpdatesModifiedQuantity() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(2); // original 3 -> modified 5
+            ev.setBonusAmountToAdd(10); // new modified should be 13
+            assertEquals(13, ev.getModifiedItemStackQuantity());
+            assertEquals(10, ev.getBonusAmountToAdd());
+            assertFalse(ev.isEffectivelyNoBonus());
+        }
+
+        @Test
+        void setBonusAmountToAddNegativeThrows() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1);
+            assertThrows(IllegalArgumentException.class, () -> ev.setBonusAmountToAdd(-1));
+        }
+
+        @Test
+        void setModifiedItemStackQuantityEqualToOriginalIsNoBonus() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(2); // 3 -> 5
+            ev.setModifiedItemStackQuantity(3); // back to original => no bonus
+            assertEquals(3, ev.getModifiedItemStackQuantity());
+            assertEquals(0, ev.getBonusAmountToAdd());
+            assertTrue(ev.isEffectivelyNoBonus());
+        }
+
+        @Test
+        void setModifiedItemStackQuantityLessThanOriginalThrows() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1);
+            assertThrows(IllegalArgumentException.class, () -> ev.setModifiedItemStackQuantity(2)); // original is 3
+        }
+
+        @Test
+        void setModifiedItemStackQuantityGreaterThanOriginalUpdatesBonus() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1); // original 3 -> modified 4
+            ev.setModifiedItemStackQuantity(12);
+            assertEquals(12, ev.getModifiedItemStackQuantity());
+            assertEquals(9, ev.getBonusAmountToAdd()); // 12 - 3
+            assertFalse(ev.isEffectivelyNoBonus());
+        }
+    }
+
+    @Nested
+    @DisplayName("Delegate passthroughs")
+    class Delegates {
+
+        @Test
+        void getPlayerPassthrough() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1);
+            assertSame(player, ev.getPlayer());
+            verify(blockDropItemEvent, atLeastOnce()).getPlayer();
+        }
+
+        @Test
+        void getBlockPassthrough() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1);
+            assertSame(block, ev.getBlock());
+            verify(blockDropItemEvent, atLeastOnce()).getBlock();
+        }
+
+        @Test
+        void getBlockStatePassthrough() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1);
+            assertSame(blockState, ev.getBlockState());
+            verify(blockDropItemEvent, atLeastOnce()).getBlockState();
+        }
+
+        @Test
+        void getItemReturnsOriginalItemEntity() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1);
+            assertSame(itemEntity, ev.getItem());
+        }
+    }
+
+    @Nested
+    @DisplayName("HandlerList plumbing")
+    class HandlerListTests {
+        @Test
+        void handlerList_isNonNull_andShared() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(1);
+            HandlerList fromInstance = ev.getHandlers();
+            HandlerList fromStatic = McMMOModifyBlockDropItemEvent.getHandlerList();
+            assertNotNull(fromInstance);
+            assertNotNull(fromStatic);
+            // Bukkit convention: same static instance
+            assertSame(fromStatic, fromInstance);
+        }
+    }
+
+    @Nested
+    @DisplayName("Object contracts")
+    class ObjectContracts {
+
+        @Test
+        void toStringContainsKeyFields() {
+            McMMOModifyBlockDropItemEvent ev = newEvent(2);
+            String s = ev.toString();
+            assertNotNull(s);
+            assertTrue(s.contains("originalBonusAmountToAdd=2"));
+            assertTrue(s.contains("modifiedItemStackQuantity=5"));
+        }
+
+        @Test
+        void equalsAndHashCodeReflectState() {
+            // Same inputs => equal (mocks are same instances)
+            McMMOModifyBlockDropItemEvent a = newEvent(2);
+            McMMOModifyBlockDropItemEvent b = newEvent(2);
+            assertEquals(a, b);
+            assertEquals(a.hashCode(), b.hashCode());
+
+            // Change cancellation and modified quantity => not equal
+            McMMOModifyBlockDropItemEvent c = newEvent(2);
+            c.setCancelled(true);
+            assertNotEquals(a, c);
+
+            McMMOModifyBlockDropItemEvent d = newEvent(2);
+            d.setModifiedItemStackQuantity(99);
+            assertNotEquals(a, d);
+
+            // Different underlying mocks => not equal
+            BlockDropItemEvent otherEvent = mock(BlockDropItemEvent.class, RETURNS_DEEP_STUBS);
+            when(otherEvent.getPlayer()).thenReturn(player);
+            when(otherEvent.getBlock()).thenReturn(block);
+            when(otherEvent.getBlockState()).thenReturn(blockState);
+
+            ItemStack otherStack = mock(ItemStack.class);
+            when(otherStack.getAmount()).thenReturn(3);
+            Item otherItem = mock(Item.class);
+            when(otherItem.getItemStack()).thenReturn(otherStack);
+
+            McMMOModifyBlockDropItemEvent e = new McMMOModifyBlockDropItemEvent(otherEvent, otherItem, 2);
+            assertNotEquals(a, e);
+        }
+    }
+}