Inseparable set of network changes - these all need each other to work

- Separated player handling and creation from network interfaces
- Rewire disconnects to make them not be recursive
- Batching now uses sessions instead of players
- Fixed DisconnectPacket getting sent to players who disconnect of their own accord

src/pocketmine/Player.php

@@ -125,7 +125,6 @@ use pocketmine\network\mcpe\protocol\types\PlayerPermissions;
 use pocketmine\network\mcpe\protocol\UpdateAttributesPacket;
 use pocketmine\network\mcpe\protocol\UpdateBlockPacket;
 use pocketmine\network\mcpe\VerifyLoginTask;
-use pocketmine\network\NetworkInterface;
 use pocketmine\permission\PermissibleBase;
 use pocketmine\permission\PermissionAttachment;
 use pocketmine\permission\PermissionAttachmentInfo;
@@ -657,13 +656,14 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 	}
 
 	/**
-	 * @param NetworkInterface $interface
+	 * @param Server         $server
-	 * @param string           $ip
+	 * @param NetworkSession $session
-	 * @param int              $port
 	 */
-	public function __construct(NetworkInterface $interface, string $ip, int $port){
+	public function __construct(Server $server, NetworkSession $session){
+		$this->server = $server;
+		$this->networkSession = $session;
+
 		$this->perm = new PermissibleBase($this);
-		$this->server = Server::getInstance();
 		$this->loaderId = Level::generateChunkLoaderId($this);
 		$this->chunksPerTick = (int) $this->server->getProperty("chunk-sending.per-tick", 4);
 		$this->spawnThreshold = (int) (($this->server->getProperty("chunk-sending.spawn-radius", 4) ** 2) * M_PI);
@@ -671,8 +671,6 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 		$this->creationTime = microtime(true);
 
 		$this->allowMovementCheats = (bool) $this->server->getProperty("player.anti-cheat.allow-movement-cheats", false);
-
-		$this->networkSession = new NetworkSession($this->server, $this, $interface, $ip, $port);
 	}
 
 	/**
@@ -906,7 +904,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 		$this->usedChunks[Level::chunkHash($x, $z)] = true;
 		$this->chunkLoadCount++;
 
-		$this->networkSession->getInterface()->putPacket($this, $payload);
+		$this->networkSession->getInterface()->putPacket($this->networkSession, $payload);
 
 		if($this->spawned){
 			foreach($this->level->getChunkEntities($x, $z) as $entity){
@@ -2938,7 +2936,7 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 			try{
 				$ip = $this->networkSession->getIp();
 				$port = $this->networkSession->getPort();
-				$this->networkSession->serverDisconnect($reason, $notify);
+				$this->networkSession->onPlayerDestroyed($reason, $notify);
 				$this->networkSession = null;
 
 				$this->server->getPluginManager()->unsubscribeFromPermission(Server::BROADCAST_CHANNEL_USERS, $this);

src/pocketmine/Server.php

@@ -71,6 +71,7 @@ use pocketmine\nbt\tag\StringTag;
 use pocketmine\network\AdvancedNetworkInterface;
 use pocketmine\network\mcpe\CompressBatchedTask;
 use pocketmine\network\mcpe\NetworkCompression;
+use pocketmine\network\mcpe\NetworkSession;
 use pocketmine\network\mcpe\PacketStream;
 use pocketmine\network\mcpe\protocol\DataPacket;
 use pocketmine\network\mcpe\protocol\PlayerListPacket;
@@ -1860,7 +1861,13 @@ class Server{
 		}
 		Timings::$playerNetworkTimer->startTiming();
 
-		$targets = array_filter($players, function(Player $player) : bool{ return $player->isConnected(); });
+		/** @var NetworkSession[] $targets */
+		$targets = [];
+		foreach($players as $player){
+			if($player->isConnected()){
+				$targets[] = $player->getNetworkSession();
+			}
+		}
 
 		if(!empty($targets)){
 			$stream = new PacketStream();
@@ -1887,13 +1894,14 @@ class Server{
 	}
 
 	/**
-	 * @param string   $payload
+	 * @param string           $payload
-	 * @param Player[] $players
+	 * @param NetworkSession[] $sessions
-	 * @param bool     $immediate
+	 * @param bool             $immediate
 	 */
-	public function broadcastPacketsCallback(string $payload, array $players, bool $immediate = false){
+	public function broadcastPacketsCallback(string $payload, array $sessions, bool $immediate = false){
-		foreach($players as $i){
+		/** @var NetworkSession $session */
-			$i->getNetworkSession()->getInterface()->putPacket($i, $payload, $immediate);
+		foreach($sessions as $session){
+			$session->getInterface()->putPacket($session, $payload, $immediate);
 		}
 	}
 

src/pocketmine/event/player/PlayerCreationEvent.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\event\player;
 
 use pocketmine\event\Event;
+use pocketmine\network\mcpe\NetworkSession;
 use pocketmine\network\NetworkInterface;
 use pocketmine\Player;
 
@@ -31,48 +32,49 @@ use pocketmine\Player;
  * Allows the creation of players overriding the base Player class
  */
 class PlayerCreationEvent extends Event{
-	/** @var NetworkInterface */
+
-	private $interface;
+	/** @var NetworkSession */
-	/** @var string */
+	private $session;
-	private $address;
-	/** @var int */
-	private $port;
 
 	/** @var Player::class */
 	private $baseClass = Player::class;
 	/** @var Player::class */
 	private $playerClass = Player::class;
 
+
 	/**
-	 * @param NetworkInterface $interface
+	 * @param NetworkSession $session
-	 * @param string           $address
-	 * @param int              $port
 	 */
-	public function __construct(NetworkInterface $interface, string $address, int $port){
+	public function __construct(NetworkSession $session){
-		$this->interface = $interface;
+		$this->session = $session;
-		$this->address = $address;
-		$this->port = $port;
 	}
 
 	/**
 	 * @return NetworkInterface
 	 */
 	public function getInterface() : NetworkInterface{
-		return $this->interface;
+		return $this->session->getInterface();
+	}
+
+	/**
+	 * @return NetworkSession
+	 */
+	public function getNetworkSession() : NetworkSession{
+		return $this->session;
 	}
 
 	/**
 	 * @return string
 	 */
 	public function getAddress() : string{
-		return $this->address;
+		return $this->session->getIp();
 	}
 
 	/**
 	 * @return int
 	 */
 	public function getPort() : int{
-		return $this->port;
+		return $this->session->getPort();
 	}
 
 	/**

src/pocketmine/network/NetworkInterface.php

@@ -26,7 +26,7 @@ declare(strict_types=1);
  */
 namespace pocketmine\network;
 
-use pocketmine\Player;
+use pocketmine\network\mcpe\NetworkSession;
 
 /**
  * Network interfaces are transport layers which can be used to transmit packets between the server and clients.
@@ -41,19 +41,19 @@ interface NetworkInterface{
 	/**
 	 * Sends a DataPacket to the interface, returns an unique identifier for the packet if $needACK is true
 	 *
-	 * @param Player $player
+	 * @param NetworkSession $session
-	 * @param string $payload
+	 * @param string         $payload
-	 * @param bool   $immediate
+	 * @param bool           $immediate
 	 */
-	public function putPacket(Player $player, string $payload, bool $immediate = true) : void;
+	public function putPacket(NetworkSession $session, string $payload, bool $immediate = true) : void;
 
 	/**
 	 * Terminates the connection
 	 *
-	 * @param Player $player
+	 * @param NetworkSession $session
-	 * @param string $reason
+	 * @param string         $reason
 	 */
-	public function close(Player $player, string $reason = "unknown reason") : void;
+	public function close(NetworkSession $session, string $reason = "unknown reason") : void;
 
 	/**
 	 * @param string $name

src/pocketmine/network/mcpe/CompressBatchedTask.php

@@ -23,7 +23,6 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe;
 
-use pocketmine\Player;
 use pocketmine\scheduler\AsyncTask;
 use pocketmine\Server;
 
@@ -33,9 +32,9 @@ class CompressBatchedTask extends AsyncTask{
 	private $data;
 
 	/**
-	 * @param PacketStream $stream
+	 * @param PacketStream     $stream
-	 * @param Player[]     $targets
+	 * @param NetworkSession[] $targets
-	 * @param int          $compressionLevel
+	 * @param int              $compressionLevel
 	 */
 	public function __construct(PacketStream $stream, array $targets, int $compressionLevel){
 		$this->data = $stream->buffer;
@@ -48,7 +47,7 @@ class CompressBatchedTask extends AsyncTask{
 	}
 
 	public function onCompletion(Server $server) : void{
-		/** @var Player[] $targets */
+		/** @var NetworkSession[] $targets */
 		$targets = $this->fetchLocal();
 
 		$server->broadcastPacketsCallback($this->getResult(), $targets);

src/pocketmine/network/mcpe/NetworkSession.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe;
 
+use pocketmine\event\player\PlayerCreationEvent;
 use pocketmine\event\server\DataPacketReceiveEvent;
 use pocketmine\event\server\DataPacketSendEvent;
 use pocketmine\network\mcpe\handler\DeathSessionHandler;
@@ -59,15 +60,36 @@ class NetworkSession{
 	/** @var SessionHandler */
 	private $handler;
 
-	public function __construct(Server $server, Player $player, NetworkInterface $interface, string $ip, int $port){
+	/** @var bool */
-		$this->server = $server;
+	private $connected = true;
-		$this->player = $player;
-		$this->interface = $interface;
 
+	public function __construct(Server $server, NetworkInterface $interface, string $ip, int $port){
+		$this->server = $server;
+		$this->interface = $interface;
 		$this->ip = $ip;
 		$this->port = $port;
 
-		$this->setHandler(new LoginSessionHandler($player, $this));
+		//TODO: this should happen later in the login sequence
+		$this->createPlayer();
+
+		$this->setHandler(new LoginSessionHandler($this->player, $this));
+	}
+
+	protected function createPlayer() : void{
+		$this->server->getPluginManager()->callEvent($ev = new PlayerCreationEvent($this));
+		$class = $ev->getPlayerClass();
+
+		/**
+		 * @var Player $player
+		 * @see Player::__construct()
+		 */
+		$this->player = new $class($this->server, $this);
+
+		$this->server->addPlayer($this->player);
+	}
+
+	public function isConnected() : bool{
+		return $this->connected;
 	}
 
 	public function getInterface() : NetworkInterface{
@@ -116,10 +138,14 @@ class NetworkSession{
 	}
 
 	public function handleEncoded(string $payload) : void{
+		if(!$this->connected){
+			return;
+		}
+
 		//TODO: decryption if enabled
 
 		$stream = new PacketStream(NetworkCompression::decompress($payload));
-		while(!$stream->feof() and $this->player->isConnected()){
+		while(!$stream->feof() and $this->connected){
 			$this->handleDataPacket(PacketPool::getPacket($stream->getString()));
 		}
 	}
@@ -160,14 +186,61 @@ class NetworkSession{
 		}
 	}
 
-	public function serverDisconnect(string $reason, bool $notify = true) : void{
+	/**
+	 * Disconnects the session, destroying the associated player (if it exists).
+	 *
+	 * @param string $reason
+	 * @param bool   $notify
+	 */
+	public function disconnect(string $reason, bool $notify = true) : void{
+		if($this->connected){
+			$this->connected = false;
+			$this->player->close($this->player->getLeaveMessage(), $reason);
+			$this->doServerDisconnect($reason, $notify);
+		}
+	}
+
+	/**
+	 * Called by the Player when it is closed (for example due to getting kicked).
+	 *
+	 * @param string $reason
+	 * @param bool   $notify
+	 */
+	public function onPlayerDestroyed(string $reason, bool $notify = true) : void{
+		if($this->connected){
+			$this->connected = false;
+			$this->doServerDisconnect($reason, $notify);
+		}
+	}
+
+	/**
+	 * Internal helper function used to handle server disconnections.
+	 *
+	 * @param string $reason
+	 * @param bool   $notify
+	 */
+	private function doServerDisconnect(string $reason, bool $notify = true) : void{
 		if($notify){
 			$pk = new DisconnectPacket();
 			$pk->message = $reason;
 			$pk->hideDisconnectionScreen = $reason === "";
 			$this->sendDataPacket($pk, true);
 		}
-		$this->interface->close($this->player, $notify ? $reason : "");
+
+		$this->interface->close($this, $notify ? $reason : "");
+	}
+
+	/**
+	 * Called by the network interface to close the session when the client disconnects without server input, for
+	 * example in a timeout condition or voluntary client disconnect.
+	 *
+	 * @param string $reason
+	 */
+	public function onClientDisconnect(string $reason) : void{
+		if($this->connected){
+			$this->connected = false;
+			$this->player->close($this->player->getLeaveMessage(), $reason);
+		}
 	}
 
 	//TODO: onEnableEncryption() step

src/pocketmine/network/mcpe/RakLibInterface.php

@@ -23,11 +23,9 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe;
 
-use pocketmine\event\player\PlayerCreationEvent;
 use pocketmine\network\AdvancedNetworkInterface;
 use pocketmine\network\mcpe\protocol\ProtocolInfo;
 use pocketmine\network\Network;
-use pocketmine\Player;
 use pocketmine\Server;
 use pocketmine\snooze\SleeperNotifier;
 use raklib\protocol\EncapsulatedPacket;
@@ -56,8 +54,8 @@ class RakLibInterface implements ServerInstance, AdvancedNetworkInterface{
 	/** @var RakLibServer */
 	private $rakLib;
 
-	/** @var Player[] */
+	/** @var NetworkSession[] */
-	private $players = [];
+	private $sessions = [];
 
 	/** @var string[] */
 	private $identifiers = [];
@@ -104,17 +102,17 @@ class RakLibInterface implements ServerInstance, AdvancedNetworkInterface{
 	}
 
 	public function closeSession(string $identifier, string $reason) : void{
-		if(isset($this->players[$identifier])){
+		if(isset($this->sessions[$identifier])){
-			$player = $this->players[$identifier];
+			$session = $this->sessions[$identifier];
-			unset($this->identifiers[spl_object_hash($player)]);
+			unset($this->identifiers[spl_object_hash($session)]);
-			unset($this->players[$identifier]);
+			unset($this->sessions[$identifier]);
-			$player->close($player->getLeaveMessage(), $reason);
+			$session->onClientDisconnect($reason);
 		}
 	}
 
-	public function close(Player $player, string $reason = "unknown reason") : void{
+	public function close(NetworkSession $session, string $reason = "unknown reason") : void{
-		if(isset($this->identifiers[$h = spl_object_hash($player)])){
+		if(isset($this->identifiers[$h = spl_object_hash($session)])){
-			unset($this->players[$this->identifiers[$h]]);
+			unset($this->sessions[$this->identifiers[$h]]);
 			$this->interface->closeSession($this->identifiers[$h], $reason);
 			unset($this->identifiers[$h]);
 		}
@@ -131,23 +129,18 @@ class RakLibInterface implements ServerInstance, AdvancedNetworkInterface{
 	}
 
 	public function openSession(string $identifier, string $address, int $port, int $clientID) : void{
-		$ev = new PlayerCreationEvent($this, $address, $port);
+		$session = new NetworkSession($this->server, $this, $address, $port);
-		$this->server->getPluginManager()->callEvent($ev);
+		$this->sessions[$identifier] = $session;
-		$class = $ev->getPlayerClass();
+		$this->identifiers[spl_object_hash($session)] = $identifier;
-
-		$player = new $class($this, $ev->getAddress(), $ev->getPort());
-		$this->players[$identifier] = $player;
-		$this->identifiers[spl_object_hash($player)] = $identifier;
-		$this->server->addPlayer($player);
 	}
 
 	public function handleEncapsulated(string $identifier, EncapsulatedPacket $packet, int $flags) : void{
-		if(isset($this->players[$identifier])){
+		if(isset($this->sessions[$identifier])){
 			//get this now for blocking in case the player was closed before the exception was raised
-			$address = $this->players[$identifier]->getNetworkSession()->getIp();
+			$address = $this->sessions[$identifier]->getIp();
 			try{
 				if($packet->buffer !== "" and $packet->buffer{0} === self::MCPE_RAKNET_PACKET_ID){ //Batch
-					$this->players[$identifier]->getNetworkSession()->handleEncoded(substr($packet->buffer, 1));
+					$this->sessions[$identifier]->handleEncoded(substr($packet->buffer, 1));
 				}
 			}catch(\Throwable $e){
 				$logger = $this->server->getLogger();
@@ -208,8 +201,8 @@ class RakLibInterface implements ServerInstance, AdvancedNetworkInterface{
 		}
 	}
 
-	public function putPacket(Player $player, string $payload, bool $immediate = true) : void{
+	public function putPacket(NetworkSession $session, string $payload, bool $immediate = true) : void{
-		if(isset($this->identifiers[$h = spl_object_hash($player)])){
+		if(isset($this->identifiers[$h = spl_object_hash($session)])){
 			$identifier = $this->identifiers[$h];
 
 			$pk = new EncapsulatedPacket();
@@ -222,8 +215,8 @@ class RakLibInterface implements ServerInstance, AdvancedNetworkInterface{
 	}
 
 	public function updatePing(string $identifier, int $pingMS) : void{
-		if(isset($this->players[$identifier])){
+		if(isset($this->sessions[$identifier])){
-			$this->players[$identifier]->getNetworkSession()->updatePing($pingMS);
+			$this->sessions[$identifier]->updatePing($pingMS);
 		}
 	}
 }

src/pocketmine/network/mcpe/handler/LoginSessionHandler.php

@@ -53,8 +53,7 @@ class LoginSessionHandler extends SessionHandler{
 			$this->session->sendDataPacket($pk, true);
 
 			//This pocketmine disconnect message will only be seen by the console (PlayStatusPacket causes the messages to be shown for the client)
-			$this->player->close(
+			$this->session->disconnect(
-				"",
 				$this->player->getServer()->getLanguage()->translateString("pocketmine.disconnect.incompatibleProtocol", [$packet->protocol]),
 				false
 			);
@@ -63,13 +62,13 @@ class LoginSessionHandler extends SessionHandler{
 		}
 
 		if(!Player::isValidUserName($packet->username)){
-			$this->player->close("", "disconnectionScreen.invalidName");
+			$this->session->disconnect("disconnectionScreen.invalidName");
 
 			return true;
 		}
 
 		if($packet->skin === null or !$packet->skin->isValid()){
-			$this->player->close("", "disconnectionScreen.invalidSkin");
+			$this->session->disconnect("disconnectionScreen.invalidSkin");
 
 			return true;
 		}

src/pocketmine/network/mcpe/handler/PreSpawnSessionHandler.php

@@ -86,7 +86,7 @@ class PreSpawnSessionHandler extends SessionHandler{
 		$this->player->sendAllInventories();
 		$this->player->getInventory()->sendCreativeContents();
 		$this->player->getInventory()->sendHeldItem($this->player);
-		$this->session->getInterface()->putPacket($this->player, $this->server->getCraftingManager()->getCraftingDataPacket());
+		$this->session->getInterface()->putPacket($this->session, $this->server->getCraftingManager()->getCraftingDataPacket());
 
 		$this->server->sendFullPlayerListData($this->player);
 	}

src/pocketmine/network/mcpe/handler/ResourcePacksSessionHandler.php

@@ -66,14 +66,14 @@ class ResourcePacksSessionHandler extends SessionHandler{
 
 	private function disconnectWithError(string $error) : void{
 		$this->player->getServer()->getLogger()->error("Error while downloading resource packs for " . $this->player->getName() . ": " . $error);
-		$this->player->close("", "disconnectionScreen.resourcePack", true);
+		$this->session->disconnect("disconnectionScreen.resourcePack");
 	}
 
 	public function handleResourcePackClientResponse(ResourcePackClientResponsePacket $packet) : bool{
 		switch($packet->status){
 			case ResourcePackClientResponsePacket::STATUS_REFUSED:
 				//TODO: add lang strings for this
-				$this->player->close("", "You must accept resource packs to join this server.", true);
+				$this->session->disconnect("You must accept resource packs to join this server.", true);
 				break;
 			case ResourcePackClientResponsePacket::STATUS_SEND_PACKS:
 				foreach($packet->packIds as $uuid){