From df2c3136c9d3cfb86aa5cc436d7d942dbcf1c093 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 2 Aug 2020 21:10:47 +0100 Subject: [PATCH 1/8] VersionString: added missing start anchor to regex --- src/pocketmine/utils/VersionString.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/utils/VersionString.php b/src/pocketmine/utils/VersionString.php index ca100a843..59a4dedda 100644 --- a/src/pocketmine/utils/VersionString.php +++ b/src/pocketmine/utils/VersionString.php @@ -52,7 +52,7 @@ class VersionString{ $this->development = $isDevBuild; $this->build = $buildNumber; - preg_match('/(\d+)\.(\d+)\.(\d+)(?:-(.*))?$/', $this->baseVersion, $matches); + preg_match('/^(\d+)\.(\d+)\.(\d+)(?:-(.*))?$/', $this->baseVersion, $matches); if(count($matches) < 4){ throw new \InvalidArgumentException("Invalid base version \"$baseVersion\", should contain at least 3 version digits"); } From 756840f11dc4378d08ed529c5e98839732c7df40 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 2 Aug 2020 23:07:47 +0100 Subject: [PATCH 2/8] Fixed matchItems corrupting CraftingTransaction internal state on repeated validation This bug became apparent while developing a more robust fix for 1.16 crafting. --- src/pocketmine/inventory/transaction/InventoryTransaction.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pocketmine/inventory/transaction/InventoryTransaction.php b/src/pocketmine/inventory/transaction/InventoryTransaction.php index 76c536021..b85395785 100644 --- a/src/pocketmine/inventory/transaction/InventoryTransaction.php +++ b/src/pocketmine/inventory/transaction/InventoryTransaction.php @@ -136,6 +136,8 @@ class InventoryTransaction{ * @throws TransactionValidationException */ protected function matchItems(array &$needItems, array &$haveItems) : void{ + $needItems = []; + $haveItems = []; foreach($this->actions as $key => $action){ if(!$action->getTargetItem()->isNull()){ $needItems[] = $action->getTargetItem(); From 5b620d964ea9494a07a00c140227aa91208bb350 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 2 Aug 2020 23:37:33 +0100 Subject: [PATCH 3/8] Do not assume the presence of a crafting transaction closing marker fixes #3655, fixes #3241 instead of guessing where the end of the transaction is, we attempt validation after every piece of the transaction, with the assumption being that a crafting transaction will not validate until it's complete. --- src/pocketmine/Player.php | 42 +++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php index eb3c2ee9c..d80abfdd5 100644 --- a/src/pocketmine/Player.php +++ b/src/pocketmine/Player.php @@ -149,6 +149,7 @@ use pocketmine\network\mcpe\protocol\types\CommandParameter; use pocketmine\network\mcpe\protocol\types\ContainerIds; use pocketmine\network\mcpe\protocol\types\DimensionIds; use pocketmine\network\mcpe\protocol\types\GameMode; +use pocketmine\network\mcpe\protocol\types\inventory\UIInventorySlotOffset; use pocketmine\network\mcpe\protocol\types\NetworkInventoryAction; use pocketmine\network\mcpe\protocol\types\PersonaPieceTintColor; use pocketmine\network\mcpe\protocol\types\PersonaSkinPiece; @@ -2392,18 +2393,20 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{ /** @var InventoryAction[] $actions */ $actions = []; $isCraftingPart = false; - $isFinalCraftingPart = false; foreach($packet->actions as $networkInventoryAction){ if( $networkInventoryAction->sourceType === NetworkInventoryAction::SOURCE_TODO and ( $networkInventoryAction->windowId === NetworkInventoryAction::SOURCE_TYPE_CRAFTING_RESULT or $networkInventoryAction->windowId === NetworkInventoryAction::SOURCE_TYPE_CRAFTING_USE_INGREDIENT + ) or ( + $this->craftingTransaction !== null && + !$networkInventoryAction->oldItem->equalsExact($networkInventoryAction->newItem) && + $networkInventoryAction->sourceType === NetworkInventoryAction::SOURCE_CONTAINER && + $networkInventoryAction->windowId === ContainerIds::UI && + $networkInventoryAction->inventorySlot === UIInventorySlotOffset::CREATED_ITEM_OUTPUT ) ){ $isCraftingPart = true; - if($networkInventoryAction->windowId === NetworkInventoryAction::SOURCE_TYPE_CRAFTING_RESULT){ - $isFinalCraftingPart = true; - } } try{ $action = $networkInventoryAction->createInventoryAction($this); @@ -2426,23 +2429,24 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{ } } - if($isFinalCraftingPart){ - //we get the actions for this in several packets, so we need to wait until we have all the pieces before - //trying to execute it - - $ret = true; - try{ - $this->craftingTransaction->execute(); - }catch(TransactionValidationException $e){ - $this->server->getLogger()->debug("Failed to execute crafting transaction for " . $this->getName() . ": " . $e->getMessage()); - $ret = false; - } - - $this->craftingTransaction = null; - return $ret; + try{ + $this->craftingTransaction->validate(); + }catch(TransactionValidationException $e){ + //transaction is incomplete - crafting transaction comes in lots of little bits, so we have to collect + //all of the parts before we can execute it + return true; } - return true; + $ret = true; + try{ + $this->craftingTransaction->execute(); + }catch(TransactionValidationException $e){ + $this->server->getLogger()->debug("Failed to execute crafting transaction for " . $this->getName() . ": " . $e->getMessage()); + $ret = false; + } + + $this->craftingTransaction = null; + return $ret; }elseif($this->craftingTransaction !== null){ $this->server->getLogger()->debug("Got unexpected normal inventory action with incomplete crafting transaction from " . $this->getName() . ", refusing to execute crafting"); $this->craftingTransaction = null; From f0a0c9a85f42976b8e755e128d6ef336ff73c7be Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 2 Aug 2020 23:49:07 +0100 Subject: [PATCH 4/8] Player: remove useless var --- src/pocketmine/Player.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php index d80abfdd5..dac971741 100644 --- a/src/pocketmine/Player.php +++ b/src/pocketmine/Player.php @@ -2437,16 +2437,15 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{ return true; } - $ret = true; try{ $this->craftingTransaction->execute(); + return true; }catch(TransactionValidationException $e){ $this->server->getLogger()->debug("Failed to execute crafting transaction for " . $this->getName() . ": " . $e->getMessage()); - $ret = false; + return false; + }finally{ + $this->craftingTransaction = null; } - - $this->craftingTransaction = null; - return $ret; }elseif($this->craftingTransaction !== null){ $this->server->getLogger()->debug("Got unexpected normal inventory action with incomplete crafting transaction from " . $this->getName() . ", refusing to execute crafting"); $this->craftingTransaction = null; From 41f7c07703bf3f7ef2d9504bbdbdf74257e75d12 Mon Sep 17 00:00:00 2001 From: VixikHD Date: Mon, 3 Aug 2020 01:20:28 +0200 Subject: [PATCH 5/8] Entity: report the class in getSaveId() unregistered entity exception (#3744) --- src/pocketmine/entity/Entity.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/entity/Entity.php b/src/pocketmine/entity/Entity.php index 594eaf7bc..51730ccd5 100644 --- a/src/pocketmine/entity/Entity.php +++ b/src/pocketmine/entity/Entity.php @@ -868,7 +868,7 @@ abstract class Entity extends Location implements Metadatable, EntityIds{ */ public function getSaveId() : string{ if(!isset(self::$saveNames[static::class])){ - throw new \InvalidStateException("Entity is not registered"); + throw new \InvalidStateException("Entity " . static::class . " is not registered"); } reset(self::$saveNames[static::class]); return current(self::$saveNames[static::class]); From 1f5e0bc96d7fabfd39a4b1c8c45634f884b26c78 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 3 Aug 2020 19:36:32 +0100 Subject: [PATCH 6/8] Updated BedrockData submodule, new recipes.json format --- src/pocketmine/inventory/CraftingManager.php | 59 +++++++++----------- src/pocketmine/resources/vanilla | 2 +- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/pocketmine/inventory/CraftingManager.php b/src/pocketmine/inventory/CraftingManager.php index 41e1f2a63..06795986a 100644 --- a/src/pocketmine/inventory/CraftingManager.php +++ b/src/pocketmine/inventory/CraftingManager.php @@ -54,39 +54,34 @@ class CraftingManager{ $recipes = json_decode(file_get_contents(\pocketmine\RESOURCE_PATH . "vanilla" . DIRECTORY_SEPARATOR . "recipes.json"), true); $itemDeserializerFunc = \Closure::fromCallable([Item::class, 'jsonDeserialize']); - foreach($recipes as $recipe){ - switch($recipe["type"]){ - case "shapeless": - if($recipe["block"] !== "crafting_table"){ //TODO: filter others out for now to avoid breaking economics - break; - } - $this->registerShapelessRecipe(new ShapelessRecipe( - array_map($itemDeserializerFunc, $recipe["input"]), - array_map($itemDeserializerFunc, $recipe["output"]) - )); - break; - case "shaped": - if($recipe["block"] !== "crafting_table"){ //TODO: filter others out for now to avoid breaking economics - break; - } - $this->registerShapedRecipe(new ShapedRecipe( - $recipe["shape"], - array_map($itemDeserializerFunc, $recipe["input"]), - array_map($itemDeserializerFunc, $recipe["output"]) - )); - break; - case "smelting": - if($recipe["block"] !== "furnace"){ //TODO: filter others out for now to avoid breaking economics - break; - } - $this->registerFurnaceRecipe(new FurnaceRecipe( - Item::jsonDeserialize($recipe["output"]), - Item::jsonDeserialize($recipe["input"])) - ); - break; - default: - break; + + foreach($recipes["shapeless"] as $recipe){ + if($recipe["block"] !== "crafting_table"){ //TODO: filter others out for now to avoid breaking economics + continue; } + $this->registerShapelessRecipe(new ShapelessRecipe( + array_map($itemDeserializerFunc, $recipe["input"]), + array_map($itemDeserializerFunc, $recipe["output"]) + )); + } + foreach($recipes["shaped"] as $recipe){ + if($recipe["block"] !== "crafting_table"){ //TODO: filter others out for now to avoid breaking economics + continue; + } + $this->registerShapedRecipe(new ShapedRecipe( + $recipe["shape"], + array_map($itemDeserializerFunc, $recipe["input"]), + array_map($itemDeserializerFunc, $recipe["output"]) + )); + } + foreach($recipes["smelting"] as $recipe){ + if($recipe["block"] !== "furnace"){ //TODO: filter others out for now to avoid breaking economics + continue; + } + $this->registerFurnaceRecipe(new FurnaceRecipe( + Item::jsonDeserialize($recipe["output"]), + Item::jsonDeserialize($recipe["input"])) + ); } $this->buildCraftingDataCache(); diff --git a/src/pocketmine/resources/vanilla b/src/pocketmine/resources/vanilla index 43edcfde6..767676e2b 160000 --- a/src/pocketmine/resources/vanilla +++ b/src/pocketmine/resources/vanilla @@ -1 +1 @@ -Subproject commit 43edcfde6b9611b7c7d643be52332707cc10cd1b +Subproject commit 767676e2b97843072220bad719c076b169c28fd3 From 3c001b310f4e38649cfa29ac976b444ead2dc96c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 3 Aug 2020 19:54:14 +0100 Subject: [PATCH 7/8] fix phpstan analyze failure --- src/pocketmine/inventory/CraftingManager.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pocketmine/inventory/CraftingManager.php b/src/pocketmine/inventory/CraftingManager.php index 06795986a..6e068cde1 100644 --- a/src/pocketmine/inventory/CraftingManager.php +++ b/src/pocketmine/inventory/CraftingManager.php @@ -28,8 +28,10 @@ use pocketmine\network\mcpe\protocol\BatchPacket; use pocketmine\network\mcpe\protocol\CraftingDataPacket; use pocketmine\Server; use pocketmine\timings\Timings; +use pocketmine\utils\AssumptionFailedError; use function array_map; use function file_get_contents; +use function is_array; use function json_decode; use function json_encode; use function usort; @@ -52,6 +54,9 @@ class CraftingManager{ public function init() : void{ $recipes = json_decode(file_get_contents(\pocketmine\RESOURCE_PATH . "vanilla" . DIRECTORY_SEPARATOR . "recipes.json"), true); + if(!is_array($recipes)){ + throw new AssumptionFailedError("recipes.json root should contain a map of recipe types"); + } $itemDeserializerFunc = \Closure::fromCallable([Item::class, 'jsonDeserialize']); From 46c224da86c51c21e8ec21dea7d322cef3b0898a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 3 Aug 2020 19:54:53 +0100 Subject: [PATCH 8/8] phpstan: remove an obsolete ignored error pattern from explicit-mixed baseline --- tests/phpstan/configs/check-explicit-mixed-baseline.neon | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/phpstan/configs/check-explicit-mixed-baseline.neon b/tests/phpstan/configs/check-explicit-mixed-baseline.neon index ac5ec53fd..541ae8aa9 100644 --- a/tests/phpstan/configs/check-explicit-mixed-baseline.neon +++ b/tests/phpstan/configs/check-explicit-mixed-baseline.neon @@ -205,11 +205,6 @@ parameters: count: 1 path: ../../../src/pocketmine/event/EventPriority.php - - - message: "#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\\.$#" - count: 1 - path: ../../../src/pocketmine/inventory/CraftingManager.php - - message: "#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\\.$#" count: 1