Merge branch 'minor-next' into major-next

src/network/mcpe/InventoryManager.php

@@ -418,17 +418,17 @@ class InventoryManager{
 		$clientSideItem = $inventoryEntry->predictions[$slot] ?? null;
 		if($clientSideItem === null || !$clientSideItem->equals($currentItem)){
 			//no prediction or incorrect - do not associate this with the currently active itemstack request
-			$this->trackItemStack($inventory, $slot, $currentItem, null);
+			$this->trackItemStack($inventoryEntry, $slot, $currentItem, null);
-			$inventoryEntry->pendingSyncs[$slot] = $slot;
+			$inventoryEntry->pendingSyncs[$slot] = $currentItem;
 		}else{
 			//correctly predicted - associate the change with the currently active itemstack request
-			$this->trackItemStack($inventory, $slot, $currentItem, $this->currentItemStackRequestId);
+			$this->trackItemStack($inventoryEntry, $slot, $currentItem, $this->currentItemStackRequestId);
 		}
 
 		unset($inventoryEntry->predictions[$slot]);
 	}
 
-	public function syncSlot(Inventory $inventory, int $slot) : void{
+	public function syncSlot(Inventory $inventory, int $slot, ItemStack $itemStack) : void{
 		$entry = $this->inventories[spl_object_id($inventory)] ?? null;
 		if($entry === null){
 			throw new \LogicException("Cannot sync an untracked inventory");
@@ -445,7 +445,7 @@ class InventoryManager{
 			$netSlot = $slot;
 		}
 
-		$itemStackWrapper = new ItemStackWrapper($itemStackInfo->getStackId(), $itemStackInfo->getItemStack());
+		$itemStackWrapper = new ItemStackWrapper($itemStackInfo->getStackId(), $itemStack);
 		if($windowId === ContainerIds::OFFHAND){
 			//TODO: HACK!
 			//The client may sometimes ignore the InventorySlotPacket for the offhand slot.
@@ -485,10 +485,11 @@ class InventoryManager{
 			$entry->predictions = [];
 			$entry->pendingSyncs = [];
 			$contents = [];
+			$typeConverter = TypeConverter::getInstance();
 			foreach($inventory->getContents(true) as $slot => $item){
-				$itemStack = TypeConverter::getInstance()->coreItemStackToNet($item);
+				$itemStack = $typeConverter->coreItemStackToNet($item);
-				$info = $this->trackItemStack($inventory, $slot, $itemStack, null);
+				$info = $this->trackItemStack($entry, $slot, $itemStack, null);
-				$contents[] = new ItemStackWrapper($info->getStackId(), $info->getItemStack());
+				$contents[] = new ItemStackWrapper($info->getStackId(), $itemStack);
 			}
 			if($entry->complexSlotMap !== null){
 				foreach($contents as $slotId => $info){
@@ -519,6 +520,7 @@ class InventoryManager{
 	}
 
 	public function syncMismatchedPredictedSlotChanges() : void{
+		$typeConverter = TypeConverter::getInstance();
 		foreach($this->inventories as $entry){
 			$inventory = $entry->inventory;
 			foreach($entry->predictions as $slot => $expectedItem){
@@ -528,7 +530,7 @@ class InventoryManager{
 
 				//any prediction that still exists at this point is a slot that was predicted to change but didn't
 				$this->session->getLogger()->debug("Detected prediction mismatch in inventory " . get_class($inventory) . "#" . spl_object_id($inventory) . " slot $slot");
-				$entry->pendingSyncs[$slot] = $slot;
+				$entry->pendingSyncs[$slot] = $typeConverter->coreItemStackToNet($inventory->getItem($slot));
 			}
 
 			$entry->predictions = [];
@@ -547,8 +549,8 @@ class InventoryManager{
 				}
 				$inventory = $entry->inventory;
 				$this->session->getLogger()->debug("Syncing slots " . implode(", ", array_keys($entry->pendingSyncs)) . " in inventory " . get_class($inventory) . "#" . spl_object_id($inventory));
-				foreach($entry->pendingSyncs as $slot){
+				foreach($entry->pendingSyncs as $slot => $itemStack){
-					$this->syncSlot($inventory, $slot);
+					$this->syncSlot($inventory, $slot, $itemStack);
 				}
 				$entry->pendingSyncs = [];
 			}
@@ -577,7 +579,7 @@ class InventoryManager{
 
 			$this->session->sendDataPacket(MobEquipmentPacket::create(
 				$this->player->getId(),
-				new ItemStackWrapper($itemStackInfo->getStackId(), $itemStackInfo->getItemStack()),
+				new ItemStackWrapper($itemStackInfo->getStackId(), TypeConverter::getInstance()->coreItemStackToNet($playerInventory->getItemInHand())),
 				$selected,
 				$selected,
 				ContainerIds::INVENTORY
@@ -608,18 +610,9 @@ class InventoryManager{
 		return $entry?->itemStackInfos[$slot] ?? null;
 	}
 
-	private function trackItemStack(Inventory $inventory, int $slotId, ItemStack $itemStack, ?int $itemStackRequestId) : ItemStackInfo{
+	private function trackItemStack(InventoryManagerEntry $entry, int $slotId, ItemStack $itemStack, ?int $itemStackRequestId) : ItemStackInfo{
-		$entry = $this->inventories[spl_object_id($inventory)] ?? null;
-		if($entry === null){
-			throw new \LogicException("Cannot track an item stack for an untracked inventory");
-		}
-		$existing = $entry->itemStackInfos[$slotId] ?? null;
-		if($existing !== null && $existing->getItemStack()->equals($itemStack) && $existing->getRequestId() === $itemStackRequestId){
-			return $existing;
-		}
-
 		//TODO: ItemStack->isNull() would be nice to have here
-		$info = new ItemStackInfo($itemStackRequestId, $itemStack->getId() === 0 ? 0 : $this->newItemStackId(), $itemStack);
+		$info = new ItemStackInfo($itemStackRequestId, $itemStack->getId() === 0 ? 0 : $this->newItemStackId());
 		return $entry->itemStackInfos[$slotId] = $info;
 	}
 }

src/network/mcpe/InventoryManagerEntry.php

@@ -41,7 +41,7 @@ final class InventoryManagerEntry{
 
 	/**
 	 * @var int[]
-	 * @phpstan-var array<int, int>
+	 * @phpstan-var array<int, ItemStack>
 	 */
 	public array $pendingSyncs = [];
 

src/network/mcpe/ItemStackInfo.php

@@ -23,19 +23,14 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe;
 
-use pocketmine\network\mcpe\protocol\types\inventory\ItemStack;
-
 final class ItemStackInfo{
 
 	public function __construct(
 		private ?int $requestId,
-		private int $stackId,
+		private int $stackId
-		private ItemStack $itemStack
 	){}
 
 	public function getRequestId() : ?int{ return $this->requestId; }
 
 	public function getStackId() : int{ return $this->stackId; }
-
-	public function getItemStack() : ItemStack{ return $this->itemStack; }
 }

src/network/mcpe/NetworkSession.php

@@ -378,7 +378,7 @@ class NetworkSession{
 				$stream = new BinaryStream($decompressed);
 				$count = 0;
 				foreach(PacketBatch::decodeRaw($stream) as $buffer){
-					if(++$count > 1300){
+					if(++$count > 100){
 						throw new PacketHandlingException("Too many packets in batch");
 					}
 					$packet = $this->packetPool->getPacket($buffer);
@@ -1191,7 +1191,12 @@ class NetworkSession{
 				$attribute->markSynchronized();
 			}
 		}
-		$this->invManager?->flushPendingUpdates();
+		Timings::$playerNetworkSendInventorySync->startTiming();
+		try{
+			$this->invManager?->flushPendingUpdates();
+		}finally{
+			Timings::$playerNetworkSendInventorySync->stopTiming();
+		}
 
 		$this->flushSendBuffer();
 	}

src/network/mcpe/handler/InGamePacketHandler.php

@@ -391,14 +391,11 @@ class InGamePacketHandler extends PacketHandler{
 				//it's technically possible to see this more than once, but a normal client should never do that.
 				$inventory = $this->player->getInventory();
 
-				$heldItemStack = $this->inventoryManager->getItemStackInfo($inventory, $inventory->getHeldItemIndex())?->getItemStack();
+				$heldItemStack = TypeConverter::getInstance()->coreItemStackToNet($inventory->getItemInHand());
-				if($heldItemStack === null){
-					throw new AssumptionFailedError("Missing itemstack info for held item");
-				}
 				$droppedItemStack = $networkInventoryAction->newItem->getItemStack();
 				//because the client doesn't tell us the expected itemstack ID, we have to deep-compare our known
 				//itemstack info with the one the client sent. This is costly, but we don't have any other option :(
-				if(!$heldItemStack->equalsWithoutCount($droppedItemStack) || $heldItemStack->getCount() < $droppedItemStack->getCount()){
+				if($heldItemStack->getCount() < $droppedItemStack->getCount() || !$heldItemStack->equalsWithoutCount($droppedItemStack)){
 					return false;
 				}
 

src/network/mcpe/handler/ItemStackResponseBuilder.php

@@ -83,12 +83,6 @@ final class ItemStackResponseBuilder{
 				if($itemStackInfo === null){
 					throw new AssumptionFailedError("ItemStackInfo should never be null for an open inventory");
 				}
-				if($itemStackInfo->getRequestId() !== $this->requestId){
-					//the itemstack may have been synced due to transaction producing results that the client did not
-					//predict correctly, which will wipe out the tracked request ID (intentionally)
-					//TODO: is this the correct behaviour?
-					continue;
-				}
 				$item = $inventory->getItem($slot);
 
 				$responseInfosByContainer[$containerInterfaceId][] = new ItemStackResponseSlotInfo(

src/timings/Timings.php

@@ -45,6 +45,7 @@ abstract class Timings{
 	public static TimingsHandler $playerNetworkSendCompressBroadcast;
 	public static TimingsHandler $playerNetworkSendCompressSessionBuffer;
 	public static TimingsHandler $playerNetworkSendEncrypt;
+	public static TimingsHandler $playerNetworkSendInventorySync;
 	public static TimingsHandler $playerNetworkReceive;
 	public static TimingsHandler $playerNetworkReceiveDecompress;
 	public static TimingsHandler $playerNetworkReceiveDecrypt;
@@ -130,6 +131,7 @@ abstract class Timings{
 		self::$playerNetworkSendCompressBroadcast = new TimingsHandler(self::INCLUDED_BY_OTHER_TIMINGS_PREFIX . "Player Network Send - Compression (Broadcast)", self::$playerNetworkSendCompress);
 		self::$playerNetworkSendCompressSessionBuffer = new TimingsHandler(self::INCLUDED_BY_OTHER_TIMINGS_PREFIX . "Player Network Send - Compression (Session Buffer)", self::$playerNetworkSendCompress);
 		self::$playerNetworkSendEncrypt = new TimingsHandler(self::INCLUDED_BY_OTHER_TIMINGS_PREFIX . "Player Network Send - Encryption", self::$playerNetworkSend);
+		self::$playerNetworkSendInventorySync = new TimingsHandler(self::INCLUDED_BY_OTHER_TIMINGS_PREFIX . "Player Network Send - Inventory Sync", self::$playerNetworkSend);
 
 		self::$playerNetworkReceive = new TimingsHandler("Player Network Receive", self::$connection);
 		self::$playerNetworkReceiveDecompress = new TimingsHandler(self::INCLUDED_BY_OTHER_TIMINGS_PREFIX . "Player Network Receive - Decompression", self::$playerNetworkReceive);