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

Jump to bottom

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

Merged
merged 3 commits into from
May 31, 2020
Merged

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

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ed444d
Player: remove move buffering, implement simple rate limited movement…
dktapps Oct 21, 2019
88ae3f6
first look at a collection of yucky BC hacks to land this on PM3
dktapps May 31, 2020
306a4cd
Revert "first look at a collection of yucky BC hacks to land this on …
dktapps May 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
131 changes: 74 additions & 57 deletions src/pocketmine/Player.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -329,10 +332,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 @@ -865,8 +869,11 @@ public function updatePing(int $pingMS){
$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{
Expand Down Expand Up @@ -1517,23 +1524,19 @@ protected function checkNearEntities(){
}
}

/**
* @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.
Expand All @@ -1545,7 +1548,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 @@ -1559,7 +1562,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 $diff > 0.0625){
$ev = new PlayerIllegalMoveEvent($this, $newPos, new Vector3($this->lastX, $this->lastY, $this->lastZ));
Expand All @@ -1570,7 +1573,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 @@ -1579,13 +1582,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 @@ -1597,41 +1612,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 @@ -1703,7 +1724,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 @@ -2238,18 +2259,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 @@ -2259,7 +2277,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 @@ -3746,12 +3764,14 @@ public function sendPosition(Vector3 $pos, float $yaw = null, float $pitch = nul
$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;
}

/**
Expand All @@ -3769,11 +3789,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