diff --git a/src/network/mcpe/InventoryManager.php b/src/network/mcpe/InventoryManager.php index 28495f67e..5ac22334f 100644 --- a/src/network/mcpe/InventoryManager.php +++ b/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); - $inventoryEntry->pendingSyncs[$slot] = $slot; + $this->trackItemStack($inventoryEntry, $slot, $currentItem, null); + $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); - $info = $this->trackItemStack($inventory, $slot, $itemStack, null); - $contents[] = new ItemStackWrapper($info->getStackId(), $info->getItemStack()); + $itemStack = $typeConverter->coreItemStackToNet($item); + $info = $this->trackItemStack($entry, $slot, $itemStack, null); + $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){ - $this->syncSlot($inventory, $slot); + foreach($entry->pendingSyncs as $slot => $itemStack){ + $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{ - $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; - } - + private function trackItemStack(InventoryManagerEntry $entry, int $slotId, ItemStack $itemStack, ?int $itemStackRequestId) : ItemStackInfo{ //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; } } diff --git a/src/network/mcpe/InventoryManagerEntry.php b/src/network/mcpe/InventoryManagerEntry.php index fa49baf87..ac2a40dfe 100644 --- a/src/network/mcpe/InventoryManagerEntry.php +++ b/src/network/mcpe/InventoryManagerEntry.php @@ -41,7 +41,7 @@ final class InventoryManagerEntry{ /** * @var int[] - * @phpstan-var array + * @phpstan-var array */ public array $pendingSyncs = []; diff --git a/src/network/mcpe/ItemStackInfo.php b/src/network/mcpe/ItemStackInfo.php index 630765bfa..ace138198 100644 --- a/src/network/mcpe/ItemStackInfo.php +++ b/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 ItemStack $itemStack + private int $stackId ){} public function getRequestId() : ?int{ return $this->requestId; } public function getStackId() : int{ return $this->stackId; } - - public function getItemStack() : ItemStack{ return $this->itemStack; } } diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 4db56b84a..422036932 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/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(); } diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 3ae1bd5a2..98e4c1836 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/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(); - if($heldItemStack === null){ - throw new AssumptionFailedError("Missing itemstack info for held item"); - } + $heldItemStack = TypeConverter::getInstance()->coreItemStackToNet($inventory->getItemInHand()); $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; } diff --git a/src/network/mcpe/handler/ItemStackResponseBuilder.php b/src/network/mcpe/handler/ItemStackResponseBuilder.php index 1c742077f..325a2b108 100644 --- a/src/network/mcpe/handler/ItemStackResponseBuilder.php +++ b/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( diff --git a/src/timings/Timings.php b/src/timings/Timings.php index 2f23e7784..e62066ea4 100644 --- a/src/timings/Timings.php +++ b/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);