From 94e8623c7506a535849431ab357027cad824bb45 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 17 Jul 2018 12:14:26 +0100 Subject: [PATCH 1/4] Server: account for default provider being missing --- src/pocketmine/Server.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pocketmine/Server.php b/src/pocketmine/Server.php index aa24df888..cce6fca0f 100644 --- a/src/pocketmine/Server.php +++ b/src/pocketmine/Server.php @@ -1070,6 +1070,9 @@ class Server{ if(($providerClass = LevelProviderManager::getProviderByName($this->getProperty("level-settings.default-format", "pmanvil"))) === null){ $providerClass = LevelProviderManager::getProviderByName("pmanvil"); + if($providerClass === null){ + throw new \InvalidStateException("Default level provider has not been registered"); + } } try{ From f6b54f51168b3fb1d62c5085a2f76f442f89b553 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 17 Jul 2018 12:18:46 +0100 Subject: [PATCH 2/4] Server: don't create levels inside catch-all Under normal circumstances, none of the boxed code will throw exceptions. Under exceptional circumstances, the caller should know about it. Usually the caller is the server. We don't want to catch unexpected exceptions because those should crash the server and generate a crashdump. --- src/pocketmine/Server.php | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/src/pocketmine/Server.php b/src/pocketmine/Server.php index 6a4687b30..2e1a21715 100644 --- a/src/pocketmine/Server.php +++ b/src/pocketmine/Server.php @@ -1025,14 +1025,7 @@ class Server{ return false; } - try{ - $level = new Level($this, $name, new $providerClass($path)); - }catch(\Throwable $e){ - - $this->logger->error($this->getLanguage()->translateString("pocketmine.level.loadError", [$name, $e->getMessage()])); - $this->logger->logException($e); - return false; - } + $level = new Level($this, $name, new $providerClass($path)); $this->levels[$level->getId()] = $level; @@ -1075,20 +1068,14 @@ class Server{ } } - try{ - $path = $this->getDataPath() . "worlds/" . $name . "/"; - /** @var LevelProvider $providerClass */ - $providerClass::generate($path, $name, $seed, $generator, $options); + $path = $this->getDataPath() . "worlds/" . $name . "/"; + /** @var LevelProvider $providerClass */ + $providerClass::generate($path, $name, $seed, $generator, $options); - $level = new Level($this, $name, new $providerClass($path)); - $this->levels[$level->getId()] = $level; + $level = new Level($this, $name, new $providerClass($path)); + $this->levels[$level->getId()] = $level; - $level->setTickRate($this->baseTickRate); - }catch(\Throwable $e){ - $this->logger->error($this->getLanguage()->translateString("pocketmine.level.generationError", [$name, $e->getMessage()])); - $this->logger->logException($e); - return false; - } + $level->setTickRate($this->baseTickRate); $this->getPluginManager()->callEvent(new LevelInitEvent($level)); From b480c630601e641ab1e7870a463f85b0fd0f6c0c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 17 Jul 2018 14:46:08 +0100 Subject: [PATCH 3/4] Fixed ItemFactory::fromString() meta handling bug introduced by 71c3c349766e348bf53a1beeefaf9a897bc0a954 --- src/pocketmine/item/ItemFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/item/ItemFactory.php b/src/pocketmine/item/ItemFactory.php index e87dde253..0e76790e8 100644 --- a/src/pocketmine/item/ItemFactory.php +++ b/src/pocketmine/item/ItemFactory.php @@ -353,7 +353,7 @@ class ItemFactory{ if(!isset($b[1])){ $meta = 0; }elseif(is_numeric($b[1])){ - $meta = $b[1]; + $meta = (int) $b[1]; }else{ throw new \InvalidArgumentException("Unable to parse \"" . $b[1] . "\" from \"" . $str . "\" as a valid meta value"); } From 6ab2fa84da4631f63c8ae639be0c172ecfa52fdb Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 17 Jul 2018 14:52:47 +0100 Subject: [PATCH 4/4] added some tests for ItemFactory::fromString() --- tests/phpunit/item/ItemTest.php | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/phpunit/item/ItemTest.php b/tests/phpunit/item/ItemTest.php index 2cb2b975d..f1be994f3 100644 --- a/tests/phpunit/item/ItemTest.php +++ b/tests/phpunit/item/ItemTest.php @@ -51,4 +51,29 @@ class ItemTest extends TestCase{ self::assertEquals(BlockFactory::isRegistered($id), ItemFactory::isRegistered($id)); } } + + public function itemFromStringProvider() : array{ + return [ + ["dye:4", ItemIds::DYE, 4], + ["351", ItemIds::DYE, 0], + ["351:4", ItemIds::DYE, 4], + ["stone:3", ItemIds::STONE, 3], + ["minecraft:string", ItemIds::STRING, 0], + ["diamond_pickaxe", ItemIds::DIAMOND_PICKAXE, 0], + ["diamond_pickaxe:5", ItemIds::DIAMOND_PICKAXE, 5] + ]; + } + + /** + * @dataProvider itemFromStringProvider + * @param string $string + * @param int $id + * @param int $meta + */ + public function testFromStringSingle(string $string, int $id, int $meta) : void{ + $item = ItemFactory::fromString($string); + + self::assertEquals($id, $item->getId()); + self::assertEquals($meta, $item->getDamage()); + } }