From b65b7a7f7497bc8e284eaedacbce61bd59c9ff88 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 16 Aug 2023 09:38:16 +0100 Subject: [PATCH 1/2] Bump tests/plugins/DevTools from `83f0db3` to `411fd5b` (#5998) Bumps [tests/plugins/DevTools](https://github.com/pmmp/DevTools) from `83f0db3` to `411fd5b`. - [Release notes](https://github.com/pmmp/DevTools/releases) - [Commits](https://github.com/pmmp/DevTools/compare/83f0db3f9e0adbf424e32ed81f7730e97b037be9...411fd5bdc002edd82cf1ea658d170bb814d59483) --- updated-dependencies: - dependency-name: tests/plugins/DevTools dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- tests/plugins/DevTools | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/plugins/DevTools b/tests/plugins/DevTools index 83f0db3f9..411fd5bdc 160000 --- a/tests/plugins/DevTools +++ b/tests/plugins/DevTools @@ -1 +1 @@ -Subproject commit 83f0db3f9e0adbf424e32ed81f7730e97b037be9 +Subproject commit 411fd5bdc002edd82cf1ea658d170bb814d59483 From 9f09acc07933d037af9c1f1f683ca0f3cd6a6215 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Fri, 18 Aug 2023 12:27:27 +0100 Subject: [PATCH 2/2] Workaround for slot IDs not changing client side when old item == new item this is a really dumb bug and seems similar to the armor bug I fixed a while ago. fixes #5987 it's unlikely that #5727 will be solved by this, but one can hope... --- src/network/mcpe/InventoryManager.php | 73 ++++++++++++++++++--------- 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/src/network/mcpe/InventoryManager.php b/src/network/mcpe/InventoryManager.php index c6d83c65e..9982ad8d3 100644 --- a/src/network/mcpe/InventoryManager.php +++ b/src/network/mcpe/InventoryManager.php @@ -59,6 +59,7 @@ use pocketmine\network\PacketHandlingException; use pocketmine\player\Player; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\ObjectSet; +use function array_fill_keys; use function array_keys; use function array_search; use function count; @@ -429,6 +430,50 @@ class InventoryManager{ unset($inventoryEntry->predictions[$slot]); } + private function sendInventorySlotPackets(int $windowId, int $netSlot, ItemStackWrapper $itemStackWrapper) : void{ + /* + * TODO: HACK! + * As of 1.20.12, the client ignores change of itemstackID in some cases when the old item == the new item. + * Notably, this happens with armor, offhand and enchanting tables, but not with main inventory. + * While we could track the items previously sent to the client, that's a waste of memory and would + * cost performance. Instead, clear the slot(s) first, then send the new item(s). + * The network cost of doing this is fortunately minimal, as an air itemstack is only 1 byte. + */ + if($itemStackWrapper->getStackId() !== 0){ + $this->session->sendDataPacket(InventorySlotPacket::create( + $windowId, + $netSlot, + new ItemStackWrapper(0, ItemStack::null()) + )); + } + //now send the real contents + $this->session->sendDataPacket(InventorySlotPacket::create( + $windowId, + $netSlot, + $itemStackWrapper + )); + } + + /** + * @param ItemStackWrapper[] $itemStackWrappers + */ + private function sendInventoryContentPackets(int $windowId, array $itemStackWrappers) : void{ + /* + * TODO: HACK! + * As of 1.20.12, the client ignores change of itemstackID in some cases when the old item == the new item. + * Notably, this happens with armor, offhand and enchanting tables, but not with main inventory. + * While we could track the items previously sent to the client, that's a waste of memory and would + * cost performance. Instead, clear the slot(s) first, then send the new item(s). + * The network cost of doing this is fortunately minimal, as an air itemstack is only 1 byte. + */ + $this->session->sendDataPacket(InventoryContentPacket::create( + $windowId, + array_fill_keys(array_keys($itemStackWrappers), new ItemStackWrapper(0, ItemStack::null())) + )); + //now send the real contents + $this->session->sendDataPacket(InventoryContentPacket::create($windowId, $itemStackWrappers)); + } + public function syncSlot(Inventory $inventory, int $slot, ItemStack $itemStack) : void{ $entry = $this->inventories[spl_object_id($inventory)] ?? null; if($entry === null){ @@ -453,24 +498,9 @@ class InventoryManager{ //This can cause a lot of problems (totems, arrows, and more...). //The workaround is to send an InventoryContentPacket instead //BDS (Bedrock Dedicated Server) also seems to work this way. - $this->session->sendDataPacket(InventoryContentPacket::create($windowId, [$itemStackWrapper])); + $this->sendInventoryContentPackets($windowId, [$itemStackWrapper]); }else{ - if($windowId === ContainerIds::ARMOR){ - //TODO: HACK! - //When right-clicking to equip armour, the client predicts the content of the armour slot, but - //doesn't report it in the transaction packet. The server then sends an InventorySlotPacket to - //the client, assuming the slot changed for some other reason, since there is no prediction for - //the slot. - //However, later requests involving that itemstack will refer to the request ID in which the - //armour was equipped, instead of the stack ID provided by the server in the outgoing - //InventorySlotPacket. (Perhaps because the item is already the same as the client actually - //predicted, but didn't tell us?) - //We work around this bug by setting the slot to air and then back to the correct item. In - //theory, setting a different count and then back again (or changing any other property) would - //also work, but this is simpler. - $this->session->sendDataPacket(InventorySlotPacket::create($windowId, $netSlot, new ItemStackWrapper(0, ItemStack::null()))); - } - $this->session->sendDataPacket(InventorySlotPacket::create($windowId, $netSlot, $itemStackWrapper)); + $this->sendInventorySlotPackets($windowId, $netSlot, $itemStackWrapper); } unset($entry->predictions[$slot], $entry->pendingSyncs[$slot]); } @@ -497,20 +527,17 @@ class InventoryManager{ $info = $this->trackItemStack($entry, $slot, $itemStack, null); $contents[] = new ItemStackWrapper($info->getStackId(), $itemStack); } + $clearSlotWrapper = new ItemStackWrapper(0, ItemStack::null()); if($entry->complexSlotMap !== null){ foreach($contents as $slotId => $info){ $packetSlot = $entry->complexSlotMap->mapCoreToNet($slotId) ?? null; if($packetSlot === null){ continue; } - $this->session->sendDataPacket(InventorySlotPacket::create( - $windowId, - $packetSlot, - $info - )); + $this->sendInventorySlotPackets($windowId, $packetSlot, $info); } }else{ - $this->session->sendDataPacket(InventoryContentPacket::create($windowId, $contents)); + $this->sendInventoryContentPackets($windowId, $contents); } } }