Some cleanup to player net session handling for connect/disconnect

src/pocketmine/Player.php

@@ -50,7 +50,6 @@ use pocketmine\event\player\PlayerChangeSkinEvent;
 use pocketmine\event\player\PlayerChatEvent;
 use pocketmine\event\player\PlayerCommandPreprocessEvent;
 use pocketmine\event\player\PlayerDeathEvent;
-use pocketmine\event\player\PlayerDuplicateLoginEvent;
 use pocketmine\event\player\PlayerEditBookEvent;
 use pocketmine\event\player\PlayerExhaustEvent;
 use pocketmine\event\player\PlayerGameModeChangeEvent;
@@ -1795,20 +1794,7 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 			$this->server->getLogger()->debug($this->getName() . " is logged into Xbox Live");
 		}
 
-		foreach($this->server->getLoggedInPlayers() as $p){
+		return $this->server->getNetwork()->getSessionManager()->kickDuplicates($this->networkSession);
-			if($p !== $this and ($p->iusername === $this->iusername or $this->getUniqueId()->equals($p->getUniqueId()))){
-				$ev = new PlayerDuplicateLoginEvent($this->networkSession, $p->networkSession);
-				$ev->call();
-				if($ev->isCancelled()){
-					$this->networkSession->disconnect($ev->getDisconnectMessage());
-					return false;
-				}
-
-				$p->networkSession->disconnect($ev->getDisconnectMessage());
-			}
-		}
-
-		return true;
 	}
 
 	public function onLoginSuccess() : void{
@@ -2843,7 +2829,6 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 			$this->loadQueue = [];
 
 			if($this->loggedIn){
-				$this->server->onPlayerLogout($this);
 				foreach($this->server->getOnlinePlayers() as $player){
 					if(!$player->canSee($this)){
 						$player->showPlayer($this);
@@ -2869,7 +2854,6 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 				$this->loggedIn = false;
 				$this->server->removeOnlinePlayer($this);
 			}
-			$this->server->removePlayer($this);
 
 			$this->server->getLogger()->info($this->getServer()->getLanguage()->translateString("pocketmine.player.logOut", [
 				TextFormat::AQUA . $this->getName() . TextFormat::WHITE,

src/pocketmine/Server.php

@@ -291,12 +291,6 @@ class Server{
 	/** @var Config */
 	private $config;
 
-	/** @var Player[] */
-	private $players = [];
-
-	/** @var Player[] */
-	private $loggedInPlayers = [];
-
 	/** @var Player[] */
 	private $playerList = [];
 
@@ -602,13 +596,6 @@ class Server{
 		return $this->commandMap;
 	}
 
-	/**
-	 * @return Player[]
-	 */
-	public function getLoggedInPlayers() : array{
-		return $this->loggedInPlayers;
-	}
-
 	/**
 	 * @return Player[]
 	 */
@@ -1654,8 +1641,8 @@ class Server{
 				$this->pluginManager->disablePlugins();
 			}
 
-			foreach($this->players as $player){
+			if($this->network instanceof Network){
-				$player->close($player->getLeaveMessage(), $this->getProperty("settings.shutdown-message", "Server closed"));
+				$this->network->getSessionManager()->close($this->getProperty("settings.shutdown-message", "Server closed"));
 			}
 
 			if($this->levelManager instanceof LevelManager){
@@ -1883,23 +1870,6 @@ class Server{
 		if($this->sendUsageTicker > 0){
 			$this->uniquePlayers[$player->getRawUniqueId()] = $player->getRawUniqueId();
 		}
-
-		$this->loggedInPlayers[$player->getRawUniqueId()] = $player;
-	}
-
-	public function onPlayerLogout(Player $player) : void{
-		unset($this->loggedInPlayers[$player->getRawUniqueId()]);
-	}
-
-	public function addPlayer(Player $player) : void{
-		$this->players[spl_object_id($player)] = $player;
-	}
-
-	/**
-	 * @param Player $player
-	 */
-	public function removePlayer(Player $player) : void{
-		unset($this->players[spl_object_id($player)]);
 	}
 
 	public function addOnlinePlayer(Player $player) : void{

src/pocketmine/level/LevelManager.php

@@ -404,8 +404,6 @@ class LevelManager{
 			foreach($level->getPlayers() as $player){
 				if($player->spawned){
 					$player->save();
-				}elseif(!$player->isConnected()){ //TODO: check if this is ever possible
-					$this->server->removePlayer($player);
 				}
 			}
 			$level->save(false);

src/pocketmine/network/Network.php

@@ -28,7 +28,6 @@ namespace pocketmine\network;
 
 use pocketmine\event\server\NetworkInterfaceRegisterEvent;
 use pocketmine\event\server\NetworkInterfaceUnregisterEvent;
-use pocketmine\network\mcpe\NetworkSession;
 use pocketmine\network\mcpe\protocol\PacketPool;
 use function get_class;
 use function spl_object_id;
@@ -46,11 +45,12 @@ class Network{
 	/** @var string */
 	private $name;
 
-	/** @var NetworkSession[] */
+	/** @var NetworkSessionManager */
-	private $updateSessions = [];
+	private $sessionManager;
 
 	public function __construct(){
 		PacketPool::init();
+		$this->sessionManager = new NetworkSessionManager();
 	}
 
 	public function addStatistics(float $upload, float $download) : void{
@@ -78,12 +78,15 @@ class Network{
 		return $this->interfaces;
 	}
 
+	/**
+	 * @return NetworkSessionManager
+	 */
+	public function getSessionManager() : NetworkSessionManager{
+		return $this->sessionManager;
+	}
+
 	public function getConnectionCount() : int{
-		$count = 0;
+		return $this->sessionManager->getSessionCount();
-		foreach($this->interfaces as $interface){
-			$count += $interface->getConnectionCount();
-		}
-		return $count;
 	}
 
 	public function tick() : void{
@@ -91,11 +94,7 @@ class Network{
 			$interface->tick();
 		}
 
-		foreach($this->updateSessions as $k => $session){
+		$this->sessionManager->tick();
-			if(!$session->isConnected() or !$session->tick()){
-				unset($this->updateSessions[$k]);
-			}
-		}
 	}
 
 	/**
@@ -187,8 +186,4 @@ class Network{
 			$interface->addRawPacketFilter($regex);
 		}
 	}
-
-	public function scheduleSessionTick(NetworkSession $session) : void{
-		$this->updateSessions[spl_object_id($session)] = $session;
-	}
 }

src/pocketmine/network/NetworkSessionManager.php

@@ -0,0 +1,129 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\network;
+
+use pocketmine\event\player\PlayerDuplicateLoginEvent;
+use pocketmine\network\mcpe\NetworkSession;
+use function count;
+use function spl_object_id;
+
+class NetworkSessionManager{
+
+	/** @var NetworkSession[] */
+	private $sessions = [];
+	/** @var NetworkSession[] */
+	private $updateSessions = [];
+
+	/**
+	 * Adds a network session to the manager. This should only be called on session creation.
+	 *
+	 * @param NetworkSession $session
+	 */
+	public function add(NetworkSession $session) : void{
+		$idx = spl_object_id($session);
+		$this->sessions[$idx] = $this->updateSessions[$idx] = $session;
+	}
+
+	/**
+	 * Removes the given network session, due to disconnect. This should only be called by a network session on
+	 * disconnection.
+	 *
+	 * @param NetworkSession $session
+	 */
+	public function remove(NetworkSession $session) : void{
+		$idx = spl_object_id($session);
+		unset($this->sessions[$idx], $this->updateSessions[$idx]);
+	}
+
+	/**
+	 * Requests an update to be scheduled on the given network session at the next tick.
+	 *
+	 * @param NetworkSession $session
+	 */
+	public function scheduleUpdate(NetworkSession $session) : void{
+		$this->updateSessions[spl_object_id($session)] = $session;
+	}
+
+	/**
+	 * Checks whether this network session is a duplicate of an already-connected session (same player connecting from
+	 * 2 locations).
+	 *
+	 * @param NetworkSession $connectingSession
+	 *
+	 * @return bool if the network session is still connected.
+	 */
+	public function kickDuplicates(NetworkSession $connectingSession) : bool{
+		foreach($this->sessions as $existingSession){
+			if($existingSession === $connectingSession){
+				continue;
+			}
+			$info = $existingSession->getPlayerInfo();
+			if($info !== null and ($info->getUsername() === $connectingSession->getPlayerInfo()->getUsername() or $info->getUuid()->equals($connectingSession->getPlayerInfo()->getUuid()))){
+				$ev = new PlayerDuplicateLoginEvent($connectingSession, $existingSession);
+				$ev->call();
+				if($ev->isCancelled()){
+					$connectingSession->disconnect($ev->getDisconnectMessage());
+					return false;
+				}
+
+				$existingSession->disconnect($ev->getDisconnectMessage());
+			}
+		}
+
+		return true;
+	}
+
+	/**
+	 * Returns the number of known connected sessions.
+	 *
+	 * @return int
+	 */
+	public function getSessionCount() : int{
+		return count($this->sessions);
+	}
+
+	/**
+	 * Updates all sessions which need it.
+	 */
+	public function tick() : void{
+		foreach($this->updateSessions as $k => $session){
+			if(!$session->tick()){
+				unset($this->updateSessions[$k]);
+			}
+		}
+	}
+
+	/**
+	 * Terminates all connected sessions with the given reason.
+	 *
+	 * @param string $reason
+	 */
+	public function close(string $reason = "") : void{
+		foreach($this->sessions as $session){
+			$session->disconnect($reason);
+		}
+		$this->sessions = [];
+		$this->updateSessions = [];
+	}
+}

src/pocketmine/network/mcpe/NetworkSession.php

@@ -42,7 +42,9 @@ use pocketmine\network\mcpe\protocol\PlayStatusPacket;
 use pocketmine\network\mcpe\protocol\ServerboundPacket;
 use pocketmine\network\mcpe\protocol\ServerToClientHandshakePacket;
 use pocketmine\network\NetworkInterface;
+use pocketmine\network\NetworkSessionManager;
 use pocketmine\Player;
+use pocketmine\PlayerInfo;
 use pocketmine\Server;
 use pocketmine\timings\Timings;
 use pocketmine\utils\BinaryDataException;
@@ -56,12 +58,16 @@ class NetworkSession{
 	private $server;
 	/** @var Player|null */
 	private $player;
+	/** @var NetworkSessionManager */
+	private $manager;
 	/** @var NetworkInterface */
 	private $interface;
 	/** @var string */
 	private $ip;
 	/** @var int */
 	private $port;
+	/** @var PlayerInfo */
+	private $info;
 	/** @var int */
 	private $ping;
 
@@ -82,8 +88,9 @@ class NetworkSession{
 	/** @var \SplQueue|CompressBatchPromise[] */
 	private $compressedQueue;
 
-	public function __construct(Server $server, NetworkInterface $interface, string $ip, int $port){
+	public function __construct(Server $server, NetworkSessionManager $manager, NetworkInterface $interface, string $ip, int $port){
 		$this->server = $server;
+		$this->manager = $manager;
 		$this->interface = $interface;
 		$this->ip = $ip;
 		$this->port = $port;
@@ -91,12 +98,13 @@ class NetworkSession{
 		$this->compressedQueue = new \SplQueue();
 
 		$this->connectTime = time();
-		$this->server->getNetwork()->scheduleSessionTick($this);
 
 		//TODO: this should happen later in the login sequence
 		$this->createPlayer();
 
 		$this->setHandler(new LoginSessionHandler($this->player, $this));
+
+		$this->manager->add($this);
 	}
 
 	protected function createPlayer() : void{
@@ -109,14 +117,29 @@ class NetworkSession{
 		 * @see Player::__construct()
 		 */
 		$this->player = new $class($this->server, $this);
-
-		$this->server->addPlayer($this->player);
 	}
 
 	public function getPlayer() : ?Player{
 		return $this->player;
 	}
 
+	public function getPlayerInfo() : ?PlayerInfo{
+		return $this->info;
+	}
+
+	/**
+	 * TODO: this shouldn't be accessible after the initial login phase
+	 *
+	 * @param PlayerInfo $info
+	 * @throws \InvalidStateException
+	 */
+	public function setPlayerInfo(PlayerInfo $info) : void{
+		if($this->info !== null){
+			throw new \InvalidStateException("Player info has already been set");
+		}
+		$this->info = $info;
+	}
+
 	public function isConnected() : bool{
 		return $this->connected;
 	}
@@ -285,7 +308,7 @@ class NetworkSession{
 				$this->sendBuffer = new PacketStream();
 			}
 			$this->sendBuffer->putPacket($packet);
-			$this->server->getNetwork()->scheduleSessionTick($this);
+			$this->manager->scheduleUpdate($this); //schedule flush at end of tick
 		}finally{
 			$timings->stopTiming();
 		}
@@ -337,6 +360,15 @@ class NetworkSession{
 		$this->interface->putPacket($this, $payload, $immediate);
 	}
 
+	private function checkDisconnect() : bool{
+		if($this->connected){
+			$this->connected = false;
+			$this->manager->remove($this);
+			return true;
+		}
+		return false;
+	}
+
 	/**
 	 * Disconnects the session, destroying the associated player (if it exists).
 	 *
@@ -344,8 +376,7 @@ class NetworkSession{
 	 * @param bool   $notify
 	 */
 	public function disconnect(string $reason, bool $notify = true) : void{
-		if($this->connected){
+		if($this->checkDisconnect()){
-			$this->connected = false;
 			$this->player->close($this->player->getLeaveMessage(), $reason);
 			$this->doServerDisconnect($reason, $notify);
 		}
@@ -358,8 +389,7 @@ class NetworkSession{
 	 * @param bool   $notify
 	 */
 	public function onPlayerDestroyed(string $reason, bool $notify = true) : void{
-		if($this->connected){
+		if($this->checkDisconnect()){
-			$this->connected = false;
 			$this->doServerDisconnect($reason, $notify);
 		}
 	}
@@ -388,8 +418,7 @@ class NetworkSession{
 	 * @param string $reason
 	 */
 	public function onClientDisconnect(string $reason) : void{
-		if($this->connected){
+		if($this->checkDisconnect()){
-			$this->connected = false;
 			$this->player->close($this->player->getLeaveMessage(), $reason);
 		}
 	}

src/pocketmine/network/mcpe/RakLibInterface.php

@@ -143,7 +143,7 @@ class RakLibInterface implements ServerInstance, AdvancedNetworkInterface{
 	}
 
 	public function openSession(int $sessionId, string $address, int $port, int $clientID) : void{
-		$session = new NetworkSession($this->server, $this, $address, $port);
+		$session = new NetworkSession($this->server, $this->network->getSessionManager(), $this, $address, $port);
 		$this->sessions[$sessionId] = $session;
 		$this->identifiers[spl_object_id($session)] = $sessionId;
 	}

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

@@ -45,6 +45,8 @@ class LoginSessionHandler extends SessionHandler{
 	}
 
 	public function handleLogin(LoginPacket $packet) : bool{
+		$this->session->setPlayerInfo($packet->playerInfo);
+
 		if(!$this->isCompatibleProtocol($packet->protocol)){
 			$pk = new PlayStatusPacket();
 			$pk->status = $packet->protocol < ProtocolInfo::CURRENT_PROTOCOL ?