InventoryManager: be aware of client-side state when syncing slots

this eliminates feedback loops during client-initiated slot changes, and also makes it possible to have a SlotChangeAction anonymous from its initiator.

src/inventory/transaction/action/SlotChangeAction.php

@@ -79,11 +79,6 @@ class SlotChangeAction extends InventoryAction{
 	 * Sets the item into the target inventory.
 	 */
 	public function execute(Player $source) : void{
-		$this->inventory->setItem($this->inventorySlot, $this->targetItem, false);
+		$this->inventory->setItem($this->inventorySlot, $this->targetItem);
-		foreach($this->inventory->getViewers() as $viewer){
-			if($viewer !== $source){
-				$viewer->getNetworkSession()->getInvManager()->syncSlot($this->inventory, $this->inventorySlot);
-			}
-		}
 	}
 }

src/network/mcpe/InventoryManager.php

@@ -31,6 +31,9 @@ use pocketmine\inventory\EnchantInventory;
 use pocketmine\inventory\FurnaceInventory;
 use pocketmine\inventory\HopperInventory;
 use pocketmine\inventory\Inventory;
+use pocketmine\inventory\transaction\action\SlotChangeAction;
+use pocketmine\inventory\transaction\InventoryTransaction;
+use pocketmine\item\Item;
 use pocketmine\network\mcpe\protocol\ContainerClosePacket;
 use pocketmine\network\mcpe\protocol\ContainerOpenPacket;
 use pocketmine\network\mcpe\protocol\ContainerSetDataPacket;
@@ -55,6 +58,12 @@ class InventoryManager{
 	/** @var int */
 	private $lastInventoryNetworkId = ContainerIds::FIRST;
 
+	/**
+	 * @var Item[][]
+	 * @phpstan-var array<int, array<int, Item>>
+	 */
+	private $initiatedSlotChanges = [];
+
 	public function __construct(Player $player, NetworkSession $session){
 		$this->player = $player;
 		$this->session = $session;
@@ -76,6 +85,15 @@ class InventoryManager{
 		return $this->windowMap[$windowId] ?? null;
 	}
 
+	public function onTransactionStart(InventoryTransaction $tx) : void{
+		foreach($tx->getActions() as $action){
+			if($action instanceof SlotChangeAction and ($windowId = $this->getWindowId($action->getInventory())) !== null){
+				//in some cases the inventory might not have a window ID, but still be referenced by a transaction (e.g. crafting grid changes), so we can't unconditionally record the change here or we might leak things
+				$this->initiatedSlotChanges[$windowId][$action->getSlot()] = $action->getTargetItem();
+			}
+		}
+	}
+
 	public function onCurrentWindowChange(Inventory $inventory) : void{
 		$this->onCurrentWindowRemove();
 		$this->windowMap[$this->lastInventoryNetworkId = max(ContainerIds::FIRST, ($this->lastInventoryNetworkId + 1) % ContainerIds::LAST)] = $inventory;
@@ -114,6 +132,7 @@ class InventoryManager{
 	public function onCurrentWindowRemove() : void{
 		if(isset($this->windowMap[$this->lastInventoryNetworkId])){
 			unset($this->windowMap[$this->lastInventoryNetworkId]);
+			unset($this->initiatedSlotChanges[$this->lastInventoryNetworkId]);
 			$this->session->sendDataPacket(ContainerClosePacket::create($this->lastInventoryNetworkId));
 		}
 	}
@@ -121,13 +140,19 @@ class InventoryManager{
 	public function syncSlot(Inventory $inventory, int $slot) : void{
 		$windowId = $this->getWindowId($inventory);
 		if($windowId !== null){
-			$this->session->sendDataPacket(InventorySlotPacket::create($windowId, $slot, $inventory->getItem($slot)));
+			$currentItem = $inventory->getItem($slot);
+			$clientSideItem = $this->initiatedSlotChanges[$windowId][$slot] ?? null;
+			if($clientSideItem === null or !$clientSideItem->equalsExact($currentItem)){
+				$this->session->sendDataPacket(InventorySlotPacket::create($windowId, $slot, $currentItem));
+			}
+			unset($this->initiatedSlotChanges[$windowId][$slot]);
 		}
 	}
 
 	public function syncContents(Inventory $inventory) : void{
 		$windowId = $this->getWindowId($inventory);
 		if($windowId !== null){
+			unset($this->initiatedSlotChanges[$windowId]);
 			$this->session->sendDataPacket(InventoryContentPacket::create($windowId, $inventory->getContents(true)));
 		}
 	}

src/network/mcpe/handler/InGamePacketHandler.php

@@ -245,6 +245,7 @@ class InGamePacketHandler extends PacketHandler{
 
 			if($isFinalCraftingPart){
 				try{
+					$this->session->getInvManager()->onTransactionStart($this->craftingTransaction);
 					$this->craftingTransaction->execute();
 				}catch(TransactionValidationException $e){
 					$this->session->getLogger()->debug("Failed to execute crafting transaction: " . $e->getMessage());
@@ -267,6 +268,7 @@ class InGamePacketHandler extends PacketHandler{
 			}
 
 			$transaction = new InventoryTransaction($this->player, $actions);
+			$this->session->getInvManager()->onTransactionStart($transaction);
 			try{
 				$transaction->execute();
 			}catch(TransactionValidationException $e){