From 63310cf764039af3524d9e32b61c6b2d3dd3d9eb Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 20 Mar 2023 23:18:43 +0000 Subject: [PATCH] Do not cache ItemStacks for every item this is very memory inefficient, and only provides a performance advantage in cold code anyway. --- src/network/mcpe/InventoryManager.php | 19 ++++++++++--------- src/network/mcpe/InventoryManagerEntry.php | 2 +- src/network/mcpe/ItemStackInfo.php | 7 +------ .../mcpe/handler/InGamePacketHandler.php | 7 ++----- 4 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/network/mcpe/InventoryManager.php b/src/network/mcpe/InventoryManager.php index a6fe63635..5aa22b002 100644 --- a/src/network/mcpe/InventoryManager.php +++ b/src/network/mcpe/InventoryManager.php @@ -413,7 +413,7 @@ class InventoryManager{ if($clientSideItem === null || !$clientSideItem->equals($currentItem)){ //no prediction or incorrect - do not associate this with the currently active itemstack request $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($inventoryEntry, $slot, $currentItem, $this->currentItemStackRequestId); @@ -422,7 +422,7 @@ class InventoryManager{ 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"); @@ -439,7 +439,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. @@ -482,7 +482,7 @@ class InventoryManager{ foreach($inventory->getContents(true) as $slot => $item){ $itemStack = TypeConverter::getInstance()->coreItemStackToNet($item); $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){ @@ -513,6 +513,7 @@ class InventoryManager{ } public function syncMismatchedPredictedSlotChanges() : void{ + $typeConverter = TypeConverter::getInstance(); foreach($this->inventories as $entry){ $inventory = $entry->inventory; foreach($entry->predictions as $slot => $expectedItem){ @@ -522,7 +523,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 = []; @@ -541,8 +542,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 = []; } @@ -571,7 +572,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 @@ -604,7 +605,7 @@ class InventoryManager{ 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/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 68e939195..67d7ebb65 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -394,14 +394,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; }