Fixed player spawning in ungenerated terrain (#4087)

fixes #4044
fixes #2724

this is significantly more complex than I hoped for, but it's a start... and it works.

src/Server.php

@@ -34,6 +34,8 @@ use pocketmine\command\ConsoleCommandSender;
 use pocketmine\command\SimpleCommandMap;
 use pocketmine\crafting\CraftingManager;
 use pocketmine\crafting\CraftingManagerFromDataHelper;
+use pocketmine\entity\EntityDataHelper;
+use pocketmine\entity\Location;
 use pocketmine\event\HandlerListManager;
 use pocketmine\event\player\PlayerCreationEvent;
 use pocketmine\event\player\PlayerDataSaveEvent;
@@ -68,6 +70,7 @@ use pocketmine\permission\DefaultPermissions;
 use pocketmine\player\GameMode;
 use pocketmine\player\OfflinePlayer;
 use pocketmine\player\Player;
+use pocketmine\player\PlayerCreationPromise;
 use pocketmine\player\PlayerInfo;
 use pocketmine\plugin\PharPluginLoader;
 use pocketmine\plugin\Plugin;
@@ -84,6 +87,7 @@ use pocketmine\stats\SendUsageTask;
 use pocketmine\timings\Timings;
 use pocketmine\timings\TimingsHandler;
 use pocketmine\updater\AutoUpdater;
+use pocketmine\utils\AssumptionFailedError;
 use pocketmine\utils\Config;
 use pocketmine\utils\Filesystem;
 use pocketmine\utils\Internet;
@@ -576,17 +580,45 @@ class Server{
 		}
 	}
 
