Crafting: nuke

This commit brings in a much-needed rewrite of crafting transaction handling.

The following classes have been removed:
- CraftingTransferMaterialAction
- CraftingTakeResultAction

The following classes have significant changes:
- CraftingTransaction
	- All API methods have been removed and are now handled in CraftItemEvent
- CraftItemEvent
	- added the following:
		- getInputs()
		- getOutputs()
		- getRepetitions() (tells how many times a recipe was crafted in this event)
- Recipe interface:
	- Removed getResult() (individual recipes may handle this differently)
- CraftingRecipe interface
	- removed the following:
		- matchItems()
		- getExtraResults()
		- getAllResults()
	- added the following
		- getResults()
		- getIngredientList() : Item[], which must return a 1D array of items that should be consumed (wildcards accepted).
		- matchesCraftingGrid(CraftingGrid)
- ShapedRecipe
	- constructor now accepts string[], Item[], Item[]
- ShapelessRecipe
	- constructor now accepts Item[], Item[]

src/pocketmine/Player.php

@@ -2302,11 +2302,17 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 				}
 			}
 
-			if($this->craftingTransaction->getPrimaryOutput() !== null){
-				//we get the actions for this in several packets, so we can't execute it until we get the result
+			if($packet->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
+
+				$result = $this->craftingTransaction->execute();
+				if(!$result){
+					$this->server->getLogger()->debug("Failed to execute crafting transaction from " . $this->getName());
+				}
 
-				$this->craftingTransaction->execute();
 				$this->craftingTransaction = null;
+				return $result;
 			}
 
 			return true;

src/pocketmine/event/inventory/CraftItemEvent.php

@@ -25,35 +25,83 @@ namespace pocketmine\event\inventory;
 
 use pocketmine\event\Cancellable;
 use pocketmine\event\Event;
+use pocketmine\inventory\CraftingRecipe;
 use pocketmine\inventory\Recipe;
 use pocketmine\inventory\transaction\CraftingTransaction;
+use pocketmine\item\Item;
 use pocketmine\Player;
 
 class CraftItemEvent extends Event implements Cancellable{
 	/** @var CraftingTransaction */
 	private $transaction;
+	/** @var CraftingRecipe */
+	private $recipe;
+	/** @var int */
+	private $repetitions;
+	/** @var Item[] */
+	private $inputs;
+	/** @var Item[] */
+	private $outputs;
 
 	/**
 	 * @param CraftingTransaction $transaction
+	 * @param CraftingRecipe      $recipe
+	 * @param int                 $repetitions
+	 * @param Item[]              $inputs
+	 * @param Item[]              $outputs
 	 */
-	public function __construct(CraftingTransaction $transaction){
+	public function __construct(CraftingTransaction $transaction, CraftingRecipe $recipe, int $repetitions, array $inputs, array $outputs){
 		$this->transaction = $transaction;
+		$this->recipe = $recipe;
+		$this->repetitions = $repetitions;
+		$this->inputs = $inputs;
+		$this->outputs = $outputs;
 	}
 
+	/**
+	 * Returns the inventory transaction involved in this crafting event.
+	 *
+	 * @return CraftingTransaction
+	 */
 	public function getTransaction() : CraftingTransaction{
 		return $this->transaction;
 	}
 
 	/**
-	 * @return Recipe
+	 * Returns the recipe crafted.
+	 *
+	 * @return CraftingRecipe
 	 */
-	public function getRecipe() : Recipe{
-		$recipe = $this->transaction->getRecipe();
-		if($recipe === null){
-			throw new \RuntimeException("This shouldn't be called if the transaction can't be executed");
-		}
+	public function getRecipe() : CraftingRecipe{
+		return $this->recipe;
+	}
 
-		return $recipe;
+	/**
+	 * Returns the number of times the recipe was crafted. This is usually 1, but might be more in the case of recipe
+	 * book shift-clicks (which craft lots of items in a batch).
+	 *
+	 * @return int
+	 */
+	public function getRepetitions() : int{
+		return $this->repetitions;
+	}
+
+	/**
+	 * Returns a list of items destroyed as ingredients of the recipe.
+	 *
+	 * @return Item[]
+	 */
+	public function getInputs() : array{
+		return $this->inputs;
+	}
+
+	/**
+	 * Returns a list of items created by crafting the recipe.
+	 *
+	 * @return Item[]
+	 */
+	public function getOutputs() : array{
+		return $this->outputs;
 	}
 
 	/**

src/pocketmine/inventory/CraftingGrid.php

@@ -23,6 +23,8 @@ declare(strict_types=1);
 
 namespace pocketmine\inventory;
 
+use pocketmine\item\Item;
+use pocketmine\item\ItemFactory;
 use pocketmine\Player;
 
 class CraftingGrid extends BaseInventory{
@@ -34,6 +36,15 @@ class CraftingGrid extends BaseInventory{
 	/** @var int */
 	private $gridWidth;
 
+	/** @var int|null */
+	private $startX;
+	/** @var int|null */
+	private $xLen;
+	/** @var int|null */
+	private $startY;
+	/** @var int|null */
+	private $yLen;
+
 	public function __construct(Player $holder, int $gridWidth){
 		$this->holder = $holder;
 		$this->gridWidth = $gridWidth;
@@ -56,6 +67,16 @@ class CraftingGrid extends BaseInventory{
 		return "Crafting";
 	}
 
+	public function setItem(int $index, Item $item, bool $send = true) : bool{
+		if(parent::setItem($index, $item, $send)){
+			$this->seekRecipeBounds();
+
+			return true;
+		}
+
+		return false;
+	}
+
 	public function sendSlot(int $index, $target) : void{
 		//we can't send a slot of a client-sided inventory window
 	}
@@ -70,4 +91,70 @@ class CraftingGrid extends BaseInventory{
 	public function getHolder(){
 		return $this->holder;
 	}
+
+	private function seekRecipeBounds() : void{
+		$minX = PHP_INT_MAX;
+		$maxX = 0;
+
+		$minY = PHP_INT_MAX;
+		$maxY = 0;
+
+		$empty = true;
+
+		for($y = 0; $y < $this->gridWidth; ++$y){
+			for($x = 0; $x < $this->gridWidth; ++$x){
+				if(!$this->isSlotEmpty($y * $this->gridWidth + $x)){
+					$minX = min($minX, $x);
+					$maxX = max($maxX, $x);
+
+					$minY = min($minY, $y);
+					$maxY = max($maxY, $y);
+
+					$empty = false;
+				}
+			}
+		}
+
+		if(!$empty){
+			$this->startX = $minX;
+			$this->xLen = $maxX - $minX + 1;
+			$this->startY = $minY;
+			$this->yLen = $maxY - $minY + 1;
+		}else{
+			$this->startX = $this->xLen = $this->startY = $this->yLen = null;
+		}
+	}
+
+	/**
+	 * Returns the item at offset x,y, offset by where the starts of the recipe rectangle are.
+	 *
+	 * @param int $x
+	 * @param int $y
+	 *
+	 * @return Item
+	 */
+	public function getIngredient(int $x, int $y) : Item{
+		if($this->startX !== null and $this->startY !== null){
+			return $this->getItem(($y + $this->startY) * $this->gridWidth + ($x + $this->startX));
+		}
+
+		throw new \InvalidStateException("No ingredients found in grid");
+	}
+
+	/**
+	 * Returns the width of the recipe we're trying to craft, based on items currently in the grid.
+	 *
+	 * @return int
+	 */
+	public function getRecipeWidth() : int{
+		return $this->xLen ?? 0;
+	}
+
+	/**
+	 * Returns the height of the recipe we're trying to craft, based on items currently in the grid.
+	 * @return int
+	 */
+	public function getRecipeHeight() : int{
+		return $this->yLen ?? 0;
+	}
 }

src/pocketmine/inventory/CraftingManager.php

@@ -59,20 +59,13 @@ class CraftingManager{
 		foreach($recipes->getAll() as $recipe){
 			switch($recipe["type"]){
 				case 0:
-					// TODO: handle multiple result items
-					$first = $recipe["output"][0];
-					$result = new ShapelessRecipe(Item::jsonDeserialize($first));
-
-					foreach($recipe["input"] as $ingredient){
-						$result->addIngredient(Item::jsonDeserialize($ingredient));
-					}
-					$this->registerRecipe($result);
+					$this->registerRecipe(new ShapelessRecipe(
+						array_map(function(array $data) : Item{ return Item::jsonDeserialize($data); }, $recipe["input"]),
+						array_map(function(array $data) : Item{ return Item::jsonDeserialize($data); }, $recipe["output"])
+					));
 					break;
 				case 1:
-					$first = array_shift($recipe["output"]);
-
 					$this->registerRecipe(new ShapedRecipe(
-						Item::jsonDeserialize($first),
 						$recipe["shape"],
 						array_map(function(array $data) : Item{ return Item::jsonDeserialize($data); }, $recipe["input"]),
 						array_map(function(array $data) : Item{ return Item::jsonDeserialize($data); }, $recipe["output"])
@@ -151,6 +144,41 @@ class CraftingManager{
 		return $retval;
 	}
 
+	/**
+	 * @param Item[] $items
+	 *
+	 * @return Item[]
+	 */
+	private static function pack(array $items) : array{
+		/** @var Item[] $result */
+		$result = [];
+
+		foreach($items as $i => $item){
+			foreach($result as $otherItem){
+				if($item->equals($otherItem)){
+					$otherItem->setCount($otherItem->getCount() + $item->getCount());
+					continue 2;
+				}
+			}
+
+			//No matching item found
+			$result[] = clone $item;
+		}
+
+		return $result;
+	}
+
+	private static function hashOutputs(array $outputs) : string{
+		$outputs = self::pack($outputs);
+		usort($outputs, [self::class, "sort"]);
+		foreach($outputs as $o){
+			//this reduces accuracy of hash, but it's necessary to deal with recipe book shift-clicking stupidity
+			$o->setCount(1);
+		}
+
+		return json_encode($outputs);
+	}
+
 	/**
 	 * @param UUID $id
 	 * @return CraftingRecipe|null
@@ -192,7 +220,7 @@ class CraftingManager{
 	 * @param ShapedRecipe $recipe
 	 */
 	public function registerShapedRecipe(ShapedRecipe $recipe) : void{
-		$this->shapedRecipes[json_encode($recipe->getResult())][json_encode($recipe->getIngredientMap())] = $recipe;
+		$this->shapedRecipes[self::hashOutputs($recipe->getResults())][] = $recipe;
 		$this->craftingDataCache = null;
 	}
 
@@ -200,9 +228,7 @@ class CraftingManager{
 	 * @param ShapelessRecipe $recipe
 	 */
 	public function registerShapelessRecipe(ShapelessRecipe $recipe) : void{
-		$ingredients = $recipe->getIngredientList();
-		usort($ingredients, [self::class, "sort"]);
-		$this->shapelessRecipes[json_encode($recipe->getResult())][json_encode($ingredients)] = $recipe;
+		$this->shapelessRecipes[self::hashOutputs($recipe->getResults())][] = $recipe;
 		$this->craftingDataCache = null;
 	}
 
@@ -216,61 +242,27 @@ class CraftingManager{
 	}
 
 	/**
-	 * Clones a map of Item objects to avoid accidental modification.
-	 *
-	 * @param Item[][] $map
-	 * @return Item[][]
-	 */
-	private function cloneItemMap(array $map) : array{
-		/** @var Item[] $row */
-		foreach($map as $y => $row){
-			foreach($row as $x => $item){
-				$map[$y][$x] = clone $item;
-			}
-		}
-
-		return $map;
-	}
-
-	/**
-	 * @param Item[][] $inputMap
-	 * @param Item     $primaryOutput
-	 * @param Item[][] $extraOutputMap
+	 * @param CraftingGrid $grid
+	 * @param Item[]       $outputs
 	 *
 	 * @return CraftingRecipe|null
 	 */
-	public function matchRecipe(array $inputMap, Item $primaryOutput, array $extraOutputMap) : ?CraftingRecipe{
+	public function matchRecipe(CraftingGrid $grid, array $outputs) : ?CraftingRecipe{
 		//TODO: try to match special recipes before anything else (first they need to be implemented!)
 
-		$outputHash = json_encode($primaryOutput);
+		$outputHash = self::hashOutputs($outputs);
+
 		if(isset($this->shapedRecipes[$outputHash])){
-			$inputHash = json_encode($inputMap);
-			$recipe = $this->shapedRecipes[$outputHash][$inputHash] ?? null;
-
-			if($recipe !== null and $recipe->matchItems($this->cloneItemMap($inputMap), $this->cloneItemMap($extraOutputMap))){ //matched a recipe by hash
-				return $recipe;
-			}
-
 			foreach($this->shapedRecipes[$outputHash] as $recipe){
-				if($recipe->matchItems($this->cloneItemMap($inputMap), $this->cloneItemMap($extraOutputMap))){
+				if($recipe->matchesCraftingGrid($grid)){
 					return $recipe;
 				}
 			}
 		}
 
 		if(isset($this->shapelessRecipes[$outputHash])){
-			$list = array_merge(...$inputMap);
-			usort($list, [self::class, "sort"]);
-
-			$inputHash = json_encode($list);
-			$recipe = $this->shapelessRecipes[$outputHash][$inputHash] ?? null;
-
-			if($recipe !== null and $recipe->matchItems($this->cloneItemMap($inputMap), $this->cloneItemMap($extraOutputMap))){
-				return $recipe;
-			}
-
 			foreach($this->shapelessRecipes[$outputHash] as $recipe){
-				if($recipe->matchItems($this->cloneItemMap($inputMap), $this->cloneItemMap($extraOutputMap))){
+				if($recipe->matchesCraftingGrid($grid)){
 					return $recipe;
 				}
 			}
@@ -293,8 +285,7 @@ class CraftingManager{
 	 */
 	public function registerRecipe(Recipe $recipe) : void{
 		if($recipe instanceof CraftingRecipe){
-			$result = $recipe->getResult();
-			$recipe->setId($uuid = UUID::fromData((string) ++self::$RECIPE_COUNT, (string) $result->getId(), (string) $result->getDamage(), (string) $result->getCount(), $result->getCompoundTag()));
+			$recipe->setId($uuid = UUID::fromData((string) ++self::$RECIPE_COUNT, json_encode(self::pack($recipe->getResults()))));
 			$this->recipes[$uuid->toBinary()] = $recipe;
 		}
 

src/pocketmine/inventory/CraftingRecipe.php

@@ -39,23 +39,25 @@ interface CraftingRecipe extends Recipe{
 	public function setId(UUID $id);
 
 	/**
-	 * @return Item[]
-	 */
-	public function getExtraResults() : array;
-
-	/**
-	 * @return Item[]
-	 */
-	public function getAllResults() : array;
-
-	/**
-	 * Returns whether the specified list of crafting grid inputs and outputs matches this recipe. Outputs DO NOT
-	 * include the primary result item.
+	 * Returns a list of items needed to craft this recipe. This MUST NOT include Air items or items with a zero count.
 	 *
-	 * @param Item[][] $input 2D array of items taken from the crafting grid
-	 * @param Item[][] $output 2D array of items put back into the crafting grid (secondary results)
+	 * @return Item[]
+	 */
+	public function getIngredientList() : array;
+
+	/**
+	 * Returns a list of items created by crafting this recipe.
+	 *
+	 * @return Item[]
+	 */
+	public function getResults() : array;
+
+	/**
+	 * Returns whether the given crafting grid meets the requirements to craft this recipe.
+	 *
+	 * @param CraftingGrid $grid
 	 *
 	 * @return bool
 	 */
-	public function matchItems(array $input, array $output) : bool;
+	public function matchesCraftingGrid(CraftingGrid $grid) : bool;
 }

src/pocketmine/inventory/Recipe.php

@@ -23,14 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\inventory;
 
-use pocketmine\item\Item;
-
 interface Recipe{
 
-	/**
-	 * @return Item
-	 */
-	public function getResult() : Item;
-
 	public function registerToCraftingManager(CraftingManager $manager) : void;
 }

src/pocketmine/inventory/ShapedRecipe.php

@@ -28,11 +28,6 @@ use pocketmine\item\ItemFactory;
 use pocketmine\utils\UUID;
 
 class ShapedRecipe implements CraftingRecipe{
-	/** @var Item */
-	private $primaryResult;
-	/** @var Item[] */
-	private $extraResults = [];
-
 	/** @var UUID|null */
 	private $id = null;
 
@@ -40,90 +35,76 @@ class ShapedRecipe implements CraftingRecipe{
 	private $shape = [];
 	/** @var Item[] char => Item map */
 	private $ingredientList = [];
+	/** @var Item[] */
+	private $results = [];
+
+	/** @var int */
+	private $height;
+	/** @var int */
+	private $width;
 
 	/**
 	 * Constructs a ShapedRecipe instance.
 	 *
-	 * @param Item     $primaryResult
-	 * @param string[] $shape<br>
+	 * @param string[] $shape <br>
 	 *     Array of 1, 2, or 3 strings representing the rows of the recipe.
 	 *     This accepts an array of 1, 2 or 3 strings. Each string should be of the same length and must be at most 3
 	 *     characters long. Each character represents a unique type of ingredient. Spaces are interpreted as air.
-	 * @param Item[]   $ingredients<br>
+	 * @param Item[] $ingredients <br>
 	 *     Char => Item map of items to be set into the shape.
 	 *     This accepts an array of Items, indexed by character. Every unique character (except space) in the shape
 	 *     array MUST have a corresponding item in this list. Space character is automatically treated as air.
-	 * @param Item[]   $extraResults<br>
-	 *     List of additional result items to leave in the crafting grid afterwards. Used for things like cake recipe
-	 *     empty buckets.
+	 * @param Item[] $results List of items that this recipe produces when crafted.
 	 *
 	 * Note: Recipes **do not** need to be square. Do NOT add padding for empty rows/columns.
 	 */
-	public function __construct(Item $primaryResult, array $shape, array $ingredients, array $extraResults = []){
-		$rowCount = count($shape);
-		if($rowCount > 3 or $rowCount <= 0){
-			throw new \InvalidArgumentException("Shaped recipes may only have 1, 2 or 3 rows, not $rowCount");
+	public function __construct(array $shape, array $ingredients, array $results){
+		$this->height = count($shape);
+		if($this->height > 3 or $this->height <= 0){
+			throw new \InvalidArgumentException("Shaped recipes may only have 1, 2 or 3 rows, not $this->height");
 		}
 
 		$shape = array_values($shape);
 
-		$columnCount = strlen($shape[0]);
-		if($columnCount > 3 or $columnCount <= 0){
-			throw new \InvalidArgumentException("Shaped recipes may only have 1, 2 or 3 columns, not $columnCount");
+		$this->width = strlen($shape[0]);
+		if($this->width > 3 or $this->width <= 0){
+			throw new \InvalidArgumentException("Shaped recipes may only have 1, 2 or 3 columns, not $this->width");
 		}
 
 		foreach($shape as $y => $row){
-			if(strlen($row) !== $columnCount){
-				throw new \InvalidArgumentException("Shaped recipe rows must all have the same length (expected $columnCount, got " . strlen($row) . ")");
+			if(strlen($row) !== $this->width){
+				throw new \InvalidArgumentException("Shaped recipe rows must all have the same length (expected $this->width, got " . strlen($row) . ")");
 			}
 
-			for($x = 0; $x < $columnCount; ++$x){
+			for($x = 0; $x < $this->width; ++$x){
 				if($row{$x} !== ' ' and !isset($ingredients[$row{$x}])){
 					throw new \InvalidArgumentException("No item specified for symbol '" . $row{$x} . "'");
 				}
 			}
 		}
-		$this->primaryResult = clone $primaryResult;
-		foreach($extraResults as $item){
-			$this->extraResults[] = clone $item;
-		}
 
 		$this->shape = $shape;
 
 		foreach($ingredients as $char => $i){
 			$this->setIngredient($char, $i);
 		}
+
+		$this->results = array_map(function(Item $item) : Item{ return clone $item; }, $results);
 	}
 
 	public function getWidth() : int{
-		return strlen($this->shape[0]);
+		return $this->width;
 	}
 
 	public function getHeight() : int{
-		return count($this->shape);
-	}
-
-	/**
-	 * @return Item
-	 */
-	public function getResult() : Item{
-		return $this->primaryResult;
+		return $this->height;
 	}
 
 	/**
 	 * @return Item[]
 	 */
-	public function getExtraResults() : array{
-		return $this->extraResults;
-	}
-
-	/**
-	 * @return Item[]
-	 */
-	public function getAllResults() : array{
-		$results = $this->extraResults;
-		array_unshift($results, $this->primaryResult);
-		return $results;
+	public function getResults() : array{
+		return array_map(function(Item $item) : Item{ return clone $item; }, $this->results);
 	}
 
 	/**
@@ -164,8 +145,8 @@ class ShapedRecipe implements CraftingRecipe{
 	public function getIngredientMap() : array{
 		$ingredients = [];
 
-		for($y = 0, $y2 = $this->getHeight(); $y < $y2; ++$y){
-			for($x = 0, $x2 = $this->getWidth(); $x < $x2; ++$x){
+		for($y = 0; $y < $this->height; ++$y){
+			for($x = 0; $x < $this->width; ++$x){
 				$ingredients[$y][$x] = $this->getIngredient($x, $y);
 			}
 		}
@@ -173,6 +154,24 @@ class ShapedRecipe implements CraftingRecipe{
 		return $ingredients;
 	}
 
+	/**
+	 * @return Item[]
+	 */
+	public function getIngredientList() : array{
+		$ingredients = [];
+
+		for($y = 0; $y < $this->height; ++$y){
+			for($x = 0; $x < $this->width; ++$x){
+				$ingredient = $this->getIngredient($x, $y);
+				if(!$ingredient->isNull()){
+					$ingredients[] = $ingredient;
+				}
+			}
+		}
+
+		return $ingredients;
+	}
+
 	/**
 	 * @param int $x
 	 * @param int $y
@@ -197,34 +196,20 @@ class ShapedRecipe implements CraftingRecipe{
 	}
 
 	/**
-	 * @param Item[][] $input
+	 * @param CraftingGrid $grid
+	 * @param bool         $reverse
 	 *
 	 * @return bool
 	 */
-	private function matchInputMap(array $input) : bool{
-		$map = $this->getIngredientMap();
+	private function matchInputMap(CraftingGrid $grid, bool $reverse) : bool{
+		for($y = 0; $y < $this->height; ++$y){
+			for($x = 0; $x < $this->width; ++$x){
 
-		//match the given items to the requested items
-		for($y = 0, $y2 = $this->getHeight(); $y < $y2; ++$y){
-			for($x = 0, $x2 = $this->getWidth(); $x < $x2; ++$x){
-				$given = $input[$y][$x] ?? null;
-				$required = $map[$y][$x];
-
-				if($given === null or !$required->equals($given, !$required->hasAnyDamageValue(), $required->hasCompoundTag()) or $required->getCount() !== $given->getCount()){
+				$given = $grid->getIngredient($reverse ? $this->width - $x - 1 : $x, $y);
+				$required = $this->getIngredient($x, $y);
+				if(!$required->equals($given, !$required->hasAnyDamageValue(), $required->hasCompoundTag()) or $required->getCount() > $given->getCount()){
 					return false;
 				}
-
-				unset($input[$y][$x]);
-			}
-		}
-
-		//check if there are any items left in the grid outside of the recipe
-		/** @var Item[] $row */
-		foreach($input as $y => $row){
-			foreach($row as $x => $needItem){
-				if(!$needItem->isNull()){
-					return false; //too many input ingredients
-				}
 			}
 		}
 
@@ -232,38 +217,15 @@ class ShapedRecipe implements CraftingRecipe{
 	}
 
 	/**
-	 * @param Item[][] $input
-	 * @param Item[][] $output
+	 * @param CraftingGrid $grid
 	 *
 	 * @return bool
 	 */
-	public function matchItems(array $input, array $output) : bool{
-		if(
-			!$this->matchInputMap($input) and //as-is
-			!$this->matchInputMap(array_map(function(array $row) : array{ return array_reverse($row, false); }, $input)) //mirrored
-		){
+	public function matchesCraftingGrid(CraftingGrid $grid) : bool{
+		if($this->width !== $grid->getRecipeWidth() or $this->height !== $grid->getRecipeHeight()){
 			return false;
 		}
 
-		//and then, finally, check that the output items are good:
-
-		/** @var Item[] $haveItems */
-		$haveItems = array_merge(...$output);
-		$needItems = $this->getExtraResults();
-		foreach($haveItems as $j => $haveItem){
-			if($haveItem->isNull()){
-				unset($haveItems[$j]);
-				continue;
-			}
-
-			foreach($needItems as $i => $needItem){
-				if($needItem->equals($haveItem, !$needItem->hasAnyDamageValue(), $needItem->hasCompoundTag()) and $needItem->getCount() === $haveItem->getCount()){
-					unset($haveItems[$j], $needItems[$i]);
-					break;
-				}
-			}
-		}
-
-		return count($haveItems) === 0 and count($needItems) === 0;
+		return $this->matchInputMap($grid, false) or $this->matchInputMap($grid, true);
 	}
 }

src/pocketmine/inventory/ShapelessRecipe.php

@@ -27,17 +27,25 @@ use pocketmine\item\Item;
 use pocketmine\utils\UUID;
 
 class ShapelessRecipe implements CraftingRecipe{
-	/** @var Item */
-	private $output;
-
 	/** @var UUID|null */
 	private $id = null;
 
 	/** @var Item[] */
 	private $ingredients = [];
+	/** @var Item[] */
+	private $results;
 
-	public function __construct(Item $result){
-		$this->output = clone $result;
+	/**
+	 * @param Item[] $ingredients No more than 9 total. This applies to sum of item stack counts, not count of array.
+	 * @param Item[] $results List of result items created by this recipe.
+	 */
+	public function __construct(array $ingredients, array $results){
+		foreach($ingredients as $item){
+			//Ensure they get split up properly
+			$this->addIngredient($item);
+		}
+
+		$this->results = array_map(function(Item $item) : Item{ return clone $item; }, $results);
 	}
 
 	/**
@@ -58,16 +66,8 @@ class ShapelessRecipe implements CraftingRecipe{
 		$this->id = $id;
 	}
 
-	public function getResult() : Item{
-		return clone $this->output;
-	}
-
-	public function getExtraResults() : array{
-		return []; //TODO
-	}
-
-	public function getAllResults() : array{
-		return [$this->getResult()]; //TODO
+	public function getResults() : array{
+		return array_map(function(Item $item) : Item{ return clone $item; }, $this->results);
 	}
 
 	/**
@@ -78,7 +78,7 @@ class ShapelessRecipe implements CraftingRecipe{
 	 * @throws \InvalidArgumentException
 	 */
 	public function addIngredient(Item $item) : ShapelessRecipe{
-		if(count($this->ingredients) >= 9){
+		if(count($this->ingredients) + $item->getCount() > 9){
 			throw new \InvalidArgumentException("Shapeless recipes cannot have more than 9 ingredients");
 		}
 
@@ -112,12 +112,7 @@ class ShapelessRecipe implements CraftingRecipe{
 	 * @return Item[]
 	 */
 	public function getIngredientList() : array{
-		$ingredients = [];
-		foreach($this->ingredients as $ingredient){
-			$ingredients[] = clone $ingredient;
-		}
-
-		return $ingredients;
+		return array_map(function(Item $item) : Item{ return clone $item; }, $this->ingredients);
 	}
 
 	/**
@@ -137,53 +132,25 @@ class ShapelessRecipe implements CraftingRecipe{
 	}
 
 	/**
-	 * @param Item[][] $input
-	 * @param Item[][] $output
+	 * @param CraftingGrid $grid
 	 *
 	 * @return bool
 	 */
-	public function matchItems(array $input, array $output) : bool{
-		/** @var Item[] $haveInputs */
-		$haveInputs = array_merge(...$input); //we don't care how the items were arranged
-		$needInputs = $this->getIngredientList();
+	public function matchesCraftingGrid(CraftingGrid $grid) : bool{
+		//don't pack the ingredients - shapeless recipes require that each ingredient be in a separate slot
+		$input = $grid->getContents();
 
-		if(!$this->matchItemList($haveInputs, $needInputs)){
-			return false;
-		}
-
-		/** @var Item[] $haveOutputs */
-		$haveOutputs = array_merge(...$output);
-		$needOutputs = $this->getExtraResults();
-
-		if(!$this->matchItemList($haveOutputs, $needOutputs)){
-			return false;
-		}
-
-		return true;
-	}
-
-	/**
-	 * @param Item[] $haveItems
-	 * @param Item[] $needItems
-	 *
-	 * @return bool
-	 */
-	private function matchItemList(array $haveItems, array $needItems) : bool{
-		foreach($haveItems as $j => $haveItem){
-			if($haveItem->isNull()){
-				unset($haveItems[$j]);
-				continue;
-			}
-
-
-			foreach($needItems as $i => $needItem){
-				if($needItem->equals($haveItem, !$needItem->hasAnyDamageValue(), $needItem->hasCompoundTag()) and $needItem->getCount() === $haveItem->getCount()){
-					unset($haveItems[$j], $needItems[$i]);
-					break;
+		foreach($this->ingredients as $needItem){
+			foreach($input as $j => $haveItem){
+				if($haveItem->equals($needItem, !$needItem->hasAnyDamageValue(), $needItem->hasCompoundTag()) and $haveItem->getCount() >= $needItem->getCount()){
+					unset($input[$j]);
+					continue 2;
 				}
 			}
+
+			return false; //failed to match the needed item to a given item
 		}
 
-		return count($haveItems) === 0 and count($needItems) === 0;
+		return empty($input); //crafting grid should be empty apart from the given ingredient stacks
 	}
 }

src/pocketmine/inventory/transaction/CraftingTransaction.php

@@ -26,132 +26,117 @@ namespace pocketmine\inventory\transaction;
 use pocketmine\event\inventory\CraftItemEvent;
 use pocketmine\inventory\CraftingRecipe;
 use pocketmine\item\Item;
-use pocketmine\item\ItemFactory;
 use pocketmine\network\mcpe\protocol\ContainerClosePacket;
 use pocketmine\network\mcpe\protocol\types\ContainerIds;
-use pocketmine\Player;
+use pocketmine\utils\MainLogger;
 
 class CraftingTransaction extends InventoryTransaction{
-
-	protected $gridSize;
-	/** @var Item[][] */
-	protected $inputs;
-	/** @var Item[][] */
-	protected $secondaryOutputs;
-	/** @var Item|null */
-	protected $primaryOutput;
-
 	/** @var CraftingRecipe|null */
-	protected $recipe = null;
+	protected $recipe;
+	/** @var int|null */
+	protected $repetitions;
 
-	public function __construct(Player $source, $actions = []){
-		$this->gridSize = $source->getCraftingGrid()->getGridWidth();
+	/** @var Item[] */
+	protected $inputs = [];
+	/** @var Item[] */
+	protected $outputs = [];
 
-		$air = ItemFactory::get(Item::AIR, 0, 0);
-		$this->inputs = array_fill(0, $this->gridSize, array_fill(0, $this->gridSize, $air));
-		$this->secondaryOutputs = array_fill(0, $this->gridSize, array_fill(0, $this->gridSize, $air));
-
-		parent::__construct($source, $actions);
-	}
-
-	public function setInput(int $index, Item $item) : void{
-		$y = (int) ($index / $this->gridSize);
-		$x = $index % $this->gridSize;
-
-		if(!isset($this->inputs[$y][$x])){
-			return;
+	/**
+	 * @param Item[] $txItems
+	 * @param Item[] $recipeItems
+	 * @param bool   $wildcards
+	 *
+	 * @return int
+	 * @throws \InvalidStateException
+	 * @throws \InvalidArgumentException
+	 */
+	protected function matchRecipeItems(array $txItems, array $recipeItems, bool $wildcards) : int{
+		if(empty($recipeItems)){
+			throw new \InvalidArgumentException("No recipe items given");
+		}
+		if(empty($txItems)){
+			throw new \InvalidArgumentException("No transaction items given");
 		}
 
-		if($this->inputs[$y][$x]->isNull()){
-			$this->inputs[$y][$x] = clone $item;
-		}elseif(!$this->inputs[$y][$x]->equals($item)){
-			throw new \RuntimeException("Input $index has already been set and does not match the current item (expected " . $this->inputs[$y][$x] . ", got " . $item . ")");
-		}
-	}
-
-	public function getInputMap() : array{
-		return $this->inputs;
-	}
-
-	public function setExtraOutput(int $index, Item $item) : void{
-		$y = (int) ($index / $this->gridSize);
-		$x = $index % $this->gridSize;
-
-		if(!isset($this->secondaryOutputs[$y][$x])){
-			return;
-		}
-
-		if($this->secondaryOutputs[$y][$x]->isNull()){
-			$this->secondaryOutputs[$y][$x] = clone $item;
-		}elseif(!$this->secondaryOutputs[$y][$x]->equals($item)){
-			throw new \RuntimeException("Output $index has already been set and does not match the current item (expected " . $this->secondaryOutputs[$y][$x] . ", got " . $item . ")");
-		}
-	}
-
-	public function getPrimaryOutput() : ?Item{
-		return $this->primaryOutput;
-	}
-
-	public function setPrimaryOutput(Item $item) : void{
-		if($this->primaryOutput === null){
-			$this->primaryOutput = clone $item;
-		}elseif(!$this->primaryOutput->equals($item)){
-			throw new \RuntimeException("Primary result item has already been set and does not match the current item (expected " . $this->primaryOutput . ", got " . $item . ")");
-		}
-	}
-
-	public function getRecipe() : ?CraftingRecipe{
-		return $this->recipe;
-	}
-
-	private function reindexInputs() : array{
-		$minX = PHP_INT_MAX;
-		$maxX = 0;
-
-		$minY = PHP_INT_MAX;
-		$maxY = 0;
-
-		$empty = true;
-
-		foreach($this->inputs as $y => $row){
-			foreach($row as $x => $item){
-				if(!$item->isNull()){
-					$minX = min($minX, $x);
-					$maxX = max($maxX, $x);
-
-					$minY = min($minY, $y);
-					$maxY = max($maxY, $y);
-
-					$empty = false;
+		$iterations = 0;
+		while(!empty($recipeItems)){
+			/** @var Item $recipeItem */
+			$recipeItem = array_pop($recipeItems);
+			$needCount = $recipeItem->getCount();
+			foreach($recipeItems as $i => $otherRecipeItem){
+				if($otherRecipeItem->equals($recipeItem)){ //make sure they have the same wildcards set
+					$needCount += $otherRecipeItem->getCount();
+					unset($recipeItems[$i]);
 				}
 			}
-		}
 
-		if($empty){
-			return [];
-		}
+			$haveCount = 0;
+			foreach($txItems as $j => $txItem){
+				if($txItem->equals($recipeItem, !$wildcards or !$recipeItem->hasAnyDamageValue(), !$wildcards or $recipeItem->hasCompoundTag())){
+					$haveCount += $txItem->getCount();
+					unset($txItems[$j]);
+				}
+			}
 
-		$air = ItemFactory::get(Item::AIR, 0, 0);
-		$reindexed = array_fill(0, $maxY - $minY + 1, array_fill(0, $maxX - $minX + 1, $air));
-		foreach($reindexed as $y => $row){
-			foreach($row as $x => $item){
-				$reindexed[$y][$x] = $this->inputs[$y + $minY][$x + $minX];
+			if($haveCount % $needCount !== 0){
+				//wrong count for this output, should divide exactly
+				throw new \InvalidStateException("Expected an exact multiple of required $recipeItem (given: $haveCount, needed: $needCount)");
+			}
+
+			$multiplier = intdiv($haveCount, $needCount);
+			if($multiplier < 1){
+				throw new \InvalidStateException("Expected more than zero items matching $recipeItem (given: $haveCount, needed: $needCount)");
+			}
+			if($iterations === 0){
+				$iterations = $multiplier;
+			}elseif($multiplier !== $iterations){
+				//wrong count for this output, should match previous outputs
+				throw new \InvalidStateException("Expected $recipeItem x$iterations, but found x$multiplier");
 			}
 		}
 
-		return $reindexed;
+		if($iterations < 1){
+			throw new \InvalidStateException("Tried to craft zero times");
+		}
+		if(!empty($txItems)){
+			//all items should be destroyed in this process
+			throw new \InvalidStateException("Expected 0 ingredients left over, have " . count($txItems));
+		}
+
+		return $iterations;
 	}
 
+
 	public function canExecute() : bool{
-		$inputs = $this->reindexInputs();
+		$this->squashDuplicateSlotChanges();
+		if(count($this->actions) < 1){
+			return false;
+		}
 
-		$this->recipe = $this->source->getServer()->getCraftingManager()->matchRecipe($inputs, $this->primaryOutput, $this->secondaryOutputs);
+		$this->matchItems($this->outputs, $this->inputs);
 
-		return $this->recipe !== null and parent::canExecute();
+		$this->recipe = $this->source->getServer()->getCraftingManager()->matchRecipe($this->source->getCraftingGrid(), $this->outputs);
+		if($this->recipe === null){
+			return false;
+		}
+
+		try{
+			$this->repetitions = $this->matchRecipeItems($this->outputs, $this->recipe->getResults(), false);
+
+			if(($inputIterations = $this->matchRecipeItems($this->inputs, $this->recipe->getIngredientList(), true)) !== $this->repetitions){
+				throw new \InvalidStateException("Tried to craft recipe $this->repetitions times in batch, but have enough inputs for $inputIterations");
+			}
+
+			return true;
+		}catch(\InvalidStateException $e){
+			MainLogger::getLogger()->debug("Failed to validate crafting transaction for " . $this->source->getName() . ": " . $e->getMessage());
+		}
+
+		return false;
 	}
 
 	protected function callExecuteEvent() : bool{
-		$this->source->getServer()->getPluginManager()->callEvent($ev = new CraftItemEvent($this));
+		$this->source->getServer()->getPluginManager()->callEvent($ev = new CraftItemEvent($this, $this->recipe, $this->repetitions, $this->inputs, $this->outputs));
 		return !$ev->isCancelled();
 	}
 
@@ -171,37 +156,39 @@ class CraftingTransaction extends InventoryTransaction{
 
 	public function execute() : bool{
 		if(parent::execute()){
-			switch($this->primaryOutput->getId()){
-				case Item::CRAFTING_TABLE:
-					$this->source->awardAchievement("buildWorkBench");
-					break;
-				case Item::WOODEN_PICKAXE:
-					$this->source->awardAchievement("buildPickaxe");
-					break;
-				case Item::FURNACE:
-					$this->source->awardAchievement("buildFurnace");
-					break;
-				case Item::WOODEN_HOE:
-					$this->source->awardAchievement("buildHoe");
-					break;
-				case Item::BREAD:
-					$this->source->awardAchievement("makeBread");
-					break;
-				case Item::CAKE:
-					$this->source->awardAchievement("bakeCake");
-					break;
-				case Item::STONE_PICKAXE:
-				case Item::GOLDEN_PICKAXE:
-				case Item::IRON_PICKAXE:
-				case Item::DIAMOND_PICKAXE:
-					$this->source->awardAchievement("buildBetterPickaxe");
-					break;
-				case Item::WOODEN_SWORD:
-					$this->source->awardAchievement("buildSword");
-					break;
-				case Item::DIAMOND:
-					$this->source->awardAchievement("diamond");
-					break;
+			foreach($this->outputs as $item){
+				switch($item->getId()){
+					case Item::CRAFTING_TABLE:
+						$this->source->awardAchievement("buildWorkBench");
+						break;
+					case Item::WOODEN_PICKAXE:
+						$this->source->awardAchievement("buildPickaxe");
+						break;
+					case Item::FURNACE:
+						$this->source->awardAchievement("buildFurnace");
+						break;
+					case Item::WOODEN_HOE:
+						$this->source->awardAchievement("buildHoe");
+						break;
+					case Item::BREAD:
+						$this->source->awardAchievement("makeBread");
+						break;
+					case Item::CAKE:
+						$this->source->awardAchievement("bakeCake");
+						break;
+					case Item::STONE_PICKAXE:
+					case Item::GOLDEN_PICKAXE:
+					case Item::IRON_PICKAXE:
+					case Item::DIAMOND_PICKAXE:
+						$this->source->awardAchievement("buildBetterPickaxe");
+						break;
+					case Item::WOODEN_SWORD:
+						$this->source->awardAchievement("buildSword");
+						break;
+					case Item::DIAMOND:
+						$this->source->awardAchievement("diamond");
+						break;
+				}
 			}
 
 			return true;

src/pocketmine/inventory/transaction/action/CraftingTakeResultAction.php

@@ -1,59 +0,0 @@
-<?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\inventory\transaction\action;
-
-use pocketmine\inventory\transaction\CraftingTransaction;
-use pocketmine\inventory\transaction\InventoryTransaction;
-use pocketmine\Player;
-
-/**
- * Action used to take the primary result item during crafting.
- */
-class CraftingTakeResultAction extends InventoryAction{
-
-	public function onAddToTransaction(InventoryTransaction $transaction) : void{
-		if($transaction instanceof CraftingTransaction){
-			$transaction->setPrimaryOutput($this->getSourceItem());
-		}else{
-			throw new \InvalidStateException(get_class($this) . " can only be added to CraftingTransactions");
-		}
-	}
-
-	public function isValid(Player $source) : bool{
-		return true;
-	}
-
-	public function execute(Player $source) : bool{
-		return true;
-	}
-
-	public function onExecuteSuccess(Player $source) : void{
-
-	}
-
-	public function onExecuteFail(Player $source) : void{
-
-	}
-
-}

src/pocketmine/inventory/transaction/action/CraftingTransferMaterialAction.php

@@ -1,73 +0,0 @@
-<?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\inventory\transaction\action;
-
-use pocketmine\inventory\transaction\CraftingTransaction;
-use pocketmine\inventory\transaction\InventoryTransaction;
-use pocketmine\item\Item;
-use pocketmine\Player;
-
-/**
- * Action used to take ingredients out of the crafting grid, or put secondary results into the crafting grid, when
- * crafting.
- */
-class CraftingTransferMaterialAction extends InventoryAction{
-	/** @var int */
-	private $slot;
-
-	public function __construct(Item $sourceItem, Item $targetItem, int $slot){
-		parent::__construct($sourceItem, $targetItem);
-		$this->slot = $slot;
-	}
-
-	public function onAddToTransaction(InventoryTransaction $transaction) : void{
-		if($transaction instanceof CraftingTransaction){
-			if($this->sourceItem->isNull()){
-				$transaction->setInput($this->slot, $this->targetItem);
-			}elseif($this->targetItem->isNull()){
-				$transaction->setExtraOutput($this->slot, $this->sourceItem);
-			}else{
-				throw new \InvalidStateException("Invalid " . get_class($this) . ", either source or target item must be air, got source: " . $this->sourceItem . ", target: " . $this->targetItem);
-			}
-		}else{
-			throw new \InvalidStateException(get_class($this) . " can only be added to CraftingTransactions");
-		}
-	}
-
-	public function isValid(Player $source) : bool{
-		return true;
-	}
-
-	public function execute(Player $source) : bool{
-		return true;
-	}
-
-	public function onExecuteSuccess(Player $source) : void{
-
-	}
-
-	public function onExecuteFail(Player $source) : void{
-
-	}
-}

src/pocketmine/network/mcpe/protocol/CraftingDataPacket.php

@@ -133,7 +133,7 @@ class CraftingDataPacket extends DataPacket{
 			$stream->putSlot($item);
 		}
 
-		$results = $recipe->getAllResults();
+		$results = $recipe->getResults();
 		$stream->putUnsignedVarInt(count($results));
 		foreach($results as $item){
 			$stream->putSlot($item);
@@ -154,7 +154,7 @@ class CraftingDataPacket extends DataPacket{
 			}
 		}
 
-		$results = $recipe->getAllResults();
+		$results = $recipe->getResults();
 		$stream->putUnsignedVarInt(count($results));
 		foreach($results as $item){
 			$stream->putSlot($item);

src/pocketmine/network/mcpe/protocol/InventoryTransactionPacket.php

@@ -56,6 +56,12 @@ class InventoryTransactionPacket extends DataPacket{
 	 * determine whether we're doing a crafting transaction.
 	 */
 	public $isCraftingPart = false;
+	/**
+	 * @var bool
+	 * NOTE: THIS FIELD DOES NOT EXIST IN THE PROTOCOL, it's merely used for convenience for PocketMine-MP to easily
+	 * determine whether we're doing a crafting transaction.
+	 */
+	public $isFinalCraftingPart = false;
 
 	/** @var NetworkInventoryAction[] */
 	public $actions = [];

src/pocketmine/network/mcpe/protocol/types/NetworkInventoryAction.php

@@ -23,8 +23,6 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe\protocol\types;
 
-use pocketmine\inventory\transaction\action\CraftingTakeResultAction;
-use pocketmine\inventory\transaction\action\CraftingTransferMaterialAction;
 use pocketmine\inventory\transaction\action\CreativeInventoryAction;
 use pocketmine\inventory\transaction\action\DropItemAction;
 use pocketmine\inventory\transaction\action\InventoryAction;
@@ -111,8 +109,10 @@ class NetworkInventoryAction{
 			case self::SOURCE_TODO:
 				$this->windowId = $packet->getVarInt();
 				switch($this->windowId){
-					case self::SOURCE_TYPE_CRAFTING_USE_INGREDIENT:
+					/** @noinspection PhpMissingBreakStatementInspection */
 					case self::SOURCE_TYPE_CRAFTING_RESULT:
+						$packet->isFinalCraftingPart = true;
+					case self::SOURCE_TYPE_CRAFTING_USE_INGREDIENT:
 						$packet->isCraftingPart = true;
 						break;
 				}
@@ -193,9 +193,8 @@ class NetworkInventoryAction{
 						$window = $player->getCraftingGrid();
 						return new SlotChangeAction($window, $this->inventorySlot, $this->oldItem, $this->newItem);
 					case self::SOURCE_TYPE_CRAFTING_RESULT:
-						return new CraftingTakeResultAction($this->oldItem, $this->newItem);
 					case self::SOURCE_TYPE_CRAFTING_USE_INGREDIENT:
-						return new CraftingTransferMaterialAction($this->oldItem, $this->newItem, $this->inventorySlot);
+						return null;
 
 					case self::SOURCE_TYPE_CONTAINER_DROP_CONTENTS:
 						//TODO: this type applies to all fake windows, not just crafting