Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reworked Vehicle PlayerMoveEvent Handling #11835

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+52 −86
Open

Reworked Vehicle PlayerMoveEvent Handling #11835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build-data/paper.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ public net.minecraft.world.entity.LivingEntity detectEquipmentUpdates()V
public net.minecraft.world.entity.LivingEntity effectsDirty
public net.minecraft.world.entity.LivingEntity entityEventForEquipmentBreak(Lnet/minecraft/world/entity/EquipmentSlot;)B
public net.minecraft.world.entity.LivingEntity getDeathSound()Lnet/minecraft/sounds/SoundEvent;
public net.minecraft.world.entity.LivingEntity getRiddenSpeed(Lnet/minecraft/world/entity/player/Player;)F
public net.minecraft.world.entity.LivingEntity getSoundVolume()F
public net.minecraft.world.entity.LivingEntity jumping
public net.minecraft.world.entity.LivingEntity lastHurt
Expand Down
36 changes: 17 additions & 19 deletions ...r-server/patches/features/0030-Optimise-collision-checking-in-player-move-packet-ha.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ Subject: [PATCH] Optimise collision checking in player move packet handling
Move collision logic to just the hasNewCollision call instead of getCubes + hasNewCollision

diff --git a/net/minecraft/server/network/ServerGamePacketListenerImpl.java b/net/minecraft/server/network/ServerGamePacketListenerImpl.java
index 650126e6445c0458c6a6649c235908bfeea428cd..d248671b2e1c6256fc4d74320bdb29ca078bad0b 100644
index f56c5cc86647025bed91a27b26d53233d82a4b56..2ae83a4b3fc7f4fb2c13ba7adbb21f42fc2abf20 100644
--- a/net/minecraft/server/network/ServerGamePacketListenerImpl.java
+++ b/net/minecraft/server/network/ServerGamePacketListenerImpl.java
@@ -561,7 +561,7 @@ public class ServerGamePacketListenerImpl
@@ -553,7 +553,7 @@ public class ServerGamePacketListenerImpl
return;
}

Expand All @@ -18,15 +18,15 @@ index 650126e6445c0458c6a6649c235908bfeea428cd..d248671b2e1c6256fc4d74320bdb29ca
d3 = d - this.vehicleLastGoodX; // Paper - diff on change, used for checking large move vectors above
d4 = d1 - this.vehicleLastGoodY; // Paper - diff on change, used for checking large move vectors above
d5 = d2 - this.vehicleLastGoodZ; // Paper - diff on change, used for checking large move vectors above
@@ -571,6 +571,7 @@ public class ServerGamePacketListenerImpl
@@ -563,6 +563,7 @@ public class ServerGamePacketListenerImpl
}

rootVehicle.move(MoverType.PLAYER, new Vec3(d3, d4, d5));
+ boolean didCollide = toX != rootVehicle.getX() || toY != rootVehicle.getY() || toZ != rootVehicle.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 verticalDelta = d4; // Paper - Decompile fix, was named d11 previously, is now gone in the source
d3 = d - rootVehicle.getX();
d4 = d1 - rootVehicle.getY();
@@ -582,14 +583,22 @@ public class ServerGamePacketListenerImpl
@@ -574,15 +575,20 @@ public class ServerGamePacketListenerImpl
d7 = d3 * d3 + d4 * d4 + d5 * d5;
boolean flag2 = false;
if (d7 > org.spigotmc.SpigotConfig.movedWronglyThreshold) { // Spigot
Expand All @@ -36,23 +36,21 @@ index 650126e6445c0458c6a6649c235908bfeea428cd..d248671b2e1c6256fc4d74320bdb29ca
}

rootVehicle.absMoveTo(d, d1, d2, f, f1);
this.player.absMoveTo(d, d1, d2, this.player.getYRot(), this.player.getXRot()); // CraftBukkit
- boolean flag3 = serverLevel.noCollision(rootVehicle, rootVehicle.getBoundingBox().deflate(0.0625));
- if (flag && (flag2 || !flag3)) {
+ // Paper start - optimise out extra getCubes
// Paper start
- boolean teleportBack = flag && (flag2 || !flag3);
+ boolean teleportBack = flag2; // violating this is always a fail
+ if (!teleportBack) {
if (!teleportBack) {
+ // note: only call after setLocation, or else getBoundingBox is wrong
+ AABB newBox = rootVehicle.getBoundingBox();
+ if (didCollide || !oldBox.equals(newBox)) {
+ teleportBack = this.hasNewCollision(serverLevel, rootVehicle, oldBox, newBox);
+ } // else: no collision at all detected, why do we care?
+ }
+ if (teleportBack) { // Paper end - optimise out extra getCubes
rootVehicle.absMoveTo(x, y, z, f, f1);
this.player.absMoveTo(x, y, z, this.player.getYRot(), this.player.getXRot()); // CraftBukkit
this.send(ClientboundMoveVehiclePacket.fromEntity(rootVehicle));
@@ -667,9 +676,32 @@ public class ServerGamePacketListenerImpl
+
// Move event - emulating from the vehicle
org.bukkit.entity.Entity rootVehicleBukkit = rootVehicle.getBukkitEntity();
Location from = new Location(rootVehicleBukkit.getWorld(), x, y, z, f, f1); // Copy from #absMoveTo on teleportBack
@@ -636,9 +642,32 @@ public class ServerGamePacketListenerImpl
}

private boolean noBlocksAround(Entity entity) {
Expand Down Expand Up @@ -88,7 +86,7 @@ index 650126e6445c0458c6a6649c235908bfeea428cd..d248671b2e1c6256fc4d74320bdb29ca
}

@Override
@@ -1361,7 +1393,7 @@ public class ServerGamePacketListenerImpl
@@ -1330,7 +1359,7 @@ public class ServerGamePacketListenerImpl
}
}

