Fix obsidian scoop lava/obsidian dupe by re-validating before giving lava

src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/ObsidianScoopingListener.java

@@ -230,7 +230,7 @@
      */
     private boolean lookForLava(PlayerInteractEvent e) {
         Player player = e.getPlayer();
-        ItemStack bucket = e.getItem();
+        EquipmentSlot hand = e.getHand() == null ? EquipmentSlot.HAND : e.getHand();
 
         // Get block player is looking at
         RayTraceResult rtBlocks = e.getPlayer().rayTraceBlocks(5, FluidCollisionMode.ALWAYS);
@@ -257,19 +257,37 @@
             }
             // Add player to cooldown set to prevent rapid scooping
             getCooldowns().add(player.getUniqueId());
-            user.sendMessage("protection.flags.OBSIDIAN_SCOOPING.scooping");
-            player.getWorld().playSound(player.getLocation(), Sound.ITEM_BUCKET_FILL_LAVA, 1F, 1F);
             e.setCancelled(true);
-            Bukkit.getScheduler().runTask(BentoBox.getInstance(), () -> givePlayerLava(player, b, bucket));
+            Bukkit.getScheduler().runTask(BentoBox.getInstance(), () -> givePlayerLava(user, b, hand));
             return true;
         }
         return false;
 
     }
 
-    private void givePlayerLava(Player player, Block b, ItemStack bucket) {
+    /**
+     * Converts the obsidian into a lava bucket. This runs a tick after the interact event,
+     * so everything checked there is re-checked here: in the meantime the obsidian may have
+     * been mined (which would give the player both the obsidian and the lava) or the bucket
+     * moved out of the hand (which would give the lava without using up the bucket).
+     *
+     * @param user the player scooping
+     * @param b the obsidian block
+     * @param hand the hand that held the empty bucket
+     * @return true if the lava was given, false if the scoop was abandoned
+     */
+    boolean givePlayerLava(User user, Block b, EquipmentSlot hand) {
+        Player player = user.getPlayer();
+        if (!player.isOnline() || !b.getType().equals(Material.OBSIDIAN)) {
+            return false;
+        }
+        ItemStack bucket = player.getInventory().getItem(hand);
+        if (bucket == null || !bucket.getType().equals(Material.BUCKET)) {
+            return false;
+        }
         // Remove one empty bucket and add a lava bucket to the player's inventory
         bucket.setAmount(bucket.getAmount() - 1);
+        player.getInventory().setItem(hand, bucket);
         Map<Integer, ItemStack> map = player.getInventory().addItem(new ItemStack(Material.LAVA_BUCKET));
         if (!map.isEmpty()) {
             map.values().forEach(i -> player.getWorld().dropItem(player.getLocation(), i));
@@ -278,6 +296,9 @@
         b.setType(Material.AIR);
         // Remove the lava tip hologram, if any, since the obsidian is gone
         removeHologramFor(b);
+        user.sendMessage("protection.flags.OBSIDIAN_SCOOPING.scooping");
+        player.getWorld().playSound(player.getLocation(), Sound.ITEM_BUCKET_FILL_LAVA, 1F, 1F);
+        return true;
     }
 
     private List<Block> getBlocksAround(Block b, int radius) {

src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/ObsidianScoopingListenerTest.java

@@ -30,6 +30,7 @@
 import org.bukkit.event.block.Action;
 import org.bukkit.event.block.BlockFormEvent;
 import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.inventory.EquipmentSlot;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 import org.bukkit.scheduler.BukkitTask;
@@ -111,7 +112,10 @@ public void setUp() throws Exception {
         ItemStack air = mock(ItemStack.class);
         when(air.getType()).thenReturn(Material.AIR);
         when(playerInventory.getItemInOffHand()).thenReturn(air);
+        when(playerInventory.getItem(EquipmentSlot.HAND)).thenReturn(item);
+        when(playerInventory.getItem(EquipmentSlot.OFF_HAND)).thenReturn(air);
         when(mockPlayer.getInventory()).thenReturn(playerInventory);
+        when(mockPlayer.isOnline()).thenReturn(true);
 
         // Addon
         when(iwm.getAddon(Mockito.any())).thenReturn(Optional.empty());
@@ -365,6 +369,65 @@ void testScoopingRemovesActiveHologramImmediately() {
         verify(mockHologram).remove();
     }
 
+    @Test
+    void testGivePlayerLava() {
+        when(item.getType()).thenReturn(Material.BUCKET);
+        when(clickedBlock.getType()).thenReturn(Material.OBSIDIAN);
+        when(clickedBlock.getLocation()).thenReturn(location);
+        when(mockPlayer.getInventory().addItem(any(ItemStack.class))).thenReturn(new HashMap<>());
+
+        assertTrue(listener.givePlayerLava(User.getInstance(mockPlayer), clickedBlock, EquipmentSlot.HAND));
+
+        verify(item).setAmount(0);
+        verify(mockPlayer.getInventory()).addItem(any(ItemStack.class));
+        verify(clickedBlock).setType(Material.AIR);
+    }
+
+    /**
+     * The lava is handed out a tick after the click. If the obsidian is mined in that
+     * window the player must not also get the lava.
+     */
+    @Test
+    void testGivePlayerLavaObsidianGone() {
+        when(item.getType()).thenReturn(Material.BUCKET);
+        when(clickedBlock.getType()).thenReturn(Material.WATER);
+
+        assertFalse(listener.givePlayerLava(User.getInstance(mockPlayer), clickedBlock, EquipmentSlot.HAND));
+
+        verify(item, never()).setAmount(Mockito.anyInt());
+        verify(mockPlayer.getInventory(), never()).addItem(any(ItemStack.class));
+        verify(clickedBlock, never()).setType(any());
+    }
+
+    /**
+     * If the empty bucket left the hand before the lava is handed out, the scoop is abandoned
+     * rather than giving lava without using up a bucket.
+     */
+    @Test
+    void testGivePlayerLavaBucketNoLongerInHand() {
+        when(clickedBlock.getType()).thenReturn(Material.OBSIDIAN);
+        ItemStack pickaxe = mock(ItemStack.class);
+        when(pickaxe.getType()).thenReturn(Material.DIAMOND_PICKAXE);
+        when(mockPlayer.getInventory().getItem(EquipmentSlot.HAND)).thenReturn(pickaxe);
+
+        assertFalse(listener.givePlayerLava(User.getInstance(mockPlayer), clickedBlock, EquipmentSlot.HAND));
+
+        verify(pickaxe, never()).setAmount(Mockito.anyInt());
+        verify(mockPlayer.getInventory(), never()).addItem(any(ItemStack.class));
+        verify(clickedBlock, never()).setType(any());
+    }
+
+    @Test
+    void testGivePlayerLavaPlayerOffline() {
+        when(item.getType()).thenReturn(Material.BUCKET);
+        when(clickedBlock.getType()).thenReturn(Material.OBSIDIAN);
+        when(mockPlayer.isOnline()).thenReturn(false);
+
+        assertFalse(listener.givePlayerLava(User.getInstance(mockPlayer), clickedBlock, EquipmentSlot.HAND));
+
+        verify(clickedBlock, never()).setType(any());
+    }
+
     @Test
     void testObsidianFormEmptyTipText() {
         // Set up solitary obsidian