From f787552e97cc4d2c4a1197924cedce476313abaa Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 4 Oct 2018 14:56:42 +0100 Subject: [PATCH] Remove LevelProvider::getProviderName() This is problematic because child level providers can forget to override the provider name of their parents, and then override them by error. Instead, they should be used in a mapping fashion to make sure they are unique and not inherited. Also, the old method did not permit registering multiple aliases for the same provider. This now makes that possible. --- .../level/format/io/LevelProvider.php | 7 ------- .../level/format/io/LevelProviderManager.php | 21 ++++++++++++------- .../level/format/io/leveldb/LevelDB.php | 4 ---- .../level/format/io/region/Anvil.php | 4 ---- .../level/format/io/region/McRegion.php | 4 ---- .../level/format/io/region/PMAnvil.php | 4 ---- .../format/io/LevelProviderManagerTest.php | 8 +++---- 7 files changed, 17 insertions(+), 35 deletions(-) diff --git a/src/pocketmine/level/format/io/LevelProvider.php b/src/pocketmine/level/format/io/LevelProvider.php index 93c6c9a2a..ab30ddfbb 100644 --- a/src/pocketmine/level/format/io/LevelProvider.php +++ b/src/pocketmine/level/format/io/LevelProvider.php @@ -33,13 +33,6 @@ interface LevelProvider{ */ public function __construct(string $path); - /** - * Returns the full provider name, like "anvil" or "mcregion", will be used to find the correct format. - * - * @return string - */ - public static function getProviderName() : string; - /** * Gets the build height limit of this world * diff --git a/src/pocketmine/level/format/io/LevelProviderManager.php b/src/pocketmine/level/format/io/LevelProviderManager.php index 437721816..75291a75a 100644 --- a/src/pocketmine/level/format/io/LevelProviderManager.php +++ b/src/pocketmine/level/format/io/LevelProviderManager.php @@ -36,10 +36,10 @@ abstract class LevelProviderManager{ private static $default = PMAnvil::class; public static function init() : void{ - self::addProvider(Anvil::class); - self::addProvider(McRegion::class); - self::addProvider(PMAnvil::class); - self::addProvider(LevelDB::class); + self::addProvider(Anvil::class, "anvil"); + self::addProvider(McRegion::class, "mcregion"); + self::addProvider(PMAnvil::class, "pmanvil"); + self::addProvider(LevelDB::class, "leveldb"); } /** @@ -61,20 +61,25 @@ abstract class LevelProviderManager{ public static function setDefault(string $class) : void{ Utils::testValidInstance($class, LevelProvider::class); - self::addProvider($class); self::$default = $class; } /** * @param string $class * - * @throws \InvalidArgumentException + * @param string $name + * @param bool $overwrite */ - public static function addProvider(string $class){ + public static function addProvider(string $class, string $name, bool $overwrite = false) : void{ Utils::testValidInstance($class, LevelProvider::class); + $name = strtolower($name); + if(!$overwrite and isset(self::$providers[$name])){ + throw new \InvalidArgumentException("Alias \"$name\" is already assigned"); + } + /** @var LevelProvider $class */ - self::$providers[strtolower($class::getProviderName())] = $class; + self::$providers[$name] = $class; } /** diff --git a/src/pocketmine/level/format/io/leveldb/LevelDB.php b/src/pocketmine/level/format/io/leveldb/LevelDB.php index 34017d2f8..14199abbb 100644 --- a/src/pocketmine/level/format/io/leveldb/LevelDB.php +++ b/src/pocketmine/level/format/io/leveldb/LevelDB.php @@ -153,10 +153,6 @@ class LevelDB extends BaseLevelProvider{ $db->close(); } - public static function getProviderName() : string{ - return "leveldb"; - } - public function getWorldHeight() : int{ return 256; } diff --git a/src/pocketmine/level/format/io/region/Anvil.php b/src/pocketmine/level/format/io/region/Anvil.php index a00455c05..b5fc66718 100644 --- a/src/pocketmine/level/format/io/region/Anvil.php +++ b/src/pocketmine/level/format/io/region/Anvil.php @@ -49,10 +49,6 @@ class Anvil extends RegionLevelProvider{ ); } - public static function getProviderName() : string{ - return "anvil"; - } - protected static function getRegionFileExtension() : string{ return "mca"; } diff --git a/src/pocketmine/level/format/io/region/McRegion.php b/src/pocketmine/level/format/io/region/McRegion.php index 890de55c1..36e903f91 100644 --- a/src/pocketmine/level/format/io/region/McRegion.php +++ b/src/pocketmine/level/format/io/region/McRegion.php @@ -172,10 +172,6 @@ class McRegion extends RegionLevelProvider{ return $result; } - public static function getProviderName() : string{ - return "mcregion"; - } - protected static function getRegionFileExtension() : string{ return "mcr"; } diff --git a/src/pocketmine/level/format/io/region/PMAnvil.php b/src/pocketmine/level/format/io/region/PMAnvil.php index 472fa9397..84ab25ac7 100644 --- a/src/pocketmine/level/format/io/region/PMAnvil.php +++ b/src/pocketmine/level/format/io/region/PMAnvil.php @@ -52,10 +52,6 @@ class PMAnvil extends RegionLevelProvider{ ); } - public static function getProviderName() : string{ - return "pmanvil"; - } - protected static function getRegionFileExtension() : string{ return "mcapm"; } diff --git a/tests/phpunit/level/format/io/LevelProviderManagerTest.php b/tests/phpunit/level/format/io/LevelProviderManagerTest.php index 64b620622..10c484a76 100644 --- a/tests/phpunit/level/format/io/LevelProviderManagerTest.php +++ b/tests/phpunit/level/format/io/LevelProviderManagerTest.php @@ -30,24 +30,24 @@ class LevelProviderManagerTest extends TestCase{ public function testAddNonClassProvider() : void{ $this->expectException(\InvalidArgumentException::class); - LevelProviderManager::addProvider("lol"); + LevelProviderManager::addProvider("lol", "nope"); } public function testAddAbstractClassProvider() : void{ $this->expectException(\InvalidArgumentException::class); - LevelProviderManager::addProvider(AbstractLevelProvider::class); + LevelProviderManager::addProvider(AbstractLevelProvider::class, "abstract"); } public function testAddInterfaceProvider() : void{ $this->expectException(\InvalidArgumentException::class); - LevelProviderManager::addProvider(InterfaceLevelProvider::class); + LevelProviderManager::addProvider(InterfaceLevelProvider::class, "interface"); } public function testAddWrongClassProvider() : void{ $this->expectException(\InvalidArgumentException::class); - LevelProviderManager::addProvider(LevelProviderManagerTest::class); + LevelProviderManager::addProvider(LevelProviderManagerTest::class, "bad_class"); } }