From f903dbfe00d7ebe3fdecf3579953740ca9f6450d Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <odigiman@gmail.com>
Date: Tue, 27 Feb 2018 11:43:02 +0000
Subject: [PATCH] Server: Removed identifiers array This is completely
 unnecessary and adds extra complexity for no good reason. Maybe it was used
 historically, but nowadays it is only used to identify players to send
 async-prepared batch packets to.

There are two alternative ways to do that:
1. use spl_object_hash() as the targets array in CompressBatchedTask
2. use ServerScheduler's object storage to retain references to the Player[] array.

I've opted for the second method.

Removing these identifiers allows great code simplification in removePlayer() and removes the need for those old stupid hacks.

This also includes a backwards-compatibility break by removing the $identifier parameter of Server->addPlayer().
---
 src/pocketmine/Server.php                     | 46 ++++++-------------
 .../network/CompressBatchedTask.php           | 10 ++--
 .../network/mcpe/RakLibInterface.php          |  2 +-
 3 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/src/pocketmine/Server.php b/src/pocketmine/Server.php
index caec8746a..1b5d11772 100644
--- a/src/pocketmine/Server.php
+++ b/src/pocketmine/Server.php
@@ -264,9 +264,6 @@ class Server{
 	/** @var Player[] */
 	private $playerList = [];
 
-	/** @var string[] */
-	private $identifiers = [];
-
 	/** @var Level[] */
 	private $levels = [];
 
@@ -899,20 +896,7 @@ class Server{
 	 * @param Player $player
 	 */
 	public function removePlayer(Player $player){
-		if(isset($this->identifiers[$hash = spl_object_hash($player)])){
-			$identifier = $this->identifiers[$hash];
-			unset($this->players[$identifier]);
-			unset($this->identifiers[$hash]);
-			return;
-		}
-
-		foreach($this->players as $identifier => $p){
-			if($player === $p){
-				unset($this->players[$identifier]);
-				unset($this->identifiers[spl_object_hash($player)]);
-				break;
-			}
-		}
+		unset($this->players[spl_object_hash($player)]);
 	}
 
 	/**
@@ -1899,14 +1883,9 @@ class Server{
 		}
 		Timings::$playerNetworkTimer->startTiming();
 
-		$targets = [];
-		foreach($players as $p){
-			if($p->isConnected()){
-				$targets[] = $this->identifiers[spl_object_hash($p)];
-			}
-		}
+		$targets = array_filter($players, function(Player $player) : bool{ return $player->isConnected(); });
 
-		if(count($targets) > 0){
+		if(!empty($targets)){
 			$pk = new BatchPacket();
 
 			foreach($packets as $p){
@@ -1931,17 +1910,19 @@ class Server{
 		Timings::$playerNetworkTimer->stopTiming();
 	}
 
-	public function broadcastPacketsCallback(BatchPacket $pk, array $identifiers, bool $immediate = false){
+	/**
+	 * @param BatchPacket $pk
+	 * @param Player[]    $players
+	 * @param bool        $immediate
+	 */
+	public function broadcastPacketsCallback(BatchPacket $pk, array $players, bool $immediate = false){
 		if(!$pk->isEncoded){
 			$pk->encode();
 		}
 
-		foreach($identifiers as $i){
-			if(isset($this->players[$i])){
-				$this->players[$i]->sendDataPacket($pk, false, $immediate);
-			}
+		foreach($players as $i){
+			$i->sendDataPacket($pk, false, $immediate);
 		}
-
 	}
 
 
@@ -2317,9 +2298,8 @@ class Server{
 		unset($this->loggedInPlayers[$player->getRawUniqueId()]);
 	}
 
-	public function addPlayer(string $identifier, Player $player){
-		$this->players[$identifier] = $player;
-		$this->identifiers[spl_object_hash($player)] = $identifier;
+	public function addPlayer(Player $player){
+		$this->players[spl_object_hash($player)] = $player;
 	}
 
 	public function addOnlinePlayer(Player $player){
diff --git a/src/pocketmine/network/CompressBatchedTask.php b/src/pocketmine/network/CompressBatchedTask.php
index a70cd5531..9ddd7baae 100644
--- a/src/pocketmine/network/CompressBatchedTask.php
+++ b/src/pocketmine/network/CompressBatchedTask.php
@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\network;
 
 use pocketmine\network\mcpe\protocol\BatchPacket;
+use pocketmine\Player;
 use pocketmine\scheduler\AsyncTask;
 use pocketmine\Server;
 
@@ -31,7 +32,6 @@ class CompressBatchedTask extends AsyncTask{
 
 	public $level = 7;
 	public $data;
-	public $targets;
 
 	/**
 	 * @param BatchPacket $batch
@@ -39,8 +39,8 @@ class CompressBatchedTask extends AsyncTask{
 	 */
 	public function __construct(BatchPacket $batch, array $targets){
 		$this->data = $batch->payload;
-		$this->targets = serialize($targets);
 		$this->level = $batch->getCompressionLevel();
+		$this->storeLocal($targets);
 	}
 
 	public function onRun(){
@@ -57,6 +57,10 @@ class CompressBatchedTask extends AsyncTask{
 	public function onCompletion(Server $server){
 		$pk = new BatchPacket($this->getResult());
 		$pk->isEncoded = true;
-		$server->broadcastPacketsCallback($pk, unserialize($this->targets));
+
+		/** @var Player[] $targets */
+		$targets = $this->fetchLocal();
+
+		$server->broadcastPacketsCallback($pk, $targets);
 	}
 }
diff --git a/src/pocketmine/network/mcpe/RakLibInterface.php b/src/pocketmine/network/mcpe/RakLibInterface.php
index 46553fac8..ca83ae4b3 100644
--- a/src/pocketmine/network/mcpe/RakLibInterface.php
+++ b/src/pocketmine/network/mcpe/RakLibInterface.php
@@ -134,7 +134,7 @@ class RakLibInterface implements ServerInstance, AdvancedSourceInterface{
 		$this->players[$identifier] = $player;
 		$this->identifiersACK[$identifier] = 0;
 		$this->identifiers[spl_object_hash($player)] = $identifier;
-		$this->server->addPlayer($identifier, $player);
+		$this->server->addPlayer($player);
 	}
 
 	public function handleEncapsulated(string $identifier, EncapsulatedPacket $packet, int $flags) : void{