From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Thu, 2 Jul 2020 12:02:43 -0700 Subject: [PATCH] Optimise collision checking in player move packet handling Move collision logic to just the hasNewCollision call instead of getCubes + hasNewCollision CHECK ME diff --git a/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java b/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java index 2898712d34dc5dd1f2e505746c515f773de2b0d8..5911fc7009a4bdf99f8016d440fbeb1ae9b44441 100644 --- a/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java +++ b/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java @@ -534,7 +534,7 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl return; } - boolean flag = worldserver.noCollision(entity, entity.getBoundingBox().deflate(0.0625D)); + AABB oldBox = entity.getBoundingBox(); // Paper - copy from player movement packet d6 = d3 - this.vehicleLastGoodX; // Paper - diff on change, used for checking large move vectors above d7 = d4 - this.vehicleLastGoodY - 1.0E-6D; // Paper - diff on change, used for checking large move vectors above @@ -550,6 +550,7 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl } entity.move(MoverType.PLAYER, new Vec3(d6, d7, d8)); + boolean didCollide = toX != entity.getX() || toY != entity.getY() || toZ != entity.getZ(); // Paper - needed here as the difference in Y can be reset - also note: this is only a guess at whether collisions took place, floating point errors can make this true when it shouldn't be... double d11 = d7; d6 = d3 - entity.getX(); @@ -563,16 +564,24 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl boolean flag2 = false; if (d10 > org.spigotmc.SpigotConfig.movedWronglyThreshold) { // Spigot - flag2 = true; + flag2 = true; // Paper - diff on change, this should be moved wrongly ServerGamePacketListenerImpl.LOGGER.warn("{} (vehicle of {}) moved wrongly! {}", new Object[]{entity.getName().getString(), this.player.getName().getString(), Math.sqrt(d10)}); } Location curPos = this.getCraftPlayer().getLocation(); // Spigot entity.absMoveTo(d3, d4, d5, f, f1); this.player.absMoveTo(d3, d4, d5, this.player.getYRot(), this.player.getXRot()); // CraftBukkit - boolean flag3 = worldserver.noCollision(entity, entity.getBoundingBox().deflate(0.0625D)); - if (flag && (flag2 || !flag3)) { + // Paper start - optimise out extra getCubes + boolean teleportBack = flag2; // violating this is always a fail + if (!teleportBack) { + // note: only call after setLocation, or else getBoundingBox is wrong + AABB newBox = entity.getBoundingBox(); + if (didCollide || !oldBox.equals(newBox)) { + teleportBack = this.hasNewCollision(worldserver, entity, oldBox, newBox); + } // else: no collision at all detected, why do we care? + } + if (teleportBack) { // Paper end - optimise out extra getCubes entity.absMoveTo(d0, d1, d2, f, f1); this.player.absMoveTo(d0, d1, d2, this.player.getYRot(), this.player.getXRot()); // CraftBukkit this.send(new ClientboundMoveVehiclePacket(entity)); @@ -658,7 +667,32 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl } private boolean noBlocksAround(Entity entity) { - return entity.level().getBlockStates(entity.getBoundingBox().inflate(0.0625D).expandTowards(0.0D, -0.55D, 0.0D)).allMatch(BlockBehaviour.BlockStateBase::isAir); + // Paper start - stop using streams, this is already a known fixed problem in Entity#move + AABB box = entity.getBoundingBox().inflate(0.0625D).expandTowards(0.0D, -0.55D, 0.0D); + int minX = Mth.floor(box.minX); + int minY = Mth.floor(box.minY); + int minZ = Mth.floor(box.minZ); + int maxX = Mth.floor(box.maxX); + int maxY = Mth.floor(box.maxY); + int maxZ = Mth.floor(box.maxZ); + + Level world = entity.level(); + BlockPos.MutableBlockPos pos = new BlockPos.MutableBlockPos(); + + for (int y = minY; y <= maxY; ++y) { + for (int z = minZ; z <= maxZ; ++z) { + for (int x = minX; x <= maxX; ++x) { + pos.set(x, y, z); + BlockState type = world.getBlockStateIfLoaded(pos); + if (type != null && !type.isAir()) { + return false; + } + } + } + } + + return true; + // Paper end - stop using streams, this is already a known fixed problem in Entity#move } @Override @@ -1235,7 +1269,7 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl } if (this.awaitingPositionFromClient != null) { - if (this.tickCount - this.awaitingTeleportTime > 20) { + if (false && this.tickCount - this.awaitingTeleportTime > 20) { // Paper - this will greatly screw with clients with > 1000ms RTT this.awaitingTeleportTime = this.tickCount; this.teleport(this.awaitingPositionFromClient.x, this.awaitingPositionFromClient.y, this.awaitingPositionFromClient.z, this.player.getYRot(), this.player.getXRot()); } @@ -1328,7 +1362,7 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl } } - AABB axisalignedbb = this.player.getBoundingBox(); + AABB axisalignedbb = this.player.getBoundingBox(); // Paper - diff on change, should be old AABB d6 = d0 - this.lastGoodX; // Paper - diff on change, used for checking large move vectors above d7 = d1 - this.lastGoodY; // Paper - diff on change, used for checking large move vectors above @@ -1370,6 +1404,7 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl this.player.move(MoverType.PLAYER, new Vec3(d6, d7, d8)); this.player.onGround = packet.isOnGround(); // CraftBukkit - SPIGOT-5810, SPIGOT-5835, SPIGOT-6828: reset by this.player.move + boolean didCollide = toX != this.player.getX() || toY != this.player.getY() || toZ != this.player.getZ(); // Paper - needed here as the difference in Y can be reset - also note: this is only a guess at whether collisions took place, floating point errors can make this true when it shouldn't be... // Paper start - prevent position desync if (this.awaitingPositionFromClient != null) { return; // ... thanks Mojang for letting move calls teleport across dimensions. @@ -1388,11 +1423,23 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl boolean flag2 = false; if (!this.player.isChangingDimension() && d10 > org.spigotmc.SpigotConfig.movedWronglyThreshold && !this.player.isSleeping() && !this.player.gameMode.isCreative() && this.player.gameMode.getGameModeForPlayer() != GameType.SPECTATOR) { // Spigot - flag2 = true; + flag2 = true; // Paper - diff on change, this should be moved wrongly ServerGamePacketListenerImpl.LOGGER.warn("{} moved wrongly!", this.player.getName().getString()); } - if (!this.player.noPhysics && !this.player.isSleeping() && (flag2 && worldserver.noCollision(this.player, axisalignedbb) || this.isPlayerCollidingWithAnythingNew(worldserver, axisalignedbb, d0, d1, d2))) { + // Paper start - optimise out extra getCubes + this.player.absMoveTo(d0, d1, d2, f, f1); // prevent desync by tping to the set position, dropped for unknown reasons by mojang + // Original for reference: + // boolean teleportBack = flag2 && worldserver.getCubes(this.player, axisalignedbb) || (didCollide && this.a((IWorldReader) worldserver, axisalignedbb)); + boolean teleportBack = flag2; // violating this is always a fail + if (!this.player.noPhysics && !this.player.isSleeping() && !teleportBack) { + AABB newBox = this.player.getBoundingBox(); + if (didCollide || !axisalignedbb.equals(newBox)) { + // note: only call after setLocation, or else getBoundingBox is wrong + teleportBack = this.hasNewCollision(worldserver, this.player, axisalignedbb, newBox); + } // else: no collision at all detected, why do we care? + } + if (!this.player.noPhysics && !this.player.isSleeping() && teleportBack) { // Paper end - optimise out extra getCubes this.internalTeleport(d3, d4, d5, f, f1, Collections.emptySet()); // CraftBukkit - SPIGOT-1807: Don't call teleport event, when the client thinks the player is falling, because the chunks are not loaded on the client yet. this.player.doCheckFallDamage(this.player.getX() - d3, this.player.getY() - d4, this.player.getZ() - d5, packet.isOnGround()); } else { @@ -1478,6 +1525,33 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl } } + // Paper start - optimise out extra getCubes + private boolean hasNewCollision(final ServerLevel world, final Entity entity, final AABB oldBox, final AABB newBox) { + final List collisionsBB = new java.util.ArrayList<>(); + final List collisionsVoxel = new java.util.ArrayList<>(); + io.papermc.paper.util.CollisionUtil.getCollisions( + world, entity, newBox, collisionsVoxel, collisionsBB, + io.papermc.paper.util.CollisionUtil.COLLISION_FLAG_COLLIDE_WITH_UNLOADED_CHUNKS | io.papermc.paper.util.CollisionUtil.COLLISION_FLAG_CHECK_BORDER, + null, null + ); + + for (int i = 0, len = collisionsBB.size(); i < len; ++i) { + final AABB box = collisionsBB.get(i); + if (!io.papermc.paper.util.CollisionUtil.voxelShapeIntersect(box, oldBox)) { + return true; + } + } + + for (int i = 0, len = collisionsVoxel.size(); i < len; ++i) { + final VoxelShape voxel = collisionsVoxel.get(i); + if (!io.papermc.paper.util.CollisionUtil.voxelShapeIntersectNoEmpty(voxel, oldBox)) { + return true; + } + } + + return false; + } + // Paper end - optimise out extra getCubes private boolean isPlayerCollidingWithAnythingNew(LevelReader world, AABB box, double newX, double newY, double newZ) { AABB axisalignedbb1 = this.player.getBoundingBox().move(newX - this.player.getX(), newY - this.player.getY(), newZ - this.player.getZ()); Iterable iterable = world.getCollisions(this.player, axisalignedbb1.deflate(9.999999747378752E-6D));