mirrors/PocketMine-MP
1
0
Fork 0
mirror of https://github.com/pmmp/PocketMine-MP.git synced 2025-04-21 08:17:34 +00:00
Code Issues Projects Releases Wiki Activity

Player: remove move buffering, implement simple rate limited movement… (#3167)

Browse Source 
Introduction
This PR is a second attempt at improving movement processing to fix #1215 , #2730 and more.

This is significantly less complex than the previous attempt #2646 -- it gets rid of the movement buffering system entirely and instead relies on a simple rate limit counter to restrict on-the-fly movement processing.

Movement is rate limited to a max average of 2 per tick. It allows up to 5 seconds' backlog to accommodate network lag. The rate limit counter is increased by 2 per tick and decreased once for every movement processed. This prevents movement processing being abused for denial of service attacks.

Changes
API changes
This PR, while obviously highly beneficial for most current users, poses some BC-breaking issues because of changes to the internal Player API.

Player->processMovement() (protected) has been removed. This is a BC concern for custom player classes which overrode it and called it as a parent. In addition, child implementations won't be called every tick any more, which could break some custom movement processing systems.
Player->newPosition (protected) has been removed. This internal field may have been accessed by custom movement implementations.
Player->isTeleporting (protected) has been removed. BC concern is same as previous point.
Player->getNextPosition() (public) has been @deprecated.
Added the following protected Player class members:
int $moveRateLimit
?Vector3 $forceMoveSync
handleMovement()
processMostRecentMovements()
revertMovement()
Behavioural changes
Player movement is now subject to less rubberbanding and has more reliable behaviour.
This commit is contained in:
Dylan T 2020-05-31 15:51:30 +01:00 committed by GitHub
parent 71e0521286
commit 485f573955
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 74 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

131
src/pocketmine/Player.php
View File

@ -217,6 +217,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.
*/
@ -329,10 +332,11 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
/** @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 */
@ -865,8 +869,11 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
$this->lastPingMeasure = $pingMS;
}
/**
* @deprecated
*/
public function getNextPosition() : Position{
return $this->newPosition !== null ? Position::fromObject($this->newPosition, $this->level) : $this->getPosition();
return $this->getPosition();
}
public function getInAirTicks() : int{
@ -1517,23 +1524,19 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
}
}
/**
* @return void
*/
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.
@ -1545,7 +1548,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
* 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;
@ -1559,7 +1562,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
$this->move($dx, $dy, $dz);
$diff = $this->distanceSquared($newPos) / $tickDiff ** 2;
$diff = $this->distanceSquared($newPos);
if($this->isSurvival() and $diff > 0.0625){
$ev = new PlayerIllegalMoveEvent($this, $newPos, new Vector3($this->lastX, $this->lastY, $this->lastZ));
@ -1570,7 +1573,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
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);
}
}
@ -1579,13 +1582,25 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
}
}
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;
@ -1597,41 +1612,47 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
$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{
@ -1703,7 +1724,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
$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;
@ -2259,8 +2280,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
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){
@ -2268,9 +2288,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
$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);
@ -2280,7 +2298,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
}
$this->setRotation($packet->yaw, $packet->pitch);
$this->newPosition = $newPos;
$this->handleMovement($newPos);
}
return true;
@ -3794,12 +3812,14 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
$pk->onGround = $this->onGround;
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;
}
/**
@ -3817,11 +3837,8 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
$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();