Skip to content

Commit

Permalink
Player: remove move buffering, implement simple rate limited movement…
Browse files Browse the repository at this point in the history 
… processing

this needs some work to fix move sync spam in some areas, but this is nothing that wasn't already a problem to begin with.
  • Loading branch information
@dktapps
dktapps committed Oct 21, 2019
1 parent 39cc590 commit 8e43cce
Showing 1 changed file with 72 additions and 54 deletions.
126 changes: 72 additions & 54 deletions src/pocketmine/Player.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
public const SPECTATOR = 3;
public const VIEW = Player::SPECTATOR;

private const MOVES_PER_TICK = 2;
private const MOVE_BACKLOG_SIZE = 100 * self::MOVES_PER_TICK; //100 ticks backlog (5 seconds)

/**
* Validates the given username.
*
Expand Down Expand Up @@ -325,10 +328,11 @@ public static function isValidUserName(?string $name) : bool{
/** @var bool[] map: raw UUID (string) => bool */
protected $hiddenPlayers = [];

/** @var int */
protected $moveRateLimit = 10 * self::MOVES_PER_TICK;
/** @var Vector3|null */
protected $newPosition;
/** @var bool */
protected $isTeleporting = false;
protected $forceMoveSync = null;

/** @var int */
protected $inAirTicks = 0;
/** @var float */
Expand Down Expand Up @@ -904,10 +908,11 @@ public function updatePing(int $pingMS){
}

/**
* @deprecated
* @return Position
*/
public function getNextPosition() : Position{
return $this->newPosition !== null ? Position::fromObject($this->newPosition, $this->level) : $this->getPosition();
return $this->getPosition();
}

public function getInAirTicks() : int{
Expand Down Expand Up @@ -1575,20 +1580,19 @@ protected function checkNearEntities(){
}
}

protected function processMovement(int $tickDiff){
if(!$this->isAlive() or !$this->spawned or $this->newPosition === null or $this->isSleeping()){
protected function handleMovement(Vector3 $newPos) : void{
$this->moveRateLimit--;
if($this->moveRateLimit < 0){
return;
}

assert($this->x !== null and $this->y !== null and $this->z !== null);
assert($this->newPosition->x !== null and $this->newPosition->y !== null and $this->newPosition->z !== null);

$newPos = $this->newPosition;
$distanceSquared = $newPos->distanceSquared($this);
$oldPos = $this->asLocation();
$distanceSquared = $newPos->distanceSquared($oldPos);

$revert = false;

if(($distanceSquared / ($tickDiff ** 2)) > 100){
if($distanceSquared > 100){
//TODO: this is probably too big if we process every movement
/* !!! BEWARE YE WHO ENTER HERE !!!
*
* This is NOT an anti-cheat check. It is a safety check.
Expand All @@ -1600,7 +1604,7 @@ protected function processMovement(int $tickDiff){
* asking for help if you suffer the consequences of messing with this.
*/
$this->server->getLogger()->debug($this->getName() . " moved too fast, reverting movement");
$this->server->getLogger()->debug("Old position: " . $this->asVector3() . ", new position: " . $this->newPosition);
$this->server->getLogger()->debug("Old position: " . $this->asVector3() . ", new position: " . $newPos);
$revert = true;
}elseif(!$this->level->isInLoadedTerrain($newPos) or !$this->level->isChunkGenerated($newPos->getFloorX() >> 4, $newPos->getFloorZ() >> 4)){
$revert = true;
Expand All @@ -1614,7 +1618,7 @@ protected function processMovement(int $tickDiff){

$this->move($dx, $dy, $dz);

$diff = $this->distanceSquared($newPos) / $tickDiff ** 2;
$diff = $this->distanceSquared($newPos);

if($this->isSurvival() and !$revert and $diff > 0.0625){
$ev = new PlayerIllegalMoveEvent($this, $newPos, new Vector3($this->lastX, $this->lastY, $this->lastZ));
Expand All @@ -1625,7 +1629,7 @@ protected function processMovement(int $tickDiff){
if(!$ev->isCancelled()){
$revert = true;
$this->server->getLogger()->debug($this->getServer()->getLanguage()->translateString("pocketmine.player.invalidMove", [$this->getName()]));
$this->server->getLogger()->debug("Old position: " . $this->asVector3() . ", new position: " . $this->newPosition);
$this->server->getLogger()->debug("Old position: " . $this->asVector3() . ", new position: " . $newPos);
}
}

Expand All @@ -1634,13 +1638,25 @@ protected function processMovement(int $tickDiff){
}
}

if($revert){
$this->revertMovement($oldPos);
}
}

/**
* Fires movement events and synchronizes player movement, every tick.
*/
protected function processMostRecentMovements() : void{
$exceededRateLimit = $this->moveRateLimit < 0;
$this->moveRateLimit = min(self::MOVE_BACKLOG_SIZE, max(0, $this->moveRateLimit) + self::MOVES_PER_TICK);

$from = new Location($this->lastX, $this->lastY, $this->lastZ, $this->lastYaw, $this->lastPitch, $this->level);
$to = $this->getLocation();

$delta = (($this->lastX - $to->x) ** 2) + (($this->lastY - $to->y) ** 2) + (($this->lastZ - $to->z) ** 2);
$deltaAngle = abs($this->lastYaw - $to->yaw) + abs($this->lastPitch - $to->pitch);

if(!$revert and ($delta > 0.0001 or $deltaAngle > 1.0)){
if($delta > 0.0001 or $deltaAngle > 1.0){
$this->lastX = $to->x;
$this->lastY = $to->y;
$this->lastZ = $to->z;
Expand All @@ -1652,41 +1668,47 @@ protected function processMovement(int $tickDiff){

$ev->call();

if(!($revert = $ev->isCancelled())){ //Yes, this is intended
if($to->distanceSquared($ev->getTo()) > 0.01){ //If plugins modify the destination
$this->teleport($ev->getTo());
}else{
$this->broadcastMovement();

$distance = sqrt((($from->x - $to->x) ** 2) + (($from->z - $to->z) ** 2));
//TODO: check swimming (adds 0.015 exhaustion in MCPE)
if($this->isSprinting()){
$this->exhaust(0.1 * $distance, PlayerExhaustEvent::CAUSE_SPRINTING);
}else{
$this->exhaust(0.01 * $distance, PlayerExhaustEvent::CAUSE_WALKING);
}
}
if($ev->isCancelled()){
$this->revertMovement($from);
return;
}
}

if($revert){
if($to->distanceSquared($ev->getTo()) > 0.01){ //If plugins modify the destination
$this->teleport($ev->getTo());
return;
}

$this->lastX = $from->x;
$this->lastY = $from->y;
$this->lastZ = $from->z;
$this->broadcastMovement();

$this->lastYaw = $from->yaw;
$this->lastPitch = $from->pitch;
$distance = sqrt((($from->x - $to->x) ** 2) + (($from->z - $to->z) ** 2));
//TODO: check swimming (adds 0.015 exhaustion in MCPE)
if($this->isSprinting()){
$this->exhaust(0.1 * $distance, PlayerExhaustEvent::CAUSE_SPRINTING);
}else{
$this->exhaust(0.01 * $distance, PlayerExhaustEvent::CAUSE_WALKING);
}

$this->setPosition($from);
$this->sendPosition($from, $from->yaw, $from->pitch, MovePlayerPacket::MODE_RESET);
}else{
if($distanceSquared != 0 and $this->nextChunkOrderRun > 20){
if($this->nextChunkOrderRun > 20){
$this->nextChunkOrderRun = 20;
}
}

$this->newPosition = null;
if($exceededRateLimit){ //client and server positions will be out of sync if this happens
$this->server->getLogger()->debug("Player " . $this->getName() . " exceeded movement rate limit, forcing to last accepted position");
$this->sendPosition($this, $this->yaw, $this->pitch, MovePlayerPacket::MODE_RESET);
}
}

protected function revertMovement(Location $from) : void{
$this->lastX = $from->x;
$this->lastY = $from->y;
$this->lastZ = $from->z;

$this->lastYaw = $from->yaw;
$this->lastPitch = $from->pitch;

$this->setPosition($from);
$this->sendPosition($from, $from->yaw, $from->pitch, MovePlayerPacket::MODE_RESET);
}

public function fall(float $fallDistance) : void{
Expand Down Expand Up @@ -1755,7 +1777,7 @@ public function onUpdate(int $currentTick) : bool{
$this->timings->startTiming();

if($this->spawned){
$this->processMovement($tickDiff);
$this->processMostRecentMovements();
$this->motion->x = $this->motion->y = $this->motion->z = 0; //TODO: HACK! (Fixes player knockback being messed up)
if($this->onGround){
$this->inAirTicks = 0;
Expand Down Expand Up @@ -2253,18 +2275,15 @@ public function chat(string $message) : bool{
public function handleMovePlayer(MovePlayerPacket $packet) : bool{
$newPos = $packet->position->subtract(0, $this->baseOffset, 0);

if($this->isTeleporting and $newPos->distanceSquared($this) > 1){ //Tolerate up to 1 block to avoid problems with client-sided physics when spawning in blocks
$this->sendPosition($this, null, null, MovePlayerPacket::MODE_RESET);
if($this->forceMoveSync !== null and $newPos->distanceSquared($this->forceMoveSync) > 1){ //Tolerate up to 1 block to avoid problems with client-sided physics when spawning in blocks
$this->server->getLogger()->debug("Got outdated pre-teleport movement from " . $this->getName() . ", received " . $newPos . ", expected " . $this->asVector3());
//Still getting movements from before teleport, ignore them
}elseif((!$this->isAlive() or !$this->spawned) and $newPos->distanceSquared($this) > 0.01){
$this->sendPosition($this, null, null, MovePlayerPacket::MODE_RESET);
$this->server->getLogger()->debug("Reverted movement of " . $this->getName() . " due to not alive or not spawned, received " . $newPos . ", locked at " . $this->asVector3());
}else{
// Once we get a movement within a reasonable distance, treat it as a teleport ACK and remove position lock
if($this->isTeleporting){
$this->isTeleporting = false;
}
$this->forceMoveSync = null;

$packet->yaw = fmod($packet->yaw, 360);
$packet->pitch = fmod($packet->pitch, 360);
Expand All @@ -2274,7 +2293,7 @@ public function handleMovePlayer(MovePlayerPacket $packet) : bool{
}

$this->setRotation($packet->yaw, $packet->pitch);
$this->newPosition = $newPos;
$this->handleMovement($newPos);
}

return true;
Expand Down Expand Up @@ -3761,12 +3780,14 @@ public function sendPosition(Vector3 $pos, float $yaw = null, float $pitch = nul
$pk->mode = $mode;

if($targets !== null){
if(in_array($this, $targets, true)){
$this->forceMoveSync = $pos->asVector3();
}
$this->server->broadcastPacket($targets, $pk);
}else{
$this->forceMoveSync = $pos->asVector3();
$this->dataPacket($pk);
}

$this->newPosition = null;
}

/**
Expand All @@ -3784,11 +3805,8 @@ public function teleport(Vector3 $pos, float $yaw = null, float $pitch = null) :

$this->resetFallDistance();
$this->nextChunkOrderRun = 0;
$this->newPosition = null;
$this->stopSleep();

$this->isTeleporting = true;

//TODO: workaround for player last pos not getting updated
//Entity::updateMovement() normally handles this, but it's overridden with an empty function in Player
$this->resetLastMovements();
Expand Down

0 comments on commit 8e43cce

Please sign in to comment.