From 69ac80518c1ef7a7794f3f1c5b24a8d5f42a3151 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 30 Mar 2017 19:33:47 +0100 Subject: [PATCH] some improvements to the horrendous mess that is the handling of joining and quitting, fixed some crashes, probably caused some other crashes I can't fix this completely because it's just too much of a fucking mess. NEED to separate network stuff from Player. --- src/pocketmine/Player.php | 109 +++++++++--------- src/pocketmine/entity/Entity.php | 3 + src/pocketmine/entity/Human.php | 2 +- src/pocketmine/permission/PermissibleBase.php | 7 +- 4 files changed, 64 insertions(+), 57 deletions(-) diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php index 053dfc36c..a67d3872c 100644 --- a/src/pocketmine/Player.php +++ b/src/pocketmine/Player.php @@ -241,6 +241,7 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade public $playedBefore; public $spawned = false; public $loggedIn = false; + public $joined = false; public $gamemode; public $lastBreak; @@ -945,6 +946,8 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade $pk->z = $pos->z; $this->dataPacket($pk); } + + $this->joined = true; } protected function orderChunks(){ @@ -1971,7 +1974,8 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade public function handleResourcePackClientResponse(ResourcePackClientResponsePacket $packet) : bool{ switch($packet->status){ case ResourcePackClientResponsePacket::STATUS_REFUSED: - $this->close("", "must accept resource packs to join", true); + //TODO: add lang strings for this + $this->close("", "You must accept resource packs to join this server.", true); break; case ResourcePackClientResponsePacket::STATUS_SEND_PACKS: $manager = $this->server->getResourceManager(); @@ -3536,21 +3540,41 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade * @param bool $notify */ public final function close($message = "", $reason = "generic reason", $notify = true){ - if($this->connected and !$this->closed){ if($notify and strlen((string) $reason) > 0){ - $pk = new DisconnectPacket; + $pk = new DisconnectPacket(); $pk->message = $reason; $this->directDataPacket($pk); } $this->connected = false; - if(strlen($this->getName()) > 0){ - $this->server->getPluginManager()->callEvent($ev = new PlayerQuitEvent($this, $message, true)); - if($this->loggedIn === true and $ev->getAutoSave()){ - $this->save(); + + $this->server->getPluginManager()->unsubscribeFromPermission(Server::BROADCAST_CHANNEL_USERS, $this); + $this->server->getPluginManager()->unsubscribeFromPermission(Server::BROADCAST_CHANNEL_ADMINISTRATIVE, $this); + + if($this->joined){ + //TODO: add events for player data saving + $this->save(); + + $this->server->getPluginManager()->callEvent($ev = new PlayerQuitEvent($this, $message)); + if($ev->getQuitMessage() != ""){ + $this->server->broadcastMessage($ev->getQuitMessage()); } } + $this->joined = false; + + if($this->isValid()){ + foreach($this->usedChunks as $index => $d){ + Level::getXZ($index, $chunkX, $chunkZ); + $this->level->unregisterChunkLoader($this, $chunkX, $chunkZ); + foreach($this->level->getChunkEntities($chunkX, $chunkZ) as $entity){ + $entity->despawnFrom($this); + } + unset($this->usedChunks[$index]); + } + } + $this->usedChunks = []; + $this->loadQueue = []; foreach($this->server->getOnlinePlayers() as $player){ if(!$player->canSee($this)){ @@ -3562,61 +3586,40 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade foreach($this->windowIndex as $window){ $this->removeWindow($window); } - - foreach($this->usedChunks as $index => $d){ - Level::getXZ($index, $chunkX, $chunkZ); - $this->level->unregisterChunkLoader($this, $chunkX, $chunkZ); - foreach($this->level->getChunkEntities($chunkX, $chunkZ) as $entity){ - $entity->despawnFrom($this, false); - } - unset($this->usedChunks[$index]); - } + $this->windows = null; + $this->windowIndex = []; parent::close(); + $this->spawned = false; $this->interface->close($this, $notify ? $reason : ""); if($this->loggedIn){ $this->server->removeOnlinePlayer($this); } - $this->loggedIn = false; - $this->server->getPluginManager()->unsubscribeFromPermission(Server::BROADCAST_CHANNEL_USERS, $this); - $this->server->getPluginManager()->unsubscribeFromPermission(Server::BROADCAST_CHANNEL_ADMINISTRATIVE, $this); - - if(isset($ev) and $this->username != "" and $this->spawned !== false and $ev->getQuitMessage() != ""){ - $this->server->broadcastMessage($ev->getQuitMessage()); - } - - $this->spawned = false; $this->server->getLogger()->info($this->getServer()->getLanguage()->translateString("pocketmine.player.logOut", [ TextFormat::AQUA . $this->getName() . TextFormat::WHITE, $this->ip, $this->port, $this->getServer()->getLanguage()->translateString($reason) ])); - $this->windows = new \SplObjectStorage(); - $this->windowIndex = []; - $this->usedChunks = []; - $this->loadQueue = []; - $this->hasSpawned = []; + $this->spawnPosition = null; + + if($this->perm !== null){ + $this->perm->clearPermissions(); + $this->perm = null; + } + + if($this->inventory !== null){ + $this->inventory = null; + $this->currentTransaction = null; + } + + $this->server->removePlayer($this); } - - if($this->perm !== null){ - $this->perm->clearPermissions(); - $this->perm = null; - } - - if($this->inventory !== null){ - $this->inventory = null; - $this->currentTransaction = null; - } - - $this->chunk = null; - - $this->server->removePlayer($this); } public function __debugInfo(){ @@ -3634,7 +3637,7 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade } parent::saveNBT(); - if($this->level instanceof Level){ + if($this->getLevel() instanceof Level){ $this->namedtag->Level = new StringTag("Level", $this->level->getFolderName()); if($this->hasValidSpawnPosition()){ $this->namedtag["SpawnLevel"] = $this->spawnPosition->getLevel()->getFolderName(); @@ -3642,17 +3645,17 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade $this->namedtag["SpawnY"] = (int) $this->spawnPosition->y; $this->namedtag["SpawnZ"] = (int) $this->spawnPosition->z; } + } - foreach($this->achievements as $achievement => $status){ - $this->namedtag->Achievements[$achievement] = new ByteTag($achievement, $status === true ? 1 : 0); - } + foreach($this->achievements as $achievement => $status){ + $this->namedtag->Achievements[$achievement] = new ByteTag($achievement, $status === true ? 1 : 0); + } - $this->namedtag["playerGameType"] = $this->gamemode; - $this->namedtag["lastPlayed"] = new LongTag("lastPlayed", floor(microtime(true) * 1000)); + $this->namedtag["playerGameType"] = $this->gamemode; + $this->namedtag["lastPlayed"] = new LongTag("lastPlayed", floor(microtime(true) * 1000)); - if($this->username != "" and $this->namedtag instanceof CompoundTag){ - $this->server->saveOfflinePlayerData($this->username, $this->namedtag, $async); - } + if($this->username != "" and $this->namedtag instanceof CompoundTag){ + $this->server->saveOfflinePlayerData($this->username, $this->namedtag, $async); } } diff --git a/src/pocketmine/entity/Entity.php b/src/pocketmine/entity/Entity.php index 9cc94f806..fa8aea85b 100644 --- a/src/pocketmine/entity/Entity.php +++ b/src/pocketmine/entity/Entity.php @@ -1661,7 +1661,10 @@ abstract class Entity extends Location implements Metadatable{ if(!$this->closed){ $this->server->getPluginManager()->callEvent(new EntityDespawnEvent($this)); $this->closed = true; + $this->despawnFromAll(); + $this->hasSpawned = []; + if($this->chunk !== null){ $this->chunk->removeEntity($this); $this->chunk = null; diff --git a/src/pocketmine/entity/Human.php b/src/pocketmine/entity/Human.php index 9dd852bfc..732dd23f8 100644 --- a/src/pocketmine/entity/Human.php +++ b/src/pocketmine/entity/Human.php @@ -504,7 +504,7 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{ public function close(){ if(!$this->closed){ - if(!($this instanceof Player) or $this->loggedIn){ + if($this->inventory !== null){ foreach($this->inventory->getViewers() as $viewer){ $viewer->removeWindow($this->inventory); } diff --git a/src/pocketmine/permission/PermissibleBase.php b/src/pocketmine/permission/PermissibleBase.php index 95acb5032..c10049a3e 100644 --- a/src/pocketmine/permission/PermissibleBase.php +++ b/src/pocketmine/permission/PermissibleBase.php @@ -188,12 +188,13 @@ class PermissibleBase implements Permissible{ } public function clearPermissions(){ + $pluginManager = Server::getInstance()->getPluginManager(); foreach(array_keys($this->permissions) as $name){ - Server::getInstance()->getPluginManager()->unsubscribeFromPermission($name, $this->parent !== null ? $this->parent : $this); + $pluginManager->unsubscribeFromPermission($name, $this->parent ?? $this); } - Server::getInstance()->getPluginManager()->unsubscribeFromDefaultPerms(false, $this->parent !== null ? $this->parent : $this); - Server::getInstance()->getPluginManager()->unsubscribeFromDefaultPerms(true, $this->parent !== null ? $this->parent : $this); + $pluginManager->unsubscribeFromDefaultPerms(false, $this->parent ?? $this); + $pluginManager->unsubscribeFromDefaultPerms(true, $this->parent ?? $this); $this->permissions = []; }