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.

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

Redundant awaitingPositionFromClient check in handleMoveVehicle #11895

Open
spring-dependency-management opened this issue Jan 3, 2025 · 0 comments

Comments

@spring-dependency-management
Copy link

spring-dependency-management commented Jan 3, 2025

Expected behavior

+ if (this.awaitingPositionFromClient != null || this.player.isImmobile() || rootVehicle.isRemoved()) {

this kinda confused me until i realized its paper logic bug.

...
//ServerGamePacketListenerImpl.handleMoveVehicle()
 } else if (!this.updateAwaitingTeleport() && this.player.hasClientLoaded()) {
            Entity rootVehicle = this.player.getRootVehicle();
            // Paper start - Don't allow vehicle movement from players while teleporting
            if (this.awaitingPositionFromClient != null || this.player.isImmobile() || rootVehicle.isRemoved()) { // ensuring awaitingPositionFromClient is null
                return;
            }
            // Paper end - Don't allow vehicle movement from players while teleporting
...

as you can see we are ensuring awaitingPositionFromClient is null, but it was already just checked inside this.updateAwaitingTeleport(), which will only return false if awaitingPositionFromClient is null

private boolean updateAwaitingTeleport() {
       if (this.awaitingPositionFromClient != null) {
           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()
               );
           }
           this.allowedPlayerTicks = 20; // CraftBukkit

           return true;
       } else {
           this.awaitingTeleportTime = this.tickCount;
           return false; // returns false here, right after this.awaitingPositionFromClient == null
       }
   }
...
//ServerGamePacketListenerImpl.handleMoveVehicle()
} else if (!this.updateAwaitingTeleport() && this.player.hasClientLoaded()) { // needs to return false in order to proceed
            Entity rootVehicle = this.player.getRootVehicle();
...

Paper version

daddcf6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant