Fix left click on air being ignored and right click on block being ha…

…ndled twice (#2153)

* Fix left click on air being ignored and right click on block being handled twice

* Factorize debouncing and cache event result

* Fix ForgePlatform#server being always null

worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitServerInterface.java

@@ -37,6 +37,7 @@
 import com.sk89q.worldedit.util.lifecycle.Lifecycled;
 import com.sk89q.worldedit.world.DataFixer;
 import com.sk89q.worldedit.world.registry.Registries;
+import io.papermc.lib.PaperLib;
 import org.bukkit.Bukkit;
 import org.bukkit.Server;
 import org.bukkit.World;
@@ -234,6 +235,14 @@ public Set<SideEffect> getSupportedSideEffects() {
         return SUPPORTED_SIDE_EFFECTS;
     }
 
+    @Override
+    public long getTickCount() {
+        if (PaperLib.isPaper()) {
+            return Bukkit.getCurrentTick();
+        }
+        return super.getTickCount();
+    }
+
     public void unregisterCommands() {
         dynamicCommands.unregisterCommands();
     }

worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/WorldEditListener.java

@@ -25,6 +25,7 @@
 import com.sk89q.worldedit.entity.Player;
 import com.sk89q.worldedit.event.platform.SessionIdleEvent;
 import com.sk89q.worldedit.extension.platform.Actor;
+import com.sk89q.worldedit.internal.event.InteractionDebouncer;
 import com.sk89q.worldedit.util.Direction;
 import com.sk89q.worldedit.util.Location;
 import com.sk89q.worldedit.world.World;
@@ -52,6 +53,7 @@
 public class WorldEditListener implements Listener {
 
     private final WorldEditPlugin plugin;
+    private final InteractionDebouncer debouncer;
 
     /**
      * Construct the object.
@@ -60,6 +62,7 @@ public class WorldEditListener implements Listener {
      */
     public WorldEditListener(WorldEditPlugin plugin) {
         this.plugin = plugin;
+        debouncer = new InteractionDebouncer(plugin.getInternalPlatform());
     }
 
     @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
@@ -93,62 +96,59 @@ public void onPlayerCommandSend(PlayerCommandSendEvent event) {
      */
     @EventHandler
     public void onPlayerInteract(PlayerInteractEvent event) {
-        if (!plugin.getInternalPlatform().isHookingEvents()) {
+        if (!plugin.getInternalPlatform().isHookingEvents()
+                || event.useItemInHand() == Result.DENY
+                || event.getHand() == EquipmentSlot.OFF_HAND
+                || event.getAction() == Action.PHYSICAL) {
             return;
         }
 
-        if (event.useItemInHand() == Result.DENY) {
-            return;
-        }
+        final Player player = plugin.wrapPlayer(event.getPlayer());
 
-        if (event.getHand() == EquipmentSlot.OFF_HAND) {
-            return;
+        if (event.getAction() != Action.LEFT_CLICK_BLOCK) {
+            Optional<Boolean> previousResult = debouncer.getDuplicateInteractionResult(player);
+            if (previousResult.isPresent()) {
+                if (previousResult.get()) {
+                    event.setCancelled(true);
+                }
+                return;
+            }
         }
 
-        final Player player = plugin.wrapPlayer(event.getPlayer());
         final World world = player.getWorld();
         final WorldEdit we = plugin.getWorldEdit();
         final Direction direction = BukkitAdapter.adapt(event.getBlockFace());
+        final Block clickedBlock = event.getClickedBlock();
+        final Location pos = clickedBlock == null ? null : new Location(world, clickedBlock.getX(), clickedBlock.getY(), clickedBlock.getZ());
+
+        boolean result = false;
+        switch (event.getAction()) {
+            case LEFT_CLICK_BLOCK:
+                result = we.handleBlockLeftClick(player, pos, direction) || we.handleArmSwing(player);
+                break;
+            case LEFT_CLICK_AIR:
+                result = we.handleArmSwing(player);
+                break;
+            case RIGHT_CLICK_BLOCK:
+                result = we.handleBlockRightClick(player, pos, direction) || we.handleRightClick(player);
+                break;
+            case RIGHT_CLICK_AIR:
+                result = we.handleRightClick(player);
+                break;
+            default:
+                break;
+        }
 
-        Action action = event.getAction();
-        if (action == Action.LEFT_CLICK_BLOCK) {
-            final Block clickedBlock = event.getClickedBlock();
-            final Location pos = new Location(world, clickedBlock.getX(), clickedBlock.getY(), clickedBlock.getZ());
-
-            if (we.handleBlockLeftClick(player, pos, direction)) {
-                event.setCancelled(true);
-            }
-
-            if (we.handleArmSwing(player)) {
-                event.setCancelled(true);
-            }
-
-        } else if (action == Action.LEFT_CLICK_AIR) {
-
-            if (we.handleArmSwing(player)) {
-                event.setCancelled(true);
-            }
-
-        } else if (action == Action.RIGHT_CLICK_BLOCK) {
-            final Block clickedBlock = event.getClickedBlock();
-            final Location pos = new Location(world, clickedBlock.getX(), clickedBlock.getY(), clickedBlock.getZ());
-
-            if (we.handleBlockRightClick(player, pos, direction)) {
-                event.setCancelled(true);
-            }
-
-            if (we.handleRightClick(player)) {
-                event.setCancelled(true);
-            }
-        } else if (action == Action.RIGHT_CLICK_AIR) {
-            if (we.handleRightClick(player)) {
-                event.setCancelled(true);
-            }
+        debouncer.setLastInteraction(player, result);
+        if (result) {
+            event.setCancelled(true);
         }
     }
 
     @EventHandler
     public void onPlayerQuit(PlayerQuitEvent event) {
+        debouncer.clearInteraction(plugin.wrapPlayer(event.getPlayer()));
+
         plugin.getWorldEdit().getEventBus().post(new SessionIdleEvent(new BukkitPlayer.SessionKeyImpl(event.getPlayer())));
     }
 }

worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/AbstractPlatform.java

@@ -84,4 +84,8 @@ public void reload() {
         getTranslationManager().reload();
     }
 
+    @Override
+    public long getTickCount() {
+        return System.nanoTime() / 50_000_000;
+    }
 }

worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/Platform.java

@@ -207,4 +207,12 @@ default void registerGameHooks() {
      * @return A set of supported side effects
      */
     Set<SideEffect> getSupportedSideEffects();
+
+    /**
+     * Get the number of ticks since the server started.
+     * On some platforms this value may be an approximation based on the JVM run time.
+     *
+     * @return The number of ticks since the server started.
+     */
+    long getTickCount();
 }

worldedit-core/src/main/java/com/sk89q/worldedit/internal/event/InteractionDebouncer.java

@@ -0,0 +1,69 @@
+/*
+ * WorldEdit, a Minecraft world manipulation toolkit
+ * Copyright (C) sk89q <http://www.sk89q.com>
+ * Copyright (C) WorldEdit team and contributors
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package com.sk89q.worldedit.internal.event;
+
+import com.sk89q.worldedit.extension.platform.Platform;
+import com.sk89q.worldedit.util.Identifiable;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.UUID;
+
+public class InteractionDebouncer {
+    private final Platform platform;
+    private final Map<UUID, Interaction> lastInteractions = new HashMap<>();
+
+    public InteractionDebouncer(Platform platform) {
+        this.platform = platform;
+    }
+
+    public void clearInteraction(Identifiable player) {
+        lastInteractions.remove(player.getUniqueId());
+    }
+
+    public void setLastInteraction(Identifiable player, boolean result) {
+        lastInteractions.put(player.getUniqueId(), new Interaction(platform.getTickCount(), result));
+    }
+
+    public Optional<Boolean> getDuplicateInteractionResult(Identifiable player) {
+        Interaction last = lastInteractions.get(player.getUniqueId());
+        if (last == null) {
+            return Optional.empty();
+        }
+
+        long now = platform.getTickCount();
+        if (now - last.tick <= 1) {
+            return Optional.of(last.result);
+        }
+
+        return Optional.empty();
+    }
+
+    private static class Interaction {
+        public final long tick;
+        public final boolean result;
+
+        public Interaction(long tick, boolean result) {
+            this.tick = tick;
+            this.result = result;
+        }
+    }
+}

worldedit-fabric/src/main/java/com/sk89q/worldedit/fabric/FabricPlatform.java

@@ -213,6 +213,11 @@ public Set<SideEffect> getSupportedSideEffects() {
             : SUPPORTED_SIDE_EFFECTS_NO_MIXIN;
     }
 
+    @Override
+    public long getTickCount() {
+        return FabricWorldEdit.LIFECYCLED_SERVER.valueOrThrow().getTickCount();
+    }
+
     @Override
     public Collection<Actor> getConnectedUsers() {
         List<Actor> users = new ArrayList<>();

worldedit-fabric/src/main/java/com/sk89q/worldedit/fabric/FabricWorldEdit.java

@@ -32,6 +32,7 @@
 import com.sk89q.worldedit.extension.platform.PlatformManager;
 import com.sk89q.worldedit.fabric.net.handler.WECUIPacketHandler;
 import com.sk89q.worldedit.internal.anvil.ChunkDeleter;
+import com.sk89q.worldedit.internal.event.InteractionDebouncer;
 import com.sk89q.worldedit.internal.util.LogManagerCompat;
 import com.sk89q.worldedit.util.Location;
 import com.sk89q.worldedit.util.lifecycle.Lifecycled;
@@ -81,6 +82,7 @@
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -113,6 +115,7 @@ public static <T> Registry<T> getRegistry(ResourceKey<Registry<T>> key) {
 
     public static FabricWorldEdit inst;
 
+    private InteractionDebouncer debouncer;
     private FabricPlatform platform;
     private FabricConfiguration config;
     private Path workingDir;
@@ -139,6 +142,7 @@ public void onInitialize() {
             }
         }
         this.platform = new FabricPlatform(this);
+        debouncer = new InteractionDebouncer(platform);
 
         WorldEdit.getInstance().getPlatformManager().register(platform);
 
@@ -250,16 +254,16 @@ private void onStopServer(MinecraftServer minecraftServer) {
         WorldEdit.getInstance().getEventBus().post(new PlatformUnreadyEvent(platform));
     }
 
-    private boolean shouldSkip() {
-        if (platform == null) {
-            return true;
-        }
+    private boolean skipEvents() {
+        return platform == null || !platform.isHookingEvents();
+    }
 
-        return !platform.isHookingEvents(); // We have to be told to catch these events
+    private boolean skipInteractionEvent(Player player, InteractionHand hand) {
+        return skipEvents() || hand != InteractionHand.MAIN_HAND || player.level().isClientSide || !(player instanceof ServerPlayer);
     }
 
     private InteractionResult onLeftClickBlock(Player playerEntity, Level world, InteractionHand hand, BlockPos blockPos, Direction direction) {
-        if (shouldSkip() || hand == InteractionHand.OFF_HAND || world.isClientSide) {
+        if (skipInteractionEvent(playerEntity, hand)) {
             return InteractionResult.PASS;
         }
 
@@ -273,19 +277,14 @@ private InteractionResult onLeftClickBlock(Player playerEntity, Level world, Int
         );
         com.sk89q.worldedit.util.Direction weDirection = FabricAdapter.adaptEnumFacing(direction);
 
-        if (we.handleBlockLeftClick(player, pos, weDirection)) {
-            return InteractionResult.SUCCESS;
-        }
-
-        if (we.handleArmSwing(player)) {
-            return InteractionResult.SUCCESS;
-        }
+        boolean result = we.handleBlockLeftClick(player, pos, weDirection) || we.handleArmSwing(player);
+        debouncer.setLastInteraction(player, result);
 
-        return InteractionResult.PASS;
+        return result ? InteractionResult.SUCCESS : InteractionResult.PASS;
     }
 
     private InteractionResult onRightClickBlock(Player playerEntity, Level world, InteractionHand hand, BlockHitResult blockHitResult) {
-        if (shouldSkip() || hand == InteractionHand.OFF_HAND || world.isClientSide) {
+        if (skipInteractionEvent(playerEntity, hand)) {
             return InteractionResult.PASS;
         }
 
@@ -299,36 +298,52 @@ private InteractionResult onRightClickBlock(Player playerEntity, Level world, In
         );
         com.sk89q.worldedit.util.Direction direction = FabricAdapter.adaptEnumFacing(blockHitResult.getDirection());
 
-        if (we.handleBlockRightClick(player, pos, direction)) {
-            return InteractionResult.SUCCESS;
+        boolean result = we.handleBlockRightClick(player, pos, direction) || we.handleRightClick(player);
+        debouncer.setLastInteraction(player, result);
+
+        return result ? InteractionResult.SUCCESS : InteractionResult.PASS;
+    }
+
+    public void onLeftClickAir(ServerPlayer playerEntity, InteractionHand hand) {
+        if (skipInteractionEvent(playerEntity, hand)) {
+            return;
         }
 
-        if (we.handleRightClick(player)) {
-            return InteractionResult.SUCCESS;
+        WorldEdit we = WorldEdit.getInstance();
+        FabricPlayer player = adaptPlayer(playerEntity);
+
+        Optional<Boolean> previousResult = debouncer.getDuplicateInteractionResult(player);
+        if (previousResult.isPresent()) {
+            return;
         }
 
-        return InteractionResult.PASS;
+        boolean result = we.handleArmSwing(player);
+        debouncer.setLastInteraction(player, result);
     }
 
     private InteractionResultHolder<ItemStack> onRightClickAir(Player playerEntity, Level world, InteractionHand hand) {
         ItemStack stackInHand = playerEntity.getItemInHand(hand);
-        if (shouldSkip() || hand == InteractionHand.OFF_HAND || world.isClientSide) {
+        if (skipInteractionEvent(playerEntity, hand)) {
             return InteractionResultHolder.pass(stackInHand);
         }
 
         WorldEdit we = WorldEdit.getInstance();
         FabricPlayer player = adaptPlayer((ServerPlayer) playerEntity);
 
-        if (we.handleRightClick(player)) {
-            return InteractionResultHolder.success(stackInHand);
+        Optional<Boolean> previousResult = debouncer.getDuplicateInteractionResult(player);
+        if (previousResult.isPresent()) {
+            return previousResult.get() ? InteractionResultHolder.success(stackInHand) : InteractionResultHolder.pass(stackInHand);
         }
 
-        return InteractionResultHolder.pass(stackInHand);
-    }
+        boolean result = we.handleRightClick(player);
+        debouncer.setLastInteraction(player, result);
 
-    // TODO Pass empty left click to server
+        return result ? InteractionResultHolder.success(stackInHand) : InteractionResultHolder.pass(stackInHand);
+    }
 
     private void onPlayerDisconnect(ServerGamePacketListenerImpl handler, MinecraftServer server) {
+        debouncer.clearInteraction(adaptPlayer(handler.player));
+
         WorldEdit.getInstance().getEventBus()
             .post(new SessionIdleEvent(new FabricPlayer.SessionKeyImpl(handler.player)));
     }