Fixes #4087

When a block is broken we don't want to allow users to still use the inventory. This allows for them to pull items out along with blocks being dropped (if the block implements that).

    This fix is in 2 parts, the main part is just preventing the inventory from being opened if the block is queued for deletion. The second smaller part is just closing the inventory for all viewers on break. We would do this during cleanup but that was a tick later, we want it done in this tick and to prevent opening before the cleaning up is ran.

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());
         }
     }

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);

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)
+                    );
                 }
             }
         };

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)
+        );
+    }
+}

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;
+    }
 }