diff --git a/changelogs/4.20.md b/changelogs/4.20.md index 357cdaced..d8f4c66b0 100644 --- a/changelogs/4.20.md +++ b/changelogs/4.20.md @@ -34,3 +34,13 @@ Released 26th April 2023. ### `pocketmine\player` - The following API methods have been added: - `public Player->openSignEditor(Vector3 $position) : void` - opens the client-side sign editor GUI for the given position + +# 4.20.1 +Released 27th April 2023. + +## Fixes +- Fixed server crash when firing a bow while holding arrows in the offhand slot. + +## Internals +- `ItemStackContainerIdTranslator::translate()` now requires an additional `int $slotId` parameter and returns `array{int, int}` (translated window ID, translated slot ID) to be used with `InventoryManager->locateWindowAndSlot()`. +- `InventoryManager->locateWindowAndSlot()` now checks if the translated slot actually exists in the requested inventory, and returns `null` if not. Previously, it would return potentially invalid slot IDs without checking them, potentially leading to crashes. diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 8f31f6509..08582cc58 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -31,7 +31,7 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; - public const BASE_VERSION = "4.20.1"; + public const BASE_VERSION = "4.20.2"; public const IS_DEVELOPMENT_BUILD = true; public const BUILD_CHANNEL = "stable"; diff --git a/src/inventory/BaseInventory.php b/src/inventory/BaseInventory.php index daf41c579..9968c283e 100644 --- a/src/inventory/BaseInventory.php +++ b/src/inventory/BaseInventory.php @@ -219,6 +219,7 @@ abstract class BaseInventory implements Inventory{ $item = $this->getItem($i); if($item->isNull()){ $emptySlots[] = $i; + continue; } if($slot->canStackWith($item) && $item->getCount() < $item->getMaxStackSize()){ diff --git a/src/network/mcpe/InventoryManager.php b/src/network/mcpe/InventoryManager.php index a651b9213..c6d83c65e 100644 --- a/src/network/mcpe/InventoryManager.php +++ b/src/network/mcpe/InventoryManager.php @@ -205,11 +205,13 @@ class InventoryManager{ if($entry === null){ return null; } + $inventory = $entry->getInventory(); $coreSlotId = $entry->mapNetToCore($netSlotId); - return $coreSlotId !== null ? [$entry->getInventory(), $coreSlotId] : null; + return $coreSlotId !== null && $inventory->slotExists($coreSlotId) ? [$inventory, $coreSlotId] : null; } - if(isset($this->networkIdToInventoryMap[$windowId])){ - return [$this->networkIdToInventoryMap[$windowId], $netSlotId]; + $inventory = $this->networkIdToInventoryMap[$windowId] ?? null; + if($inventory !== null && $inventory->slotExists($netSlotId)){ + return [$inventory, $netSlotId]; } return null; } diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 732c8817d..329bcd6ef 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -356,8 +356,8 @@ class InGamePacketHandler extends PacketHandler{ //rejects the transaction. The most common example of this is equipping armor by right-click, which doesn't send //a legacy prediction action for the destination armor slot. foreach($packet->requestChangedSlots as $containerInfo){ - $windowId = ItemStackContainerIdTranslator::translate($containerInfo->getContainerId(), $this->inventoryManager->getCurrentWindowId()); - foreach($containerInfo->getChangedSlotIndexes() as $slot){ + foreach($containerInfo->getChangedSlotIndexes() as $netSlot){ + [$windowId, $slot] = ItemStackContainerIdTranslator::translate($containerInfo->getContainerId(), $this->inventoryManager->getCurrentWindowId(), $netSlot); $inventoryAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slot); if($inventoryAndSlot !== null){ //trigger the normal slot sync logic $this->inventoryManager->onSlotChange($inventoryAndSlot[0], $inventoryAndSlot[1]); diff --git a/src/network/mcpe/handler/ItemStackContainerIdTranslator.php b/src/network/mcpe/handler/ItemStackContainerIdTranslator.php index a0ad3b26c..f9ea9ef5f 100644 --- a/src/network/mcpe/handler/ItemStackContainerIdTranslator.php +++ b/src/network/mcpe/handler/ItemStackContainerIdTranslator.php @@ -33,15 +33,21 @@ final class ItemStackContainerIdTranslator{ //NOOP } - public static function translate(int $containerInterfaceId, int $currentWindowId) : int{ + /** + * @return int[] + * @phpstan-return array{int, int} + * @throws PacketHandlingException + */ + public static function translate(int $containerInterfaceId, int $currentWindowId, int $slotId) : array{ return match($containerInterfaceId){ - ContainerUIIds::ARMOR => ContainerIds::ARMOR, + ContainerUIIds::ARMOR => [ContainerIds::ARMOR, $slotId], ContainerUIIds::HOTBAR, ContainerUIIds::INVENTORY, - ContainerUIIds::COMBINED_HOTBAR_AND_INVENTORY => ContainerIds::INVENTORY, + ContainerUIIds::COMBINED_HOTBAR_AND_INVENTORY => [ContainerIds::INVENTORY, $slotId], - ContainerUIIds::OFFHAND => ContainerIds::OFFHAND, + //TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70 (though this doesn't really matter since the offhand has only 1 slot anyway) + ContainerUIIds::OFFHAND => [ContainerIds::OFFHAND, 0], ContainerUIIds::ANVIL_INPUT, ContainerUIIds::ANVIL_MATERIAL, @@ -68,7 +74,7 @@ final class ItemStackContainerIdTranslator{ ContainerUIIds::TRADE2_INGREDIENT1, ContainerUIIds::TRADE2_INGREDIENT2, ContainerUIIds::TRADE_INGREDIENT1, - ContainerUIIds::TRADE_INGREDIENT2 => ContainerIds::UI, + ContainerUIIds::TRADE_INGREDIENT2 => [ContainerIds::UI, $slotId], ContainerUIIds::BARREL, ContainerUIIds::BLAST_FURNACE_INGREDIENT, @@ -80,7 +86,7 @@ final class ItemStackContainerIdTranslator{ ContainerUIIds::FURNACE_RESULT, ContainerUIIds::LEVEL_ENTITY, //chest ContainerUIIds::SHULKER_BOX, - ContainerUIIds::SMOKER_INGREDIENT => $currentWindowId, + ContainerUIIds::SMOKER_INGREDIENT => [$currentWindowId, $slotId], //all preview slots are ignored, since the client shouldn't be modifying those directly diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index b7f9b1604..f9532291c 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -111,12 +111,7 @@ class ItemStackRequestExecutor{ * @throws ItemStackRequestProcessException */ protected function getBuilderInventoryAndSlot(ItemStackRequestSlotInfo $info) : array{ - $windowId = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId()); - $slotId = $info->getSlotId(); - if($info->getContainerId() === ContainerUIIds::OFFHAND && $slotId === 1){ - //TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70 - $slotId = 0; - } + [$windowId, $slotId] = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId(), $info->getSlotId()); $windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slotId); if($windowAndSlot === null){ throw new ItemStackRequestProcessException("No open inventory matches container UI ID: " . $info->getContainerId() . ", slot ID: " . $info->getSlotId()); diff --git a/src/network/mcpe/handler/ItemStackResponseBuilder.php b/src/network/mcpe/handler/ItemStackResponseBuilder.php index 2a55c2d95..09af69f2a 100644 --- a/src/network/mcpe/handler/ItemStackResponseBuilder.php +++ b/src/network/mcpe/handler/ItemStackResponseBuilder.php @@ -53,11 +53,7 @@ final class ItemStackResponseBuilder{ * @phpstan-return array{Inventory, int} */ private function getInventoryAndSlot(int $containerInterfaceId, int $slotId) : ?array{ - if($containerInterfaceId === ContainerUIIds::OFFHAND && $slotId === 1){ - //TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70 - $slotId = 0; - } - $windowId = ItemStackContainerIdTranslator::translate($containerInterfaceId, $this->inventoryManager->getCurrentWindowId()); + [$windowId, $slotId] = ItemStackContainerIdTranslator::translate($containerInterfaceId, $this->inventoryManager->getCurrentWindowId(), $slotId); $windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slotId); if($windowAndSlot === null){ return null;