Fix: Allow players to hit air (#5369)

This also implements blocking block breaking & attacking entities while steering boats to match the Java clients behavior.
Further, it now also updates the shifting state before sending inputs to the Java client, also matching behavior there.

README.md

@@ -15,7 +15,7 @@ The ultimate goal of this project is to allow Minecraft: Bedrock Edition users t
 Special thanks to the DragonProxy project for being a trailblazer in protocol translation and for all the team members who have joined us here!
 
 ## Supported Versions
-Geyser is currently supporting Minecraft Bedrock 1.21.40 - 1.21.60 and Minecraft Java 1.21.4. For more information, please see [here](https://geysermc.org/wiki/geyser/supported-versions/).
+Geyser is currently supporting Minecraft Bedrock 1.21.40 - 1.21.61 and Minecraft Java 1.21.4. For more information, please see [here](https://geysermc.org/wiki/geyser/supported-versions/).
 
 ## Setting Up
 Take a look [here](https://geysermc.org/wiki/geyser/setup/) for how to set up Geyser.

core/src/main/java/org/geysermc/geyser/dump/DumpInfo.java

@@ -178,9 +178,8 @@ public static class NetworkInfo {
 
         NetworkInfo() {
             if (AsteriskSerializer.showSensitive) {
-                try {
+                try (Socket socket = new Socket()) {
                     // This is the most reliable for getting the main local IP
-                    Socket socket = new Socket();
                     socket.connect(new InetSocketAddress("geysermc.org", 80));
                     this.internalIP = socket.getLocalAddress().getHostAddress();
                 } catch (IOException e1) {

core/src/main/java/org/geysermc/geyser/session/GeyserSession.java

@@ -536,10 +536,17 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
 
     /**
      * Counts how many ticks have occurred since an arm animation started.
-     * -1 means there is no active arm swing; -2 means an arm swing will start in a tick.
+     * -1 means there is no active arm swing
      */
     private int armAnimationTicks = -1;
 
+    /**
+     * The tick in which the player last hit air.
+     * Used to ensure we dont send two sing packets for one hit.
+     */
+    @Setter
+    private int lastAirHitTick;
+
     /**
      * Controls whether the daylight cycle gamerule has been sent to the client, so the sun/moon remain motionless.
      */
@@ -1113,13 +1120,10 @@ public ScheduledFuture<?> scheduleInEventLoop(Runnable runnable, long duration,
 
     public void updateTickingState(float tickRate, boolean frozen) {
         tickThread.cancel(false);
-
         this.tickingFrozen = frozen;
 
         tickRate = MathUtils.clamp(tickRate, 1.0f, 10000.0f);
-
         millisecondsPerTick = 1000.0f / tickRate;
-
         nanosecondsPerTick = MathUtils.ceil(1000000000.0f / tickRate);
         tickThread = tickEventLoop.scheduleAtFixedRate(this::tick, nanosecondsPerTick, nanosecondsPerTick, TimeUnit.NANOSECONDS);
     }
@@ -1132,7 +1136,6 @@ private void executeRunnable(Runnable runnable) {
         } catch (Throwable e) {
             geyser.getLogger().error("Error thrown in " + this.bedrockUsername() + "'s event loop!", e);
         }
-
     }
 
     /**
@@ -1365,13 +1368,10 @@ public void activateArmAnimationTicking() {
     }
 
     /**
-     * For <a href="https://github.com/GeyserMC/Geyser/issues/2113">issue 2113</a> and combating arm ticking activating being delayed in
-     * BedrockAnimateTranslator.
+     * You can't break blocks, attack entities, or use items while driving in a boat
      */
-    public void armSwingPending() {
-        if (armAnimationTicks == -1) {
-            armAnimationTicks = -2;
-        }
+    public boolean isHandsBusy() {
+        return steeringRight || steeringLeft;
     }
 
     /**

core/src/main/java/org/geysermc/geyser/session/cache/InputCache.java

@@ -32,8 +32,11 @@
 import org.cloudburstmc.protocol.bedrock.data.InputMode;
 import org.cloudburstmc.protocol.bedrock.data.PlayerAuthInputData;
 import org.cloudburstmc.protocol.bedrock.packet.PlayerAuthInputPacket;
+import org.geysermc.geyser.entity.type.player.PlayerEntity;
 import org.geysermc.geyser.session.GeyserSession;
+import org.geysermc.mcprotocollib.protocol.data.game.entity.player.PlayerState;
 import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.level.ServerboundPlayerInputPacket;
+import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.player.ServerboundPlayerCommandPacket;
 
 import java.util.Set;
 
@@ -53,7 +56,7 @@ public InputCache(GeyserSession session) {
         this.session = session;
     }
 
-    public void processInputs(PlayerAuthInputPacket packet) {
+    public void processInputs(PlayerEntity entity, PlayerAuthInputPacket packet) {
         // Input is sent to the server before packet positions, as of 1.21.2
         Set<PlayerAuthInputData> bedrockInput = packet.getInputData();
         var oldInputPacket = this.inputPacket;
@@ -74,16 +77,29 @@ public void processInputs(PlayerAuthInputPacket packet) {
             right = analogMovement.getX() < 0;
         }
 
+        boolean sneaking = bedrockInput.contains(PlayerAuthInputData.SNEAKING);
+
         // TODO when is UP_LEFT, etc. used?
         this.inputPacket = this.inputPacket
             .withForward(up)
             .withBackward(down)
             .withLeft(left)
             .withRight(right)
             .withJump(bedrockInput.contains(PlayerAuthInputData.JUMPING)) // Looks like this only triggers when the JUMP key input is being pressed. There's also JUMP_DOWN?
-            .withShift(bedrockInput.contains(PlayerAuthInputData.SNEAKING))
+            .withShift(sneaking)
             .withSprint(bedrockInput.contains(PlayerAuthInputData.SPRINTING)); // SPRINTING will trigger even if the player isn't moving
 
+        // Send sneaking state before inputs, matches Java client
+        if (oldInputPacket.isShift() != sneaking) {
+            if (sneaking) {
+                session.sendDownstreamGamePacket(new ServerboundPlayerCommandPacket(entity.javaId(), PlayerState.START_SNEAKING));
+                session.startSneaking();
+            } else {
+                session.sendDownstreamGamePacket(new ServerboundPlayerCommandPacket(entity.javaId(), PlayerState.STOP_SNEAKING));
+                session.stopSneaking();
+            }
+        }
+
         if (oldInputPacket != this.inputPacket) { // Simple equality check is fine since we're checking for an instance change.
             session.sendDownstreamGamePacket(this.inputPacket);
         }

core/src/main/java/org/geysermc/geyser/translator/level/block/entity/BrushableBlockEntityTranslator.java

@@ -64,7 +64,7 @@ public void translateTag(GeyserSession session, NbtMapBuilder bedrockNbt, @Nulla
         }
         NbtMapBuilder itemBuilder = NbtMap.builder()
             .putString("Name", mapping.getBedrockIdentifier())
-            .putByte("Count", (byte) itemTag.getByte("Count"));
+            .putByte("Count", itemTag.getByte("Count"));
 
         bedrockNbt.putCompound("item", itemBuilder.build());
         // controls which side the item protrudes from

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockAnimateTranslator.java

@@ -45,10 +45,22 @@ public void translate(GeyserSession session, AnimatePacket packet) {
         }
 
         if (packet.getAction() == AnimatePacket.Action.SWING_ARM) {
-            session.armSwingPending();
-            // Delay so entity damage can be processed first
+
+            // If this is the case, we just hit the air. Poor air.
+            // Touch devices send PlayerAuthInputPackets with MISSED_SWING, and then the animate packet.
+            // This tends to happen 1-2 ticks after the auth input packet.
+            if (session.getTicks() - session.getLastAirHitTick() < 3) {
+                return;
+            }
+
+            // Windows unfortunately sends the animate packet first, then the auth input packet with the MISSED_SWING.
+            // Often, these are sent in the same tick. In that case, the wait here ensures the auth input packet is processed first.
+            // Other times, there is a 1-tick-delay, which would result in the swing packet sent here. The BedrockAuthInputTranslator's
+            // MISSED_SWING case also accounts for that by checking if a swing was sent a tick ago here.
+
+            // Also, delay the swing so entity damage can be processed first
             session.scheduleInEventLoop(() -> {
-                    if (session.getArmAnimationTicks() != 0) {
+                    if (session.getArmAnimationTicks() != 0 && (session.getTicks() - session.getLastAirHitTick() > 2)) {
                         // So, generally, a Java player can only do one *thing* at a time.
                         // If a player right-clicks, for example, then there's probably only one action associated with
                         // that right-click that will send a swing.
@@ -61,12 +73,12 @@ public void translate(GeyserSession session, AnimatePacket packet) {
                         // This behavior was last touched on with ViaVersion 4.5.1 (with its packet limiter), Java 1.16.5,
                         // and Bedrock 1.19.51.
                         // Note for the future: we should probably largely ignore this packet and instead replicate
-                        // all actions on our end, and send swings where needed.
+                        // all actions on our end, and send swings where needed. Can be done once we replicate Block and Item interactions fully.
                         session.sendDownstreamGamePacket(new ServerboundSwingPacket(Hand.MAIN_HAND));
                         session.activateArmAnimationTicking();
                     }
                 },
-                25,
+                (long) (session.getMillisecondsPerTick() * 0.5),
                 TimeUnit.MILLISECONDS
             );
         }

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockInventoryTransactionTranslator.java

@@ -454,6 +454,11 @@ public void translate(GeyserSession session, InventoryTransactionPacket packet)
                 switch (packet.getActionType()) {
                     case 0 -> processEntityInteraction(session, packet, entity); // Interact
                     case 1 -> { // Attack
+                        if (session.isHandsBusy()) {
+                            // See Minecraft#startAttack and LocalPlayer#isHandsBusy
+                            return;
+                        }
+
                         int entityId;
                         if (entity.getDefinition() == EntityDefinitions.ENDER_DRAGON) {
                             // Redirects the attack to its body entity, this only happens when

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/entity/player/input/BedrockBlockActions.java

@@ -32,7 +32,6 @@
 import org.cloudburstmc.protocol.bedrock.data.PlayerBlockActionData;
 import org.cloudburstmc.protocol.bedrock.data.definitions.ItemDefinition;
 import org.cloudburstmc.protocol.bedrock.packet.LevelEventPacket;
-import org.geysermc.geyser.GeyserImpl;
 import org.geysermc.geyser.api.block.custom.CustomBlockState;
 import org.geysermc.geyser.entity.type.Entity;
 import org.geysermc.geyser.entity.type.ItemFrameEntity;
@@ -84,6 +83,10 @@ private static void handle(GeyserSession session, PlayerBlockActionData blockAct
                     break;
                 }
 
+                if (!canMine(session, vector)) {
+                    return;
+                }
+
                 // Start the block breaking animation
                 int blockState = session.getGeyser().getWorldManager().getBlockAt(session, vector);
                 LevelEventPacket startBreak = new LevelEventPacket();
@@ -126,6 +129,11 @@ private static void handle(GeyserSession session, PlayerBlockActionData blockAct
                 if (session.getGameMode() == GameMode.CREATIVE) {
                     break;
                 }
+
+                if (!canMine(session, vector)) {
+                    return;
+                }
+
                 int breakingBlock = session.getBreakingBlock();
                 if (breakingBlock == -1) {
                     breakingBlock = Block.JAVA_AIR_ID;
@@ -187,6 +195,7 @@ private static void handle(GeyserSession session, PlayerBlockActionData blockAct
                 stopBreak.setPosition(vector.toFloat());
                 stopBreak.setData(0);
                 session.setBreakingBlock(-1);
+                session.setBlockBreakStartTime(0);
                 session.sendUpstreamPacket(stopBreak);
             }
             // Handled in BedrockInventoryTransactionTranslator
@@ -195,6 +204,22 @@ private static void handle(GeyserSession session, PlayerBlockActionData blockAct
         }
     }
 
+    private static boolean canMine(GeyserSession session, Vector3i vector) {
+        if (session.isHandsBusy()) {
+            session.setBreakingBlock(-1);
+            session.setBlockBreakStartTime(0);
+
+            LevelEventPacket stopBreak = new LevelEventPacket();
+            stopBreak.setType(LevelEvent.BLOCK_STOP_BREAK);
+            stopBreak.setPosition(vector.toFloat());
+            stopBreak.setData(0);
+            session.setBreakingBlock(-1);
+            session.sendUpstreamPacket(stopBreak);
+            return false;
+        }
+        return true;
+    }
+
     private static void spawnBlockBreakParticles(GeyserSession session, Direction direction, Vector3i position, BlockState blockState) {
         LevelEventPacket levelEventPacket = new LevelEventPacket();
         switch (direction) {

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/entity/player/input/BedrockPlayerAuthInputTranslator.java

@@ -29,14 +29,17 @@
 import org.cloudburstmc.math.vector.Vector2f;
 import org.cloudburstmc.math.vector.Vector3f;
 import org.cloudburstmc.math.vector.Vector3i;
+import org.cloudburstmc.protocol.bedrock.data.InputMode;
 import org.cloudburstmc.protocol.bedrock.data.LevelEvent;
 import org.cloudburstmc.protocol.bedrock.data.PlayerActionType;
 import org.cloudburstmc.protocol.bedrock.data.PlayerAuthInputData;
 import org.cloudburstmc.protocol.bedrock.data.entity.EntityFlag;
 import org.cloudburstmc.protocol.bedrock.data.inventory.transaction.ItemUseTransaction;
+import org.cloudburstmc.protocol.bedrock.packet.AnimatePacket;
 import org.cloudburstmc.protocol.bedrock.packet.LevelEventPacket;
 import org.cloudburstmc.protocol.bedrock.packet.PlayerActionPacket;
 import org.cloudburstmc.protocol.bedrock.packet.PlayerAuthInputPacket;
+import org.geysermc.geyser.GeyserImpl;
 import org.geysermc.geyser.entity.EntityDefinitions;
 import org.geysermc.geyser.entity.type.BoatEntity;
 import org.geysermc.geyser.entity.type.Entity;
@@ -53,6 +56,7 @@
 import org.geysermc.geyser.util.CooldownUtils;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.object.Direction;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.player.GameMode;
+import org.geysermc.mcprotocollib.protocol.data.game.entity.player.Hand;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.player.InteractAction;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.player.PlayerAction;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.player.PlayerState;
@@ -61,6 +65,7 @@
 import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.player.ServerboundPlayerAbilitiesPacket;
 import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.player.ServerboundPlayerActionPacket;
 import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.player.ServerboundPlayerCommandPacket;
+import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.player.ServerboundSwingPacket;
 
 import java.util.Set;
 
@@ -72,7 +77,7 @@ public void translate(GeyserSession session, PlayerAuthInputPacket packet) {
         SessionPlayerEntity entity = session.getPlayerEntity();
 
         boolean wasJumping = session.getInputCache().wasJumping();
-        session.getInputCache().processInputs(packet);
+        session.getInputCache().processInputs(entity, packet);
 
         BedrockMovePlayer.translate(session, packet);
 
@@ -83,18 +88,6 @@ public void translate(GeyserSession session, PlayerAuthInputPacket packet) {
             switch (input) {
                 case PERFORM_ITEM_INTERACTION -> processItemUseTransaction(session, packet.getItemUseTransaction());
                 case PERFORM_BLOCK_ACTIONS -> BedrockBlockActions.translate(session, packet.getPlayerActions());
-                case START_SNEAKING -> {
-                    ServerboundPlayerCommandPacket startSneakPacket = new ServerboundPlayerCommandPacket(entity.getEntityId(), PlayerState.START_SNEAKING);
-                    session.sendDownstreamGamePacket(startSneakPacket);
-
-                    session.startSneaking();
-                }
-                case STOP_SNEAKING -> {
-                    ServerboundPlayerCommandPacket stopSneakPacket = new ServerboundPlayerCommandPacket(entity.getEntityId(), PlayerState.STOP_SNEAKING);
-                    session.sendDownstreamGamePacket(stopSneakPacket);
-
-                    session.stopSneaking();
-                }
                 case START_SPRINTING -> {
                     if (!entity.getFlag(EntityFlag.SWIMMING)) {
                         ServerboundPlayerCommandPacket startSprintPacket = new ServerboundPlayerCommandPacket(entity.getEntityId(), PlayerState.START_SPRINTING);
@@ -154,7 +147,25 @@ public void translate(GeyserSession session, PlayerAuthInputPacket packet) {
                     sendPlayerGlideToggle(session, entity);
                 }
                 case STOP_GLIDING -> sendPlayerGlideToggle(session, entity);
-                case MISSED_SWING -> CooldownUtils.sendCooldown(session); // Java edition sends a cooldown when hitting air.
+                case MISSED_SWING -> {
+                    session.setLastAirHitTick(session.getTicks());
+
+                    if (session.getArmAnimationTicks() != 0 && session.getArmAnimationTicks() != 1) {
+                        session.sendDownstreamGamePacket(new ServerboundSwingPacket(Hand.MAIN_HAND));
+                        session.activateArmAnimationTicking();
+                    }
+
+                    // Touch devices expect an animation packet sent back to them
+                    if (packet.getInputMode().equals(InputMode.TOUCH)) {
+                        AnimatePacket animatePacket = new AnimatePacket();
+                        animatePacket.setAction(AnimatePacket.Action.SWING_ARM);
+                        animatePacket.setRuntimeEntityId(session.getPlayerEntity().getGeyserId());
+                        session.sendUpstreamPacket(animatePacket);
+                    }
+
+                    // Java edition sends a cooldown when hitting air.
+                    CooldownUtils.sendCooldown(session);
+                }
             }
         }
         if (entity.getVehicle() instanceof BoatEntity) {

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/world/BedrockLevelSoundEventTranslator.java

@@ -28,7 +28,6 @@
 import org.cloudburstmc.math.vector.Vector3f;
 import org.cloudburstmc.math.vector.Vector3i;
 import org.cloudburstmc.protocol.bedrock.data.SoundEvent;
-import org.cloudburstmc.protocol.bedrock.packet.AnimatePacket;
 import org.cloudburstmc.protocol.bedrock.packet.LevelSoundEventPacket;
 import org.geysermc.geyser.level.block.property.Properties;
 import org.geysermc.geyser.level.block.type.BlockState;
@@ -38,7 +37,6 @@
 import org.geysermc.geyser.util.CooldownUtils;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.object.Direction;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.player.Hand;
-import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.player.ServerboundSwingPacket;
 import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.player.ServerboundUseItemOnPacket;
 
 @Translator(packet = LevelSoundEventPacket.class)
@@ -56,23 +54,6 @@ public void translate(GeyserSession session, LevelSoundEventPacket packet) {
             CooldownUtils.sendCooldown(session);
         }
 
-        if (packet.getSound() == SoundEvent.ATTACK_NODAMAGE && session.getArmAnimationTicks() == -1) {
-            // https://github.com/GeyserMC/Geyser/issues/2113
-            // Seems like consoles and Android with keyboard send the animation packet on 1.19.51, hence the animation
-            // tick check - the animate packet is sent first.
-            // ATTACK_NODAMAGE = player clicked air
-            // This should only be revisited if Bedrock packets get full Java parity, or Bedrock starts sending arm
-            // animation packets after ATTACK_NODAMAGE, OR ATTACK_NODAMAGE gets removed/isn't sent in the same spot
-            session.sendDownstreamGamePacket(new ServerboundSwingPacket(Hand.MAIN_HAND));
-            session.activateArmAnimationTicking();
-
-            // Send packet to Bedrock so it knows
-            AnimatePacket animatePacket = new AnimatePacket();
-            animatePacket.setRuntimeEntityId(session.getPlayerEntity().getGeyserId());
-            animatePacket.setAction(AnimatePacket.Action.SWING_ARM);
-            session.sendUpstreamPacket(animatePacket);
-        }
-
         // Used by client to get book from lecterns in survial mode since 1.20.70
         if (packet.getSound() == SoundEvent.HIT) {
             Vector3f position = packet.getPosition();