diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockListener.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockListener.java index 5fdc72007..1d7a75df2 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockListener.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockListener.java @@ -18,6 +18,7 @@ import org.bukkit.block.BlockFace; import org.bukkit.block.BlockState; import org.bukkit.block.data.BlockData; import org.bukkit.enchantments.Enchantment; +import org.bukkit.entity.HumanEntity; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; @@ -220,6 +221,14 @@ public class BlockListener implements Listener { } drops.addAll(sfItem.getDrops()); + // Partial fix for #4087 - We don't want the inventory to be usable post break, close it for anyone still inside + // The main fix is in SlimefunItemInteractListener preventing opening to begin with + // Close the inventory for all viewers of this block + // TODO(future): Remove this check when MockBukkit supports viewers + if (!Slimefun.instance().isUnitTest()) { + BlockStorage.getInventory(e.getBlock()).toInventory().getViewers().forEach(HumanEntity::closeInventory); + } + // Remove the block data BlockStorage.clearBlockInfo(e.getBlock()); } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java index a9931b2cc..195f6ac0c 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java @@ -57,6 +57,13 @@ public class SlimefunItemInteractListener implements Listener { return; } + // Fixes #4087 - Prevents players from interacting with a block that is about to be deleted + // We especially don't want to open inventories as that can cause duplication + if (Slimefun.getTickerTask().isDeletedSoon(e.getClickedBlock().getLocation())) { + e.setCancelled(true); + return; + } + // Fire our custom Event PlayerRightClickEvent event = new PlayerRightClickEvent(e); Bukkit.getPluginManager().callEvent(event); diff --git a/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/interfaces/InventoryBlock.java b/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/interfaces/InventoryBlock.java index 199b60cae..9c368d67a 100644 --- a/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/interfaces/InventoryBlock.java +++ b/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/interfaces/InventoryBlock.java @@ -64,7 +64,11 @@ public interface InventoryBlock { if (p.hasPermission("slimefun.inventory.bypass")) { return true; } else { - return item.canUse(p, false) && Slimefun.getProtectionManager().hasPermission(p, b.getLocation(), Interaction.INTERACT_BLOCK); + return item.canUse(p, false) && ( + // Protection manager doesn't exist in unit tests + Slimefun.instance().isUnitTest() + || Slimefun.getProtectionManager().hasPermission(p, b.getLocation(), Interaction.INTERACT_BLOCK) + ); } } }; diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java new file mode 100644 index 000000000..e40324339 --- /dev/null +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java @@ -0,0 +1,137 @@ +package io.github.thebusybiscuit.slimefun4.implementation.listeners; + +import io.github.thebusybiscuit.slimefun4.api.events.SlimefunBlockBreakEvent; +import org.bukkit.World; +import org.bukkit.block.Block; +import org.bukkit.block.BlockFace; +import org.bukkit.entity.Player; +import org.bukkit.event.Event.Result; +import org.bukkit.event.block.Action; +import org.bukkit.event.block.BlockBreakEvent; +import org.bukkit.event.player.PlayerInteractEvent; +import org.bukkit.inventory.EquipmentSlot; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import io.github.thebusybiscuit.slimefun4.api.events.PlayerRightClickEvent; +import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem; +import io.github.thebusybiscuit.slimefun4.api.recipes.RecipeType; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; +import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems; +import io.github.thebusybiscuit.slimefun4.implementation.items.electric.machines.ElectricFurnace; +import io.github.thebusybiscuit.slimefun4.test.TestUtilities; +import me.mrCookieSlime.Slimefun.api.BlockStorage; +import me.mrCookieSlime.Slimefun.api.inventory.BlockMenuPreset; +import be.seeseemelk.mockbukkit.MockBukkit; +import be.seeseemelk.mockbukkit.ServerMock; + +class TestSlimefunItemInteractListener { + + private static ServerMock server; + private static Slimefun plugin; + private static SlimefunItem slimefunItem; + + @BeforeAll + public static void load() { + server = MockBukkit.mock(); + plugin = MockBukkit.load(Slimefun.class); + + // Register block listener (for place + break) and our interact listener for inventory handling + new BlockListener(plugin); + new SlimefunItemInteractListener(plugin); + + // Enable tickers so the electric furnace can be registered + Slimefun.getCfg().setValue("URID.enable-tickers", true); + + slimefunItem = new ElectricFurnace( + TestUtilities.getItemGroup(plugin, "test"), SlimefunItems.ELECTRIC_FURNACE, RecipeType.NULL, new ItemStack[]{} + ) + .setCapacity(100) + .setEnergyConsumption(10) + .setProcessingSpeed(1); + slimefunItem.register(plugin); + } + + @AfterAll + public static void unload() { + MockBukkit.unmock(); + } + + @AfterEach + public void afterEach() { + server.getPluginManager().clearEvents(); + } + + // Test for dupe bug - issue #4087 + @Test + void testCannotOpenInvOfBrokenBlock() { + // Place down an electric furnace + Player player = server.addPlayer(); + ItemStack itemStack = slimefunItem.getItem(); + player.getInventory().setItemInMainHand(itemStack); + + // Create a world and place the block + World world = TestUtilities.createWorld(server); + Block block = TestUtilities.placeSlimefunBlock(server, itemStack, world, player); + + // Right click on the block + PlayerInteractEvent playerInteractEvent = new PlayerInteractEvent( + player, Action.RIGHT_CLICK_BLOCK, itemStack, block, BlockFace.UP, EquipmentSlot.HAND + ); + + server.getPluginManager().callEvent(playerInteractEvent); + server.getPluginManager().assertEventFired(PlayerInteractEvent.class, e -> { + // We cancel the event on inventory open + Assertions.assertSame(e.useInteractedBlock(), Result.DENY); + return true; + }); + + // Assert our right click event fired and the block usage was not denied + server.getPluginManager().assertEventFired(PlayerRightClickEvent.class, e -> { + Assertions.assertNotSame(e.useBlock(), Result.DENY); + return true; + }); + + // Assert we do have an inventory which would be opened + // TODO: Create an event for open inventory so this isn't guess work + Assertions.assertTrue(BlockMenuPreset.isInventory(slimefunItem.getId())); + Assertions.assertTrue(BlockStorage.getStorage(block.getWorld()).hasInventory(block.getLocation())); + // TODO(future): Check viewers - MockBukkit does not implement this today + + // Break the block + BlockBreakEvent blockBreakEvent = new BlockBreakEvent(block, player); + server.getPluginManager().callEvent(blockBreakEvent); + server.getPluginManager().assertEventFired(SlimefunBlockBreakEvent.class, e -> { + Assertions.assertEquals(slimefunItem.getId(), e.getSlimefunItem().getId()); + return true; + }); + + // Assert the block is queued for removal + Assertions.assertTrue(Slimefun.getTickerTask().isDeletedSoon(block.getLocation())); + + // Clear event queue since we'll be running duplicate events + server.getPluginManager().clearEvents(); + + // Right click on the block again now that it's broken + PlayerInteractEvent secondPlayerInteractEvent = new PlayerInteractEvent( + player, Action.RIGHT_CLICK_BLOCK, itemStack, block, BlockFace.UP, EquipmentSlot.HAND + ); + + server.getPluginManager().callEvent(secondPlayerInteractEvent); + server.getPluginManager().assertEventFired(PlayerInteractEvent.class, e -> { + // We cancelled the event due to the block being removed + Assertions.assertSame(e.useInteractedBlock(), Result.DENY); + return true; + }); + + // Assert our right click event was not fired due to the block being broken + Assertions.assertThrows( + AssertionError.class, + () -> server.getPluginManager().assertEventFired(PlayerRightClickEvent.class, e -> true) + ); + } +} diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java b/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java index 56f517da2..d5f5b134c 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java @@ -10,17 +10,26 @@ import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nonnull; import javax.annotation.ParametersAreNonnullByDefault; +import org.bukkit.Location; import org.bukkit.Material; import org.bukkit.NamespacedKey; import org.bukkit.OfflinePlayer; +import org.bukkit.World; +import org.bukkit.block.Block; +import org.bukkit.entity.Player; +import org.bukkit.event.block.BlockPlaceEvent; import org.bukkit.event.inventory.InventoryType; +import org.bukkit.inventory.EquipmentSlot; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.ItemStack; import org.bukkit.plugin.Plugin; import org.junit.jupiter.api.Assertions; import org.mockito.Mockito; +import be.seeseemelk.mockbukkit.ServerMock; +import be.seeseemelk.mockbukkit.block.BlockMock; import io.github.bakedlibs.dough.items.CustomItemStack; +import io.github.thebusybiscuit.slimefun4.api.events.SlimefunBlockPlaceEvent; import io.github.thebusybiscuit.slimefun4.api.items.ItemGroup; import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem; import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; @@ -28,6 +37,7 @@ import io.github.thebusybiscuit.slimefun4.api.recipes.RecipeType; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.implementation.items.VanillaItem; import io.github.thebusybiscuit.slimefun4.test.mocks.MockSlimefunItem; +import me.mrCookieSlime.Slimefun.api.BlockStorage; public final class TestUtilities { @@ -89,4 +99,26 @@ public final class TestUtilities { public static @Nonnull int randomInt(int upperBound) { return random.nextInt(upperBound); } + + public static World createWorld(ServerMock server) { + World world = server.addSimpleWorld("world_" + randomInt()); + Slimefun.getRegistry().getWorlds().put(world.getName(), new BlockStorage(world)); + return world; + } + + public static Block placeSlimefunBlock(ServerMock server, ItemStack item, World world, Player player) { + int x = TestUtilities.randomInt(); + int z = TestUtilities.randomInt(); + Block block = new BlockMock(item.getType(), new Location(world, x, 0, z)); + Block blockAgainst = new BlockMock(Material.GRASS, new Location(world, x, 1, z)); + + BlockPlaceEvent blockPlaceEvent = new BlockPlaceEvent( + block, block.getState(), blockAgainst, item, player, true, EquipmentSlot.HAND + ); + + server.getPluginManager().callEvent(blockPlaceEvent); + server.getPluginManager().assertEventFired(SlimefunBlockPlaceEvent.class, e -> true); + + return block; + } }