From 47cebf2cc2830eac69af3e4e5cd0d20cf9d81c4e Mon Sep 17 00:00:00 2001 From: nossr50 Date: Wed, 13 May 2026 17:23:29 -0700 Subject: [PATCH] Fix excavation treasure drops broken by BlockDropItemEvent refactor block.getType() returns AIR by the time BlockDropItemEvent fires because the block is removed before the event is dispatched. Capture the pre-break material from event.getBlockState().getType() and thread it through to affectedByGigaDrillBreaker and rollAndCollectTreasureDrops so treasure lookups use the correct material instead of AIR. --- .../nossr50/listeners/BlockListener.java | 7 ++- .../nossr50/skills/excavation/Excavation.java | 16 ++++++- .../skills/excavation/ExcavationManager.java | 47 ++++++++++++++----- .../skills/excavation/ExcavationTest.java | 33 +++++++------ 4 files changed, 73 insertions(+), 30 deletions(-) diff --git a/src/main/java/com/gmail/nossr50/listeners/BlockListener.java b/src/main/java/com/gmail/nossr50/listeners/BlockListener.java index 8b22e3a9a..a7a92908f 100644 --- a/src/main/java/com/gmail/nossr50/listeners/BlockListener.java +++ b/src/main/java/com/gmail/nossr50/listeners/BlockListener.java @@ -161,16 +161,19 @@ public class BlockListener implements Listener { // Excavation treasure injection — rolls happen here so drops enter the // BlockDropItemEvent item list and are visible to Telekinesis-style enchant plugins. + // block.getType() is AIR by event time; event.getBlockState() holds the pre-break + // snapshot, so we unwrap its material once here and thread it through. + final Material excavationBlockMaterial = event.getBlockState().getType(); final Player excavationPlayer = event.getPlayer(); final McMMOPlayer excavationMmoPlayer = UserManager.getPlayer(excavationPlayer); if (excavationMmoPlayer != null - && BlockUtils.affectedByGigaDrillBreaker(block) + && BlockUtils.affectedByGigaDrillBreaker(excavationBlockMaterial) && ItemUtils.isShovel(excavationPlayer.getInventory().getItemInMainHand()) && mcMMO.p.getSkillTools().doesPlayerHaveSkillPermission( excavationPlayer, PrimarySkillType.EXCAVATION) && !mcMMO.getUserBlockTracker().isIneligible(block)) { final List treasureDrops = excavationMmoPlayer.getExcavationManager() - .rollAndCollectTreasureDrops(block); + .rollAndCollectTreasureDrops(block, excavationBlockMaterial); if (!treasureDrops.isEmpty()) { final World blockWorld = block.getWorld(); final Location dropLocation = block.getLocation().add(0.5, 0.5, 0.5); diff --git a/src/main/java/com/gmail/nossr50/skills/excavation/Excavation.java b/src/main/java/com/gmail/nossr50/skills/excavation/Excavation.java index 47b69955b..86efe581f 100644 --- a/src/main/java/com/gmail/nossr50/skills/excavation/Excavation.java +++ b/src/main/java/com/gmail/nossr50/skills/excavation/Excavation.java @@ -6,6 +6,7 @@ import com.gmail.nossr50.config.treasure.TreasureConfig; import com.gmail.nossr50.datatypes.treasure.ExcavationTreasure; import java.util.ArrayList; import java.util.List; +import org.bukkit.Material; import org.bukkit.block.Block; public class Excavation { @@ -17,7 +18,20 @@ public class Excavation { * @return the list of treasures that could be found */ protected static List getTreasures(Block block) { - String friendly = getMaterialConfigString(block.getBlockData().getMaterial()); + return getTreasures(block.getType()); + } + + /** + * Get the list of possible {@link ExcavationTreasure|ExcavationTreasures} for a given material. + * + *

Prefer this overload when the block has already been broken and its type is AIR — + * e.g. when called from {@link org.bukkit.event.block.BlockDropItemEvent}. + * + * @param material the material of the block before it was broken + * @return the list of treasures that could be found + */ + protected static List getTreasures(Material material) { + String friendly = getMaterialConfigString(material); if (TreasureConfig.getInstance().excavationMap.containsKey(friendly)) { return TreasureConfig.getInstance().excavationMap.get(friendly); } diff --git a/src/main/java/com/gmail/nossr50/skills/excavation/ExcavationManager.java b/src/main/java/com/gmail/nossr50/skills/excavation/ExcavationManager.java index 3bb45e9dd..93c5a4f6a 100644 --- a/src/main/java/com/gmail/nossr50/skills/excavation/ExcavationManager.java +++ b/src/main/java/com/gmail/nossr50/skills/excavation/ExcavationManager.java @@ -22,6 +22,7 @@ import com.gmail.nossr50.util.skills.SkillUtils; import java.util.ArrayList; import java.util.List; import org.bukkit.Location; +import org.bukkit.Material; import org.bukkit.block.Block; import org.bukkit.block.BlockState; import org.bukkit.entity.Player; @@ -54,12 +55,17 @@ public class ExcavationManager extends SkillManager { @Deprecated(forRemoval = true, since = "2.2.024") public List getTreasures(@NotNull BlockState blockState) { requireNonNull(blockState, "blockState cannot be null"); - return getTreasures(blockState.getBlock()); + return getTreasures(blockState.getType()); } public List getTreasures(@NotNull Block block) { requireNonNull(block, "block cannot be null"); - return Excavation.getTreasures(block); + return getTreasures(block.getType()); + } + + public List getTreasures(@NotNull Material material) { + requireNonNull(material, "material cannot be null"); + return Excavation.getTreasures(material); } @VisibleForTesting @@ -70,30 +76,47 @@ public class ExcavationManager extends SkillManager { } /** - * Rolls for excavation treasures, spawns XP orbs, and applies treasure XP for each - * successful roll. Returns the list of {@link ItemStack}s to inject into - * {@link org.bukkit.event.block.BlockDropItemEvent} so that Telekinesis-style enchant - * plugins can intercept them through the standard Bukkit event pipeline. + * Rolls for excavation treasures. This is the primary production entry point, called from + * {@link org.bukkit.event.block.BlockDropItemEvent}. * - *

When {@link SuperAbilityType#GIGA_DRILL_BREAKER} is active the roll count is tripled, - * preserving the legacy behaviour where {@link #excavationBlockCheck} was called three - * times for the centre block (once normally and twice inside {@link #gigaDrillBreaker}). + *

{@code block.getType()} is AIR by event time because the block has already been removed + * from the world. Pass {@code material} from {@code event.getBlockState().getType()} (the + * pre-break snapshot) so treasure lookup uses the correct material. * - * @param block the block that was broken + * @param block the broken block — used only for spawn location + * @param material the material of the block before it was broken * @return list of treasure {@link ItemStack}s from all successful rolls */ + public @NotNull List rollAndCollectTreasureDrops( + @NotNull Block block, @NotNull Material material) { + requireNonNull(block, "block cannot be null"); + requireNonNull(material, "material cannot be null"); + return doRollAndCollectTreasureDrops(material, Misc.getBlockCenter(block)); + } + + /** + * @deprecated Use {@link #rollAndCollectTreasureDrops(Block, Material)} instead. When called + * during {@link org.bukkit.event.block.BlockDropItemEvent}, {@code block.getType()} + * returns AIR, causing treasure lookup to fail silently. + */ + @Deprecated(forRemoval = true, since = "2.2.053") public @NotNull List rollAndCollectTreasureDrops(@NotNull Block block) { + requireNonNull(block, "block cannot be null"); + return doRollAndCollectTreasureDrops(block.getType(), Misc.getBlockCenter(block)); + } + + private @NotNull List doRollAndCollectTreasureDrops( + @NotNull Material material, @NotNull Location centerOfBlock) { if (!Permissions.isSubSkillEnabled(getPlayer(), SubSkillType.EXCAVATION_ARCHAEOLOGY)) { return List.of(); } - final List treasures = getTreasures(block); + final List treasures = getTreasures(material); if (treasures.isEmpty()) { return List.of(); } final int skillLevel = getSkillLevel(); - final Location centerOfBlock = Misc.getBlockCenter(block); // GDB called excavationBlockCheck 3 times (1 regular + 2 via gigaDrillBreaker), // giving 3 independent treasure rolls for the centre block. Mirror that here. diff --git a/src/test/java/com/gmail/nossr50/skills/excavation/ExcavationTest.java b/src/test/java/com/gmail/nossr50/skills/excavation/ExcavationTest.java index 9ec43ec0c..20b996eb1 100644 --- a/src/test/java/com/gmail/nossr50/skills/excavation/ExcavationTest.java +++ b/src/test/java/com/gmail/nossr50/skills/excavation/ExcavationTest.java @@ -70,18 +70,19 @@ class ExcavationTest extends MMOTestEnvironment { mmoPlayer.modifySkill(PrimarySkillType.EXCAVATION, 1000); final Block block = Mockito.mock(Block.class); - when(block.getType()).thenReturn(Material.SAND); when(block.getDrops(any())).thenReturn(null); when(block.getLocation()).thenReturn(new Location(world, 0, 64, 0)); final ExcavationManager excavationManager = Mockito.spy(new ExcavationManager(mmoPlayer)); - doReturn(getGuaranteedTreasureDrops()).when(excavationManager).getTreasures(block); + doReturn(getGuaranteedTreasureDrops()).when(excavationManager).getTreasures(Material.SAND); - // When: treasure roll happens inside BlockDropItemEvent - final List drops = excavationManager.rollAndCollectTreasureDrops(block); + // When: treasure roll happens inside BlockDropItemEvent; + // Material.SAND simulates what event.getBlockState().getType() returns + final List drops = excavationManager.rollAndCollectTreasureDrops(block, + Material.SAND); // Then: at least one treasure was returned for injection into the event - verify(excavationManager, atLeastOnce()).getTreasures(block); + verify(excavationManager, atLeastOnce()).getTreasures(Material.SAND); org.junit.jupiter.api.Assertions.assertFalse(drops.isEmpty(), "Expected at least one treasure drop from a guaranteed roll"); } @@ -92,15 +93,15 @@ class ExcavationTest extends MMOTestEnvironment { mmoPlayer.modifySkill(PrimarySkillType.EXCAVATION, 1000); final Block block = Mockito.mock(Block.class); - when(block.getType()).thenReturn(Material.SAND); when(block.getDrops(any())).thenReturn(null); when(block.getLocation()).thenReturn(new Location(world, 0, 64, 0)); final ExcavationManager excavationManager = Mockito.spy(new ExcavationManager(mmoPlayer)); - doReturn(getImpossibleTreasureDrops()).when(excavationManager).getTreasures(block); + doReturn(getImpossibleTreasureDrops()).when(excavationManager).getTreasures(Material.SAND); // When - final List drops = excavationManager.rollAndCollectTreasureDrops(block); + final List drops = excavationManager.rollAndCollectTreasureDrops(block, + Material.SAND); // Then: no treasure was returned org.junit.jupiter.api.Assertions.assertTrue(drops.isEmpty(), @@ -131,15 +132,16 @@ class ExcavationTest extends MMOTestEnvironment { mmoPlayer.modifySkill(PrimarySkillType.EXCAVATION, 1000); final Block block = Mockito.mock(Block.class); - when(block.getType()).thenReturn(Material.SAND); when(block.getLocation()).thenReturn(new Location(world, 1, 64, 1)); final ExcavationManager excavationManager = Mockito.spy( new ExcavationManager(mmoPlayer)); - doReturn(getGuaranteedTreasureDrops()).when(excavationManager).getTreasures(block); + doReturn(getGuaranteedTreasureDrops()).when(excavationManager) + .getTreasures(Material.SAND); - // When: treasure roll happens inside BlockDropItemEvent - final List drops = excavationManager.rollAndCollectTreasureDrops(block); + // When: treasure roll happens with the pre-break material threaded from the listener + final List drops = excavationManager.rollAndCollectTreasureDrops(block, + Material.SAND); // Then: at least one treasure was returned for injection into the event, // not spawned directly via world.dropItem() @@ -155,15 +157,16 @@ class ExcavationTest extends MMOTestEnvironment { mmoPlayer.modifySkill(PrimarySkillType.EXCAVATION, 1000); final Block block = Mockito.mock(Block.class); - when(block.getType()).thenReturn(Material.SAND); when(block.getLocation()).thenReturn(new Location(world, 2, 64, 2)); final ExcavationManager excavationManager = Mockito.spy( new ExcavationManager(mmoPlayer)); - doReturn(getImpossibleTreasureDrops()).when(excavationManager).getTreasures(block); + doReturn(getImpossibleTreasureDrops()).when(excavationManager) + .getTreasures(Material.SAND); // When - final List drops = excavationManager.rollAndCollectTreasureDrops(block); + final List drops = excavationManager.rollAndCollectTreasureDrops(block, + Material.SAND); // Then: empty list — impossible treasure never triggers org.junit.jupiter.api.Assertions.assertTrue(drops.isEmpty(),