Merge pull request #1824 from Axionize/fix-modern-client-legacy-serve…

…r-walls

Fix Modern Client + Legacy Server Walls

src/main/java/ac/grim/grimac/predictionengine/movementtick/MovementTicker.java

@@ -47,7 +47,7 @@ public static void handleEntityCollisions(GrimPlayer player) {
         if (!player.compensatedEntities.getSelf().inVehicle()) {
             // Calculate the offset of the player to colliding other stuff
             SimpleCollisionBox playerBox = GetBoundingBox.getBoundingBoxFromPosAndSize(player, player.lastX, player.lastY, player.lastZ, 0.6f, 1.8f);
-            playerBox.union(GetBoundingBox.getBoundingBoxFromPosAndSize(player, player.x, player.y, player.z, 0.6f, 1.8f).expand(player.getMovementThreshold()));
+            playerBox.encompass(GetBoundingBox.getBoundingBoxFromPosAndSize(player, player.x, player.y, player.z, 0.6f, 1.8f).expand(player.getMovementThreshold()));
             playerBox.expand(0.2);
 
             final TeamHandler teamHandler = player.checkManager.getPacketCheck(TeamHandler.class);

src/main/java/ac/grim/grimac/utils/collisions/blocks/connecting/DynamicCollisionWall.java

@@ -19,6 +19,8 @@ public class DynamicCollisionWall extends DynamicConnecting implements Collision
     // https://bugs.mojang.com/browse/MC-9565
     // https://bugs.mojang.com/browse/MC-94016
     private static final CollisionBox[] COLLISION_BOXES = makeShapes(4.0F, 3.0F, 24.0F, 0.0F, 24.0F, false, 1);
+    private static final boolean isNewServer = PacketEvents.getAPI().getServerManager().getVersion().isNewerThan(ServerVersion.V_1_12_2);
+
 
     /**
      * @deprecated use DynamicHitboxWall
@@ -115,60 +117,64 @@ public CollisionBox fetchRegularBox(GrimPlayer player, WrappedBlockState state,
         return new SimpleCollisionBox(f, 0.0F, f2, f1, 1, f3);
     }
 
+    /*
+     * This implementation together with the simulation engine have some limitations.
+     * Running into/being knocked into corner walls on a legacy server on a modern client and vice versa
+     * Lead to simulation falses. Fixing this rare edge case requires lots more effort than worth and is low priority
+     */
     @Override
     public CollisionBox fetch(GrimPlayer player, ClientVersion version, WrappedBlockState block, int x, int y, int z) {
-        boolean north;
-        boolean south;
-        boolean west;
-        boolean east;
-        boolean up;
-
-        if (PacketEvents.getAPI().getServerManager().getVersion().isNewerThanOrEquals(ServerVersion.V_1_13)
-                && version.isNewerThan(ClientVersion.V_1_12_2)) {
-            east = block.getEast() != East.NONE;
-            north = block.getNorth() != North.NONE;
-            south = block.getSouth() != South.NONE;
-            west = block.getWest() != West.NONE;
-            up = block.isUp();
-        } else {
-            north = connectsTo(player, version, x, y, z, BlockFace.NORTH);
-            south = connectsTo(player, version, x, y, z, BlockFace.SOUTH);
-            west = connectsTo(player, version, x, y, z, BlockFace.WEST);
-            east = connectsTo(player, version, x, y, z, BlockFace.EAST);
-            up = true;
+        boolean isNewClient = version.isNewerThan(ClientVersion.V_1_12_2);
+
+        // Fast path for new client + new server
+        if (isNewServer && isNewClient) {
+            boolean north = block.getNorth() != North.NONE;
+            boolean south = block.getSouth() != South.NONE;
+            boolean west = block.getWest() != West.NONE;
+            boolean east = block.getEast() != East.NONE;
+
+            return block.isUp()
+                    ? COLLISION_BOXES[getAABBIndex(north, east, south, west)].copy().union(new HexCollisionBox(4, 0, 4, 12, 24, 12))
+                    : COLLISION_BOXES[getAABBIndex(north, east, south, west)].copy();
         }
 
-        // On 1.13+ clients the bounding box is much more complicated
-        if (player.getClientVersion().isNewerThanOrEquals(ClientVersion.V_1_13)) {
-            // Proper and faster way would be to compute all this beforehand
-            if (up) {
-                return COLLISION_BOXES[getAABBIndex(north, east, south, west)].copy().union(new HexCollisionBox(4, 0, 4, 12, 24, 12));
+        // Handle connections for old server or old client
+        boolean north = isNewServer ? block.getNorth() != North.NONE : connectsTo(player, version, x, y, z, BlockFace.NORTH);
+        boolean south = isNewServer ? block.getSouth() != South.NONE : connectsTo(player, version, x, y, z, BlockFace.SOUTH);
+        boolean west = isNewServer ? block.getWest() != West.NONE : connectsTo(player, version, x, y, z, BlockFace.WEST);
+        boolean east = isNewServer ? block.getEast() != East.NONE : connectsTo(player, version, x, y, z, BlockFace.EAST);
+
+        // Only calculate up for new client on old server
+        if (!isNewServer && isNewClient) {
+            boolean up = connectsTo(player, version, x, y, z, BlockFace.UP);
+
+            if (!up) {
+                WrappedBlockState currBlock = player.compensatedWorld.getWrappedBlockStateAt(x, y, z);
+                StateType currType = currBlock.getType();
+
+                boolean selfNorth = currType == player.compensatedWorld.getWrappedBlockStateAt(x, y, z + 1).getType();
+                boolean selfSouth = currType == player.compensatedWorld.getWrappedBlockStateAt(x, y, z - 1).getType();
+                boolean selfWest = currType == player.compensatedWorld.getWrappedBlockStateAt(x - 1, y, z).getType();
+                boolean selfEast = currType == player.compensatedWorld.getWrappedBlockStateAt(x + 1, y, z).getType();
+
+                up = (!selfNorth || !selfSouth || selfWest || selfEast) &&
+                        (!selfWest || !selfEast || selfNorth || selfSouth);
+                return up
+                        ? COLLISION_BOXES[getAABBIndex(north, east, south, west)].copy().union(new HexCollisionBox(4, 0, 4, 12, 24, 12))
+                        : COLLISION_BOXES[getAABBIndex(north, east, south, west)].copy();
             }
-
-            return COLLISION_BOXES[getAABBIndex(north, east, south, west)].copy();
         }
 
-        // Magic 1.8 code for walls that I copied over, 1.12 below uses this mess
+        // Old client collision box calculation
         float f = 0.25F;
         float f1 = 0.75F;
         float f2 = 0.25F;
         float f3 = 0.75F;
 
-        if (north) {
-            f2 = 0.0F;
-        }
-
-        if (south) {
-            f3 = 1.0F;
-        }
-
-        if (west) {
-            f = 0.0F;
-        }
-
-        if (east) {
-            f1 = 1.0F;
-        }
+        if (north) f2 = 0.0F;
+        if (south) f3 = 1.0F;
+        if (west) f = 0.0F;
+        if (east) f1 = 1.0F;
 
         if (north && south && !west && !east) {
             f = 0.3125F;

src/main/java/ac/grim/grimac/utils/collisions/datatypes/DynamicCollisionBox.java

@@ -23,7 +23,7 @@ public DynamicCollisionBox(GrimPlayer player, ClientVersion version, CollisionFa
 
     // Untested but currently unused
     // *should* work because every single one of these eventually becomes a Complex, Simple, or NoCollision Box
-    @Override
+//    @Override
     public CollisionBox union(SimpleCollisionBox other) {
         CollisionBox dynamicBox = box.fetch(player, version, block, x, y, z).offset(x, y, z);
         return dynamicBox.union(other);

src/main/java/ac/grim/grimac/utils/collisions/datatypes/SimpleCollisionBox.java

@@ -164,8 +164,7 @@ public Vector[] corners() {
         return vectors;
     }
 
-    @Override
-    public CollisionBox union(SimpleCollisionBox other) {
+    public CollisionBox encompass(SimpleCollisionBox other) {
         this.minX = Math.min(this.minX, other.minX);
         this.minY = Math.min(this.minY, other.minY);
         this.minZ = Math.min(this.minZ, other.minZ);
@@ -214,6 +213,11 @@ public SimpleCollisionBox combineToMinimum(double x, double y, double z) {
         return this;
     }
 
+    @Override
+    public CollisionBox union(SimpleCollisionBox other) {
+        return new ComplexCollisionBox(2, this, other);
+    }
+
     @Override
     public boolean isCollided(SimpleCollisionBox other) {
         return other.maxX >= this.minX && other.minX <= this.maxX