The bug
The Minecraft server checks if each player's move distance is valid according to the number of ServerboundMovePlayerPacket
s received per tick. If a player reaches this limit, the server will give a warning message (... moved too quickly! ...). In addition, if the server receives more than 5 ServerboundMovePlayerPacket
s per tick, it will give a debug message (... is sending move packets too frequently (... packets since last tick)), effectively reducing the number of packets received to 1. This often causes a rollback of the position for a player moving too quickly.
The problem is that ServerboundMovePlayerPacket$Rot
, which does not affect the player's position, increments the number of the move packets. This means that the more frequently a player moves its rotation, the more likely it is to reach the distance limit. This behavior would be unintentional, given the intent of the code, which limits movement speed.
Code analysis
net.minecraft.server.network.ServerGamePacketListenerImpl
@Override
public void handleMovePlayer(ServerboundMovePlayerPacket $$0) {
// ...
++this.receivedMovePacketCount;
int $$16 = this.receivedMovePacketCount - this.knownMovePacketCount;
if ($$16 > 5) {
LOGGER.debug("{} is sending move packets too frequently ({} packets since last tick)", this.player.getName().getString(), $$16);
$$16 = 1; // the number of move packets is reduced to 1
}
if (!this.player.isChangingDimension()
&& (!this.player.getLevel().getGameRules().getBoolean(GameRules.RULE_DISABLE_ELYTRA_MOVEMENT_CHECK) || !this.player.isFallFlying())) {
float $$17 = this.player.isFallFlying() ? 300.0F : 100.0F;
if ($$15 - $$14 > (double)($$17 * (float)$$16) /* distance limit */ && !this.isSingleplayerOwner()) {
LOGGER.warn("{} moved too quickly! {},{},{}", new Object[]{this.player.getName().getString(), $$11, $$12, $$13});
// reset the position and return
this.teleport(this.player.getX(), this.player.getY(), this.player.getZ(), this.player.getYRot(), this.player.getXRot());
return;
}
}
// ...
}
The way the move packet rate limit is implemented is kind of misguided in general, since it allows a sustained rate of 5 packets per tick. The client has no excuse to send more than 20 per second, so sending 5 in one tick should require 5 skipped packets soon afterwards. (The purpose of the limit is presumably to account for network fluctuations, not for clients running at 100 TPS.) Also, it should be based on wall clock time, not TPS. Because the maximum speed is scaled by the number of received packets (actually the square root of that number due to an algebra mistake in the code), this leniency increases the speed limit to 5x (actually 2.24x) the intended value.
That said, I don't know if the specific behavior this report is about is actually problematic. It kind of makes sense to count rotation-only packets, since the client will on each tick send either a packet that contains the position, or one that doesn't, so they both follow the same tick rate. And of course, an increased rotation packet rate could give an aim/reaction time advantage in combat.
edit: As long as it only affects the speed limit it indeed doesn't make that much sense; and I don't think the vanilla server checks that you're actually aiming at the entity you're hitting either (shouldn't be too hard to store entity positions a few ticks back, then check those against the rotation and an estimate of the currect latency (based on pings and the client's send rate); but still way more involved than anything in vanilla anticheat). I still think the idea of rate limiting rotation packets would make sense if it actually had any effect, but oh well.