Expand All @@ -97,15 +95,15 @@ index 650126e6445c0458c6a6649c235908bfeea428cd..d248671b2e1c6256fc4d74320bdb29ca
d3 = d - this.lastGoodX; // Paper - diff on change, used for checking large move vectors above
d4 = d1 - this.lastGoodY; // Paper - diff on change, used for checking large move vectors above
d5 = d2 - this.lastGoodZ; // Paper - diff on change, used for checking large move vectors above
@@ -1400,6 +1432,7 @@ public class ServerGamePacketListenerImpl
@@ -1369,6 +1398,7 @@ public class ServerGamePacketListenerImpl
boolean flag1 = this.player.verticalCollisionBelow;
this.player.move(MoverType.PLAYER, new Vec3(d3, d4, d5));
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.
@@ -1432,7 +1465,17 @@ public class ServerGamePacketListenerImpl
@@ -1401,7 +1431,17 @@ public class ServerGamePacketListenerImpl
}

// Paper start - Add fail move event
Expand All @@ -124,7 +122,7 @@ index 650126e6445c0458c6a6649c235908bfeea428cd..d248671b2e1c6256fc4d74320bdb29ca
if (teleportBack) {
io.papermc.paper.event.player.PlayerFailMoveEvent event = fireFailMove(io.papermc.paper.event.player.PlayerFailMoveEvent.FailReason.CLIPPED_INTO_BLOCK,
toX, toY, toZ, toYaw, toPitch, false);
@@ -1568,7 +1611,7 @@ public class ServerGamePacketListenerImpl
@@ -1530,7 +1570,7 @@ public class ServerGamePacketListenerImpl

private boolean updateAwaitingTeleport() {
if (this.awaitingPositionFromClient != null) {
Expand All @@ -133,7 +131,7 @@ index 650126e6445c0458c6a6649c235908bfeea428cd..d248671b2e1c6256fc4d74320bdb29ca
this.awaitingTeleportTime = this.tickCount;
this.teleport(
this.awaitingPositionFromClient.x,
@@ -1587,6 +1630,33 @@ public class ServerGamePacketListenerImpl
@@ -1549,6 +1589,33 @@ public class ServerGamePacketListenerImpl
}
}

Expand Down
101 changes: 34 additions & 67 deletions ...rver/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@
this.player.setLastClientInput(packet.input());
}

@@ -390,17 +_,29 @@
@@ -390,17 +_,22 @@
public void handleMoveVehicle(ServerboundMoveVehiclePacket packet) {
PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
if (containsInvalidValues(packet.position().x(), packet.position().y(), packet.position().z(), packet.yRot(), packet.xRot())) {
Expand All @@ -182,13 +182,6 @@
+ // Paper end - Don't allow vehicle movement from players while teleporting
if (rootVehicle != this.player && rootVehicle.getControllingPassenger() == this.player && rootVehicle == this.lastVehicle) {
ServerLevel serverLevel = this.player.serverLevel();
+ // CraftBukkit - store current player position
+ double prevX = this.player.getX();
+ double prevY = this.player.getY();
+ double prevZ = this.player.getZ();
+ float prevYaw = this.player.getYRot();
+ float prevPitch = this.player.getXRot();
+ // CraftBukkit end
double x = rootVehicle.getX();
double y = rootVehicle.getY();
double z = rootVehicle.getZ();
Expand Down Expand Up @@ -276,7 +269,7 @@
d3 = d - rootVehicle.getX();
d4 = d1 - rootVehicle.getY();
if (d4 > -0.5 || d4 < 0.5) {
@@ -435,18 +_,71 @@
@@ -435,14 +_,44 @@
d5 = d2 - rootVehicle.getZ();
d7 = d3 * d3 + d4 * d4 + d5 * d5;
boolean flag2 = false;
Expand All @@ -287,68 +280,42 @@
}

rootVehicle.absMoveTo(d, d1, d2, f, f1);
+ this.player.absMoveTo(d, d1, d2, this.player.getYRot(), this.player.getXRot()); // CraftBukkit
boolean flag3 = serverLevel.noCollision(rootVehicle, rootVehicle.getBoundingBox().deflate(0.0625));
if (flag && (flag2 || !flag3)) {
- if (flag && (flag2 || !flag3)) {
+ // Paper start
+ boolean teleportBack = flag && (flag2 || !flag3);
+ if (!teleportBack) {
+ // Move event - emulating from the vehicle
+ org.bukkit.entity.Entity rootVehicleBukkit = rootVehicle.getBukkitEntity();
+ Location from = new Location(rootVehicleBukkit.getWorld(), x, y, z, f, f1); // Copy from #absMoveTo on teleportBack
+ Location to = rootVehicleBukkit.getLocation();
+
+ // Prevent 40 event-calls for less than a single pixel of movement >.>
+ double delta = Math.pow(from.getX() - to.getX(), 2) + Math.pow(from.getY() - to.getY(), 2) + Math.pow(from.getZ() - to.getZ(), 2);
+ float deltaAngle = Math.abs(from.getYaw() - to.getYaw()) + Math.abs(from.getPitch() - to.getPitch());
+
+ if ((delta > 1f / 256 || deltaAngle > 10f) && !this.player.isImmobile()) {
+ Location oldLoc = to.clone();
+ PlayerMoveEvent event = new PlayerMoveEvent(this.player.getBukkitEntity(), from, to);
+ if (event.callEvent()) {
+ if (event.getTo() != to || !oldLoc.equals(event.getTo())) { // If instance changed or value change, resync
+ Location newTo = event.getTo();
+ rootVehicle.absMoveTo(newTo.x(), newTo.y(), newTo.z(), newTo.getYaw(), newTo.getPitch());
+ this.send(ClientboundMoveVehiclePacket.fromEntity(rootVehicle)); // resync the vehicle
+ }
+ } else {
+ teleportBack = true;
+ // Reset the above tick -- as ignore checks that they are flying
+ // Most notable if they are in air.
+ this.aboveGroundTickCount = 0;
+ }
+ }
+ }
+ if (teleportBack) {
+ // Paper end
rootVehicle.absMoveTo(x, y, z, f, f1);
+ this.player.absMoveTo(x, y, z, this.player.getYRot(), this.player.getXRot()); // CraftBukkit
this.send(ClientboundMoveVehiclePacket.fromEntity(rootVehicle));
return;
}
+
+ // CraftBukkit start - fire PlayerMoveEvent
+ org.bukkit.entity.Player player = this.getCraftPlayer();
+ if (!this.hasMoved) {
+ this.lastPosX = prevX;
+ this.lastPosY = prevY;
+ this.lastPosZ = prevZ;
+ this.lastYaw = prevYaw;
+ this.lastPitch = prevPitch;
+ this.hasMoved = true;
+ }
+ Location from = new Location(player.getWorld(), this.lastPosX, this.lastPosY, this.lastPosZ, this.lastYaw, this.lastPitch); // Get the Players previous Event location.
+ Location to = CraftLocation.toBukkit(packet.position(), player.getWorld(), packet.yRot(), packet.xRot());
+
+ // Prevent 40 event-calls for less than a single pixel of movement >.>
+ double delta = Math.pow(this.lastPosX - to.getX(), 2) + Math.pow(this.lastPosY - to.getY(), 2) + Math.pow(this.lastPosZ - to.getZ(), 2);
+ float deltaAngle = Math.abs(this.lastYaw - to.getYaw()) + Math.abs(this.lastPitch - to.getPitch());
+
+ if ((delta > 1f / 256 || deltaAngle > 10f) && !this.player.isImmobile()) {
+ this.lastPosX = to.getX();
+ this.lastPosY = to.getY();
+ this.lastPosZ = to.getZ();
+ this.lastYaw = to.getYaw();
+ this.lastPitch = to.getPitch();
+
+ Location oldTo = to.clone();
+ PlayerMoveEvent event = new PlayerMoveEvent(player, from, to);
+ this.cserver.getPluginManager().callEvent(event);
+
+ // If the event is cancelled we move the player back to their old location.
+ if (event.isCancelled()) {
+ this.teleport(from);
+ return;
+ }
+
+ // If a Plugin has changed the To destination then we teleport the Player
+ // there to avoid any 'Moved wrongly' or 'Moved too quickly' errors.
+ // We only do this if the Event was not cancelled.
+ if (!oldTo.equals(event.getTo()) && !event.isCancelled()) {
+ this.player.getBukkitEntity().teleport(event.getTo(), PlayerTeleportEvent.TeleportCause.PLUGIN);
+ return;
+ }
+
+ // Check to see if the Players Location has some how changed during the call of the event.
+ // This can happen due to a plugin teleporting the player instead of using .setTo()
+ if (!from.equals(this.getCraftPlayer().getLocation()) && this.justTeleported) {
+ this.justTeleported = false;
+ return;
+ }
+ }
+ // CraftBukkit end

this.player.serverLevel().getChunkSource().move(this.player);
rootVehicle.recordMovementThroughBlocks(new Vec3(x, y, z), rootVehicle.position());
@@ -455,7 +_,7 @@
rootVehicle.setOnGroundWithMovement(packet.onGround(), vec3);
rootVehicle.doCheckFallDamage(vec3.x, vec3.y, vec3.z, packet.onGround());
Expand Down
Loading