Improved Promise API - separate resolver and consumer APIs

this makes creating a promise slightly more cumbersome, but I'm more concerned about people who might try to call 'new Promise' directly.

src/Server.php

@@ -98,6 +98,7 @@ use pocketmine\utils\NotCloneable;
 use pocketmine\utils\NotSerializable;
 use pocketmine\utils\Process;
 use pocketmine\utils\Promise;
+use pocketmine\utils\PromiseResolver;
 use pocketmine\utils\SignalHandler;
 use pocketmine\utils\Terminal;
 use pocketmine\utils\TextFormat;
@@ -548,11 +549,11 @@ class Server{
 			$playerPos = null;
 			$spawn = $world->getSpawnLocation();
 		}
-		$playerPromise = new Promise();
+		$playerPromiseResolver = new PromiseResolver();
 		$world->requestChunkPopulation($spawn->getFloorX() >> Chunk::COORD_BIT_SIZE, $spawn->getFloorZ() >> Chunk::COORD_BIT_SIZE, null)->onCompletion(
-			function() use ($playerPromise, $class, $session, $playerInfo, $authenticated, $world, $playerPos, $spawn, $offlinePlayerData) : void{
+			function() use ($playerPromiseResolver, $class, $session, $playerInfo, $authenticated, $world, $playerPos, $spawn, $offlinePlayerData) : void{
 				if(!$session->isConnected()){
-					$playerPromise->reject();
+					$playerPromiseResolver->reject();
 					return;
 				}
 
@@ -572,16 +573,16 @@ class Server{
 				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
 				}
-				$playerPromise->resolve($player);
+				$playerPromiseResolver->resolve($player);
 			},
-			static function() use ($playerPromise, $session) : void{
+			static function() use ($playerPromiseResolver, $session) : void{
 				if($session->isConnected()){
 					$session->disconnect("Spawn terrain generation failed");
 				}
-				$playerPromise->reject();
+				$playerPromiseResolver->reject();
 			}
 		);
-		return $playerPromise;
+		return $playerPromiseResolver->getPromise();
 	}
 
 	/**

src/utils/Promise.php

@@ -30,64 +30,22 @@ use function spl_object_id;
  */
 final class Promise{
 	/**
-	 * @var \Closure[]
+	 * @internal Do NOT call this directly; create a new Resolver and call Resolver->promise()
-	 * @phpstan-var array<int, \Closure(TValue) : void>
+	 * @see PromiseResolver
+	 * @phpstan-param PromiseSharedData<TValue> $shared
 	 */
-	private array $onSuccess = [];
+	public function __construct(private PromiseSharedData $shared){}
-
-	/**
-	 * @var \Closure[]
-	 * @phpstan-var array<int, \Closure() : void>
-	 */
-	private array $onFailure = [];
-
-	private bool $resolved = false;
-
-	/**
-	 * @var mixed
-	 * @phpstan-var TValue|null
-	 */
-	private $result = null;
 
 	/**
 	 * @phpstan-param \Closure(TValue) : void $onSuccess
 	 * @phpstan-param \Closure() : void $onFailure
 	 */
 	public function onCompletion(\Closure $onSuccess, \Closure $onFailure) : void{
-		if($this->resolved){
+		if($this->shared->resolved){
-			$this->result === null ? $onFailure() : $onSuccess($this->result);
+			$this->shared->result === null ? $onFailure() : $onSuccess($this->shared->result);
 		}else{
-			$this->onSuccess[spl_object_id($onSuccess)] = $onSuccess;
+			$this->shared->onSuccess[spl_object_id($onSuccess)] = $onSuccess;
-			$this->onFailure[spl_object_id($onFailure)] = $onFailure;
+			$this->shared->onFailure[spl_object_id($onFailure)] = $onFailure;
 		}
 	}
-
-	/**
-	 * @param mixed $value
-	 * @phpstan-param TValue $value
-	 */
-	public function resolve($value) : void{
-		if($this->resolved){
-			throw new \InvalidStateException("Promise has already been resolved/rejected");
-		}
-		$this->resolved = true;
-		$this->result = $value;
-		foreach($this->onSuccess as $c){
-			$c($value);
-		}
-		$this->onSuccess = [];
-		$this->onFailure = [];
-	}
-
-	public function reject() : void{
-		if($this->resolved){
-			throw new \InvalidStateException("Promise has already been resolved/rejected");
-		}
-		$this->resolved = true;
-		foreach($this->onFailure as $c){
-			$c();
-		}
-		$this->onSuccess = [];
-		$this->onFailure = [];
-	}
 }

src/utils/PromiseResolver.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\utils;
+
+/**
+ * @phpstan-template TValue
+ */
+final class PromiseResolver{
+	/** @phpstan-var PromiseSharedData<TValue> */
+	private PromiseSharedData $shared;
+	/** @phpstan-var Promise<TValue> */
+	private Promise $promise;
+
+	public function __construct(){
+		$this->shared = new PromiseSharedData();
+		$this->promise = new Promise($this->shared);
+	}
+
+	/**
+	 * @param mixed $value
+	 * @phpstan-param TValue $value
+	 */
+	public function resolve($value) : void{
+		if($this->shared->resolved){
+			throw new \InvalidStateException("Promise has already been resolved/rejected");
+		}
+		$this->shared->resolved = true;
+		$this->shared->result = $value;
+		foreach($this->shared->onSuccess as $c){
+			$c($value);
+		}
+		$this->shared->onSuccess = [];
+		$this->shared->onFailure = [];
+	}
+
+	public function reject() : void{
+		if($this->shared->resolved){
+			throw new \InvalidStateException("Promise has already been resolved/rejected");
+		}
+		$this->shared->resolved = true;
+		foreach($this->shared->onFailure as $c){
+			$c();
+		}
+		$this->shared->onSuccess = [];
+		$this->shared->onFailure = [];
+	}
+
+	/**
+	 * @phpstan-return Promise<TValue>
+	 */
+	public function getPromise() : Promise{
+		return $this->promise;
+	}
+}

src/utils/PromiseSharedData.php

@@ -0,0 +1,51 @@
+<?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\utils;
+
+/**
+ * @internal
+ * @see PromiseResolver
+ * @phpstan-template TValue
+ */
+final class PromiseSharedData{
+	/**
+	 * @var \Closure[]
+	 * @phpstan-var array<int, \Closure(TValue) : void>
+	 */
+	public array $onSuccess = [];
+
+	/**
+	 * @var \Closure[]
+	 * @phpstan-var array<int, \Closure() : void>
+	 */
+	public array $onFailure = [];
+
+	public bool $resolved = false;
+
+	/**
+	 * @var mixed
+	 * @phpstan-var TValue|null
+	 */
+	public $result = null;
+}

src/world/World.php

@@ -72,6 +72,7 @@ use pocketmine\timings\Timings;
 use pocketmine\utils\AssumptionFailedError;
 use pocketmine\utils\Limits;
 use pocketmine\utils\Promise;
+use pocketmine\utils\PromiseResolver;
 use pocketmine\utils\ReversePriorityQueue;
 use pocketmine\world\biome\Biome;
 use pocketmine\world\biome\BiomeRegistry;
@@ -253,8 +254,8 @@ class World implements ChunkManager{
 	/** @var int */
 	private $maxConcurrentChunkPopulationTasks = 2;
 	/**
-	 * @var Promise[] chunkHash => promise
+	 * @var PromiseResolver[] chunkHash => promise
-	 * @phpstan-var array<int, Promise<Chunk>>
+	 * @phpstan-var array<int, PromiseResolver<Chunk>>
 	 */
 	private array $chunkPopulationRequestMap = [];
 	/**
@@ -2717,16 +2718,16 @@ class World implements ChunkManager{
 	private function enqueuePopulationRequest(int $chunkX, int $chunkZ, ?ChunkLoader $associatedChunkLoader) : Promise{
 		$chunkHash = World::chunkHash($chunkX, $chunkZ);
 		$this->chunkPopulationRequestQueue->enqueue($chunkHash);
-		$promise = $this->chunkPopulationRequestMap[$chunkHash] = new Promise();
+		$resolver = $this->chunkPopulationRequestMap[$chunkHash] = new PromiseResolver();
 		if($associatedChunkLoader === null){
 			$temporaryLoader = new class implements ChunkLoader{};
 			$this->registerChunkLoader($temporaryLoader, $chunkX, $chunkZ);
-			$promise->onCompletion(
+			$resolver->getPromise()->onCompletion(
 				fn() => $this->unregisterChunkLoader($temporaryLoader, $chunkX, $chunkZ),
 				static function() : void{}
 			);
 		}
-		return $promise;
+		return $resolver->getPromise();
 	}
 
 	private function drainPopulationRequestQueue() : void{
@@ -2763,14 +2764,14 @@ class World implements ChunkManager{
 	 */
 	public function requestChunkPopulation(int $chunkX, int $chunkZ, ?ChunkLoader $associatedChunkLoader) : Promise{
 		$chunkHash = World::chunkHash($chunkX, $chunkZ);
-		$promise = $this->chunkPopulationRequestMap[$chunkHash] ?? null;
+		$resolver = $this->chunkPopulationRequestMap[$chunkHash] ?? null;
-		if($promise !== null && isset($this->activeChunkPopulationTasks[$chunkHash])){
+		if($resolver !== null && isset($this->activeChunkPopulationTasks[$chunkHash])){
 			//generation is already running
-			return $promise;
+			return $resolver->getPromise();
 		}
 		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 $resolver?->getPromise() ?? $this->enqueuePopulationRequest($chunkX, $chunkZ, $associatedChunkLoader);
 		}
 		return $this->orderChunkPopulation($chunkX, $chunkZ, $associatedChunkLoader);
 	}
@@ -2787,16 +2788,16 @@ class World implements ChunkManager{
 	 */
 	public function orderChunkPopulation(int $x, int $z, ?ChunkLoader $associatedChunkLoader) : Promise{
 		$index = World::chunkHash($x, $z);
-		$promise = $this->chunkPopulationRequestMap[$index] ?? null;
+		$resolver = $this->chunkPopulationRequestMap[$index] ?? null;
-		if($promise !== null && isset($this->activeChunkPopulationTasks[$index])){
+		if($resolver !== null && isset($this->activeChunkPopulationTasks[$index])){
 			//generation is already running
-			return $promise;
+			return $resolver->getPromise();
 		}
 		for($xx = -1; $xx <= 1; ++$xx){
 			for($zz = -1; $zz <= 1; ++$zz){
 				if($this->isChunkLocked($x + $xx, $z + $zz)){
 					//chunk is already in use by another generation request; queue the request for later
-					return $promise ?? $this->enqueuePopulationRequest($x, $z, $associatedChunkLoader);
+					return $resolver?->getPromise() ?? $this->enqueuePopulationRequest($x, $z, $associatedChunkLoader);
 				}
 			}
 		}
@@ -2808,9 +2809,9 @@ class World implements ChunkManager{
 			Timings::$population->startTiming();
 
 			$this->activeChunkPopulationTasks[$index] = true;
-			if($promise === null){
+			if($resolver === null){
-				$promise = new Promise();
+				$resolver = new PromiseResolver();
-				$this->chunkPopulationRequestMap[$index] = $promise;
+				$this->chunkPopulationRequestMap[$index] = $resolver;
 			}
 
 			for($xx = -1; $xx <= 1; ++$xx){
@@ -2830,15 +2831,15 @@ class World implements ChunkManager{
 			$this->workerPool->submitTaskToWorker($task, $workerId);
 
 			Timings::$population->stopTiming();
-			return $promise;
+			return $resolver->getPromise();
 		}
 
 		$this->unregisterChunkLoader($temporaryChunkLoader, $x, $z);
 
 		//chunk is already populated; return a pre-resolved promise that will directly fire callbacks assigned
-		$result = new Promise();
+		$result = new PromiseResolver();
 		$result->resolve($chunk);
-		return $result;
+		return $result->getPromise();
 	}
 
 	/**