ItemFactory::get() now consistently throws SavedDataLoadingException on any error, including unknown items

src/crafting/CraftingManagerFromDataHelper.php

@@ -24,7 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\crafting;
 
 use pocketmine\data\bedrock\item\ItemTypeDeserializeException;
-use pocketmine\item\Durable;
+use pocketmine\data\SavedDataLoadingException;
 use pocketmine\item\Item;
 use pocketmine\item\ItemFactory;
 use pocketmine\utils\AssumptionFailedError;
@@ -37,26 +37,6 @@ use function is_int;
 use function json_decode;
 
 final class CraftingManagerFromDataHelper{
-
-	/**
-	 * @param Item[] $items
-	 */
-	private static function containsUnknownItems(array $items) : bool{
-		$factory = ItemFactory::getInstance();
-		foreach($items as $item){
-			if($item instanceof Durable){
-				//TODO: this check is imperfect and might cause problems if meta 0 isn't used for some reason
-				if(!$factory->isRegistered($item->getId())){
-					return true;
-				}
-			}elseif(!$factory->isRegistered($item->getId(), $item->getMeta())){
-				return true;
-			}
-		}
-
-		return false;
-	}
-
 	/**
 	 * @param mixed[] $data
 	 */
@@ -76,9 +56,14 @@ final class CraftingManagerFromDataHelper{
 		}
 
 		//TODO: we need to stop using jsonDeserialize for this
-		$item = Item::jsonDeserialize($data);
+		try{
+			$item = Item::jsonDeserialize($data);
+		}catch(SavedDataLoadingException){
+			//unknown item
+			return null;
+		}
 
-		return self::containsUnknownItems([$item]) ? null : new ExactRecipeIngredient($item);
+		return new ExactRecipeIngredient($item);
 	}
 
 	public static function make(string $filePath) : CraftingManager{
@@ -109,8 +94,10 @@ final class CraftingManagerFromDataHelper{
 				}
 				$inputs[] = $input;
 			}