-	public function createPlayer(NetworkSession $session, PlayerInfo $playerInfo, bool $authenticated, ?CompoundTag $offlinePlayerData) : Player{
+	public function createPlayer(NetworkSession $session, PlayerInfo $playerInfo, bool $authenticated, ?CompoundTag $offlinePlayerData) : PlayerCreationPromise{
 		$ev = new PlayerCreationEvent($session);
 		$ev->call();
 		$class = $ev->getPlayerClass();
 
+		if($offlinePlayerData !== null and ($world = $this->worldManager->getWorldByName($offlinePlayerData->getString("Level", ""))) !== null){
+			$spawn = EntityDataHelper::parseLocation($offlinePlayerData, $world);
+			$onGround = $offlinePlayerData->getByte("OnGround", 1) === 1;
+		}else{
+			$world = $this->worldManager->getDefaultWorld();
+			if($world === null){
+				throw new AssumptionFailedError("Default world should always be loaded");
+			}
+			$spawn = Location::fromObject($world->getSafeSpawn(), $world);
+			$onGround = true;
+		}
+		$playerPromise = new PlayerCreationPromise();
+		$world->requestChunkPopulation($spawn->getFloorX() >> 4, $spawn->getFloorZ() >> 4, null)->onCompletion(
+			function() use ($playerPromise, $class, $session, $playerInfo, $authenticated, $spawn, $offlinePlayerData, $onGround) : void{
+				if(!$session->isConnected()){
+					$playerPromise->reject();
+					return;
+				}
 				/**
 				 * @see Player::__construct()
 				 * @var Player $player
 				 */
-		$player = new $class($this, $session, $playerInfo, $authenticated, $offlinePlayerData);
+				$player = new $class($this, $session, $playerInfo, $authenticated, $spawn, $offlinePlayerData);
-		return $player;
+				$player->onGround = $onGround;  //TODO: this hack is needed for new players in-air ticks - they don't get detected as on-ground until they move
+				$playerPromise->resolve($player);
+			},
+			static function() use ($playerPromise, $session) : void{
+				if($session->isConnected()){
+					$session->disconnect("Spawn terrain generation failed");
+				}
+				$playerPromise->reject();
+			}
+		);
+		return $playerPromise;
 	}
 
 	/**

src/network/mcpe/NetworkSession.php

@@ -234,8 +234,18 @@ class NetworkSession{
 	}
 
 	protected function createPlayer() : void{
-		$this->player = $this->server->createPlayer($this, $this->info, $this->authenticated, $this->cachedOfflinePlayerData);
+		$this->server->createPlayer($this, $this->info, $this->authenticated, $this->cachedOfflinePlayerData)->onCompletion(
+			\Closure::fromCallable([$this, 'onPlayerCreated']),
+			fn() => $this->disconnect("Player creation failed") //TODO: this should never actually occur... right?
+		);
+	}
 
+	private function onPlayerCreated(Player $player) : void{
+		if(!$this->isConnected()){
+			//the remote player might have disconnected before spawn terrain generation was finished
+			return;
+		}
+		$this->player = $player;
 		$this->invManager = new InventoryManager($this->player, $this);
 
 		$effectManager = $this->player->getEffects();
@@ -259,6 +269,7 @@ class NetworkSession{
 		$this->disposeHooks->add(static function() use ($permissionHooks, $permHook) : void{
 			$permissionHooks->remove($permHook);
 		});
+		$this->beginSpawnSequence();
 	}
 
 	public function getPlayer() : ?Player{
@@ -680,13 +691,11 @@ class NetworkSession{
 
 		$this->logger->debug("Initiating resource packs phase");
 		$this->setHandler(new ResourcePacksPacketHandler($this, $this->server->getResourcePackManager(), function() : void{
-			$this->beginSpawnSequence();
+			$this->createPlayer();
 		}));
 	}
 
 	private function beginSpawnSequence() : void{
-		$this->createPlayer();
-
 		$this->setHandler(new PreSpawnPacketHandler($this->server, $this->player, $this));
 		$this->player->setImmobile(); //TODO: HACK: fix client-side falling pre-spawn
 

src/player/Player.php

@@ -34,7 +34,6 @@ use pocketmine\entity\animation\ArmSwingAnimation;
 use pocketmine\entity\animation\CriticalHitAnimation;
 use pocketmine\entity\effect\VanillaEffects;
 use pocketmine\entity\Entity;
-use pocketmine\entity\EntityDataHelper;
 use pocketmine\entity\Human;
 use pocketmine\entity\Living;
 use pocketmine\entity\Location;
@@ -261,7 +260,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 	/** @var SurvivalBlockBreakHandler|null */
 	protected $blockBreakHandler = null;
 
-	public function __construct(Server $server, NetworkSession $session, PlayerInfo $playerInfo, bool $authenticated, ?CompoundTag $namedtag){
+	public function __construct(Server $server, NetworkSession $session, PlayerInfo $playerInfo, bool $authenticated, Location $spawnLocation, ?CompoundTag $namedtag){
 		$username = TextFormat::clean($playerInfo->getUsername());
 		$this->logger = new \PrefixedLogger($server->getLogger(), "Player: $username");
 
@@ -286,24 +285,15 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 		$this->spawnThreshold = (int) (($this->server->getConfigGroup()->getProperty("chunk-sending.spawn-radius", 4) ** 2) * M_PI);
 		$this->chunkSelector = new ChunkSelector();
 
-		if($namedtag !== null and ($world = $this->server->getWorldManager()->getWorldByName($namedtag->getString("Level", ""))) !== null){
+		$this->chunkLoader = new PlayerChunkLoader($spawnLocation);
-			$spawn = EntityDataHelper::parseLocation($namedtag, $world);
-			$onGround = $namedtag->getByte("OnGround", 1) === 1;
-		}else{
-			$world = $this->server->getWorldManager()->getDefaultWorld();
-			$spawn = Location::fromObject($world->getSafeSpawn(), $world);
-			$onGround = true;
-		}
-
-		$this->chunkLoader = new PlayerChunkLoader($spawn);
 
+		$world = $spawnLocation->getWorld();
 		//load the spawn chunk so we can see the terrain
-		$world->registerChunkLoader($this->chunkLoader, $spawn->getFloorX() >> 4, $spawn->getFloorZ() >> 4, true);
+		$world->registerChunkLoader($this->chunkLoader, $spawnLocation->getFloorX() >> 4, $spawnLocation->getFloorZ() >> 4, true);
-		$world->registerChunkListener($this, $spawn->getFloorX() >> 4, $spawn->getFloorZ() >> 4);
+		$world->registerChunkListener($this, $spawnLocation->getFloorX() >> 4, $spawnLocation->getFloorZ() >> 4);
-		$this->usedChunks[World::chunkHash($spawn->getFloorX() >> 4, $spawn->getFloorZ() >> 4)] = UsedChunkStatus::NEEDED();
+		$this->usedChunks[World::chunkHash($spawnLocation->getFloorX() >> 4, $spawnLocation->getFloorZ() >> 4)] = UsedChunkStatus::NEEDED();
 
-		parent::__construct($spawn, $this->playerInfo->getSkin(), $namedtag);
+		parent::__construct($spawnLocation, $this->playerInfo->getSkin(), $namedtag);
-		$this->onGround = $onGround; //TODO: this hack is needed for new players in-air ticks - they don't get detected as on-ground until they move
 
 		$ev = new PlayerLoginEvent($this, "Plugin reason");
 		$ev->call();
@@ -712,6 +702,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 		Timings::$playerChunkSend->startTiming();
 
 		$count = 0;
+		$world = $this->getWorld();
 		foreach($this->loadQueue as $index => $distance){
 			if($count >= $this->chunksPerTick){
 				break;
@@ -728,10 +719,11 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 			$this->getWorld()->registerChunkLoader($this->chunkLoader, $X, $Z, true);
 			$this->getWorld()->registerChunkListener($this, $X, $Z);
 
-			if(!$this->getWorld()->requestChunkPopulation($X, $Z)){
+			$this->getWorld()->requestChunkPopulation($X, $Z, $this->chunkLoader)->onCompletion(
-				continue;
+				function() use ($X, $Z, $index, $world) : void{
+					if(!$this->isConnected() || !isset($this->usedChunks[$index]) || $world !== $this->getWorld()){
+						return;
 					}
-
 					unset($this->loadQueue[$index]);
 					$this->usedChunks[$index] = UsedChunkStatus::REQUESTED();
 
@@ -752,6 +744,11 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 							$this->getNetworkSession()->notifyTerrainReady();
 						}
 					});
+				},
+				static function() : void{
+					//NOOP: we'll re-request this if it fails anyway
+				}
+			);
 		}
 
 		Timings::$playerChunkSend->stopTiming();

src/player/PlayerCreationPromise.php

@@ -0,0 +1,75 @@
+<?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\player;
+
+use function spl_object_id;
+
+final class PlayerCreationPromise{
+	/**
+	 * @var \Closure[]
+	 * @phpstan-var array<int, \Closure(Player) : void>
+	 */
+	private array $onSuccess = [];
+
+	/**
+	 * @var \Closure[]
+	 * @phpstan-var array<int, \Closure() : void>
+	 */
+	private array $onFailure = [];
+
+	private bool $resolved = false;
+	private ?Player $result = null;
+
+	/**
+	 * @phpstan-param \Closure(Player) : void $onSuccess
+	 * @phpstan-param \Closure() : void $onFailure
+	 */
+	public function onCompletion(\Closure $onSuccess, \Closure $onFailure) : void{
+		if($this->resolved){
+			$this->result === null ? $onFailure() : $onSuccess($this->result);
+		}else{
+			$this->onSuccess[spl_object_id($onSuccess)] = $onSuccess;
+			$this->onFailure[spl_object_id($onFailure)] = $onFailure;
+		}
+	}
+
+	public function resolve(Player $player) : void{
+		$this->resolved = true;
+		$this->result = $player;
+		foreach($this->onSuccess as $c){
+			$c($player);
+		}
+		$this->onSuccess = [];
+		$this->onFailure = [];
+	}
+
+	public function reject() : void{
+		$this->resolved = true;
+		foreach($this->onFailure as $c){
+			$c();
+		}
+		$this->onSuccess = [];
+		$this->onFailure = [];
+	}
+}

src/world/ChunkPopulationPromise.php

@@ -0,0 +1,76 @@
+<?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\world;
+
+use function spl_object_id;
+
+final class ChunkPopulationPromise{
+	/**
+	 * @var \Closure[]
+	 * @phpstan-var array<int, \Closure() : void>
+	 */
+	private array $onSuccess = [];
+	/**
+	 * @var \Closure[]
+	 * @phpstan-var array<int, \Closure() : void>
+	 */
+	private array $onFailure = [];
+
+	private ?bool $success = null;
+
+	/**
+	 * @phpstan-param \Closure() : void $onSuccess
+	 * @phpstan-param \Closure() : void $onFailure
+	 */
+	public function onCompletion(\Closure $onSuccess, \Closure $onFailure) : void{
+		if($this->success !== null){
+			$this->success ? $onSuccess() : $onFailure();
+		}else{
+			$this->onSuccess[spl_object_id($onSuccess)] = $onSuccess;
+			$this->onFailure[spl_object_id($onFailure)] = $onFailure;
+		}
+	}
+
+	public function resolve() : void{
+		$this->success = true;
+		foreach($this->onSuccess as $callback){
+			$callback();
+		}
+		$this->onSuccess = [];
+		$this->onFailure = [];
+	}
+
+	public function reject() : void{
+		$this->success = false;
+		foreach($this->onFailure as $callback){
+			$callback();
+		}
+		$this->onSuccess = [];
+		$this->onFailure = [];
+	}
+
+	public function isCompleted() : bool{
+		return $this->success !== null;
+	}
+}

src/world/World.php

@@ -238,6 +238,16 @@ class World implements ChunkManager{
 	private $chunkLock = [];
 	/** @var int */
 	private $maxConcurrentChunkPopulationTasks = 2;
+	/**
+	 * @var ChunkPopulationPromise[] chunkHash => promise
+	 * @phpstan-var array<int, ChunkPopulationPromise>
+	 */
+	private array $chunkPopulationRequestMap = [];
+	/**
+	 * @var \SplQueue (queue of chunkHashes)
+	 * @phpstan-var \SplQueue<int>
+	 */
+	private \SplQueue $chunkPopulationRequestQueue;
 	/** @var bool[] */
 	private $generatorRegisteredWorkers = [];
 
@@ -390,6 +400,20 @@ class World implements ChunkManager{
 		$this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.level.preparing", [$this->displayName]));
 		$this->generator = GeneratorManager::getInstance()->getGenerator($this->provider->getWorldData()->getGenerator(), true);
 		//TODO: validate generator options
+		$this->chunkPopulationRequestQueue = new \SplQueue();
+		$this->addOnUnloadCallback(function() : void{
+			$this->logger->debug("Cancelling unfulfilled generation requests");
+
+			foreach($this->chunkPopulationRequestMap as $chunkHash => $promise){
+				$promise->reject();
+				unset($this->chunkPopulationRequestMap[$chunkHash]);
+			}
+			if(count($this->chunkPopulationRequestMap) !== 0){
+				//TODO: this might actually get hit because generation rejection callbacks might try to schedule new
+				//requests, and we can't prevent that right now because there's no way to detect "unloading" state
+				throw new AssumptionFailedError("New generation requests scheduled during unload");
+			}
+		});
 
 		$this->folderName = $name;
 
@@ -626,6 +650,10 @@ class World implements ChunkManager{
 			if(count($this->chunkLoaders[$chunkHash]) === 0){
 				unset($this->chunkLoaders[$chunkHash]);
 				$this->unloadChunkRequest($chunkX, $chunkZ, true);
+				if(isset($this->chunkPopulationRequestMap[$chunkHash]) && !isset($this->activeChunkPopulationTasks[$chunkHash])){
+					$this->chunkPopulationRequestMap[$chunkHash]->reject();
+					unset($this->chunkPopulationRequestMap[$chunkHash]);
+				}
 			}
 
 			if(--$this->loaderCounter[$loaderId] === 0){
@@ -2005,9 +2033,33 @@ class World implements ChunkManager{
 		return isset($this->chunkLock[World::chunkHash($chunkX, $chunkZ)]);
 	}
 
+	private function drainPopulationRequestQueue() : void{
+		$failed = [];
+		while(count($this->activeChunkPopulationTasks) < $this->maxConcurrentChunkPopulationTasks && !$this->chunkPopulationRequestQueue->isEmpty()){
+			$nextChunkHash = $this->chunkPopulationRequestQueue->dequeue();
+			World::getXZ($nextChunkHash, $nextChunkX, $nextChunkZ);
+			if(isset($this->chunkPopulationRequestMap[$nextChunkHash])){
+				assert(!isset($this->activeChunkPopulationTasks[$nextChunkHash]), "Population for chunk $nextChunkX $nextChunkZ already running");
+				$this->logger->debug("Fulfilling population request for chunk $nextChunkX $nextChunkZ");
+				$this->orderChunkPopulation($nextChunkX, $nextChunkZ, null);
+				if(!isset($this->activeChunkPopulationTasks[$nextChunkHash])){
+					$failed[] = $nextChunkHash;
+				}
+			}else{
+				$this->logger->debug("Population request for chunk $nextChunkX $nextChunkZ was discarded before it could be fulfilled");
+			}
+		}
+
+		//these requests failed even though they weren't rate limited; we can't directly re-add them to the back of the
+		//queue because it would result in an infinite loop
+		foreach($failed as $hash){
+			$this->chunkPopulationRequestQueue->enqueue($hash);
+		}
+	}
+
 	public function generateChunkCallback(int $x, int $z, ?Chunk $chunk) : void{
 		Timings::$generationCallback->startTiming();
-		if(isset($this->activeChunkPopulationTasks[$index = World::chunkHash($x, $z)])){
+		if(isset($this->chunkPopulationRequestMap[$index = World::chunkHash($x, $z)]) && isset($this->activeChunkPopulationTasks[$index])){
 			if($chunk === null){
 				throw new AssumptionFailedError("Primary chunk should never be NULL");
 			}
@@ -2016,7 +2068,6 @@ class World implements ChunkManager{
 					$this->unlockChunk($x + $xx, $z + $zz);
 				}
 			}
-			unset($this->activeChunkPopulationTasks[$index]);
 
 			$oldChunk = $this->loadChunk($x, $z);
 			$this->setChunk($x, $z, $chunk, false);
@@ -2027,11 +2078,17 @@ class World implements ChunkManager{
 					$listener->onChunkPopulated($x, $z, $chunk);
 				}
 			}
+			unset($this->activeChunkPopulationTasks[$index]);
+			$this->chunkPopulationRequestMap[$index]->resolve();
+			unset($this->chunkPopulationRequestMap[$index]);
+
+			$this->drainPopulationRequestQueue();
 		}elseif($this->isChunkLocked($x, $z)){
 			$this->unlockChunk($x, $z);
 			if($chunk !== null){
 				$this->setChunk($x, $z, $chunk, false);
 			}
+			$this->drainPopulationRequestQueue();
 		}elseif($chunk !== null){
 			$this->setChunk($x, $z, $chunk, false);
 		}
@@ -2464,6 +2521,11 @@ class World implements ChunkManager{
 		unset($this->blockCache[$chunkHash]);
 		unset($this->changedBlocks[$chunkHash]);
 
+		if(array_key_exists($chunkHash, $this->chunkPopulationRequestMap)){
+			$this->chunkPopulationRequestMap[$chunkHash]->reject();
+			unset($this->chunkPopulationRequestMap[$chunkHash]);
+		}
+
 		$this->timings->doChunkUnload->stopTiming();
 
 		return true;
@@ -2605,43 +2667,64 @@ class World implements ChunkManager{
 		}
 	}
 
+	private function enqueuePopulationRequest(int $chunkX, int $chunkZ, ?ChunkLoader $associatedChunkLoader) : ChunkPopulationPromise{
+		$chunkHash = World::chunkHash($chunkX, $chunkZ);
+		$this->chunkPopulationRequestQueue->enqueue($chunkHash);
+		$promise = $this->chunkPopulationRequestMap[$chunkHash] = new ChunkPopulationPromise();
+		if($associatedChunkLoader === null){
+			$temporaryLoader = new class implements ChunkLoader{};
+			$this->registerChunkLoader($temporaryLoader, $chunkX, $chunkZ);
+			$promise->onCompletion(
+				static function() : void{},
+				fn() => $this->unregisterChunkLoader($temporaryLoader, $chunkX, $chunkZ)
+			);
+		}
+		return $promise;
+	}
+
 	/**
 	 * Attempts to initiate asynchronous generation/population of the target chunk, if it's currently reasonable to do
 	 * so (and if it isn't already generated/populated).
+	 * If the generator is busy, the request will be put into a queue and delayed until a better time.
 	 *
-	 * This method can fail for the following reasons:
+	 * A ChunkLoader can be associated with the generation request to ensure that the generation request is cancelled if
-	 * - The generation queue for this world is currently full (intended to prevent CPU overload with non-essential generation)
+	 * no loaders are attached to the target chunk. If no loader is provided, one will be assigned (and automatically
-	 * - The target chunk is already being generated/populated
+	 * removed when the generation request completes).
-	 * - The target chunk is locked for use by another async operation (usually population)
-	 *
-	 * @return bool whether the chunk has been successfully populated already
-	 * TODO: the return values don't make a lot of sense, but currently stuff depends on them :<
 	 */
-	public function requestChunkPopulation(int $chunkX, int $chunkZ) : bool{
+	public function requestChunkPopulation(int $chunkX, int $chunkZ, ?ChunkLoader $associatedChunkLoader) : ChunkPopulationPromise{
-		if(count($this->activeChunkPopulationTasks) >= $this->maxConcurrentChunkPopulationTasks){
+		$chunkHash = World::chunkHash($chunkX, $chunkZ);
-			return false;
+		$promise = $this->chunkPopulationRequestMap[$chunkHash] ?? null;
+		if($promise !== null && isset($this->activeChunkPopulationTasks[$chunkHash])){
+			//generation is already running
+			return $promise;
 		}
-		return $this->orderChunkPopulation($chunkX, $chunkZ);
+		if(count($this->activeChunkPopulationTasks) >= $this->maxConcurrentChunkPopulationTasks){
+			//too many chunks are already generating; delay resolution of the request until later
+			return $promise ?? $this->enqueuePopulationRequest($chunkX, $chunkZ, $associatedChunkLoader);
+		}
+		return $this->orderChunkPopulation($chunkX, $chunkZ, $associatedChunkLoader);
 	}
 
 	/**
 	 * Initiates asynchronous generation/population of the target chunk, if it's not already generated/populated.
+	 * If generation has already been requested for the target chunk, the promise for the already active request will be
+	 * returned directly.
 	 *
-	 * This method can fail for the following reasons:
+	 * If the chunk is currently locked (for example due to another chunk using it for async generation), the request
-	 * - The target chunk is already being generated/populated
+	 * will be queued and executed at the earliest opportunity.
-	 * - The target chunk is locked for use by another async operation (usually population)
-	 *
-	 * @return bool whether the chunk has been successfully populated already
-	 * TODO: the return values don't make sense, but currently stuff depends on them :<
 	 */
-	public function orderChunkPopulation(int $x, int $z) : bool{
+	public function orderChunkPopulation(int $x, int $z, ?ChunkLoader $associatedChunkLoader) : ChunkPopulationPromise{
-		if(isset($this->activeChunkPopulationTasks[$index = World::chunkHash($x, $z)])){
+		$index = World::chunkHash($x, $z);
-			return false;
+		$promise = $this->chunkPopulationRequestMap[$index] ?? null;
+		if($promise !== null && isset($this->activeChunkPopulationTasks[$index])){
+			//generation is already running
+			return $promise;
 		}
 		for($xx = -1; $xx <= 1; ++$xx){
 			for($zz = -1; $zz <= 1; ++$zz){
 				if($this->isChunkLocked($x + $xx, $z + $zz)){
-					return false;
+					//chunk is already in use by another generation request; queue the request for later
+					return $promise ?? $this->enqueuePopulationRequest($x, $z, $associatedChunkLoader);
 				}
 			}
 		}
@@ -2651,6 +2734,11 @@ class World implements ChunkManager{
 			Timings::$population->startTiming();
 
 			$this->activeChunkPopulationTasks[$index] = true;
+			if($promise === null){
+				$promise = new ChunkPopulationPromise();
+				$this->chunkPopulationRequestMap[$index] = $promise;
+			}
+
 			for($xx = -1; $xx <= 1; ++$xx){
 				for($zz = -1; $zz <= 1; ++$zz){
 					$this->lockChunk($x + $xx, $z + $zz);
@@ -2665,10 +2753,13 @@ class World implements ChunkManager{
 			$this->workerPool->submitTaskToWorker($task, $workerId);
 
 			Timings::$population->stopTiming();
-			return false;
+			return $promise;
 		}
 
-		return true;
+		//chunk is already populated; return a pre-resolved promise that will directly fire callbacks assigned
+		$result = new ChunkPopulationPromise();
+		$result->resolve();
+		return $result;
 	}
 
 	public function doChunkGarbageCollection() : void{

src/world/WorldManager.php

@@ -285,7 +285,7 @@ class WorldManager{
 
 			foreach((new ChunkSelector())->selectChunks(3, $centerX, $centerZ) as $index){
 				World::getXZ($index, $chunkX, $chunkZ);
-				$world->orderChunkPopulation($chunkX, $chunkZ);
+				$world->orderChunkPopulation($chunkX, $chunkZ, null);
 			}
 		}