From 289e86e899d74e7d439af70c11721546ccdbb449 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Sat, 14 Jan 2023 17:55:00 +0000
Subject: [PATCH] Make use of World::requestSafeSpawn()

---
 src/Server.php        | 62 ++++++++++++++++++++-----------------------
 src/player/Player.php | 11 ++++----
 2 files changed, 34 insertions(+), 39 deletions(-)

diff --git a/src/Server.php b/src/Server.php
index 8f6d732e6..bd20c8616 100644
--- a/src/Server.php
+++ b/src/Server.php
@@ -106,12 +106,12 @@ use pocketmine\utils\SignalHandler;
 use pocketmine\utils\Terminal;
 use pocketmine\utils\TextFormat;
 use pocketmine\utils\Utils;
-use pocketmine\world\format\Chunk;
 use pocketmine\world\format\io\WorldProviderManager;
 use pocketmine\world\format\io\WritableWorldProviderManagerEntry;
 use pocketmine\world\generator\Generator;
 use pocketmine\world\generator\GeneratorManager;
 use pocketmine\world\generator\InvalidGeneratorOptionsException;
+use pocketmine\world\Position;
 use pocketmine\world\World;
 use pocketmine\world\WorldCreationOptions;
 use pocketmine\world\WorldManager;
@@ -554,49 +554,45 @@ class Server{
 
 		if($offlinePlayerData !== null && ($world = $this->worldManager->getWorldByName($offlinePlayerData->getString(Player::TAG_LEVEL, ""))) !== null){
 			$playerPos = EntityDataHelper::parseLocation($offlinePlayerData, $world);
-			$spawn = $playerPos->asVector3();
 		}else{
 			$world = $this->worldManager->getDefaultWorld();
 			if($world === null){
 				throw new AssumptionFailedError("Default world should always be loaded");
 			}
 			$playerPos = null;
-			$spawn = $world->getSpawnLocation();
 		}
 		/** @phpstan-var PromiseResolver<Player> $playerPromiseResolver */
 		$playerPromiseResolver = new PromiseResolver();
-		$world->requestChunkPopulation($spawn->getFloorX() >> Chunk::COORD_BIT_SIZE, $spawn->getFloorZ() >> Chunk::COORD_BIT_SIZE, null)->onCompletion(
-			function() use ($playerPromiseResolver, $class, $session, $playerInfo, $authenticated, $world, $playerPos, $spawn, $offlinePlayerData) : void{
-				if(!$session->isConnected()){
-					$playerPromiseResolver->reject();
-					return;
-				}
 
-				/* Stick with the original spawn at the time of generation request, even if it changed since then.
-				 * This is because we know for sure that that chunk will be generated, but the one at the new location
-				 * might not be, and it would be much more complex to go back and redo the whole thing.
-				 *
-				 * TODO: this relies on the assumption that getSafeSpawn() will only alter the Y coordinate of the
-				 * provided position. If this assumption is broken, we'll start seeing crashes in here.
-				 */
-
-				/**
-				 * @see Player::__construct()
-				 * @var Player $player
-				 */
-				$player = new $class($this, $session, $playerInfo, $authenticated, $playerPos ?? Location::fromObject($world->getSafeSpawn($spawn), $world), $offlinePlayerData);
-				if(!$player->hasPlayedBefore()){
-					$player->onGround = true;  //TODO: this hack is needed for new players in-air ticks - they don't get detected as on-ground until they move
-				}
-				$playerPromiseResolver->resolve($player);
-			},
-			static function() use ($playerPromiseResolver, $session) : void{
-				if($session->isConnected()){
-					$session->disconnect("Spawn terrain generation failed");
-				}
-				$playerPromiseResolver->reject();
+		$createPlayer = function(Location $location) use ($playerPromiseResolver, $class, $session, $playerInfo, $authenticated, $offlinePlayerData) : void{
+			$player = new $class($this, $session, $playerInfo, $authenticated, $location, $offlinePlayerData);
+			if(!$player->hasPlayedBefore()){
+				$player->onGround = true; //TODO: this hack is needed for new players in-air ticks - they don't get detected as on-ground until they move
 			}
-		);
+			$playerPromiseResolver->resolve($player);
+		};
+
+		if($playerPos === null){ //new player or no valid position due to world not being loaded
+			$world->requestSafeSpawn()->onCompletion(
+				function(Position $spawn) use ($createPlayer, $playerPromiseResolver, $session, $world) : void{
+					if(!$session->isConnected()){
+						$playerPromiseResolver->reject();
+						return;
+					}
+					$createPlayer(Location::fromObject($spawn, $world));
+				},
+				function() use ($playerPromiseResolver, $session) : void{
+					if($session->isConnected()){
+						//TODO: this needs to be localized - this might be reached if the spawn world was unloaded while the player was logging in
+						$session->disconnect("Failed to find a safe spawn location");
+					}
+					$playerPromiseResolver->reject();
+				}
+			);
+		}else{ //returning player with a valid position - safe spawn not required
+			$createPlayer($playerPos);
+		}
+
 		return $playerPromiseResolver->getPromise();
 	}
 
diff --git a/src/player/Player.php b/src/player/Player.php
index f4d5c8762..8d9afcbc1 100644
--- a/src/player/Player.php
+++ b/src/player/Player.php
@@ -2311,16 +2311,15 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 		}
 		$this->respawnLocked = true;
 
-		$this->logger->debug("Waiting for spawn terrain generation for respawn");
+		$this->logger->debug("Waiting for safe respawn position to be located");
 		$spawn = $this->getSpawn();
-		$spawn->getWorld()->orderChunkPopulation($spawn->getFloorX() >> Chunk::COORD_BIT_SIZE, $spawn->getFloorZ() >> Chunk::COORD_BIT_SIZE, null)->onCompletion(
-			function() use ($spawn) : void{
+		$spawn->getWorld()->requestSafeSpawn($spawn)->onCompletion(
+			function(Position $safeSpawn) : void{
 				if(!$this->isConnected()){
 					return;
 				}
-				$this->logger->debug("Spawn terrain generation done, completing respawn");
-				$spawn = $spawn->getWorld()->getSafeSpawn($spawn);
-				$ev = new PlayerRespawnEvent($this, $spawn);
+				$this->logger->debug("Respawn position located, completing respawn");
+				$ev = new PlayerRespawnEvent($this, $safeSpawn);
 				$ev->call();
 
 				$realSpawn = Position::fromObject($ev->getRespawnPosition()->add(0.5, 0, 0.5), $ev->getRespawnPosition()->getWorld());