Merge branch 'master' into fix/write-race-condition

Author: AlexProgrammerDE

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.62 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

@@ -341,7 +341,6 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
 
     @Setter
     private Vector2i lastChunkPosition = null;
-    @Setter
     private int clientRenderDistance = -1;
     private int serverRenderDistance = -1;
 
@@ -536,10 +535,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 +1119,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 +1135,6 @@ private void executeRunnable(Runnable runnable) {
         } catch (Throwable e) {
             geyser.getLogger().error("Error thrown in " + this.bedrockUsername() + "'s event loop!", e);
         }
-
     }
 
     /**
@@ -1365,13 +1367,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;
     }
 
     /**
@@ -1453,11 +1452,31 @@ public void sendCommand(String command) {
         sendDownstreamGamePacket(new ServerboundChatCommandSignedPacket(command, Instant.now().toEpochMilli(), 0L, Collections.emptyList(), 0, new BitSet()));
     }
 
+    public void setClientRenderDistance(int clientRenderDistance) {
+        boolean oldSquareToCircle = this.clientRenderDistance < this.serverRenderDistance;
+        this.clientRenderDistance = clientRenderDistance;
+        boolean newSquareToCircle = this.clientRenderDistance < this.serverRenderDistance;
+
+        if (this.serverRenderDistance != -1 && oldSquareToCircle != newSquareToCircle) {
+            recalculateBedrockRenderDistance();
+        }
+    }
+
     public void setServerRenderDistance(int renderDistance) {
         // Ensure render distance is not above 96 as sending a larger value at any point crashes mobile clients and 96 is the max of any bedrock platform
         renderDistance = Math.min(renderDistance, 96);
         this.serverRenderDistance = renderDistance;
 
+        recalculateBedrockRenderDistance();
+    }
+
+    /**
+     * Ensures that the ChunkRadiusUpdatedPacket uses the correct render distance for whatever the client distance is set as.
+     * If the server render distance is larger than the client's, then account for this and add some extra padding.
+     * We don't want to apply this for every render distance, if at all possible, because
+     */
+    private void recalculateBedrockRenderDistance() {
+        int renderDistance = ChunkUtils.squareToCircle(this.serverRenderDistance);
         ChunkRadiusUpdatedPacket chunkRadiusUpdatedPacket = new ChunkRadiusUpdatedPacket();
         chunkRadiusUpdatedPacket.setRadius(renderDistance);
         upstream.sendPacket(chunkRadiusUpdatedPacket);

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/session/cache/registry/JavaRegistries.java

@@ -27,6 +27,7 @@
 
 import net.kyori.adventure.key.Key;
 import org.checkerframework.checker.nullness.qual.Nullable;
+import org.geysermc.geyser.inventory.item.BannerPattern;
 import org.geysermc.geyser.item.enchantment.Enchantment;
 import org.geysermc.geyser.item.type.Item;
 import org.geysermc.geyser.level.block.type.Block;
@@ -48,6 +49,7 @@ public class JavaRegistries {
     public static final JavaRegistryKey<Block> BLOCK = create("block", BlockRegistries.JAVA_BLOCKS, Block::javaId);
     public static final JavaRegistryKey<Item> ITEM = create("item", Registries.JAVA_ITEMS, Item::javaId);
     public static final JavaRegistryKey<Enchantment> ENCHANTMENT = create("enchantment", RegistryCache::enchantments);
+    public static final JavaRegistryKey<BannerPattern> BANNER_PATTERNS = create("banner_pattern", RegistryCache::bannerPatterns);
 
     private static <T> JavaRegistryKey<T> create(String key, JavaRegistryKey.NetworkSerializer<T> networkSerializer, JavaRegistryKey.NetworkDeserializer<T> networkDeserializer) {
         JavaRegistryKey<T> registry = new JavaRegistryKey<>(MinecraftKey.key(key), networkSerializer, networkDeserializer);

core/src/main/java/org/geysermc/geyser/translator/inventory/LoomInventoryTranslator.java

@@ -25,8 +25,7 @@
 
 package org.geysermc.geyser.translator.inventory;
 
-import it.unimi.dsi.fastutil.objects.Object2IntMap;
-import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
+import net.kyori.adventure.key.Key;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.cloudburstmc.nbt.NbtMap;
 import org.cloudburstmc.nbt.NbtType;
@@ -39,15 +38,19 @@
 import org.cloudburstmc.protocol.bedrock.data.inventory.itemstack.request.action.ItemStackRequestAction;
 import org.cloudburstmc.protocol.bedrock.data.inventory.itemstack.request.action.ItemStackRequestActionType;
 import org.cloudburstmc.protocol.bedrock.data.inventory.itemstack.response.ItemStackResponse;
+import org.geysermc.geyser.GeyserImpl;
 import org.geysermc.geyser.inventory.BedrockContainerSlot;
 import org.geysermc.geyser.inventory.GeyserItemStack;
 import org.geysermc.geyser.inventory.Inventory;
 import org.geysermc.geyser.inventory.SlotType;
+import org.geysermc.geyser.inventory.item.BannerPattern;
 import org.geysermc.geyser.inventory.updater.UIInventoryUpdater;
 import org.geysermc.geyser.item.type.BannerItem;
 import org.geysermc.geyser.item.type.DyeItem;
 import org.geysermc.geyser.level.block.Blocks;
 import org.geysermc.geyser.session.GeyserSession;
+import org.geysermc.geyser.session.cache.registry.JavaRegistries;
+import org.geysermc.geyser.session.cache.tags.Tag;
 import org.geysermc.mcprotocollib.protocol.data.game.item.component.BannerPatternLayer;
 import org.geysermc.mcprotocollib.protocol.data.game.item.component.DataComponentTypes;
 import org.geysermc.mcprotocollib.protocol.packet.ingame.serverbound.inventory.ServerboundContainerButtonClickPacket;
@@ -56,47 +59,8 @@
 import java.util.List;
 
 public class LoomInventoryTranslator extends AbstractBlockInventoryTranslator {
-    /**
-     * A map of Bedrock patterns to Java index. Used to request for a specific banner pattern.
-     */
-    private static final Object2IntMap<String> PATTERN_TO_INDEX = new Object2IntOpenHashMap<>();
-
-    static {
-        // Added from left-to-right then up-to-down in the order Java presents it
-        int index = 0;
-        PATTERN_TO_INDEX.put("bl", index++);
-        PATTERN_TO_INDEX.put("br", index++);
-        PATTERN_TO_INDEX.put("tl", index++);
-        PATTERN_TO_INDEX.put("tr", index++);
-        PATTERN_TO_INDEX.put("bs", index++);
-        PATTERN_TO_INDEX.put("ts", index++);
-        PATTERN_TO_INDEX.put("ls", index++);
-        PATTERN_TO_INDEX.put("rs", index++);
-        PATTERN_TO_INDEX.put("cs", index++);
-        PATTERN_TO_INDEX.put("ms", index++);
-        PATTERN_TO_INDEX.put("drs", index++);
-        PATTERN_TO_INDEX.put("dls", index++);
-        PATTERN_TO_INDEX.put("ss", index++);
-        PATTERN_TO_INDEX.put("cr", index++);
-        PATTERN_TO_INDEX.put("sc", index++);
-        PATTERN_TO_INDEX.put("bt", index++);
-        PATTERN_TO_INDEX.put("tt", index++);
-        PATTERN_TO_INDEX.put("bts", index++);
-        PATTERN_TO_INDEX.put("tts", index++);
-        PATTERN_TO_INDEX.put("ld", index++);
-        PATTERN_TO_INDEX.put("rd", index++);
-        PATTERN_TO_INDEX.put("lud", index++);
-        PATTERN_TO_INDEX.put("rud", index++);
-        PATTERN_TO_INDEX.put("mc", index++);
-        PATTERN_TO_INDEX.put("mr", index++);
-        PATTERN_TO_INDEX.put("vh", index++);
-        PATTERN_TO_INDEX.put("hh", index++);
-        PATTERN_TO_INDEX.put("vhr", index++);
-        PATTERN_TO_INDEX.put("hhb", index++);
-        PATTERN_TO_INDEX.put("bo", index++);
-        PATTERN_TO_INDEX.put("gra", index++);
-        PATTERN_TO_INDEX.put("gru", index);
-    }
+
+    private static final Tag<BannerPattern> NO_ITEMS_REQUIRED = new Tag<>(JavaRegistries.BANNER_PATTERNS, Key.key("no_item_required"));
 
     public LoomInventoryTranslator() {
         super(4, Blocks.LOOM, ContainerType.LOOM, UIInventoryUpdater.INSTANCE);
@@ -136,8 +100,13 @@ public ItemStackResponse translateSpecialRequest(GeyserSession session, Inventor
 
         String bedrockPattern = ((CraftLoomAction) headerData).getPatternId();
 
-        // Get the Java index of this pattern
-        int index = PATTERN_TO_INDEX.getOrDefault(bedrockPattern, -1);
+        BannerPattern requestedPattern = BannerPattern.getByBedrockIdentifier(bedrockPattern);
+        if (requestedPattern == null) {
+            GeyserImpl.getInstance().getLogger().warning("Unknown Bedrock pattern id: " + bedrockPattern);
+            return rejectRequest(request);
+        }
+
+        int index = session.getTagCache().get(NO_ITEMS_REQUIRED).indexOf(requestedPattern);
         if (index == -1) {
             return rejectRequest(request);
         }

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