Update player bounding box in more scenarios (#2377)

Co-authored-by: Camotoy <20743703+Camotoy@users.noreply.github.com>

connector/src/main/java/org/geysermc/connector/entity/player/PlayerEntity.java

@@ -155,10 +155,6 @@ public class PlayerEntity extends LivingEntity {
         setRotation(rotation);
         this.position = Vector3f.from(position.getX() + relX, position.getY() + relY, position.getZ() + relZ);
 
-        // If this is the player logged in through this Geyser session
-        if (geyserId == 1) {
-            session.getCollisionManager().updatePlayerBoundingBox(position);
-        }
         setOnGround(isOnGround);
 
         MovePlayerPacket movePlayerPacket = new MovePlayerPacket();
@@ -230,18 +226,7 @@ public class PlayerEntity extends LivingEntity {
 
     @Override
     public void setPosition(Vector3f position) {
-        setPosition(position, true);
+        super.setPosition(position.add(0, entityType.getOffset(), 0));
-    }
-
-    /**
-     * Set the player position and specify if the entity type's offset should be added. Set to false when the player
-     * sends us a move packet where the offset is already added
-     *
-     * @param position the new position of the Bedrock player
-     * @param includeOffset whether to include the offset
-     */
-    public void setPosition(Vector3f position, boolean includeOffset) {
-        this.position = includeOffset ? position.add(0, entityType.getOffset(), 0) : position;
     }
 
     @Override

connector/src/main/java/org/geysermc/connector/entity/player/SessionPlayerEntity.java

@@ -74,9 +74,9 @@ public class SessionPlayerEntity extends PlayerEntity {
     }
 
     @Override
-    public void moveAbsolute(GeyserSession session, Vector3f position, Vector3f rotation, boolean isOnGround, boolean teleported) {
+    public void moveRelative(GeyserSession session, double relX, double relY, double relZ, Vector3f rotation, boolean isOnGround) {
-        session.getCollisionManager().updatePlayerBoundingBox(position);
+        super.moveRelative(session, relX, relY, relZ, rotation, isOnGround);
-        super.moveAbsolute(session, position, rotation, isOnGround, teleported);
+        session.getCollisionManager().updatePlayerBoundingBox(this.position.down(entityType.getOffset()));
     }
 
     @Override
@@ -87,6 +87,17 @@ public class SessionPlayerEntity extends PlayerEntity {
         super.setPosition(position);
     }
 
+    /**
+     * Set the player's position without applying an offset or moving the bounding box
+     * This is used in BedrockMovePlayerTranslator which receives the player's position
+     * with the offset pre-applied
+     *
+     * @param position the new position of the Bedrock player
+     */
+    public void setPositionManual(Vector3f position) {
+        this.position = position;
+    }
+
     @Override
     public void updateBedrockMetadata(EntityMetadata entityMetadata, GeyserSession session) {
         super.updateBedrockMetadata(entityMetadata, session);

connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java

@@ -1132,9 +1132,9 @@ public class GeyserSession implements CommandSender {
         }
     }
 
-    public boolean confirmTeleport(Vector3d position) {
+    public void confirmTeleport(Vector3d position) {
         if (teleportMap.size() == 0) {
-            return true;
+            return;
         }
         int teleportID = -1;
 
@@ -1185,8 +1185,6 @@ public class GeyserSession implements CommandSender {
                         teleport.getYaw(), teleport.getPitch(), playerEntity.isOnGround(), true);
             }
         }
-
-        return true;
     }
 
     /**

connector/src/main/java/org/geysermc/connector/network/translators/bedrock/entity/player/BedrockMovePlayerTranslator.java

@@ -35,7 +35,7 @@ import com.nukkitx.protocol.bedrock.packet.MoveEntityAbsolutePacket;
 import com.nukkitx.protocol.bedrock.packet.MovePlayerPacket;
 import org.geysermc.connector.GeyserConnector;
 import org.geysermc.connector.common.ChatColor;
-import org.geysermc.connector.entity.player.PlayerEntity;
+import org.geysermc.connector.entity.player.SessionPlayerEntity;
 import org.geysermc.connector.entity.type.EntityType;
 import org.geysermc.connector.network.session.GeyserSession;
 import org.geysermc.connector.network.translators.PacketTranslator;
@@ -54,7 +54,7 @@ public class BedrockMovePlayerTranslator extends PacketTranslator<MovePlayerPack
 
     @Override
     public void translate(MovePlayerPacket packet, GeyserSession session) {
-        PlayerEntity entity = session.getPlayerEntity();
+        SessionPlayerEntity entity = session.getPlayerEntity();
         if (!session.isSpawned()) return;
 
         if (!session.getUpstream().isInitialized()) {
@@ -73,72 +73,71 @@ public class BedrockMovePlayerTranslator extends PacketTranslator<MovePlayerPack
         // Send book update before the player moves
         session.getBookEditCache().checkForSend();
 
-        if (session.confirmTeleport(packet.getPosition().toDouble().sub(0, EntityType.PLAYER.getOffset(), 0))) {
+        session.confirmTeleport(packet.getPosition().toDouble().sub(0, EntityType.PLAYER.getOffset(), 0));
-            // head yaw, pitch, head yaw
+        // head yaw, pitch, head yaw
-            Vector3f rotation = Vector3f.from(packet.getRotation().getY(), packet.getRotation().getX(), packet.getRotation().getY());
+        Vector3f rotation = Vector3f.from(packet.getRotation().getY(), packet.getRotation().getX(), packet.getRotation().getY());
 
-            boolean positionChanged = !entity.getPosition().equals(packet.getPosition());
+        boolean positionChanged = !entity.getPosition().equals(packet.getPosition());
-            boolean rotationChanged = !entity.getRotation().equals(rotation);
+        boolean rotationChanged = !entity.getRotation().equals(rotation);
 
-            // If only the pitch and yaw changed
+        // If only the pitch and yaw changed
-            // This isn't needed, but it makes the packets closer to vanilla
+        // This isn't needed, but it makes the packets closer to vanilla
-            // It also means you can't "lag back" while only looking, in theory
+        // It also means you can't "lag back" while only looking, in theory
-            if (!positionChanged && rotationChanged) {
+        if (!positionChanged && rotationChanged) {
-                ClientPlayerRotationPacket playerRotationPacket = new ClientPlayerRotationPacket(
+            ClientPlayerRotationPacket playerRotationPacket = new ClientPlayerRotationPacket(
-                        packet.isOnGround(), packet.getRotation().getY(), packet.getRotation().getX());
+                    packet.isOnGround(), packet.getRotation().getY(), packet.getRotation().getX());
 
-                entity.setRotation(rotation);
+            entity.setRotation(rotation);
-                entity.setOnGround(packet.isOnGround());
+            entity.setOnGround(packet.isOnGround());
 
-                session.sendDownstreamPacket(playerRotationPacket);
+            session.sendDownstreamPacket(playerRotationPacket);
-            } else {
+        } else {
-                Vector3d position = session.getCollisionManager().adjustBedrockPosition(packet.getPosition(), packet.isOnGround());
+            Vector3d position = session.getCollisionManager().adjustBedrockPosition(packet.getPosition(), packet.isOnGround());
-                if (position != null) { // A null return value cancels the packet
+            if (position != null) { // A null return value cancels the packet
-                    if (isValidMove(session, packet.getMode(), entity.getPosition(), packet.getPosition())) {
+                if (isValidMove(session, packet.getMode(), entity.getPosition(), packet.getPosition())) {
-                        Packet movePacket;
+                    Packet movePacket;
-                        if (rotationChanged) {
+                    if (rotationChanged) {
-                            // Send rotation updates as well
+                        // Send rotation updates as well
-                            movePacket = new ClientPlayerPositionRotationPacket(packet.isOnGround(), position.getX(), position.getY(), position.getZ(),
+                        movePacket = new ClientPlayerPositionRotationPacket(packet.isOnGround(), position.getX(), position.getY(), position.getZ(),
-                                    packet.getRotation().getY(), packet.getRotation().getX());
+                                packet.getRotation().getY(), packet.getRotation().getX());
-                            entity.setRotation(rotation);
+                        entity.setRotation(rotation);
-                        } else {
-                            // Rotation did not change; don't send an update with rotation
-                            movePacket = new ClientPlayerPositionPacket(packet.isOnGround(), position.getX(), position.getY(), position.getZ());
-                        }
-
-                        // Compare positions here for void floor fix below before the player's position variable is set to the packet position
-                        boolean notMovingUp = entity.getPosition().getY() >= packet.getPosition().getY();
-
-                        entity.setPosition(packet.getPosition(), false);
-                        entity.setOnGround(packet.isOnGround());
-
-                        // Send final movement changes
-                        session.sendDownstreamPacket(movePacket);
-
-                        if (notMovingUp) {
-                            int floorY = position.getFloorY();
-                            // If the client believes the world has extended height, then it also believes the void floor
-                            // still exists, just at a lower spot
-                            boolean extendedWorld = session.getChunkCache().isExtendedHeight();
-                            if (floorY <= (extendedWorld ? BEDROCK_OVERWORLD_VOID_FLOOR_LOWER_Y : -38)
-                                    && floorY >= (extendedWorld ? BEDROCK_OVERWORLD_VOID_FLOOR_UPPER_Y : -40)) {
-                                // Work around there being a floor at the bottom of the world and teleport the player below it
-                                // Moving from below to above the void floor works fine
-                                entity.setPosition(entity.getPosition().sub(0, 4f, 0));
-                                MovePlayerPacket movePlayerPacket = new MovePlayerPacket();
-                                movePlayerPacket.setRuntimeEntityId(entity.getGeyserId());
-                                movePlayerPacket.setPosition(entity.getPosition());
-                                movePlayerPacket.setRotation(entity.getBedrockRotation());
-                                movePlayerPacket.setMode(MovePlayerPacket.Mode.TELEPORT);
-                                movePlayerPacket.setTeleportationCause(MovePlayerPacket.TeleportationCause.BEHAVIOR);
-                                session.sendUpstreamPacket(movePlayerPacket);
-                            }
-                        }
                     } else {
-                        // Not a valid move
+                        // Rotation did not change; don't send an update with rotation
-                        session.getConnector().getLogger().debug("Recalculating position...");
+                        movePacket = new ClientPlayerPositionPacket(packet.isOnGround(), position.getX(), position.getY(), position.getZ());
-                        session.getCollisionManager().recalculatePosition();
                     }
+
+                    // Compare positions here for void floor fix below before the player's position variable is set to the packet position
+                    boolean notMovingUp = entity.getPosition().getY() >= packet.getPosition().getY();
+
+                    entity.setPositionManual(packet.getPosition());
+                    entity.setOnGround(packet.isOnGround());
+
+                    // Send final movement changes
+                    session.sendDownstreamPacket(movePacket);
+
+                    if (notMovingUp) {
+                        int floorY = position.getFloorY();
+                        // If the client believes the world has extended height, then it also believes the void floor
+                        // still exists, just at a lower spot
+                        boolean extendedWorld = session.getChunkCache().isExtendedHeight();
+                        if (floorY <= (extendedWorld ? BEDROCK_OVERWORLD_VOID_FLOOR_LOWER_Y : -38)
+                                && floorY >= (extendedWorld ? BEDROCK_OVERWORLD_VOID_FLOOR_UPPER_Y : -40)) {
+                            // Work around there being a floor at the bottom of the world and teleport the player below it
+                            // Moving from below to above the void floor works fine
+                            entity.setPosition(entity.getPosition().sub(0, 4f, 0));
+                            MovePlayerPacket movePlayerPacket = new MovePlayerPacket();
+                            movePlayerPacket.setRuntimeEntityId(entity.getGeyserId());
+                            movePlayerPacket.setPosition(entity.getPosition());
+                            movePlayerPacket.setRotation(entity.getBedrockRotation());
+                            movePlayerPacket.setMode(MovePlayerPacket.Mode.TELEPORT);
+                            movePlayerPacket.setTeleportationCause(MovePlayerPacket.TeleportationCause.BEHAVIOR);
+                            session.sendUpstreamPacket(movePlayerPacket);
+                        }
+                    }
+                } else {
+                    // Not a valid move
+                    session.getConnector().getLogger().debug("Recalculating position...");
+                    session.getCollisionManager().recalculatePosition();
                 }
             }
         }

connector/src/main/java/org/geysermc/connector/network/translators/collision/CollisionManager.java

@@ -54,7 +54,7 @@ public class CollisionManager {
     private final GeyserSession session;
 
     @Getter
-    private BoundingBox playerBoundingBox;
+    private final BoundingBox playerBoundingBox;
 
     /**
      * Whether the player is inside scaffolding
@@ -105,26 +105,16 @@ public class CollisionManager {
     }
 
     /**
-     * Updates the stored bounding box without passing a position, which currently just changes the height depending on if the player is sneaking.
+     * Updates the height of the stored bounding box
      */
     public void updatePlayerBoundingBox() {
-        if (playerBoundingBox == null) {
+        // According to the Minecraft Wiki, when sneaking:
-            Vector3f playerPosition;
+        // - In Bedrock Edition, the height becomes 1.65 blocks, allowing movement through spaces as small as 1.75 (2 - 1⁄4) blocks high.
-            if (session.getPlayerEntity() == null) {
+        // - In Java Edition, the height becomes 1.5 blocks.
-                // Temporary position to prevent NullPointerException
+        // Other instances have the player's bounding box become as small as 0.6 or 0.2.
-                playerPosition = Vector3f.ZERO;
+        double playerHeight = session.getPlayerEntity().getMetadata().getFloat(EntityData.BOUNDING_BOX_HEIGHT);
-            } else {
+        playerBoundingBox.setMiddleY(playerBoundingBox.getMiddleY() - (playerBoundingBox.getSizeY() / 2.0) + (playerHeight / 2.0));
-                playerPosition = session.getPlayerEntity().getPosition();
+        playerBoundingBox.setSizeY(playerHeight);
-            }
-            playerBoundingBox = new BoundingBox(playerPosition.getX(), playerPosition.getY() + 0.9, playerPosition.getZ(),
-                    EntityType.PLAYER.getWidth(), EntityType.PLAYER.getHeight(), EntityType.PLAYER.getLength());
-        } else {
-            // According to the Minecraft Wiki, when sneaking:
-            // - In Bedrock Edition, the height becomes 1.65 blocks, allowing movement through spaces as small as 1.75 (2 - 1⁄4) blocks high.
-            // - In Java Edition, the height becomes 1.5 blocks.
-            // Other instances have the player's bounding box become as small as 0.6 or 0.2.
-            playerBoundingBox.setSizeY(session.getPlayerEntity().getMetadata().getFloat(EntityData.BOUNDING_BOX_HEIGHT));
-        }
     }
 
     /**
@@ -265,9 +255,15 @@ public class CollisionManager {
         if (collision != null) {
             // Determine, if the player's bounding box *were* at full height, if it would intersect with the block
             // at the current location.
+            double originalY = playerBoundingBox.getMiddleY();
+            double originalHeight = playerBoundingBox.getSizeY();
+            double standingY = originalY - (originalHeight / 2.0) + (EntityType.PLAYER.getHeight() / 2.0);
+
             playerBoundingBox.setSizeY(EntityType.PLAYER.getHeight());
+            playerBoundingBox.setMiddleY(standingY);
             boolean result = collision.checkIntersection(playerBoundingBox);
-            playerBoundingBox.setSizeY(session.getPlayerEntity().getMetadata().getFloat(EntityData.BOUNDING_BOX_HEIGHT));
+            playerBoundingBox.setSizeY(originalHeight);
+            playerBoundingBox.setMiddleY(originalY);
             return result;
         }
         return false;

connector/src/main/java/org/geysermc/connector/network/translators/java/entity/player/JavaPlayerPositionRotationTranslator.java

@@ -124,8 +124,18 @@ public class JavaPlayerPositionRotationTranslator extends PacketTranslator<Serve
         session.getConnector().getLogger().debug("Teleport from " + entity.getPosition().getX() + " " + (entity.getPosition().getY() - EntityType.PLAYER.getOffset()) + " " + entity.getPosition().getZ());
 
         session.addTeleport(new TeleportCache(newX, newY, newZ, newPitch, newYaw, packet.getTeleportId()));
-        entity.moveAbsolute(session, Vector3f.from(newX, newY, newZ), newYaw, newPitch, true, true);
+
+        Vector3f lastPlayerPosition = entity.getPosition().down(EntityType.PLAYER.getOffset());
+        float lastPlayerPitch = entity.getBedrockRotation().getX();
+        Vector3f teleportDestination = Vector3f.from(newX, newY, newZ);
+        entity.moveAbsolute(session, teleportDestination, newYaw, newPitch, true, true);
 
         session.getConnector().getLogger().debug("to " + entity.getPosition().getX() + " " + (entity.getPosition().getY() - EntityType.PLAYER.getOffset()) + " " + entity.getPosition().getZ());
+
+        // Bedrock ignores teleports that are extremely close to the player's original position and orientation,
+        // so check if we can immediately confirm the teleport
+        if (lastPlayerPosition.distanceSquared(teleportDestination) < 0.001 && Math.abs(newPitch - lastPlayerPitch) < 5) {
+            session.confirmTeleport(lastPlayerPosition.toDouble());
+        }
     }
 }