mirror of
https://github.com/mcMMO-Dev/mcMMO.git
synced 2026-08-18 11:26:06 +00:00
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.
This commit is contained in:
@ -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<ItemStack> 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);
|
||||
|
||||
@ -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<ExcavationTreasure> 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.
|
||||
*
|
||||
* <p>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<ExcavationTreasure> getTreasures(Material material) {
|
||||
String friendly = getMaterialConfigString(material);
|
||||
if (TreasureConfig.getInstance().excavationMap.containsKey(friendly)) {
|
||||
return TreasureConfig.getInstance().excavationMap.get(friendly);
|
||||
}
|
||||
|
||||
@ -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<ExcavationTreasure> getTreasures(@NotNull BlockState blockState) {
|
||||
requireNonNull(blockState, "blockState cannot be null");
|
||||
return getTreasures(blockState.getBlock());
|
||||
return getTreasures(blockState.getType());
|
||||
}
|
||||
|
||||
public List<ExcavationTreasure> getTreasures(@NotNull Block block) {
|
||||
requireNonNull(block, "block cannot be null");
|
||||
return Excavation.getTreasures(block);
|
||||
return getTreasures(block.getType());
|
||||
}
|
||||
|
||||
public List<ExcavationTreasure> 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}.
|
||||
*
|
||||
* <p>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}).
|
||||
* <p>{@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<ItemStack> 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<ItemStack> rollAndCollectTreasureDrops(@NotNull Block block) {
|
||||
requireNonNull(block, "block cannot be null");
|
||||
return doRollAndCollectTreasureDrops(block.getType(), Misc.getBlockCenter(block));
|
||||
}
|
||||
|
||||
private @NotNull List<ItemStack> doRollAndCollectTreasureDrops(
|
||||
@NotNull Material material, @NotNull Location centerOfBlock) {
|
||||
if (!Permissions.isSubSkillEnabled(getPlayer(), SubSkillType.EXCAVATION_ARCHAEOLOGY)) {
|
||||
return List.of();
|
||||
}
|
||||
|
||||
final List<ExcavationTreasure> treasures = getTreasures(block);
|
||||
final List<ExcavationTreasure> 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.
|
||||
|
||||
@ -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<ItemStack> drops = excavationManager.rollAndCollectTreasureDrops(block);
|
||||
// When: treasure roll happens inside BlockDropItemEvent;
|
||||
// Material.SAND simulates what event.getBlockState().getType() returns
|
||||
final List<ItemStack> 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<ItemStack> drops = excavationManager.rollAndCollectTreasureDrops(block);
|
||||
final List<ItemStack> 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<ItemStack> drops = excavationManager.rollAndCollectTreasureDrops(block);
|
||||
// When: treasure roll happens with the pre-break material threaded from the listener
|
||||
final List<ItemStack> 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<ItemStack> drops = excavationManager.rollAndCollectTreasureDrops(block);
|
||||
final List<ItemStack> drops = excavationManager.rollAndCollectTreasureDrops(block,
|
||||
Material.SAND);
|
||||
|
||||
// Then: empty list — impossible treasure never triggers
|
||||
org.junit.jupiter.api.Assertions.assertTrue(drops.isEmpty(),
|
||||
|
||||
Reference in New Issue
Block a user