From d3244b137620d33b793e6cb92012ca53e925ac3f Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 30 Apr 2025 18:26:53 +0100 Subject: [PATCH 1/9] Second look at extracting generator executors from World this is a greatly simplified alternative to #6668. I realized we probably don't actually want to let the executor decide how promises are managed, since the system is built to be async anyway. This new iteration makes it far less complex to implement a custom executor, without needing to concern oneself with complex promise management. This means it should be fairly practical to make custom executors with this. --- src/world/World.php | 80 +++++------- src/world/generator/GeneratorRegisterTask.php | 24 ++-- .../generator/GeneratorUnregisterTask.php | 11 +- .../executor/AsyncGeneratorExecutor.php | 121 ++++++++++++++++++ .../generator/executor/GeneratorExecutor.php | 40 ++++++ .../GeneratorExecutorSetupParameters.php | 38 ++++++ 6 files changed, 242 insertions(+), 72 deletions(-) create mode 100644 src/world/generator/executor/AsyncGeneratorExecutor.php create mode 100644 src/world/generator/executor/GeneratorExecutor.php create mode 100644 src/world/generator/executor/GeneratorExecutorSetupParameters.php diff --git a/src/world/World.php b/src/world/World.php index 7ffe8e00b1c..6e635e96e59 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -93,9 +93,10 @@ use pocketmine\world\format\io\WritableWorldProvider; use pocketmine\world\format\LightArray; use pocketmine\world\format\SubChunk; +use pocketmine\world\generator\executor\AsyncGeneratorExecutor; +use pocketmine\world\generator\executor\GeneratorExecutor; +use pocketmine\world\generator\executor\GeneratorExecutorSetupParameters; use pocketmine\world\generator\GeneratorManager; -use pocketmine\world\generator\GeneratorRegisterTask; -use pocketmine\world\generator\GeneratorUnregisterTask; use pocketmine\world\generator\PopulationTask; use pocketmine\world\light\BlockLightUpdate; use pocketmine\world\light\LightPopulationTask; @@ -336,11 +337,7 @@ class World implements ChunkManager{ */ private array $chunkPopulationRequestQueueIndex = []; - /** - * @var true[] - * @phpstan-var array - */ - private array $generatorRegisteredWorkers = []; + private readonly GeneratorExecutor $generatorExecutor; private bool $autoSave = true; @@ -360,9 +357,6 @@ class World implements ChunkManager{ private bool $doingTick = false; - /** @phpstan-var class-string<\pocketmine\world\generator\Generator> */ - private string $generator; - private bool $unloaded = false; /** * @var \Closure[] @@ -498,7 +492,21 @@ public function __construct( $generator = GeneratorManager::getInstance()->getGenerator($this->provider->getWorldData()->getGenerator()) ?? throw new AssumptionFailedError("WorldManager should already have checked that the generator exists"); $generator->validateGeneratorOptions($this->provider->getWorldData()->getGeneratorOptions()); - $this->generator = $generator->getGeneratorClass(); + + $executorSetupParameters = new GeneratorExecutorSetupParameters( + worldMinY: $this->minY, + worldMaxY: $this->maxY, + generatorSeed: $this->getSeed(), + generatorClass: $generator->getGeneratorClass(), + generatorSettings: $this->provider->getWorldData()->getGeneratorOptions() + ); + $this->generatorExecutor = new AsyncGeneratorExecutor( + $this->logger, + $this->workerPool, + $executorSetupParameters, + $this->worldId + ); + $this->chunkPopulationRequestQueue = new \SplQueue(); $this->addOnUnloadCallback(function() : void{ $this->logger->debug("Cancelling unfulfilled generation requests"); @@ -534,17 +542,6 @@ public function __construct( $this->initRandomTickBlocksFromConfig($cfg); $this->timings = new WorldTimings($this); - - $this->workerPool->addWorkerStartHook($workerStartHook = function(int $workerId) : void{ - if(array_key_exists($workerId, $this->generatorRegisteredWorkers)){ - $this->logger->debug("Worker $workerId with previously registered generator restarted, flagging as unregistered"); - unset($this->generatorRegisteredWorkers[$workerId]); - } - }); - $workerPool = $this->workerPool; - $this->addOnUnloadCallback(static function() use ($workerPool, $workerStartHook) : void{ - $workerPool->removeWorkerStartHook($workerStartHook); - }); } private function initRandomTickBlocksFromConfig(ServerConfigGroup $cfg) : void{ @@ -585,21 +582,6 @@ public function getTickRateTime() : float{ return $this->tickRateTime; } - public function registerGeneratorToWorker(int $worker) : void{ - $this->logger->debug("Registering generator on worker $worker"); - $this->workerPool->submitTaskToWorker(new GeneratorRegisterTask($this, $this->generator, $this->provider->getWorldData()->getGeneratorOptions()), $worker); - $this->generatorRegisteredWorkers[$worker] = true; - } - - public function unregisterGenerator() : void{ - foreach($this->workerPool->getRunningWorkers() as $i){ - if(isset($this->generatorRegisteredWorkers[$i])){ - $this->workerPool->submitTaskToWorker(new GeneratorUnregisterTask($this), $i); - } - } - $this->generatorRegisteredWorkers = []; - } - public function getServer() : Server{ return $this->server; } @@ -657,7 +639,7 @@ public function onUnload() : void{ $this->save(); - $this->unregisterGenerator(); + $this->generatorExecutor->shutdown(); $this->provider->close(); $this->blockCache = []; @@ -3486,29 +3468,25 @@ private function internalOrderChunkPopulation(int $chunkX, int $chunkZ, ?ChunkLo $centerChunk = $this->loadChunk($chunkX, $chunkZ); $adjacentChunks = $this->getAdjacentChunks($chunkX, $chunkZ); - $task = new PopulationTask( - $this->worldId, + + $this->generatorExecutor->populate( $chunkX, $chunkZ, $centerChunk, - $adjacentChunks, - function(Chunk $centerChunk, array $adjacentChunks) use ($chunkPopulationLockId, $chunkX, $chunkZ, $temporaryChunkLoader) : void{ + $adjacentChunks + )->onCompletion( + function(array $results) use ($chunkPopulationLockId, $chunkX, $chunkZ, $temporaryChunkLoader) : void{ if(!$this->isLoaded()){ return; } + [$centerChunk, $adjacentChunks] = $results; $this->generateChunkCallback($chunkPopulationLockId, $chunkX, $chunkZ, $centerChunk, $adjacentChunks, $temporaryChunkLoader); + }, + function() : void{ + throw new AssumptionFailedError("Not expecting executor to reject this promise"); } ); - $workerId = $this->workerPool->selectWorker(); - if(!isset($this->workerPool->getRunningWorkers()[$workerId]) && isset($this->generatorRegisteredWorkers[$workerId])){ - $this->logger->debug("Selected worker $workerId previously had generator registered, but is now offline"); - unset($this->generatorRegisteredWorkers[$workerId]); - } - if(!isset($this->generatorRegisteredWorkers[$workerId])){ - $this->registerGeneratorToWorker($workerId); - } - $this->workerPool->submitTaskToWorker($task, $workerId); return $resolver->getPromise(); }finally{ diff --git a/src/world/generator/GeneratorRegisterTask.php b/src/world/generator/GeneratorRegisterTask.php index e2e773a35ec..9609f3f3226 100644 --- a/src/world/generator/GeneratorRegisterTask.php +++ b/src/world/generator/GeneratorRegisterTask.php @@ -24,34 +24,30 @@ namespace pocketmine\world\generator; use pocketmine\scheduler\AsyncTask; -use pocketmine\world\World; +use pocketmine\thread\NonThreadSafeValue; +use pocketmine\world\generator\executor\GeneratorExecutorSetupParameters; class GeneratorRegisterTask extends AsyncTask{ - public int $seed; - public int $worldId; - public int $worldMinY; - public int $worldMaxY; + /** @phpstan-var NonThreadSafeValue */ + private NonThreadSafeValue $setupParameters; /** * @phpstan-param class-string $generatorClass */ public function __construct( - World $world, - public string $generatorClass, - public string $generatorSettings + GeneratorExecutorSetupParameters $setupParameters, + private readonly int $contextId ){ - $this->seed = $world->getSeed(); - $this->worldId = $world->getId(); - $this->worldMinY = $world->getMinY(); - $this->worldMaxY = $world->getMaxY(); + $this->setupParameters = new NonThreadSafeValue($setupParameters); } public function onRun() : void{ + $setupParameters = $this->setupParameters->deserialize(); /** * @var Generator $generator * @see Generator::__construct() */ - $generator = new $this->generatorClass($this->seed, $this->generatorSettings); - ThreadLocalGeneratorContext::register(new ThreadLocalGeneratorContext($generator, $this->worldMinY, $this->worldMaxY), $this->worldId); + $generator = new $setupParameters->generatorClass($setupParameters->generatorSeed, $setupParameters->generatorSettings); + ThreadLocalGeneratorContext::register(new ThreadLocalGeneratorContext($generator, $setupParameters->worldMinY, $setupParameters->worldMaxY), $this->contextId); } } diff --git a/src/world/generator/GeneratorUnregisterTask.php b/src/world/generator/GeneratorUnregisterTask.php index 41b4cd80824..e1dc35fa459 100644 --- a/src/world/generator/GeneratorUnregisterTask.php +++ b/src/world/generator/GeneratorUnregisterTask.php @@ -24,16 +24,13 @@ namespace pocketmine\world\generator; use pocketmine\scheduler\AsyncTask; -use pocketmine\world\World; class GeneratorUnregisterTask extends AsyncTask{ - public int $worldId; - - public function __construct(World $world){ - $this->worldId = $world->getId(); - } + public function __construct( + private readonly int $contextId + ){} public function onRun() : void{ - ThreadLocalGeneratorContext::unregister($this->worldId); + ThreadLocalGeneratorContext::unregister($this->contextId); } } diff --git a/src/world/generator/executor/AsyncGeneratorExecutor.php b/src/world/generator/executor/AsyncGeneratorExecutor.php new file mode 100644 index 00000000000..7d19b3f466b --- /dev/null +++ b/src/world/generator/executor/AsyncGeneratorExecutor.php @@ -0,0 +1,121 @@ + + */ + private array $generatorRegisteredWorkers = []; + + public function __construct( + \Logger $logger, + private readonly AsyncPool $workerPool, + private readonly GeneratorExecutorSetupParameters $setupParameters, + int $asyncContextId = null + ){ + $this->logger = new \PrefixedLogger($logger, "AsyncGeneratorExecutor"); + + //TODO: we only allow setting this for PM5 because of PopulationTask uses in plugins + $this->asyncContextId = $asyncContextId ?? self::$nextAsyncContextId++; + + $this->workerStartHook = function(int $workerId) : void{ + if(array_key_exists($workerId, $this->generatorRegisteredWorkers)){ + $this->logger->debug("Worker $workerId with previously registered generator restarted, flagging as unregistered"); + unset($this->generatorRegisteredWorkers[$workerId]); + } + }; + $this->workerPool->addWorkerStartHook($this->workerStartHook); + } + + private function registerGeneratorToWorker(int $worker) : void{ + $this->logger->debug("Registering generator on worker $worker"); + $this->workerPool->submitTaskToWorker(new GeneratorRegisterTask($this->setupParameters, $this->asyncContextId), $worker); + $this->generatorRegisteredWorkers[$worker] = true; + } + + private function unregisterGenerator() : void{ + foreach($this->workerPool->getRunningWorkers() as $i){ + if(isset($this->generatorRegisteredWorkers[$i])){ + $this->workerPool->submitTaskToWorker(new GeneratorUnregisterTask($this->asyncContextId), $i); + } + } + $this->generatorRegisteredWorkers = []; + } + + public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks) : Promise{ + /** @phpstan-var PromiseResolver}> $resolver */ + $resolver = new PromiseResolver(); + + $task = new PopulationTask( + $this->asyncContextId, + $chunkX, + $chunkZ, + $centerChunk, + $adjacentChunks, + function(Chunk $centerChunk, array $adjacentChunks) use ($resolver) : void{ + $resolver->resolve([$centerChunk, $adjacentChunks]); + } + ); + $workerId = $this->workerPool->selectWorker(); + if(!isset($this->workerPool->getRunningWorkers()[$workerId]) && isset($this->generatorRegisteredWorkers[$workerId])){ + $this->logger->debug("Selected worker $workerId previously had generator registered, but is now offline"); + unset($this->generatorRegisteredWorkers[$workerId]); + } + if(!isset($this->generatorRegisteredWorkers[$workerId])){ + $this->registerGeneratorToWorker($workerId); + } + $this->workerPool->submitTaskToWorker($task, $workerId); + + return $resolver->getPromise(); + } + + public function shutdown() : void{ + $this->unregisterGenerator(); + $this->workerPool->removeWorkerStartHook($this->workerStartHook); + } +} diff --git a/src/world/generator/executor/GeneratorExecutor.php b/src/world/generator/executor/GeneratorExecutor.php new file mode 100644 index 00000000000..75b542170bb --- /dev/null +++ b/src/world/generator/executor/GeneratorExecutor.php @@ -0,0 +1,40 @@ + $adjacentChunks + * + * @phpstan-return Promise}> + */ + public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks) : Promise; + + public function shutdown() : void; + +} diff --git a/src/world/generator/executor/GeneratorExecutorSetupParameters.php b/src/world/generator/executor/GeneratorExecutorSetupParameters.php new file mode 100644 index 00000000000..b8ddc2b241a --- /dev/null +++ b/src/world/generator/executor/GeneratorExecutorSetupParameters.php @@ -0,0 +1,38 @@ + $generatorClass + */ + public function __construct( + public readonly int $worldMinY, + public readonly int $worldMaxY, + public readonly int $generatorSeed, + public readonly string $generatorClass, + public readonly string $generatorSettings, + ){} +} From 4ea83187ada5cb04c46498e462c3152e05743ca3 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 30 Apr 2025 18:31:19 +0100 Subject: [PATCH 2/9] tidy --- src/world/generator/PopulationTask.php | 7 +++++++ src/world/generator/executor/AsyncGeneratorExecutor.php | 6 ++---- .../AsyncGeneratorRegisterTask.php} | 6 +++--- .../AsyncGeneratorUnregisterTask.php} | 4 ++-- .../{ => executor}/ThreadLocalGeneratorContext.php | 4 +++- 5 files changed, 17 insertions(+), 10 deletions(-) rename src/world/generator/{GeneratorRegisterTask.php => executor/AsyncGeneratorRegisterTask.php} (91%) rename src/world/generator/{GeneratorUnregisterTask.php => executor/AsyncGeneratorUnregisterTask.php} (90%) rename src/world/generator/{ => executor}/ThreadLocalGeneratorContext.php (94%) diff --git a/src/world/generator/PopulationTask.php b/src/world/generator/PopulationTask.php index a8366a30632..971349a5b6e 100644 --- a/src/world/generator/PopulationTask.php +++ b/src/world/generator/PopulationTask.php @@ -27,11 +27,18 @@ use pocketmine\utils\AssumptionFailedError; use pocketmine\world\format\Chunk; use pocketmine\world\format\io\FastChunkSerializer; +use pocketmine\world\generator\executor\ThreadLocalGeneratorContext; use function array_map; use function igbinary_serialize; use function igbinary_unserialize; /** + * @internal + * + * TODO: this should be moved to the executor namespace, but plugins have unfortunately used it directly due to the + * difficulty of regenerating chunks. This should be addressed in the future. + * For the remainder of PM5, we can't relocate this class. + * * @phpstan-type OnCompletion \Closure(Chunk $centerChunk, array $adjacentChunks) : void */ class PopulationTask extends AsyncTask{ diff --git a/src/world/generator/executor/AsyncGeneratorExecutor.php b/src/world/generator/executor/AsyncGeneratorExecutor.php index 7d19b3f466b..fa71e86f5b1 100644 --- a/src/world/generator/executor/AsyncGeneratorExecutor.php +++ b/src/world/generator/executor/AsyncGeneratorExecutor.php @@ -27,8 +27,6 @@ use pocketmine\promise\PromiseResolver; use pocketmine\scheduler\AsyncPool; use pocketmine\world\format\Chunk; -use pocketmine\world\generator\GeneratorRegisterTask; -use pocketmine\world\generator\GeneratorUnregisterTask; use pocketmine\world\generator\PopulationTask; use pocketmine\world\World; use function array_key_exists; @@ -74,14 +72,14 @@ public function __construct( private function registerGeneratorToWorker(int $worker) : void{ $this->logger->debug("Registering generator on worker $worker"); - $this->workerPool->submitTaskToWorker(new GeneratorRegisterTask($this->setupParameters, $this->asyncContextId), $worker); + $this->workerPool->submitTaskToWorker(new AsyncGeneratorRegisterTask($this->setupParameters, $this->asyncContextId), $worker); $this->generatorRegisteredWorkers[$worker] = true; } private function unregisterGenerator() : void{ foreach($this->workerPool->getRunningWorkers() as $i){ if(isset($this->generatorRegisteredWorkers[$i])){ - $this->workerPool->submitTaskToWorker(new GeneratorUnregisterTask($this->asyncContextId), $i); + $this->workerPool->submitTaskToWorker(new AsyncGeneratorUnregisterTask($this->asyncContextId), $i); } } $this->generatorRegisteredWorkers = []; diff --git a/src/world/generator/GeneratorRegisterTask.php b/src/world/generator/executor/AsyncGeneratorRegisterTask.php similarity index 91% rename from src/world/generator/GeneratorRegisterTask.php rename to src/world/generator/executor/AsyncGeneratorRegisterTask.php index 9609f3f3226..de3af21f732 100644 --- a/src/world/generator/GeneratorRegisterTask.php +++ b/src/world/generator/executor/AsyncGeneratorRegisterTask.php @@ -21,13 +21,13 @@ declare(strict_types=1); -namespace pocketmine\world\generator; +namespace pocketmine\world\generator\executor; use pocketmine\scheduler\AsyncTask; use pocketmine\thread\NonThreadSafeValue; -use pocketmine\world\generator\executor\GeneratorExecutorSetupParameters; +use pocketmine\world\generator\Generator; -class GeneratorRegisterTask extends AsyncTask{ +class AsyncGeneratorRegisterTask extends AsyncTask{ /** @phpstan-var NonThreadSafeValue */ private NonThreadSafeValue $setupParameters; diff --git a/src/world/generator/GeneratorUnregisterTask.php b/src/world/generator/executor/AsyncGeneratorUnregisterTask.php similarity index 90% rename from src/world/generator/GeneratorUnregisterTask.php rename to src/world/generator/executor/AsyncGeneratorUnregisterTask.php index e1dc35fa459..c771903f586 100644 --- a/src/world/generator/GeneratorUnregisterTask.php +++ b/src/world/generator/executor/AsyncGeneratorUnregisterTask.php @@ -21,11 +21,11 @@ declare(strict_types=1); -namespace pocketmine\world\generator; +namespace pocketmine\world\generator\executor; use pocketmine\scheduler\AsyncTask; -class GeneratorUnregisterTask extends AsyncTask{ +class AsyncGeneratorUnregisterTask extends AsyncTask{ public function __construct( private readonly int $contextId ){} diff --git a/src/world/generator/ThreadLocalGeneratorContext.php b/src/world/generator/executor/ThreadLocalGeneratorContext.php similarity index 94% rename from src/world/generator/ThreadLocalGeneratorContext.php rename to src/world/generator/executor/ThreadLocalGeneratorContext.php index bcf99882b56..bea8bb032b0 100644 --- a/src/world/generator/ThreadLocalGeneratorContext.php +++ b/src/world/generator/executor/ThreadLocalGeneratorContext.php @@ -21,7 +21,9 @@ declare(strict_types=1); -namespace pocketmine\world\generator; +namespace pocketmine\world\generator\executor; + +use pocketmine\world\generator\Generator; /** * Manages thread-local caches for generators and the things needed to support them From ee72460c8dd69c8d884c81c7b10c5284f668bfa8 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 30 Apr 2025 18:45:35 +0100 Subject: [PATCH 3/9] Added sync executor & made Flat use it (this was WAY easier than the first time) --- src/world/World.php | 15 +++-- src/world/generator/GeneratorManager.php | 7 +- src/world/generator/GeneratorManagerEntry.php | 5 +- .../executor/AsyncGeneratorRegisterTask.php | 6 +- .../GeneratorExecutorSetupParameters.php | 11 +++ .../executor/SyncGeneratorExecutor.php | 67 +++++++++++++++++++ 6 files changed, 96 insertions(+), 15 deletions(-) create mode 100644 src/world/generator/executor/SyncGeneratorExecutor.php diff --git a/src/world/World.php b/src/world/World.php index 6e635e96e59..0a7cbc77b6d 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -96,6 +96,7 @@ use pocketmine\world\generator\executor\AsyncGeneratorExecutor; use pocketmine\world\generator\executor\GeneratorExecutor; use pocketmine\world\generator\executor\GeneratorExecutorSetupParameters; +use pocketmine\world\generator\executor\SyncGeneratorExecutor; use pocketmine\world\generator\GeneratorManager; use pocketmine\world\generator\PopulationTask; use pocketmine\world\light\BlockLightUpdate; @@ -500,12 +501,14 @@ public function __construct( generatorClass: $generator->getGeneratorClass(), generatorSettings: $this->provider->getWorldData()->getGeneratorOptions() ); - $this->generatorExecutor = new AsyncGeneratorExecutor( - $this->logger, - $this->workerPool, - $executorSetupParameters, - $this->worldId - ); + $this->generatorExecutor = $generator->isFast() ? + new SyncGeneratorExecutor($executorSetupParameters) : + new AsyncGeneratorExecutor( + $this->logger, + $this->workerPool, + $executorSetupParameters, + $this->worldId + ); $this->chunkPopulationRequestQueue = new \SplQueue(); $this->addOnUnloadCallback(function() : void{ diff --git a/src/world/generator/GeneratorManager.php b/src/world/generator/GeneratorManager.php index 291ea91de0f..a1b00480ea7 100644 --- a/src/world/generator/GeneratorManager.php +++ b/src/world/generator/GeneratorManager.php @@ -50,7 +50,7 @@ public function __construct(){ }catch(InvalidGeneratorOptionsException $e){ return $e; } - }); + }, fast: true); $this->addGenerator(Normal::class, "normal", fn() => null); $this->addAlias("normal", "default"); $this->addGenerator(Nether::class, "nether", fn() => null); @@ -62,6 +62,7 @@ public function __construct(){ * @param string $name Alias for this generator type that can be written in configs * @param \Closure $presetValidator Callback to validate generator options for new worlds * @param bool $overwrite Whether to force overwriting any existing registered generator with the same name + * @param bool $fast Whether this generator is fast enough to run without async tasks * * @phpstan-param \Closure(string) : ?InvalidGeneratorOptionsException $presetValidator * @@ -69,7 +70,7 @@ public function __construct(){ * * @throws \InvalidArgumentException */ - public function addGenerator(string $class, string $name, \Closure $presetValidator, bool $overwrite = false) : void{ + public function addGenerator(string $class, string $name, \Closure $presetValidator, bool $overwrite = false, bool $fast = false) : void{ Utils::testValidInstance($class, Generator::class); $name = strtolower($name); @@ -77,7 +78,7 @@ public function addGenerator(string $class, string $name, \Closure $presetValida throw new \InvalidArgumentException("Alias \"$name\" is already assigned"); } - $this->list[$name] = new GeneratorManagerEntry($class, $presetValidator); + $this->list[$name] = new GeneratorManagerEntry($class, $presetValidator, $fast); } /** diff --git a/src/world/generator/GeneratorManagerEntry.php b/src/world/generator/GeneratorManagerEntry.php index 256ed27d5cb..942f6ee79ca 100644 --- a/src/world/generator/GeneratorManagerEntry.php +++ b/src/world/generator/GeneratorManagerEntry.php @@ -31,12 +31,15 @@ final class GeneratorManagerEntry{ */ public function __construct( private string $generatorClass, - private \Closure $presetValidator + private \Closure $presetValidator, + private readonly bool $fast ){} /** @phpstan-return class-string */ public function getGeneratorClass() : string{ return $this->generatorClass; } + public function isFast() : bool{ return $this->fast; } + /** * @throws InvalidGeneratorOptionsException */ diff --git a/src/world/generator/executor/AsyncGeneratorRegisterTask.php b/src/world/generator/executor/AsyncGeneratorRegisterTask.php index de3af21f732..c340c2cbcde 100644 --- a/src/world/generator/executor/AsyncGeneratorRegisterTask.php +++ b/src/world/generator/executor/AsyncGeneratorRegisterTask.php @@ -43,11 +43,7 @@ public function __construct( public function onRun() : void{ $setupParameters = $this->setupParameters->deserialize(); - /** - * @var Generator $generator - * @see Generator::__construct() - */ - $generator = new $setupParameters->generatorClass($setupParameters->generatorSeed, $setupParameters->generatorSettings); + $generator = $setupParameters->createGenerator(); ThreadLocalGeneratorContext::register(new ThreadLocalGeneratorContext($generator, $setupParameters->worldMinY, $setupParameters->worldMaxY), $this->contextId); } } diff --git a/src/world/generator/executor/GeneratorExecutorSetupParameters.php b/src/world/generator/executor/GeneratorExecutorSetupParameters.php index b8ddc2b241a..92b9de179e2 100644 --- a/src/world/generator/executor/GeneratorExecutorSetupParameters.php +++ b/src/world/generator/executor/GeneratorExecutorSetupParameters.php @@ -23,6 +23,8 @@ namespace pocketmine\world\generator\executor; +use pocketmine\world\generator\Generator; + final class GeneratorExecutorSetupParameters{ /** @@ -35,4 +37,13 @@ public function __construct( public readonly string $generatorClass, public readonly string $generatorSettings, ){} + + public function createGenerator() : Generator{ + /** + * @var Generator $generator + * @see Generator::__construct() + */ + $generator = new $this->generatorClass($this->generatorSeed, $this->generatorSettings); + return $generator; + } } diff --git a/src/world/generator/executor/SyncGeneratorExecutor.php b/src/world/generator/executor/SyncGeneratorExecutor.php new file mode 100644 index 00000000000..19713322a12 --- /dev/null +++ b/src/world/generator/executor/SyncGeneratorExecutor.php @@ -0,0 +1,67 @@ +generator = $setupParameters->createGenerator(); + $this->worldMinY = $setupParameters->worldMinY; + $this->worldMaxY = $setupParameters->worldMaxY; + } + + public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks) : Promise{ + [$centerChunk, $adjacentChunks] = PopulationUtils::populateChunkWithAdjacents( + $this->worldMinY, + $this->worldMaxY, + $this->generator, + $chunkX, + $chunkZ, + $centerChunk, + $adjacentChunks + ); + + /** @phpstan-var PromiseResolver}> $resolver */ + $resolver = new PromiseResolver(); + $resolver->resolve([$centerChunk, $adjacentChunks]); + + return $resolver->getPromise(); + } + + public function shutdown() : void{ + //NOOP + } +} From 8eccbfcae9223264646bebc1a5309d7eb371029e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 30 Apr 2025 18:47:40 +0100 Subject: [PATCH 4/9] Fix PHPStan --- .../executor/AsyncGeneratorRegisterTask.php | 5 +---- tests/phpstan/configs/actual-problems.neon | 12 ++++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/world/generator/executor/AsyncGeneratorRegisterTask.php b/src/world/generator/executor/AsyncGeneratorRegisterTask.php index c340c2cbcde..9762474c947 100644 --- a/src/world/generator/executor/AsyncGeneratorRegisterTask.php +++ b/src/world/generator/executor/AsyncGeneratorRegisterTask.php @@ -25,15 +25,12 @@ use pocketmine\scheduler\AsyncTask; use pocketmine\thread\NonThreadSafeValue; -use pocketmine\world\generator\Generator; class AsyncGeneratorRegisterTask extends AsyncTask{ /** @phpstan-var NonThreadSafeValue */ private NonThreadSafeValue $setupParameters; - /** - * @phpstan-param class-string $generatorClass - */ + public function __construct( GeneratorExecutorSetupParameters $setupParameters, private readonly int $contextId diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index d3adde422d6..2030a0dad26 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -1272,18 +1272,18 @@ parameters: count: 1 path: ../../../src/world/format/io/region/RegionLoader.php - - - message: '#^Dynamic new is not allowed\.$#' - identifier: pocketmine.new.dynamic - count: 1 - path: ../../../src/world/generator/GeneratorRegisterTask.php - - message: '#^Method pocketmine\\world\\generator\\biome\\BiomeSelector\:\:pickBiome\(\) should return pocketmine\\world\\biome\\Biome but returns pocketmine\\world\\biome\\Biome\|null\.$#' identifier: return.type count: 1 path: ../../../src/world/generator/biome/BiomeSelector.php + - + message: '#^Dynamic new is not allowed\.$#' + identifier: pocketmine.new.dynamic + count: 1 + path: ../../../src/world/generator/executor/GeneratorExecutorSetupParameters.php + - message: '#^Cannot call method getBiomeId\(\) on pocketmine\\world\\format\\Chunk\|null\.$#' identifier: method.nonObject From 70daea3fc10499a73b8deb2a5d00a68a071006ac Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 30 Apr 2025 18:55:08 +0100 Subject: [PATCH 5/9] simpler to not use a promise ??? --- src/world/World.php | 9 ++------- .../generator/executor/AsyncGeneratorExecutor.php | 11 ++--------- src/world/generator/executor/GeneratorExecutor.php | 5 ++--- .../generator/executor/SyncGeneratorExecutor.php | 8 ++------ 4 files changed, 8 insertions(+), 25 deletions(-) diff --git a/src/world/World.php b/src/world/World.php index 0a7cbc77b6d..792681a89e9 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -3476,18 +3476,13 @@ private function internalOrderChunkPopulation(int $chunkX, int $chunkZ, ?ChunkLo $chunkX, $chunkZ, $centerChunk, - $adjacentChunks - )->onCompletion( - function(array $results) use ($chunkPopulationLockId, $chunkX, $chunkZ, $temporaryChunkLoader) : void{ + $adjacentChunks, + function(Chunk $centerChunk, array $adjacentChunks) use ($chunkPopulationLockId, $chunkX, $chunkZ, $temporaryChunkLoader) : void{ if(!$this->isLoaded()){ return; } - [$centerChunk, $adjacentChunks] = $results; $this->generateChunkCallback($chunkPopulationLockId, $chunkX, $chunkZ, $centerChunk, $adjacentChunks, $temporaryChunkLoader); - }, - function() : void{ - throw new AssumptionFailedError("Not expecting executor to reject this promise"); } ); diff --git a/src/world/generator/executor/AsyncGeneratorExecutor.php b/src/world/generator/executor/AsyncGeneratorExecutor.php index fa71e86f5b1..efe2289c716 100644 --- a/src/world/generator/executor/AsyncGeneratorExecutor.php +++ b/src/world/generator/executor/AsyncGeneratorExecutor.php @@ -85,19 +85,14 @@ private function unregisterGenerator() : void{ $this->generatorRegisteredWorkers = []; } - public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks) : Promise{ - /** @phpstan-var PromiseResolver}> $resolver */ - $resolver = new PromiseResolver(); - + public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks, \Closure $onCompletion) : void{ $task = new PopulationTask( $this->asyncContextId, $chunkX, $chunkZ, $centerChunk, $adjacentChunks, - function(Chunk $centerChunk, array $adjacentChunks) use ($resolver) : void{ - $resolver->resolve([$centerChunk, $adjacentChunks]); - } + $onCompletion ); $workerId = $this->workerPool->selectWorker(); if(!isset($this->workerPool->getRunningWorkers()[$workerId]) && isset($this->generatorRegisteredWorkers[$workerId])){ @@ -108,8 +103,6 @@ function(Chunk $centerChunk, array $adjacentChunks) use ($resolver) : void{ $this->registerGeneratorToWorker($workerId); } $this->workerPool->submitTaskToWorker($task, $workerId); - - return $resolver->getPromise(); } public function shutdown() : void{ diff --git a/src/world/generator/executor/GeneratorExecutor.php b/src/world/generator/executor/GeneratorExecutor.php index 75b542170bb..f727bda9022 100644 --- a/src/world/generator/executor/GeneratorExecutor.php +++ b/src/world/generator/executor/GeneratorExecutor.php @@ -30,10 +30,9 @@ interface GeneratorExecutor{ /** * @param Chunk[]|null[] $adjacentChunks * @phpstan-param array $adjacentChunks - * - * @phpstan-return Promise}> + * @phpstan-param \Closure(Chunk $centerChunk, array $adjacentChunks) : void $onCompletion */ - public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks) : Promise; + public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks, \Closure $onCompletion) : void; public function shutdown() : void; diff --git a/src/world/generator/executor/SyncGeneratorExecutor.php b/src/world/generator/executor/SyncGeneratorExecutor.php index 19713322a12..719825d6d94 100644 --- a/src/world/generator/executor/SyncGeneratorExecutor.php +++ b/src/world/generator/executor/SyncGeneratorExecutor.php @@ -43,7 +43,7 @@ public function __construct( $this->worldMaxY = $setupParameters->worldMaxY; } - public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks) : Promise{ + public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $adjacentChunks, \Closure $onCompletion) : void{ [$centerChunk, $adjacentChunks] = PopulationUtils::populateChunkWithAdjacents( $this->worldMinY, $this->worldMaxY, @@ -54,11 +54,7 @@ public function populate(int $chunkX, int $chunkZ, ?Chunk $centerChunk, array $a $adjacentChunks ); - /** @phpstan-var PromiseResolver}> $resolver */ - $resolver = new PromiseResolver(); - $resolver->resolve([$centerChunk, $adjacentChunks]); - - return $resolver->getPromise(); + $onCompletion($centerChunk, $adjacentChunks); } public function shutdown() : void{ From 1aabd98905be7c3d4ce8cc170c334b0076b73c56 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 30 Apr 2025 18:56:10 +0100 Subject: [PATCH 6/9] bruh --- src/world/generator/executor/AsyncGeneratorExecutor.php | 2 -- src/world/generator/executor/GeneratorExecutor.php | 1 - src/world/generator/executor/SyncGeneratorExecutor.php | 2 -- 3 files changed, 5 deletions(-) diff --git a/src/world/generator/executor/AsyncGeneratorExecutor.php b/src/world/generator/executor/AsyncGeneratorExecutor.php index efe2289c716..6d6f6e86339 100644 --- a/src/world/generator/executor/AsyncGeneratorExecutor.php +++ b/src/world/generator/executor/AsyncGeneratorExecutor.php @@ -23,8 +23,6 @@ namespace pocketmine\world\generator\executor; -use pocketmine\promise\Promise; -use pocketmine\promise\PromiseResolver; use pocketmine\scheduler\AsyncPool; use pocketmine\world\format\Chunk; use pocketmine\world\generator\PopulationTask; diff --git a/src/world/generator/executor/GeneratorExecutor.php b/src/world/generator/executor/GeneratorExecutor.php index f727bda9022..d3f62d410b9 100644 --- a/src/world/generator/executor/GeneratorExecutor.php +++ b/src/world/generator/executor/GeneratorExecutor.php @@ -23,7 +23,6 @@ namespace pocketmine\world\generator\executor; -use pocketmine\promise\Promise; use pocketmine\world\format\Chunk; interface GeneratorExecutor{ diff --git a/src/world/generator/executor/SyncGeneratorExecutor.php b/src/world/generator/executor/SyncGeneratorExecutor.php index 719825d6d94..79b5fdd00b9 100644 --- a/src/world/generator/executor/SyncGeneratorExecutor.php +++ b/src/world/generator/executor/SyncGeneratorExecutor.php @@ -23,8 +23,6 @@ namespace pocketmine\world\generator\executor; -use pocketmine\promise\Promise; -use pocketmine\promise\PromiseResolver; use pocketmine\world\format\Chunk; use pocketmine\world\generator\Generator; use pocketmine\world\generator\PopulationUtils; From 9736d5589dd8c6066746d96c3b90b6166c7339d3 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 30 Apr 2025 19:06:08 +0100 Subject: [PATCH 7/9] Make GeneratorExecutorSetupParameters thread-safe --- .../executor/AsyncGeneratorRegisterTask.php | 12 +++--------- .../executor/GeneratorExecutorSetupParameters.php | 3 ++- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/world/generator/executor/AsyncGeneratorRegisterTask.php b/src/world/generator/executor/AsyncGeneratorRegisterTask.php index 9762474c947..5bc67834dea 100644 --- a/src/world/generator/executor/AsyncGeneratorRegisterTask.php +++ b/src/world/generator/executor/AsyncGeneratorRegisterTask.php @@ -24,22 +24,16 @@ namespace pocketmine\world\generator\executor; use pocketmine\scheduler\AsyncTask; -use pocketmine\thread\NonThreadSafeValue; class AsyncGeneratorRegisterTask extends AsyncTask{ - /** @phpstan-var NonThreadSafeValue */ - private NonThreadSafeValue $setupParameters; - public function __construct( - GeneratorExecutorSetupParameters $setupParameters, + private readonly GeneratorExecutorSetupParameters $setupParameters, private readonly int $contextId - ){ - $this->setupParameters = new NonThreadSafeValue($setupParameters); - } + ){} public function onRun() : void{ - $setupParameters = $this->setupParameters->deserialize(); + $setupParameters = $this->setupParameters; $generator = $setupParameters->createGenerator(); ThreadLocalGeneratorContext::register(new ThreadLocalGeneratorContext($generator, $setupParameters->worldMinY, $setupParameters->worldMaxY), $this->contextId); } diff --git a/src/world/generator/executor/GeneratorExecutorSetupParameters.php b/src/world/generator/executor/GeneratorExecutorSetupParameters.php index 92b9de179e2..b5fdb7bf913 100644 --- a/src/world/generator/executor/GeneratorExecutorSetupParameters.php +++ b/src/world/generator/executor/GeneratorExecutorSetupParameters.php @@ -23,9 +23,10 @@ namespace pocketmine\world\generator\executor; +use pmmp\thread\ThreadSafe; use pocketmine\world\generator\Generator; -final class GeneratorExecutorSetupParameters{ +final class GeneratorExecutorSetupParameters extends ThreadSafe{ /** * @phpstan-param class-string $generatorClass From e45c9a001cfebca926d016cd54fb05a8d8001694 Mon Sep 17 00:00:00 2001 From: "Dylan T." Date: Sat, 3 May 2025 15:31:31 +0100 Subject: [PATCH 8/9] Update AsyncGeneratorExecutor.php --- src/world/generator/executor/AsyncGeneratorExecutor.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/world/generator/executor/AsyncGeneratorExecutor.php b/src/world/generator/executor/AsyncGeneratorExecutor.php index 6d6f6e86339..01773f1d889 100644 --- a/src/world/generator/executor/AsyncGeneratorExecutor.php +++ b/src/world/generator/executor/AsyncGeneratorExecutor.php @@ -29,9 +29,6 @@ use pocketmine\world\World; use function array_key_exists; -/** - * @phpstan-import-type ChunkPosHash from World - */ final class AsyncGeneratorExecutor implements GeneratorExecutor{ private static int $nextAsyncContextId = 1; From 2382d5f16cce7069ffc8eebe442d0294687a1cc4 Mon Sep 17 00:00:00 2001 From: "Dylan T." Date: Sat, 3 May 2025 15:35:28 +0100 Subject: [PATCH 9/9] Update AsyncGeneratorExecutor.php --- src/world/generator/executor/AsyncGeneratorExecutor.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/world/generator/executor/AsyncGeneratorExecutor.php b/src/world/generator/executor/AsyncGeneratorExecutor.php index 01773f1d889..d19b6e6617a 100644 --- a/src/world/generator/executor/AsyncGeneratorExecutor.php +++ b/src/world/generator/executor/AsyncGeneratorExecutor.php @@ -26,7 +26,6 @@ use pocketmine\scheduler\AsyncPool; use pocketmine\world\format\Chunk; use pocketmine\world\generator\PopulationTask; -use pocketmine\world\World; use function array_key_exists; final class AsyncGeneratorExecutor implements GeneratorExecutor{