Crafting tables now work the same way as anvils and enchanting tables

Removing almost all special-case logic for crafting tables.

src/block/CraftingTable.php

@@ -23,7 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\block;
 
-use pocketmine\crafting\CraftingGrid;
+use pocketmine\block\inventory\CraftingTableInventory;
 use pocketmine\item\Item;
 use pocketmine\math\Vector3;
 use pocketmine\player\Player;
@@ -32,7 +32,7 @@ class CraftingTable extends Opaque{
 
 	public function onInteract(Item $item, int $face, Vector3 $clickVector, ?Player $player = null) : bool{
 		if($player instanceof Player){
-			$player->setCraftingGrid(new CraftingGrid($player, CraftingGrid::SIZE_BIG));
+			$player->setCurrentWindow(new CraftingTableInventory($this->position));
 		}
 
 		return true;

src/block/inventory/CraftingTableInventory.php

@@ -0,0 +1,46 @@
+<?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\block\inventory;
+
+use pocketmine\crafting\CraftingGrid;
+use pocketmine\player\Player;
+use pocketmine\world\Position;
+
+final class CraftingTableInventory extends CraftingGrid implements BlockInventory{
+	use BlockInventoryTrait;
+
+	public function __construct(Position $holder){
+		$this->holder = $holder;
+		parent::__construct(CraftingGrid::SIZE_BIG);
+	}
+
+	public function onClose(Player $who) : void{
+		parent::onClose($who);
+
+		foreach($this->getContents() as $item){
+			$who->dropItem($item);
+		}
+		$this->clearAll();
+	}
+}

src/crafting/CraftingGrid.php

@@ -25,17 +25,14 @@ namespace pocketmine\crafting;
 
 use pocketmine\inventory\SimpleInventory;
 use pocketmine\item\Item;
-use pocketmine\player\Player;
 use function max;
 use function min;
 use const PHP_INT_MAX;
 
-class CraftingGrid extends SimpleInventory{
+abstract class CraftingGrid extends SimpleInventory{
 	public const SIZE_SMALL = 2;
 	public const SIZE_BIG = 3;
 
-	/** @var Player */
-	protected $holder;
 	/** @var int */
 	private $gridWidth;
 
@@ -48,8 +45,7 @@ class CraftingGrid extends SimpleInventory{
 	/** @var int|null */
 	private $yLen;
 
-	public function __construct(Player $holder, int $gridWidth){
+	public function __construct(int $gridWidth){
-		$this->holder = $holder;
 		$this->gridWidth = $gridWidth;
 		parent::__construct($this->getGridWidth() ** 2);
 	}
@@ -63,13 +59,6 @@ class CraftingGrid extends SimpleInventory{
 		$this->seekRecipeBounds();
 	}
 
-	/**
-	 * @return Player
-	 */
-	public function getHolder(){
-		return $this->holder;
-	}
-
 	private function seekRecipeBounds() : void{
 		$minX = PHP_INT_MAX;
 		$maxX = 0;

src/inventory/PlayerCraftingInventory.php

@@ -0,0 +1,36 @@
+<?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;
+
+use pocketmine\crafting\CraftingGrid;
+use pocketmine\player\Player;
+
+final class PlayerCraftingInventory extends CraftingGrid{
+
+	public function __construct(private Player $holder){
+		parent::__construct(CraftingGrid::SIZE_SMALL);
+	}
+
+	public function getHolder() : Player{ return $this->holder; }
+}

src/network/mcpe/InventoryManager.php

@@ -26,6 +26,7 @@ namespace pocketmine\network\mcpe;
 use pocketmine\block\inventory\AnvilInventory;
 use pocketmine\block\inventory\BlockInventory;
 use pocketmine\block\inventory\BrewingStandInventory;
+use pocketmine\block\inventory\CraftingTableInventory;
 use pocketmine\block\inventory\EnchantInventory;
 use pocketmine\block\inventory\FurnaceInventory;
 use pocketmine\block\inventory\HopperInventory;
@@ -67,7 +68,6 @@ class InventoryManager{
 	//effect on the behaviour of inventory transactions I don't currently plan to integrate these into the main system.
 	private const RESERVED_WINDOW_ID_RANGE_START = ContainerIds::LAST - 10;
 	private const RESERVED_WINDOW_ID_RANGE_END = ContainerIds::LAST;
-	public const HARDCODED_CRAFTING_GRID_WINDOW_ID = self::RESERVED_WINDOW_ID_RANGE_START + 1;
 	public const HARDCODED_INVENTORY_WINDOW_ID = self::RESERVED_WINDOW_ID_RANGE_START + 2;
 
 	/** @var Player */
@@ -178,6 +178,7 @@ class InventoryManager{
 				$inv instanceof BrewingStandInventory => WindowTypes::BREWING_STAND,
 				$inv instanceof AnvilInventory => WindowTypes::ANVIL,
 				$inv instanceof HopperInventory => WindowTypes::HOPPER,
+				$inv instanceof CraftingTableInventory => WindowTypes::WORKBENCH,
 				default => WindowTypes::CONTAINER
 			};
 			return [ContainerOpenPacket::blockInv($id, $windowType, $blockPosition)];

src/network/mcpe/convert/TypeConverter.php

@@ -24,9 +24,9 @@ namespace pocketmine\network\mcpe\convert;
 
 use pocketmine\block\BlockLegacyIds;
 use pocketmine\block\inventory\AnvilInventory;
+use pocketmine\block\inventory\CraftingTableInventory;
 use pocketmine\block\inventory\EnchantInventory;
 use pocketmine\block\inventory\LoomInventory;
-use pocketmine\crafting\CraftingGrid;
 use pocketmine\inventory\Inventory;
 use pocketmine\inventory\transaction\action\CreateItemAction;
 use pocketmine\inventory\transaction\action\DestroyItemAction;
@@ -290,11 +290,7 @@ class TypeConverter{
 					$pSlot = $action->inventorySlot;
 
 					$craftingGrid = $player->getCraftingGrid();
-					$mapped =
+					$mapped = $this->mapUIInventory($pSlot, UIInventorySlotOffset::CRAFTING2X2_INPUT, $craftingGrid, fn() => true);
-						$this->mapUIInventory($pSlot, UIInventorySlotOffset::CRAFTING2X2_INPUT, $craftingGrid,
-							function(Inventory $i) : bool{ return $i instanceof CraftingGrid && $i->getGridWidth() === CraftingGrid::SIZE_SMALL; }) ??
-						$this->mapUIInventory($pSlot, UIInventorySlotOffset::CRAFTING3X3_INPUT, $craftingGrid,
-							function(Inventory $i) : bool{ return $i instanceof CraftingGrid && $i->getGridWidth() === CraftingGrid::SIZE_BIG; });
 					if($mapped === null){
 						$current = $player->getCurrentWindow();
 						$mapped =
@@ -303,7 +299,9 @@ class TypeConverter{
 							$this->mapUIInventory($pSlot, UIInventorySlotOffset::ENCHANTING_TABLE, $current,
 								function(Inventory $i) : bool{ return $i instanceof EnchantInventory; }) ??
 							$this->mapUIInventory($pSlot, UIInventorySlotOffset::LOOM, $current,
-								fn(Inventory $i) => $i instanceof LoomInventory);
+								fn(Inventory $i) => $i instanceof LoomInventory) ??
+							$this->mapUIInventory($pSlot, UIInventorySlotOffset::CRAFTING3X3_INPUT, $current,
+								fn(Inventory $i) => $i instanceof CraftingTableInventory);
 					}
 					if($mapped === null){
 						throw new TypeConversionException("Unmatched UI inventory slot offset $pSlot");

src/network/mcpe/handler/InGamePacketHandler.php

@@ -26,7 +26,6 @@ namespace pocketmine\network\mcpe\handler;
 use pocketmine\block\BaseSign;
 use pocketmine\block\ItemFrame;
 use pocketmine\block\utils\SignText;
-use pocketmine\crafting\CraftingGrid;
 use pocketmine\entity\animation\ConsumingItemAnimation;
 use pocketmine\entity\InvalidSkinException;
 use pocketmine\event\player\PlayerEditBookEvent;
@@ -309,14 +308,6 @@ class InGamePacketHandler extends PacketHandler{
 				foreach($this->craftingTransaction->getInventories() as $inventory){
 					$this->inventoryManager->syncContents($inventory);
 				}
-				/*
-				 * TODO: HACK!
-				 * we can't resend the contents of the crafting window, so we force the client to close it instead.
-				 * So people don't whine about messy desync issues when someone cancels CraftItemEvent, or when a crafting
-				 * transaction goes wrong.
-				 */
-				$this->session->sendDataPacket(ContainerClosePacket::create(InventoryManager::HARDCODED_CRAFTING_GRID_WINDOW_ID, true));
-
 				return false;
 			}finally{
 				$this->craftingTransaction = null;
@@ -380,18 +371,6 @@ class InGamePacketHandler extends PacketHandler{
 				$vBlockPos = new Vector3($blockPos->getX(), $blockPos->getY(), $blockPos->getZ());
 				if(!$this->player->interactBlock($vBlockPos, $data->getFace(), $clickPos)){
 					$this->onFailedBlockAction($vBlockPos, $data->getFace());
-				}elseif(
-					!array_key_exists($windowId = InventoryManager::HARDCODED_CRAFTING_GRID_WINDOW_ID, $this->openHardcodedWindows) &&
-					$this->player->getCraftingGrid()->getGridWidth() === CraftingGrid::SIZE_BIG
-				){
-					//TODO: HACK! crafting grid doesn't fit very well into the current PM container system, so this hack
-					//allows it to carry on working approximately the same way as it did in 1.14
-					$this->openHardcodedWindows[$windowId] = true;
-					$this->session->sendDataPacket(ContainerOpenPacket::blockInv(
-						InventoryManager::HARDCODED_CRAFTING_GRID_WINDOW_ID,
-						WindowTypes::WORKBENCH,
-						$blockPos
-					));
 				}
 				return true;
 			case UseItemTransactionData::ACTION_BREAK_BLOCK:

src/player/Player.php

@@ -75,6 +75,7 @@ use pocketmine\form\Form;
 use pocketmine\form\FormValidationException;
 use pocketmine\inventory\CallbackInventoryListener;
 use pocketmine\inventory\Inventory;
+use pocketmine\inventory\PlayerCraftingInventory;
 use pocketmine\inventory\PlayerCursorInventory;
 use pocketmine\inventory\transaction\action\DropItemAction;
 use pocketmine\inventory\transaction\InventoryTransaction;
@@ -182,7 +183,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 	/** @var Inventory[] */
 	protected array $permanentWindows = [];
 	protected PlayerCursorInventory $cursorInventory;
-	protected CraftingGrid $craftingGrid;
+	protected PlayerCraftingInventory $craftingGrid;
 
 	protected int $messageCounter = 2;
 
@@ -2301,7 +2302,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 
 	protected function addDefaultWindows() : void{
 		$this->cursorInventory = new PlayerCursorInventory($this);
-		$this->craftingGrid = new CraftingGrid($this, CraftingGrid::SIZE_SMALL);
+		$this->craftingGrid = new PlayerCraftingInventory($this);
 
 		$this->addPermanentInventories($this->inventory, $this->armorInventory, $this->cursorInventory, $this->offHandInventory);
 
@@ -2316,10 +2317,6 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 		return $this->craftingGrid;
 	}
 
-	public function setCraftingGrid(CraftingGrid $grid) : void{
-		$this->craftingGrid = $grid;
-	}
-
 	/**
 	 * @internal Called to clean up crafting grid and cursor inventory when it is detected that the player closed their
 	 * inventory.
@@ -2363,10 +2360,6 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 				throw new AssumptionFailedError("This server-generated transaction should never be invalid", 0, $e);
 			}
 		}
-
-		if($this->craftingGrid->getGridWidth() > CraftingGrid::SIZE_SMALL){
-			$this->craftingGrid = new CraftingGrid($this, CraftingGrid::SIZE_SMALL);
-		}
 	}
 
 	/**