-			$outputs = array_map($itemDeserializerFunc, $recipe["output"]);
+			try{
-			if(self::containsUnknownItems($outputs)){
+				$outputs = array_map($itemDeserializerFunc, $recipe["output"]);
+			}catch(SavedDataLoadingException){
+				//unknown output item
 				continue;
 			}
 			$result->registerShapelessRecipe(new ShapelessRecipe(
@@ -131,8 +118,10 @@ final class CraftingManagerFromDataHelper{
 				}
 				$inputs[$symbol] = $input;
 			}
-			$outputs = array_map($itemDeserializerFunc, $recipe["output"]);
+			try{
-			if(self::containsUnknownItems($outputs)){
+				$outputs = array_map($itemDeserializerFunc, $recipe["output"]);
+			}catch(SavedDataLoadingException){
+				//unknown output item
 				continue;
 			}
 			$result->registerShapedRecipe(new ShapedRecipe(
@@ -152,9 +141,13 @@ final class CraftingManagerFromDataHelper{
 			if($furnaceType === null){
 				continue;
 			}
-			$output = Item::jsonDeserialize($recipe["output"]);
+			try{
+				$output = Item::jsonDeserialize($recipe["output"]);
+			}catch(SavedDataLoadingException){
+				continue;
+			}
 			$input = self::deserializeIngredient($recipe["input"]);
-			if($input === null || self::containsUnknownItems([$output])){
+			if($input === null){
 				continue;
 			}
 			$result->getFurnaceRecipeManager($furnaceType)->register(new FurnaceRecipe(
@@ -163,11 +156,12 @@ final class CraftingManagerFromDataHelper{
 			));
 		}
 		foreach($recipes["potion_type"] as $recipe){
-			$input = Item::jsonDeserialize($recipe["input"]);
+			try{
-			$ingredient = Item::jsonDeserialize($recipe["ingredient"]);
+				$input = Item::jsonDeserialize($recipe["input"]);
-			$output = Item::jsonDeserialize($recipe["output"]);
+				$ingredient = Item::jsonDeserialize($recipe["ingredient"]);
-
+				$output = Item::jsonDeserialize($recipe["output"]);
-			if(self::containsUnknownItems([$input, $ingredient, $output])){
+			}catch(SavedDataLoadingException){
+				//unknown item
 				continue;
 			}
 			$result->registerPotionTypeRecipe(new PotionTypeRecipe(
@@ -177,11 +171,12 @@ final class CraftingManagerFromDataHelper{
 			));
 		}
 		foreach($recipes["potion_container_change"] as $recipe){
-			$input = ItemFactory::getInstance()->get($recipe["input_item_id"]);
+			try{
-			$ingredient = Item::jsonDeserialize($recipe["ingredient"]);
+				$input = ItemFactory::getInstance()->get($recipe["input_item_id"]);
-			$output = ItemFactory::getInstance()->get($recipe["output_item_id"]);
+				$ingredient = Item::jsonDeserialize($recipe["ingredient"]);
-
+				$output = ItemFactory::getInstance()->get($recipe["output_item_id"]);
-			if(self::containsUnknownItems([$input, $ingredient, $output])){
+			}catch(SavedDataLoadingException){
+				//unknown item
 				continue;
 			}
 			$result->registerPotionContainerChangeRecipe(new PotionContainerChangeRecipe(

src/data/bedrock/item/upgrade/ItemDataUpgrader.php

@@ -102,6 +102,8 @@ final class ItemDataUpgrader{
 
 	/**
 	 * This function replaces the legacy ItemFactory::get().
+	 *
+	 * @throws SavedDataLoadingException if the legacy numeric ID doesn't map to a string ID
 	 */
 	public function upgradeItemTypeDataInt(int $legacyNumericId, int $meta, int $count, ?CompoundTag $nbt) : SavedItemStackData{
 		$rawNameId = $this->legacyIntToStringIdMap->legacyToString($legacyNumericId);

src/inventory/CreativeInventory.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\inventory;
 
+use pocketmine\data\SavedDataLoadingException;
 use pocketmine\item\Durable;
 use pocketmine\item\Item;
 use pocketmine\utils\SingletonTrait;
@@ -40,8 +41,10 @@ final class CreativeInventory{
 		$creativeItems = json_decode(file_get_contents(Path::join(\pocketmine\BEDROCK_DATA_PATH, "creativeitems.json")), true);
 
 		foreach($creativeItems as $data){
-			$item = Item::jsonDeserialize($data);
+			try{
-			if($item->getName() === "Unknown"){
+				$item = Item::jsonDeserialize($data);
+			}catch(SavedDataLoadingException){
+				//unknown item
 				continue;
 			}
 			$this->add($item);

src/item/Item.php

@@ -38,7 +38,6 @@ use pocketmine\item\enchantment\EnchantmentInstance;
 use pocketmine\math\Vector3;
 use pocketmine\nbt\LittleEndianNbtSerializer;
 use pocketmine\nbt\NBT;
-use pocketmine\nbt\NbtDataException;
 use pocketmine\nbt\NbtException;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\ListTag;
@@ -606,8 +605,7 @@ class Item implements \JsonSerializable{
 	 * Returns an Item from properties created in an array by {@link Item#jsonSerialize}
 	 * @param mixed[] $data
 	 *
-	 * @throws NbtDataException
+	 * @throws SavedDataLoadingException
-	 * @throws \InvalidArgumentException
 	 */
 	final public static function jsonDeserialize(array $data) : Item{
 		$nbt = "";

src/item/ItemFactory.php

@@ -36,6 +36,7 @@ use pocketmine\data\bedrock\CompoundTypeIds;
 use pocketmine\data\bedrock\DyeColorIdMap;
 use pocketmine\data\bedrock\EntityLegacyIds;
 use pocketmine\data\bedrock\PotionTypeIdMap;
+use pocketmine\data\SavedDataLoadingException;
 use pocketmine\entity\Entity;
 use pocketmine\entity\Location;
 use pocketmine\entity\Squid;
@@ -50,6 +51,7 @@ use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\utils\SingletonTrait;
 use pocketmine\world\format\io\GlobalBlockStateHandlers;
 use pocketmine\world\World;
+use function min;
 
 /**
  * Manages deserializing item types from their legacy ID/metadata.
@@ -451,26 +453,24 @@ class ItemFactory{
 	 *
 	 * Deserializes an item from the provided legacy ID, legacy meta, count and NBT.
 	 *
-	 * @throws \InvalidArgumentException
+	 * @throws SavedDataLoadingException
-	 * @throws NbtException
 	 */
 	public function get(int $id, int $meta = 0, int $count = 1, ?CompoundTag $tags = null) : Item{
 		/** @var Item|null $item */
 		$item = null;
 
+		if($id < -0x8000 || $id > 0x7fff){
+			throw new SavedDataLoadingException("Legacy ID must be in the range " . -0x8000 . " ... " . 0x7fff);
+		}
 		if($meta < 0 || $meta > 0x7ffe){ //0x7fff would cause problems with recipe wildcards
-			throw new \InvalidArgumentException("Meta cannot be negative or larger than " . 0x7ffe);
+			throw new SavedDataLoadingException("Meta cannot be negative or larger than " . 0x7ffe);
 		}
 
 		if(isset($this->list[$offset = self::getListOffset($id, $meta)])){
 			$item = clone $this->list[$offset];
 		}elseif(isset($this->list[$zero = self::getListOffset($id, 0)]) && $this->list[$zero] instanceof Durable){
-			if($meta <= $this->list[$zero]->getMaxDurability()){
+			$item = clone $this->list[$zero];
-				$item = clone $this->list[$zero];
+			$item->setDamage(min($meta, $this->list[$zero]->getMaxDurability()));
-				$item->setDamage($meta);
-			}else{
-				$item = new Item(new IID($id, $meta));
-			}
 		}elseif($id < 256){ //intentionally includes negatives, for extended block IDs
 			//TODO: do not assume that item IDs and block IDs are the same or related
 			$blockStateData = GlobalBlockStateHandlers::getUpgrader()->upgradeIntIdMeta(self::itemToBlockId($id), $meta & 0xf);
@@ -479,20 +479,22 @@ class ItemFactory{
 					$blockStateId = GlobalBlockStateHandlers::getDeserializer()->deserialize($blockStateData);
 					$item = new ItemBlock(new IID($id, $meta), BlockFactory::getInstance()->fromFullBlock($blockStateId));
 				}catch(BlockStateDeserializeException $e){
-					\GlobalLogger::get()->logException($e);
+					throw new SavedDataLoadingException("Failed to deserialize itemblock: " . $e->getMessage(), 0, $e);
-					//fallthru
 				}
 			}
 		}
 
 		if($item === null){
-			//negative damage values will fallthru to here, to avoid crazy shit with crafting wildcard hacks
+			throw new SavedDataLoadingException("No registered item is associated with this ID and meta");
-			$item = new Item(new IID($id, $meta));
 		}
 
 		$item->setCount($count);
 		if($tags !== null){
-			$item->setNamedTag($tags);
+			try{
+				$item->setNamedTag($tags);
+			}catch(NbtException $e){
+				throw new SavedDataLoadingException("Invalid item NBT: " . $e->getMessage(), 0, $e);
+			}
 		}
 		return $item;
 	}

src/item/LegacyStringToItemParser.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\item;
 
+use pocketmine\data\SavedDataLoadingException;
 use pocketmine\utils\AssumptionFailedError;
 use pocketmine\utils\SingletonTrait;
 use pocketmine\utils\Utils;
@@ -104,15 +105,16 @@ final class LegacyStringToItemParser{
 			$meta = 0;
 		}elseif(is_numeric($b[1])){
 			$meta = (int) $b[1];
-			if($meta < 0 || $meta > 0x7ffe){
-				throw new LegacyStringToItemParserException("Meta value $meta is outside the range 0 - " . 0x7ffe);
-			}
 		}else{
 			throw new LegacyStringToItemParserException("Unable to parse \"" . $b[1] . "\" from \"" . $input . "\" as a valid meta value");
 		}
 
 		if(isset($this->map[strtolower($b[0])])){
-			$item = $this->itemFactory->get($this->map[strtolower($b[0])], $meta);
+			try{
+				$item = $this->itemFactory->get($this->map[strtolower($b[0])], $meta);
+			}catch(SavedDataLoadingException $e){
+				throw new LegacyStringToItemParserException($e->getMessage(), 0, $e);
+			}
 		}else{
 			throw new LegacyStringToItemParserException("Unable to resolve \"" . $input . "\" to a valid item");
 		}

src/network/mcpe/convert/TypeConverter.php

@@ -32,6 +32,7 @@ use pocketmine\block\VanillaBlocks;
 use pocketmine\crafting\ExactRecipeIngredient;
 use pocketmine\crafting\MetaWildcardRecipeIngredient;
 use pocketmine\crafting\RecipeIngredient;
+use pocketmine\data\SavedDataLoadingException;
 use pocketmine\inventory\transaction\action\CreateItemAction;
 use pocketmine\inventory\transaction\action\DestroyItemAction;
 use pocketmine\inventory\transaction\action\DropItemAction;
@@ -239,13 +240,11 @@ class TypeConverter{
 				$compound = null;
 			}
 			if($meta !== null){
-				if($id !== null && ($id < -0x8000 || $id >= 0x7fff)){
+				try{
-					throw new TypeConversionException("Item ID must be in range " . -0x8000 . " ... " . 0x7fff . " (received $id)");
+					$itemResult = ItemFactory::getInstance()->get($id ?? $itemResult->getId(), $meta);
+				}catch(SavedDataLoadingException $e){
+					throw new TypeConversionException("Failed loading network item: " . $e->getMessage(), 0, $e);
 				}
-				if($meta < 0 || $meta >= 0x7ffe){ //this meta value may have been restored from the NBT
-					throw new TypeConversionException("Item meta must be in range 0 ... " . 0x7ffe . " (received $meta)");
-				}
-				$itemResult = ItemFactory::getInstance()->get($id ?? $itemResult->getId(), $meta);
 			}
 		}