Added support for creation-time validation of generator options, closes #2717

resources/locale

@@ -1 +1 @@
-Subproject commit 0fe963d087c1408b1dffa82f33e67f34ad8deaf5
+Subproject commit 17fc6a10501c2cd48ec08f81ab41a640864f8d1d

src/Server.php

@@ -106,6 +106,7 @@ 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\World;
 use pocketmine\world\WorldCreationOptions;
 use pocketmine\world\WorldManager;
@@ -964,15 +965,25 @@ class Server{
 			$this->pluginManager->loadPlugins($this->pluginPath);
 			$this->enablePlugins(PluginEnableOrder::STARTUP());
 
-			$getGenerator = function(string $generatorName, string $worldName) : ?string{
+			$getGenerator = function(string $generatorName, string $generatorOptions, string $worldName) : ?string{
-				$generatorClass = GeneratorManager::getInstance()->getGenerator($generatorName);
+				$generatorEntry = GeneratorManager::getInstance()->getGenerator($generatorName);
-				if($generatorClass === null){
+				if($generatorEntry === null){
 					$this->logger->error($this->language->translate(KnownTranslationFactory::pocketmine_level_generationError(
 						$worldName,
 						KnownTranslationFactory::pocketmine_level_unknownGenerator($generatorName)
 					)));
+					return null;
 				}
-				return $generatorClass;
+				try{
+					$generatorEntry->validateGeneratorOptions($generatorOptions);
+				}catch(InvalidGeneratorOptionsException $e){
+					$this->logger->error($this->language->translate(KnownTranslationFactory::pocketmine_level_generationError(
+						$worldName,
+						KnownTranslationFactory::pocketmine_level_invalidGeneratorOptions($generatorOptions, $generatorName, $e->getMessage())
+					)));
+					return null;
+				}
+				return $generatorEntry->getGeneratorClass();
 			};
 
 			foreach((array) $this->configGroup->getProperty("worlds", []) as $name => $options){
@@ -985,19 +996,20 @@ class Server{
 					$creationOptions = WorldCreationOptions::create();
 					//TODO: error checking
 
-					if(isset($options["generator"])){
+					$generatorName = $options["generator"] ?? "default";
-						$generatorClass = $getGenerator($options["generator"], $name);
+					$generatorOptions = isset($options["preset"]) && is_string($options["preset"]) ? $options["preset"] : "";
-						if($generatorClass === null){
+
-							continue;
+					$generatorClass = $getGenerator($generatorName, $generatorOptions, $name);
-						}
+					if($generatorClass === null){
-						$creationOptions->setGeneratorClass($generatorClass);
+						continue;
 					}
+					$creationOptions->setGeneratorClass($generatorClass);
+					$creationOptions->setGeneratorOptions($generatorOptions);
+
 					if(isset($options["difficulty"]) && is_string($options["difficulty"])){
 						$creationOptions->setDifficulty(World::getDifficultyFromString($options["difficulty"]));
 					}
-					if(isset($options["preset"]) && is_string($options["preset"])){
+
-						$creationOptions->setGeneratorOptions($options["preset"]);
-					}
 					if(isset($options["seed"])){
 						$convertedSeed = Generator::convertSeed((string) ($options["seed"] ?? ""));
 						if($convertedSeed !== null){
@@ -1017,11 +1029,13 @@ class Server{
 					$this->configGroup->setConfigString("level-name", "world");
 				}
 				if(!$this->worldManager->loadWorld($default, true)){
-					$generatorClass = $getGenerator($this->configGroup->getConfigString("level-type"), $default);
+					$generatorName = $this->configGroup->getConfigString("level-type");
+					$generatorOptions = $this->configGroup->getConfigString("generator-settings");
+					$generatorClass = $getGenerator($generatorName, $generatorOptions, $default);
 					if($generatorClass !== null){
 						$creationOptions = WorldCreationOptions::create()
 							->setGeneratorClass($generatorClass)
-							->setGeneratorOptions($this->configGroup->getConfigString("generator-settings"));
+							->setGeneratorOptions($generatorOptions);
 						$convertedSeed = Generator::convertSeed($this->configGroup->getConfigString("level-seed"));
 						if($convertedSeed !== null){
 							$creationOptions->setSeed($convertedSeed);

src/lang/KnownTranslationFactory.php

@@ -1575,6 +1575,14 @@ final class KnownTranslationFactory{
 		]);
 	}
 
+	public static function pocketmine_level_invalidGeneratorOptions(Translatable|string $preset, Translatable|string $generatorName, Translatable|string $details) : Translatable{
+		return new Translatable(KnownTranslationKeys::POCKETMINE_LEVEL_INVALIDGENERATOROPTIONS, [
+			"preset" => $preset,
+			"generatorName" => $generatorName,
+			"details" => $details,
+		]);
+	}
+
 	public static function pocketmine_level_loadError(Translatable|string $param0, Translatable|string $param1) : Translatable{
 		return new Translatable(KnownTranslationKeys::POCKETMINE_LEVEL_LOADERROR, [
 			0 => $param0,

src/lang/KnownTranslationKeys.php

@@ -338,6 +338,7 @@ final class KnownTranslationKeys{
 	public const POCKETMINE_LEVEL_CORRUPTED = "pocketmine.level.corrupted";
 	public const POCKETMINE_LEVEL_DEFAULTERROR = "pocketmine.level.defaultError";
 	public const POCKETMINE_LEVEL_GENERATIONERROR = "pocketmine.level.generationError";
+	public const POCKETMINE_LEVEL_INVALIDGENERATOROPTIONS = "pocketmine.level.invalidGeneratorOptions";
 	public const POCKETMINE_LEVEL_LOADERROR = "pocketmine.level.loadError";
 	public const POCKETMINE_LEVEL_NOTFOUND = "pocketmine.level.notFound";
 	public const POCKETMINE_LEVEL_PREPARING = "pocketmine.level.preparing";

src/world/World.php

@@ -411,9 +411,10 @@ class World implements ChunkManager{
 		$this->maxY = $this->provider->getWorldMaxY();
 
 		$this->server->getLogger()->info($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_level_preparing($this->displayName)));
-		$this->generator = GeneratorManager::getInstance()->getGenerator($this->provider->getWorldData()->getGenerator()) ??
+		$generator = GeneratorManager::getInstance()->getGenerator($this->provider->getWorldData()->getGenerator()) ??
 			throw new AssumptionFailedError("WorldManager should already have checked that the generator exists");
-		//TODO: validate generator options
+		$generator->validateGeneratorOptions($this->provider->getWorldData()->getGeneratorOptions());
+		$this->generator = $generator->getGeneratorClass();
 		$this->chunkPopulationRequestQueue = new \SplQueue();
 		$this->addOnUnloadCallback(function() : void{
 			$this->logger->debug("Cancelling unfulfilled generation requests");

src/world/format/io/FormatConverter.php

@@ -115,7 +115,7 @@ class FormatConverter{
 		$this->newProvider->generate($convertedOutput, $data->getName(), WorldCreationOptions::create()
 			//TODO: defaulting to NORMAL here really isn't very good behaviour, but it's consistent with what we already
 			//did previously; besides, WorldManager checks for unknown generators before this is reached anyway.
-			->setGeneratorClass(GeneratorManager::getInstance()->getGenerator($data->getGenerator()) ?? Normal::class)
+			->setGeneratorClass(GeneratorManager::getInstance()->getGenerator($data->getGenerator())?->getGeneratorClass() ?? Normal::class)
 			->setGeneratorOptions($data->getGeneratorOptions())
 			->setSeed($data->getSeed())
 			->setSpawnPosition($data->getSpawn())

src/world/generator/GeneratorManager.php

@@ -34,35 +34,49 @@ final class GeneratorManager{
 	use SingletonTrait;
 
 	/**
-	 * @var string[] name => classname mapping
+	 * @var GeneratorManagerEntry[] name => classname mapping
-	 * @phpstan-var array<string, class-string<Generator>>
+	 * @phpstan-var array<string, GeneratorManagerEntry>
 	 */
 	private $list = [];
 
 	public function __construct(){
-		$this->addGenerator(Flat::class, "flat");
+		$this->addGenerator(Flat::class, "flat", \Closure::fromCallable(function(string $preset) : ?InvalidGeneratorOptionsException{
-		$this->addGenerator(Normal::class, "normal");
+			if($preset === ""){
-		$this->addGenerator(Normal::class, "default");
+				return null;
-		$this->addGenerator(Nether::class, "hell");
+			}
-		$this->addGenerator(Nether::class, "nether");
+			try{
+				FlatGeneratorOptions::parsePreset($preset);
+				return null;
+			}catch(InvalidGeneratorOptionsException $e){
+				return $e;
+			}
+		}));
+		$this->addGenerator(Normal::class, "normal", fn() => null);
+		$this->addGenerator(Normal::class, "default", fn() => null);
+		$this->addGenerator(Nether::class, "hell", fn() => null);
+		$this->addGenerator(Nether::class, "nether", fn() => null);
 	}
 
 	/**
-	 * @param string $class Fully qualified name of class that extends \pocketmine\world\generator\Generator
+	 * @param string                          $class Fully qualified name of class that extends \pocketmine\world\generator\Generator
-	 * @param string $name Alias for this generator type that can be written in configs
+	 * @param string                          $name Alias for this generator type that can be written in configs
-	 * @param bool   $overwrite Whether to force overwriting any existing registered generator with the same name
+	 * @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
+	 *
+	 * @phpstan-param \Closure(string) : ?InvalidGeneratorOptionsException $presetValidator
+	 *
 	 * @phpstan-param class-string<Generator> $class
 	 *
 	 * @throws \InvalidArgumentException
 	 */
-	public function addGenerator(string $class, string $name, bool $overwrite = false) : void{
+	public function addGenerator(string $class, string $name, \Closure $presetValidator, bool $overwrite = false) : void{
 		Utils::testValidInstance($class, Generator::class);
 
 		if(!$overwrite and isset($this->list[$name = strtolower($name)])){
 			throw new \InvalidArgumentException("Alias \"$name\" is already assigned");
 		}
 
-		$this->list[$name] = $class;
+		$this->list[$name] = new GeneratorManagerEntry($class, $presetValidator);
 	}
 
 	/**
@@ -75,12 +89,9 @@ final class GeneratorManager{
 	}
 
 	/**
-	 * Returns a class name of a registered Generator matching the given name.
+	 * Returns the generator entry of a registered Generator matching the given name, or null if not found.
-	 *
-	 * @return string|null Name of class that extends Generator, or null if no generator is mapped to that name
-	 * @phpstan-return class-string<Generator>|null
 	 */
-	public function getGenerator(string $name) : ?string{
+	public function getGenerator(string $name) : ?GeneratorManagerEntry{
 		return $this->list[strtolower($name)] ?? null;
 	}
 
@@ -95,7 +106,7 @@ final class GeneratorManager{
 	public function getGeneratorName(string $class) : string{
 		Utils::testValidInstance($class, Generator::class);
 		foreach($this->list as $name => $c){
-			if($c === $class){
+			if($c->getGeneratorClass() === $class){
 				return $name;
 			}
 		}

src/world/generator/GeneratorManagerEntry.php

@@ -0,0 +1,48 @@
+<?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\generator;
+
+final class GeneratorManagerEntry{
+
+	/**
+	 * @phpstan-param class-string<Generator> $generatorClass
+	 * @phpstan-param \Closure(string) : ?InvalidGeneratorOptionsException $presetValidator
+	 */
+	public function __construct(
+		private string $generatorClass,
+		private \Closure $presetValidator
+	){}
+
+	/** @phpstan-return class-string<Generator> */
+	public function getGeneratorClass() : string{ return $this->generatorClass; }
+
+	/**
+	 * @throws InvalidGeneratorOptionsException
+	 */
+	public function validateGeneratorOptions(string $generatorOptions) : void{
+		if(($exception = ($this->presetValidator)($generatorOptions)) !== null){
+			throw $exception;
+		}
+	}
+}