Working crafting :woohoo:

src/crafting/CraftingManager.php

@@ -45,6 +45,12 @@ class CraftingManager{
 	 */
 	protected $shapelessRecipes = [];
 
+	/**
+	 * @var CraftingRecipe[]
+	 * @phpstan-var array<int, CraftingRecipe>
+	 */
+	private array $craftingRecipeIndex = [];
+
 	/**
 	 * @var FurnaceRecipeManager[]
 	 * @phpstan-var array<int, FurnaceRecipeManager>
@@ -153,6 +159,14 @@ class CraftingManager{
 		return $this->shapedRecipes;
 	}
 
+	/**
+	 * @return CraftingRecipe[]
+	 * @phpstan-return array<int, CraftingRecipe>
+	 */
+	public function getCraftingRecipeIndex() : array{
+		return $this->craftingRecipeIndex;
+	}
+
 	public function getFurnaceRecipeManager(FurnaceType $furnaceType) : FurnaceRecipeManager{
 		return $this->furnaceRecipeManagers[$furnaceType->id()];
 	}
@@ -175,6 +189,7 @@ class CraftingManager{
 
 	public function registerShapedRecipe(ShapedRecipe $recipe) : void{
 		$this->shapedRecipes[self::hashOutputs($recipe->getResults())][] = $recipe;
+		$this->craftingRecipeIndex[] = $recipe;
 
 		foreach($this->recipeRegisteredCallbacks as $callback){
 			$callback();
@@ -183,6 +198,7 @@ class CraftingManager{
 
 	public function registerShapelessRecipe(ShapelessRecipe $recipe) : void{
 		$this->shapelessRecipes[self::hashOutputs($recipe->getResults())][] = $recipe;
+		$this->craftingRecipeIndex[] = $recipe;
 
 		foreach($this->recipeRegisteredCallbacks as $callback){
 			$callback();

src/inventory/transaction/CraftingTransaction.php

@@ -60,9 +60,11 @@ class CraftingTransaction extends InventoryTransaction{
 
 	private CraftingManager $craftingManager;
 
-	public function __construct(Player $source, CraftingManager $craftingManager, array $actions = []){
+	public function __construct(Player $source, CraftingManager $craftingManager, array $actions = [], ?CraftingRecipe $recipe = null, ?int $repetitions = null){
 		parent::__construct($source, $actions);
 		$this->craftingManager = $craftingManager;
+		$this->recipe = $recipe;
+		$this->repetitions = $repetitions;
 	}
 
 	/**
@@ -123,6 +125,18 @@ class CraftingTransaction extends InventoryTransaction{
 		return $iterations;
 	}
 
+	private function validateRecipe(CraftingRecipe $recipe, ?int $expectedRepetitions) : int{
+		//compute number of times recipe was crafted
+		$repetitions = $this->matchRecipeItems($this->outputs, $recipe->getResultsFor($this->source->getCraftingGrid()), false);
+		if($expectedRepetitions !== null && $repetitions !== $expectedRepetitions){
+			throw new TransactionValidationException("Expected $expectedRepetitions repetitions, got $repetitions");
+		}
+		//assert that $repetitions x recipe ingredients should be consumed
+		$this->matchRecipeItems($this->inputs, $recipe->getIngredientList(), true, $repetitions);
+
+		return $repetitions;
+	}
+
 	public function validate() : void{
 		$this->squashDuplicateSlotChanges();
 		if(count($this->actions) < 1){
@@ -131,25 +145,24 @@ class CraftingTransaction extends InventoryTransaction{
 
 		$this->matchItems($this->outputs, $this->inputs);
 
-		$failed = 0;
-		foreach($this->craftingManager->matchRecipeByOutputs($this->outputs) as $recipe){
-			try{
-				//compute number of times recipe was crafted
-				$this->repetitions = $this->matchRecipeItems($this->outputs, $recipe->getResultsFor($this->source->getCraftingGrid()), false);
-				//assert that $repetitions x recipe ingredients should be consumed
-				$this->matchRecipeItems($this->inputs, $recipe->getIngredientList(), true, $this->repetitions);
-
-				//Success!
-				$this->recipe = $recipe;
-				break;
-			}catch(TransactionValidationException $e){
-				//failed
-				++$failed;
-			}
-		}
-
 		if($this->recipe === null){
-			throw new TransactionValidationException("Unable to match a recipe to transaction (tried to match against $failed recipes)");
+			$failed = 0;
+			foreach($this->craftingManager->matchRecipeByOutputs($this->outputs) as $recipe){
+				try{
+					$this->repetitions = $this->validateRecipe($recipe, $this->repetitions);
+					$this->recipe = $recipe;
+					break;
+				}catch(TransactionValidationException $e){
+					//failed
+					++$failed;
+				}
+			}
+
+			if($this->recipe === null){
+				throw new TransactionValidationException("Unable to match a recipe to transaction (tried to match against $failed recipes)");
+			}
+		}else{
+			$this->repetitions = $this->validateRecipe($this->recipe, $this->repetitions);
 		}
 	}
 

src/network/mcpe/cache/CraftingDataCache.php

@@ -25,6 +25,8 @@ namespace pocketmine\network\mcpe\cache;
 
 use pocketmine\crafting\CraftingManager;
 use pocketmine\crafting\FurnaceType;
+use pocketmine\crafting\ShapedRecipe;
+use pocketmine\crafting\ShapelessRecipe;
 use pocketmine\crafting\ShapelessRecipeType;
 use pocketmine\item\Item;
 use pocketmine\network\mcpe\convert\ItemTranslator;
@@ -76,12 +78,12 @@ final class CraftingDataCache{
 	private function buildCraftingDataCache(CraftingManager $manager) : CraftingDataPacket{
 		Timings::$craftingDataCacheRebuild->startTiming();
 
-		$counter = 0;
 		$nullUUID = Uuid::fromString(Uuid::NIL);
 		$converter = TypeConverter::getInstance();
 		$recipesWithTypeIds = [];
-		foreach($manager->getShapelessRecipes() as $list){
-			foreach($list as $recipe){
+
+		foreach($manager->getCraftingRecipeIndex() as $index => $recipe){
+			if($recipe instanceof ShapelessRecipe){
 				$typeTag = match($recipe->getType()->id()){
 					ShapelessRecipeType::CRAFTING()->id() => CraftingRecipeBlockName::CRAFTING_TABLE,
 					ShapelessRecipeType::STONECUTTER()->id() => CraftingRecipeBlockName::STONECUTTER,
@@ -89,7 +91,7 @@ final class CraftingDataCache{
 				};
 				$recipesWithTypeIds[] = new ProtocolShapelessRecipe(
 					CraftingDataPacket::ENTRY_SHAPELESS,
-					Binary::writeInt(++$counter),
+					Binary::writeInt($index),
 					array_map(function(Item $item) use ($converter) : RecipeIngredient{
 						return $converter->coreItemStackToRecipeIngredient($item);
 					}, $recipe->getIngredientList()),
@@ -99,12 +101,9 @@ final class CraftingDataCache{
 					$nullUUID,
 					$typeTag,
 					50,
-					$counter
+					$index
 				);
-			}
-		}
-		foreach($manager->getShapedRecipes() as $list){
-			foreach($list as $recipe){
+			}elseif($recipe instanceof ShapedRecipe){
 				$inputs = [];
 
 				for($row = 0, $height = $recipe->getHeight(); $row < $height; ++$row){
@@ -114,7 +113,7 @@ final class CraftingDataCache{
 				}
 				$recipesWithTypeIds[] = $r = new ProtocolShapedRecipe(
 					CraftingDataPacket::ENTRY_SHAPED,
-					Binary::writeInt(++$counter),
+					Binary::writeInt($index),
 					$inputs,
 					array_map(function(Item $item) use ($converter) : ItemStack{
 						return $converter->coreItemStackToNet($item);
@@ -122,8 +121,10 @@ final class CraftingDataCache{
 					$nullUUID,
 					CraftingRecipeBlockName::CRAFTING_TABLE,
 					50,
-					$counter
+					$index
 				);
+			}else{
+				//TODO: probably special recipe types
 			}
 		}
 

src/network/mcpe/handler/ItemStackRequestExecutor.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe\handler;
 
+use pocketmine\crafting\CraftingGrid;
 use pocketmine\inventory\transaction\action\DestroyItemAction;
 use pocketmine\inventory\transaction\action\DropItemAction;
 use pocketmine\inventory\transaction\CraftingTransaction;
@@ -30,8 +31,8 @@ use pocketmine\inventory\transaction\InventoryTransaction;
 use pocketmine\inventory\transaction\TransactionBuilder;
 use pocketmine\inventory\transaction\TransactionBuilderInventory;
 use pocketmine\item\Item;
-use pocketmine\item\VanillaItems;
 use pocketmine\network\mcpe\InventoryManager;
+use pocketmine\network\mcpe\protocol\types\inventory\ContainerUIIds;
 use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\CraftingConsumeInputStackRequestAction;
 use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\CraftingMarkSecondaryResultStackRequestAction;
 use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\CraftRecipeAutoStackRequestAction;
@@ -46,8 +47,10 @@ use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\PlaceStackRequ
 use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\SwapStackRequestAction;
 use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\TakeStackRequestAction;
 use pocketmine\network\mcpe\protocol\types\inventory\stackresponse\ItemStackResponse;
+use pocketmine\network\mcpe\protocol\types\inventory\UIInventorySlotOffset;
 use pocketmine\network\PacketHandlingException;
 use pocketmine\player\Player;
+use pocketmine\utils\AssumptionFailedError;
 use function get_class;
 
 final class ItemStackRequestExecutor{
@@ -56,7 +59,11 @@ final class ItemStackRequestExecutor{
 	/** @var ItemStackRequestSlotInfo[] */
 	private array $requestSlotInfos = [];
 
-	private bool $crafting = false;
+	private ?InventoryTransaction $specialTransaction = null;
+
+	/** @var Item[] */
+	private array $craftingResults = [];
+	private int $craftingOutputIndex = 0;
 
 	public function __construct(
 		private Player $player,
@@ -91,37 +98,91 @@ final class ItemStackRequestExecutor{
 	}
 
 	private function transferItems(ItemStackRequestSlotInfo $source, ItemStackRequestSlotInfo $destination, int $count) : void{
-		[$sourceInventory, $sourceSlot] = $this->getBuilderInventoryAndSlot($source);
-		[$targetInventory, $targetSlot] = $this->getBuilderInventoryAndSlot($destination);
-
-		$oldSourceItem = $sourceInventory->getItem($sourceSlot);
-		$oldTargetItem = $targetInventory->getItem($targetSlot);
-
-		if(!$targetInventory->isSlotEmpty($targetSlot) && !$oldTargetItem->canStackWith($oldSourceItem)){
-			throw new PacketHandlingException("Can only transfer items into an empty slot, or a slot containing the same item");
-		}
-		[$newSourceItem, $newTargetItem] = $this->splitStack($oldSourceItem, $count, $oldTargetItem->getCount());
-
-		$sourceInventory->setItem($sourceSlot, $newSourceItem);
-		$targetInventory->setItem($targetSlot, $newTargetItem);
+		$removed = $this->removeItemFromSlot($source, $count);
+		$this->addItemToSlot($destination, $removed, $count);
 	}
 
 	/**
-	 * @phpstan-return array{Item, Item}
+	 * Deducts items from an inventory slot, returning a stack containing the removed items.
 	 */
-	private function splitStack(Item $item, int $transferredCount, int $targetCount) : array{
-		if($item->getCount() < $transferredCount){
-			throw new PacketHandlingException("Cannot take $transferredCount items from a stack of " . $item->getCount());
+	private function removeItemFromSlot(ItemStackRequestSlotInfo $slotInfo, int $count) : Item{
+		$this->requestSlotInfos[] = $slotInfo;
+		[$inventory, $slot] = $this->getBuilderInventoryAndSlot($slotInfo);
+
+		$existingItem = $inventory->getItem($slot);
+		if($existingItem->getCount() < $count){
+			throw new PacketHandlingException("Cannot take $count items from a stack of " . $existingItem->getCount());
 		}
 
+		$removed = $existingItem->pop($count);
+		$inventory->setItem($slot, $existingItem);
+
+		return $removed;
+	}
+
+	/**
+	 * Adds items to the target slot, if they are stackable.
+	 */
+	private function addItemToSlot(ItemStackRequestSlotInfo $slotInfo, Item $item, int $count) : Item{
+		$this->requestSlotInfos[] = $slotInfo;
+		[$inventory, $slot] = $this->getBuilderInventoryAndSlot($slotInfo);
+
+		$existingItem = $inventory->getItem($slot);
+		if(!$existingItem->isNull() && !$existingItem->canStackWith($item)){
+			throw new PacketHandlingException("Can only add items to an empty slot, or a slot containing the same item");
+		}
+
+		//we can't use the existing item here; it may be an empty stack
+		$newItem = clone $item;
+		$newItem->setCount($existingItem->getCount() + $count);
+		$inventory->setItem($slot, $newItem);
+
 		$leftover = clone $item;
-		$removed = $leftover->pop($transferredCount);
-		$removed->setCount($removed->getCount() + $targetCount);
-		if($leftover->isNull()){
-			$leftover = VanillaItems::AIR();
+		$leftover->pop($count);
+
+		return $leftover;
+	}
+
+	private function beginCrafting(int $recipeId, int $repetitions) : void{
+		if($this->specialTransaction !== null){
+			throw new PacketHandlingException("Cannot perform more than 1 special action per request");
+		}
+		if($repetitions < 1){ //TODO: upper bound?
+			throw new PacketHandlingException("Cannot craft a recipe less than 1 time");
+		}
+		$craftingManager = $this->player->getServer()->getCraftingManager();
+		$recipe = $craftingManager->getCraftingRecipeIndex()[$recipeId] ?? null;
+		if($recipe === null){
+			throw new PacketHandlingException("Unknown crafting recipe ID $recipeId");
 		}
 
-		return [$leftover, $removed];
+		$this->specialTransaction = new CraftingTransaction($this->player, $craftingManager, [], $recipe, $repetitions);
+
+		$currentWindow = $this->player->getCurrentWindow();
+		if($currentWindow !== null && !($currentWindow instanceof CraftingGrid)){
+			throw new PacketHandlingException("Cannot complete crafting when the player's current window is not a crafting grid");
+		}
+		$craftingGrid = $currentWindow ?? $this->player->getCraftingGrid();
+
+		$craftingResults = $recipe->getResultsFor($craftingGrid);
+		foreach($craftingResults as $k => $craftingResult){
+			$craftingResult->setCount($craftingResult->getCount() * $repetitions);
+			$this->craftingResults[$k] = $craftingResult;
+		}
+	}
+
+	private function takeCraftingResult(ItemStackRequestSlotInfo $destination, int $count) : void{
+		$recipeResultItem = $this->craftingResults[$this->craftingOutputIndex] ?? null;
+		if($recipeResultItem === null){
+			throw new PacketHandlingException("Cannot refer to nonexisting crafting output index " . $this->craftingOutputIndex);
+		}
+
+		$availableCount = $recipeResultItem->getCount();
+		if($availableCount < $count){
+			throw new PacketHandlingException("Tried to take too many results from crafting");
+		}
+
+		$this->craftingResults[$this->craftingOutputIndex] = $this->addItemToSlot($destination, $recipeResultItem, $count);
 	}
 
 	private function processItemStackRequestAction(ItemStackRequestAction $action) : void{
@@ -129,9 +190,14 @@ final class ItemStackRequestExecutor{
 			$action instanceof TakeStackRequestAction ||
 			$action instanceof PlaceStackRequestAction
 		){
-			$this->requestSlotInfos[] = $action->getSource();
-			$this->requestSlotInfos[] = $action->getDestination();
-			$this->transferItems($action->getSource(), $action->getDestination(), $action->getCount());
+			$source = $action->getSource();
+			$destination = $action->getDestination();
+
+			if($source->getContainerId() === ContainerUIIds::CREATED_OUTPUT && $source->getSlotId() === UIInventorySlotOffset::CREATED_ITEM_OUTPUT){
+				$this->takeCraftingResult($destination, $action->getCount());
+			}else{
+				$this->transferItems($source, $destination, $action->getCount());
+			}
 		}elseif($action instanceof SwapStackRequestAction){
 			$this->requestSlotInfos[] = $action->getSlot1();
 			$this->requestSlotInfos[] = $action->getSlot2();
@@ -144,35 +210,35 @@ final class ItemStackRequestExecutor{
 			$inventory1->setItem($slot1, $item2);
 			$inventory2->setItem($slot2, $item1);
 		}elseif($action instanceof DropStackRequestAction){
-			$this->requestSlotInfos[] = $action->getSource();
-			[$inventory, $slot] = $this->getBuilderInventoryAndSlot($action->getSource());
-
-			$oldItem = $inventory->getItem($slot);
-			[$leftover, $dropped] = $this->splitStack($oldItem, $action->getCount(), 0);
-
 			//TODO: this action has a "randomly" field, I have no idea what it's used for
-			$inventory->setItem($slot, $leftover);
+			$dropped = $this->removeItemFromSlot($action->getSource(), $action->getCount());
 			$this->builder->addAction(new DropItemAction($dropped));
+
 		}elseif($action instanceof DestroyStackRequestAction){
-			$this->requestSlotInfos[] = $action->getSource();
-			[$inventory, $slot] = $this->getBuilderInventoryAndSlot($action->getSource());
-
-			$oldItem = $inventory->getItem($slot);
-			[$leftover, $destroyed] = $this->splitStack($oldItem, $action->getCount(), 0);
-
-			$inventory->setItem($slot, $leftover);
+			$destroyed = $this->removeItemFromSlot($action->getSource(), $action->getCount());
 			$this->builder->addAction(new DestroyItemAction($destroyed));
+
+		}elseif($action instanceof CraftRecipeStackRequestAction){
+			$this->beginCrafting($action->getRecipeId(), 1);
+		}elseif($action instanceof CraftRecipeAutoStackRequestAction){
+			$this->beginCrafting($action->getRecipeId(), $action->getRepetitions());
 		}elseif($action instanceof CraftingConsumeInputStackRequestAction){
-			//we don't need this for the PM system
-			$this->requestSlotInfos[] = $action->getSource();
-			$this->crafting = true;
-		}elseif(
-			$action instanceof CraftRecipeStackRequestAction || //TODO
-			$action instanceof CraftRecipeAutoStackRequestAction || //TODO
-			$action instanceof CraftingMarkSecondaryResultStackRequestAction || //no obvious use
-			$action instanceof DeprecatedCraftingResultsStackRequestAction //no obvious use
-		){
-			$this->crafting = true;
+			if(!$this->specialTransaction instanceof CraftingTransaction){
+				throw new PacketHandlingException("Cannot consume crafting input when no crafting transaction is in progress");
+			}
+			$this->removeItemFromSlot($action->getSource(), $action->getCount()); //output discarded - we allow CraftingTransaction to verify the balance
+
+		}elseif($action instanceof CraftingMarkSecondaryResultStackRequestAction){
+			if(!$this->specialTransaction instanceof CraftingTransaction){
+				throw new AssumptionFailedError("Cannot mark crafting result index when no crafting transaction is in progress");
+			}
+			$outputIndex = $action->getCraftingGridSlot();
+			if($outputIndex < 0){
+				throw new PacketHandlingException("Crafting result index cannot be negative");
+			}
+			$this->craftingOutputIndex = $outputIndex;
+		}elseif($action instanceof DeprecatedCraftingResultsStackRequestAction){
+			//no obvious use
 		}else{
 			throw new PacketHandlingException("Unhandled item stack request action: " . get_class($action));
 		}
@@ -184,9 +250,12 @@ final class ItemStackRequestExecutor{
 		}
 		$inventoryActions = $this->builder->generateActions();
 
-		return $this->crafting ?
-			new CraftingTransaction($this->player, $this->player->getServer()->getCraftingManager(), $inventoryActions) :
-			new InventoryTransaction($this->player, $inventoryActions);
+		$transaction = $this->specialTransaction ?? new InventoryTransaction($this->player);
+		foreach($inventoryActions as $action){
+			$transaction->addAction($action);
+		}
+
+		return $transaction;
 	}
 
 	public function buildItemStackResponse(bool $success) : ItemStackResponse{

src/network/mcpe/handler/ItemStackResponseBuilder.php

@@ -25,6 +25,7 @@ namespace pocketmine\network\mcpe\handler;
 
 use pocketmine\inventory\Inventory;
 use pocketmine\network\mcpe\InventoryManager;
+use pocketmine\network\mcpe\protocol\types\inventory\ContainerUIIds;
 use pocketmine\network\mcpe\protocol\types\inventory\stackresponse\ItemStackResponse;
 use pocketmine\network\mcpe\protocol\types\inventory\stackresponse\ItemStackResponseContainerInfo;
 use pocketmine\network\mcpe\protocol\types\inventory\stackresponse\ItemStackResponseSlotInfo;
@@ -67,6 +68,9 @@ final class ItemStackResponseBuilder{
 	public function build(bool $success) : ItemStackResponse{
 		$responseInfosByContainer = [];
 		foreach($this->changedSlots as $containerInterfaceId => $slotIds){
+			if($containerInterfaceId === ContainerUIIds::CREATED_OUTPUT){
+				continue;
+			}
 			foreach($slotIds as $slotId){
 				[$inventory, $slot] = $this->getInventoryAndSlot($containerInterfaceId, $slotId);