diff --git a/composer.json b/composer.json index 055d75e08..442598875 100644 --- a/composer.json +++ b/composer.json @@ -37,7 +37,7 @@ "pocketmine/bedrock-block-upgrade-schema": "~1.1.1+bedrock-1.19.70", "pocketmine/bedrock-data": "~2.1.1+bedrock-1.19.70", "pocketmine/bedrock-item-upgrade-schema": "~1.1.0+bedrock-1.19.70", - "pocketmine/bedrock-protocol": "~20.0.0+bedrock-1.19.70", + "pocketmine/bedrock-protocol": "~20.1.0+bedrock-1.19.70", "pocketmine/binaryutils": "^0.2.1", "pocketmine/callback-validator": "^1.0.2", "pocketmine/classloader": "^0.3.0", diff --git a/composer.lock b/composer.lock index e03cb4d66..dfbc6b1c0 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "500661d6d723c3853f8e3b66c838a66b", + "content-hash": "43708f57042d7d40c732c3a3b87a4202", "packages": [ { "name": "adhocore/json-comment", @@ -328,16 +328,16 @@ }, { "name": "pocketmine/bedrock-protocol", - "version": "20.0.0+bedrock-1.19.70", + "version": "20.1.0+bedrock-1.19.70", "source": { "type": "git", "url": "https://github.com/pmmp/BedrockProtocol.git", - "reference": "4892a5020187da805d7b46ab522d8185b0283726" + "reference": "91d67c8b1bced3c82d0841b1041c0c1f4e93eb68" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/pmmp/BedrockProtocol/zipball/4892a5020187da805d7b46ab522d8185b0283726", - "reference": "4892a5020187da805d7b46ab522d8185b0283726", + "url": "https://api.github.com/repos/pmmp/BedrockProtocol/zipball/91d67c8b1bced3c82d0841b1041c0c1f4e93eb68", + "reference": "91d67c8b1bced3c82d0841b1041c0c1f4e93eb68", "shasum": "" }, "require": { @@ -351,7 +351,7 @@ "ramsey/uuid": "^4.1" }, "require-dev": { - "phpstan/phpstan": "1.10.1", + "phpstan/phpstan": "1.10.7", "phpstan/phpstan-phpunit": "^1.0.0", "phpstan/phpstan-strict-rules": "^1.0.0", "phpunit/phpunit": "^9.5" @@ -369,9 +369,9 @@ "description": "An implementation of the Minecraft: Bedrock Edition protocol in PHP", "support": { "issues": "https://github.com/pmmp/BedrockProtocol/issues", - "source": "https://github.com/pmmp/BedrockProtocol/tree/20.0.0+bedrock-1.19.70" + "source": "https://github.com/pmmp/BedrockProtocol/tree/20.1.0+bedrock-1.19.70" }, - "time": "2023-03-14T17:06:38+00:00" + "time": "2023-03-20T01:17:00+00:00" }, { "name": "pocketmine/binaryutils", diff --git a/src/crafting/CraftingManager.php b/src/crafting/CraftingManager.php index 4f34b291b..42d11ce9f 100644 --- a/src/crafting/CraftingManager.php +++ b/src/crafting/CraftingManager.php @@ -173,6 +173,10 @@ class CraftingManager{ return $this->craftingRecipeIndex; } + public function getCraftingRecipeFromIndex(int $index) : ?CraftingRecipe{ + return $this->craftingRecipeIndex[$index] ?? null; + } + public function getFurnaceRecipeManager(FurnaceType $furnaceType) : FurnaceRecipeManager{ return $this->furnaceRecipeManagers[$furnaceType->id()]; } diff --git a/src/inventory/transaction/TransactionBuilderInventory.php b/src/inventory/transaction/TransactionBuilderInventory.php index 4995284a0..95b6c4a14 100644 --- a/src/inventory/transaction/TransactionBuilderInventory.php +++ b/src/inventory/transaction/TransactionBuilderInventory.php @@ -50,6 +50,10 @@ final class TransactionBuilderInventory extends BaseInventory{ $this->changedSlots = new \SplFixedArray($this->actualInventory->getSize()); } + public function getActualInventory() : Inventory{ + return $this->actualInventory; + } + protected function internalSetContents(array $items) : void{ for($i = 0, $size = $this->getSize(); $i < $size; ++$i){ if(!isset($items[$i])){ diff --git a/src/network/mcpe/ComplexWindowMapEntry.php b/src/network/mcpe/ComplexInventoryMapEntry.php similarity index 97% rename from src/network/mcpe/ComplexWindowMapEntry.php rename to src/network/mcpe/ComplexInventoryMapEntry.php index c2792297b..dfd3e999a 100644 --- a/src/network/mcpe/ComplexWindowMapEntry.php +++ b/src/network/mcpe/ComplexInventoryMapEntry.php @@ -25,7 +25,7 @@ namespace pocketmine\network\mcpe; use pocketmine\inventory\Inventory; -final class ComplexWindowMapEntry{ +final class ComplexInventoryMapEntry{ /** * @var int[] diff --git a/src/network/mcpe/InventoryManager.php b/src/network/mcpe/InventoryManager.php index f4fa76482..28495f67e 100644 --- a/src/network/mcpe/InventoryManager.php +++ b/src/network/mcpe/InventoryManager.php @@ -61,8 +61,11 @@ use pocketmine\network\PacketHandlingException; use pocketmine\player\Player; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\ObjectSet; +use function array_keys; use function array_search; +use function count; use function get_class; +use function implode; use function is_int; use function max; use function spl_object_id; @@ -71,26 +74,25 @@ use function spl_object_id; * @phpstan-type ContainerOpenClosure \Closure(int $id, Inventory $inventory) : (list|null) */ class InventoryManager{ - /** @var Inventory[] */ - private array $windowMap = []; /** - * @var ComplexWindowMapEntry[] - * @phpstan-var array + * @var InventoryManagerEntry[] spl_object_id(Inventory) => InventoryManagerEntry + * @phpstan-var array */ - private array $complexWindows = []; + private array $inventories = []; + /** - * @var ComplexWindowMapEntry[] - * @phpstan-var array + * @var Inventory[] network window ID => Inventory + * @phpstan-var array */ - private array $complexSlotToWindowMap = []; + private array $networkIdToInventoryMap = []; + /** + * @var ComplexInventoryMapEntry[] net slot ID => ComplexWindowMapEntry + * @phpstan-var array + */ + private array $complexSlotToInventoryMap = []; private int $lastInventoryNetworkId = ContainerIds::FIRST; - /** - * @var Item[][] - * @phpstan-var array - */ - private array $initiatedSlotChanges = []; private int $clientSelectedHotbarSlot = -1; /** @phpstan-var ObjectSet */ @@ -103,11 +105,7 @@ class InventoryManager{ private int $nextItemStackId = 1; private ?int $currentItemStackRequestId = null; - /** - * @var int[][] - * @phpstan-var array> - */ - private array $itemStackInfos = []; + private bool $fullSyncRequested = false; public function __construct( private Player $player, @@ -127,14 +125,27 @@ class InventoryManager{ }); } + private function associateIdWithInventory(int $id, Inventory $inventory) : void{ + $this->networkIdToInventoryMap[$id] = $inventory; + } + + private function getNewWindowId() : int{ + $this->lastInventoryNetworkId = max(ContainerIds::FIRST, ($this->lastInventoryNetworkId + 1) % ContainerIds::LAST); + return $this->lastInventoryNetworkId; + } + private function add(int $id, Inventory $inventory) : void{ - $this->windowMap[$id] = $inventory; + if(isset($this->inventories[spl_object_id($inventory)])){ + throw new \InvalidArgumentException("Inventory " . get_class($inventory) . " is already tracked"); + } + $this->inventories[spl_object_id($inventory)] = new InventoryManagerEntry($inventory); + $this->associateIdWithInventory($id, $inventory); } private function addDynamic(Inventory $inventory) : int{ - $this->lastInventoryNetworkId = max(ContainerIds::FIRST, ($this->lastInventoryNetworkId + 1) % ContainerIds::LAST); - $this->add($this->lastInventoryNetworkId, $inventory); - return $this->lastInventoryNetworkId; + $id = $this->getNewWindowId(); + $this->add($id, $inventory); + return $id; } /** @@ -142,29 +153,45 @@ class InventoryManager{ * @phpstan-param array|int $slotMap */ private function addComplex(array|int $slotMap, Inventory $inventory) : void{ - $entry = new ComplexWindowMapEntry($inventory, is_int($slotMap) ? [$slotMap => 0] : $slotMap); - $this->complexWindows[spl_object_id($inventory)] = $entry; - foreach($entry->getSlotMap() as $netSlot => $coreSlot){ - $this->complexSlotToWindowMap[$netSlot] = $entry; + if(isset($this->inventories[spl_object_id($inventory)])){ + throw new \InvalidArgumentException("Inventory " . get_class($inventory) . " is already tracked"); + } + $complexSlotMap = new ComplexInventoryMapEntry($inventory, is_int($slotMap) ? [$slotMap => 0] : $slotMap); + $this->inventories[spl_object_id($inventory)] = new InventoryManagerEntry( + $inventory, + $complexSlotMap + ); + foreach($complexSlotMap->getSlotMap() as $netSlot => $coreSlot){ + $this->complexSlotToInventoryMap[$netSlot] = $complexSlotMap; } } + /** + * @param int[]|int $slotMap + * @phpstan-param array|int $slotMap + */ + private function addComplexDynamic(array|int $slotMap, Inventory $inventory) : int{ + $this->addComplex($slotMap, $inventory); + $id = $this->getNewWindowId(); + $this->associateIdWithInventory($id, $inventory); + return $id; + } + private function remove(int $id) : void{ - $inventory = $this->windowMap[$id]; - unset($this->windowMap[$id]); + $inventory = $this->networkIdToInventoryMap[$id]; + unset($this->networkIdToInventoryMap[$id]); if($this->getWindowId($inventory) === null){ - $splObjectId = spl_object_id($inventory); - unset($this->initiatedSlotChanges[$splObjectId], $this->itemStackInfos[$splObjectId], $this->complexWindows[$splObjectId]); - foreach($this->complexSlotToWindowMap as $netSlot => $entry){ + unset($this->inventories[spl_object_id($inventory)]); + foreach($this->complexSlotToInventoryMap as $netSlot => $entry){ if($entry->getInventory() === $inventory){ - unset($this->complexSlotToWindowMap[$netSlot]); + unset($this->complexSlotToInventoryMap[$netSlot]); } } } } public function getWindowId(Inventory $inventory) : ?int{ - return ($id = array_search($inventory, $this->windowMap, true)) !== false ? $id : null; + return ($id = array_search($inventory, $this->networkIdToInventoryMap, true)) !== false ? $id : null; } public function getCurrentWindowId() : int{ @@ -176,22 +203,21 @@ class InventoryManager{ */ public function locateWindowAndSlot(int $windowId, int $netSlotId) : ?array{ if($windowId === ContainerIds::UI){ - $entry = $this->complexSlotToWindowMap[$netSlotId] ?? null; + $entry = $this->complexSlotToInventoryMap[$netSlotId] ?? null; if($entry === null){ return null; } $coreSlotId = $entry->mapNetToCore($netSlotId); return $coreSlotId !== null ? [$entry->getInventory(), $coreSlotId] : null; } - if(isset($this->windowMap[$windowId])){ - return [$this->windowMap[$windowId], $netSlotId]; + if(isset($this->networkIdToInventoryMap[$windowId])){ + return [$this->networkIdToInventoryMap[$windowId], $netSlotId]; } return null; } private function addPredictedSlotChange(Inventory $inventory, int $slot, ItemStack $item) : void{ - $predictions = ($this->initiatedSlotChanges[spl_object_id($inventory)] ??= new InventoryManagerPredictedChanges($inventory)); - $predictions->add($slot, $item); + $this->inventories[spl_object_id($inventory)]->predictions[$slot] = $item; } public function addTransactionPredictedSlotChanges(InventoryTransaction $tx) : void{ @@ -281,9 +307,10 @@ class InventoryManager{ $this->onCurrentWindowRemove(); $this->openWindowDeferred(function() use ($inventory) : void{ - $windowId = $this->addDynamic($inventory); if(($slotMap = $this->createComplexSlotMapping($inventory)) !== null){ - $this->addComplex($slotMap, $inventory); + $windowId = $this->addComplexDynamic($slotMap, $inventory); + }else{ + $windowId = $this->addDynamic($inventory); } foreach($this->containerOpenCallbacks as $callback){ @@ -339,7 +366,8 @@ class InventoryManager{ $this->onCurrentWindowRemove(); $this->openWindowDeferred(function() : void{ - $windowId = $this->addDynamic($this->player->getInventory()); + $windowId = $this->getNewWindowId(); + $this->associateIdWithInventory($windowId, $this->player->getInventory()); $this->session->sendDataPacket(ContainerOpenPacket::entityInv( $windowId, @@ -350,7 +378,7 @@ class InventoryManager{ } public function onCurrentWindowRemove() : void{ - if(isset($this->windowMap[$this->lastInventoryNetworkId])){ + if(isset($this->networkIdToInventoryMap[$this->lastInventoryNetworkId])){ $this->remove($this->lastInventoryNetworkId); $this->session->sendDataPacket(ContainerClosePacket::create($this->lastInventoryNetworkId, true)); if($this->pendingCloseWindowId !== null){ @@ -362,7 +390,7 @@ class InventoryManager{ public function onClientRemoveWindow(int $id) : void{ if($id === $this->lastInventoryNetworkId){ - if(isset($this->windowMap[$id]) && $id !== $this->pendingCloseWindowId){ + if(isset($this->networkIdToInventoryMap[$id]) && $id !== $this->pendingCloseWindowId){ $this->remove($id); $this->player->removeCurrentWindow(); } @@ -386,28 +414,32 @@ class InventoryManager{ public function onSlotChange(Inventory $inventory, int $slot) : void{ $currentItem = TypeConverter::getInstance()->coreItemStackToNet($inventory->getItem($slot)); - $predictions = $this->initiatedSlotChanges[spl_object_id($inventory)] ?? null; - $clientSideItem = $predictions?->getSlot($slot); + $inventoryEntry = $this->inventories[spl_object_id($inventory)]; + $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->syncSlot($inventory, $slot); + $inventoryEntry->pendingSyncs[$slot] = $slot; }else{ //correctly predicted - associate the change with the currently active itemstack request $this->trackItemStack($inventory, $slot, $currentItem, $this->currentItemStackRequestId); } - $predictions?->remove($slot); + + unset($inventoryEntry->predictions[$slot]); } public function syncSlot(Inventory $inventory, int $slot) : void{ - $itemStackInfo = $this->getItemStackInfo($inventory, $slot); + $entry = $this->inventories[spl_object_id($inventory)] ?? null; + if($entry === null){ + throw new \LogicException("Cannot sync an untracked inventory"); + } + $itemStackInfo = $entry->itemStackInfos[$slot]; if($itemStackInfo === null){ throw new \LogicException("Cannot sync an untracked inventory slot"); } - $slotMap = $this->complexWindows[spl_object_id($inventory)] ?? null; - if($slotMap !== null){ + if($entry->complexSlotMap !== null){ $windowId = ContainerIds::UI; - $netSlot = $slotMap->mapCoreToNet($slot) ?? throw new AssumptionFailedError("We already have an ItemStackInfo, so this should not be null"); + $netSlot = $entry->complexSlotMap->mapCoreToNet($slot) ?? throw new AssumptionFailedError("We already have an ItemStackInfo, so this should not be null"); }else{ $windowId = $this->getWindowId($inventory) ?? throw new AssumptionFailedError("We already have an ItemStackInfo, so this should not be null"); $netSlot = $slot; @@ -422,7 +454,7 @@ class InventoryManager{ //BDS (Bedrock Dedicated Server) also seems to work this way. $this->session->sendDataPacket(InventoryContentPacket::create($windowId, [$itemStackWrapper])); }else{ - if($this->currentItemStackRequestId !== null){ + 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 @@ -439,28 +471,28 @@ class InventoryManager{ } $this->session->sendDataPacket(InventorySlotPacket::create($windowId, $netSlot, $itemStackWrapper)); } - $predictions = $this->initiatedSlotChanges[spl_object_id($inventory)] ?? null; - $predictions?->remove($slot); + unset($entry->predictions[$slot], $entry->pendingSyncs[$slot]); } public function syncContents(Inventory $inventory) : void{ - $slotMap = $this->complexWindows[spl_object_id($inventory)] ?? null; - if($slotMap !== null){ + $entry = $this->inventories[spl_object_id($inventory)]; + if($entry->complexSlotMap !== null){ $windowId = ContainerIds::UI; }else{ $windowId = $this->getWindowId($inventory); } if($windowId !== null){ - unset($this->initiatedSlotChanges[spl_object_id($inventory)]); + $entry->predictions = []; + $entry->pendingSyncs = []; $contents = []; 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()); } - if($slotMap !== null){ + if($entry->complexSlotMap !== null){ foreach($contents as $slotId => $info){ - $packetSlot = $slotMap->mapCoreToNet($slotId) ?? null; + $packetSlot = $entry->complexSlotMap->mapCoreToNet($slotId) ?? null; if($packetSlot === null){ continue; } @@ -477,29 +509,50 @@ class InventoryManager{ } public function syncAll() : void{ - foreach($this->windowMap as $inventory){ - $this->syncContents($inventory); - } - foreach($this->complexWindows as $entry){ - $this->syncContents($entry->getInventory()); + foreach($this->inventories as $entry){ + $this->syncContents($entry->inventory); } } + public function requestSyncAll() : void{ + $this->fullSyncRequested = true; + } + public function syncMismatchedPredictedSlotChanges() : void{ - foreach($this->initiatedSlotChanges as $predictions){ - $inventory = $predictions->getInventory(); - foreach($predictions->getSlots() as $slot => $expectedItem){ - if(!$inventory->slotExists($slot) || $this->getItemStackInfo($inventory, $slot) === null){ + foreach($this->inventories as $entry){ + $inventory = $entry->inventory; + foreach($entry->predictions as $slot => $expectedItem){ + if(!$inventory->slotExists($slot) || $entry->itemStackInfos[$slot] === null){ continue; //TODO: size desync ??? } //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"); - $this->syncSlot($inventory, $slot); + $entry->pendingSyncs[$slot] = $slot; + } + + $entry->predictions = []; + } + } + + public function flushPendingUpdates() : void{ + if($this->fullSyncRequested){ + $this->fullSyncRequested = false; + $this->session->getLogger()->debug("Full inventory sync requested, sending contents of " . count($this->inventories) . " inventories"); + $this->syncAll(); + }else{ + foreach($this->inventories as $entry){ + if(count($entry->pendingSyncs) === 0){ + continue; + } + $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); + } + $entry->pendingSyncs = []; } } - - $this->initiatedSlotChanges = []; } public function syncData(Inventory $inventory, int $propertyId, int $value) : void{ @@ -517,7 +570,10 @@ class InventoryManager{ $playerInventory = $this->player->getInventory(); $selected = $playerInventory->getHeldItemIndex(); if($selected !== $this->clientSelectedHotbarSlot){ - $itemStackInfo = $this->itemStackInfos[spl_object_id($playerInventory)][$selected]; + $itemStackInfo = $this->getItemStackInfo($playerInventory, $selected); + if($itemStackInfo === null){ + throw new AssumptionFailedError("Player inventory slots should always be tracked"); + } $this->session->sendDataPacket(MobEquipmentPacket::create( $this->player->getId(), @@ -548,40 +604,22 @@ class InventoryManager{ } public function getItemStackInfo(Inventory $inventory, int $slot) : ?ItemStackInfo{ - return $this->itemStackInfos[spl_object_id($inventory)][$slot] ?? null; + $entry = $this->inventories[spl_object_id($inventory)] ?? null; + return $entry?->itemStackInfos[$slot] ?? null; } private function trackItemStack(Inventory $inventory, int $slotId, ItemStack $itemStack, ?int $itemStackRequestId) : ItemStackInfo{ - $existing = $this->itemStackInfos[spl_object_id($inventory)][$slotId] ?? null; + $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); - return $this->itemStackInfos[spl_object_id($inventory)][$slotId] = $info; - } - - public function matchItemStack(Inventory $inventory, int $slotId, int $clientItemStackId) : bool{ - $inventoryObjectId = spl_object_id($inventory); - if(!isset($this->itemStackInfos[$inventoryObjectId])){ - $this->session->getLogger()->debug("Attempted to match item preimage unsynced inventory " . get_class($inventory) . "#" . $inventoryObjectId); - return false; - } - $info = $this->itemStackInfos[$inventoryObjectId][$slotId] ?? null; - if($info === null){ - $this->session->getLogger()->debug("Attempted to match item preimage for unsynced slot $slotId in " . get_class($inventory) . "#$inventoryObjectId that isn't synced"); - return false; - } - - if(!($clientItemStackId < 0 ? $info->getRequestId() === $clientItemStackId : $info->getStackId() === $clientItemStackId)){ - $this->session->getLogger()->debug( - "Mismatched expected itemstack: " . get_class($inventory) . "#" . $inventoryObjectId . ", " . - "slot: $slotId, client expected: $clientItemStackId, server actual: " . $info->getStackId() . ", last modified by request: " . ($info->getRequestId() ?? "none") - ); - return false; - } - - return true; + return $entry->itemStackInfos[$slotId] = $info; } } diff --git a/src/network/mcpe/InventoryManagerPredictedChanges.php b/src/network/mcpe/InventoryManagerEntry.php similarity index 63% rename from src/network/mcpe/InventoryManagerPredictedChanges.php rename to src/network/mcpe/InventoryManagerEntry.php index 8264e83fb..fa49baf87 100644 --- a/src/network/mcpe/InventoryManagerPredictedChanges.php +++ b/src/network/mcpe/InventoryManagerEntry.php @@ -26,36 +26,27 @@ namespace pocketmine\network\mcpe; use pocketmine\inventory\Inventory; use pocketmine\network\mcpe\protocol\types\inventory\ItemStack; -final class InventoryManagerPredictedChanges{ +final class InventoryManagerEntry{ /** * @var ItemStack[] * @phpstan-var array */ - private array $slots = []; - - public function __construct( - private Inventory $inventory - ){} - - public function getInventory() : Inventory{ return $this->inventory; } + public array $predictions = []; /** - * @return ItemStack[] - * @phpstan-return array + * @var ItemStackInfo[] + * @phpstan-var array */ - public function getSlots() : array{ - return $this->slots; - } + public array $itemStackInfos = []; - public function getSlot(int $slot) : ?ItemStack{ - return $this->slots[$slot] ?? null; - } + /** + * @var int[] + * @phpstan-var array + */ + public array $pendingSyncs = []; - public function add(int $slot, ItemStack $item) : void{ - $this->slots[$slot] = $item; - } - - public function remove(int $slot) : void{ - unset($this->slots[$slot]); - } + public function __construct( + public Inventory $inventory, + public ?ComplexInventoryMapEntry $complexSlotMap = null + ){} } diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 355c5c6bd..33c45fd2d 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -1264,6 +1264,7 @@ class NetworkSession{ $attribute->markSynchronized(); } } + $this->invManager?->flushPendingUpdates(); $this->flushSendBuffer(); } diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index ed00dbab9..3ae1bd5a2 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -33,10 +33,10 @@ use pocketmine\entity\Attribute; use pocketmine\entity\InvalidSkinException; use pocketmine\event\player\PlayerEditBookEvent; use pocketmine\inventory\transaction\action\DropItemAction; -use pocketmine\inventory\transaction\CraftingTransaction; use pocketmine\inventory\transaction\InventoryTransaction; use pocketmine\inventory\transaction\TransactionBuilder; -use pocketmine\inventory\transaction\TransactionException; +use pocketmine\inventory\transaction\TransactionCancelledException; +use pocketmine\inventory\transaction\TransactionValidationException; use pocketmine\item\VanillaItems; use pocketmine\item\WritableBook; use pocketmine\item\WritableBookPage; @@ -110,11 +110,13 @@ use pocketmine\player\Player; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\Limits; use pocketmine\utils\TextFormat; +use pocketmine\utils\Utils; use pocketmine\world\format\Chunk; use function array_push; use function base64_encode; use function count; use function fmod; +use function implode; use function in_array; use function is_bool; use function is_infinite; @@ -134,8 +136,6 @@ use const JSON_THROW_ON_ERROR; class InGamePacketHandler extends PacketHandler{ private const MAX_FORM_RESPONSE_DEPTH = 2; //modal/simple will be 1, custom forms 2 - they will never contain anything other than string|int|float|bool|null - protected ?CraftingTransaction $craftingTransaction = null; - protected float $lastRightClickTime = 0.0; protected ?UseItemTransactionData $lastRightClickData = null; @@ -333,7 +333,7 @@ class InGamePacketHandler extends PacketHandler{ $result = $this->handleNormalTransaction($packet->trData, $packet->requestId); }elseif($packet->trData instanceof MismatchTransactionData){ $this->session->getLogger()->debug("Mismatch transaction received"); - $this->inventoryManager->syncAll(); + $this->inventoryManager->requestSyncAll(); $result = true; }elseif($packet->trData instanceof UseItemTransactionData){ $result = $this->handleUseItemTransaction($packet->trData); @@ -343,9 +343,7 @@ class InGamePacketHandler extends PacketHandler{ $result = $this->handleReleaseItemTransaction($packet->trData); } - if($this->craftingTransaction === null){ //don't sync if we're waiting to complete a crafting transaction - $this->inventoryManager->syncMismatchedPredictedSlotChanges(); - } + $this->inventoryManager->syncMismatchedPredictedSlotChanges(); $this->inventoryManager->setCurrentItemStackRequestId(null); return $result; } @@ -357,9 +355,14 @@ class InGamePacketHandler extends PacketHandler{ $this->inventoryManager->addTransactionPredictedSlotChanges($transaction); try{ $transaction->execute(); - }catch(TransactionException $e){ + }catch(TransactionValidationException $e){ + $this->inventoryManager->requestSyncAll(); $logger = $this->session->getLogger(); - $logger->debug("Failed to execute inventory transaction: " . $e->getMessage()); + $logger->debug("Invalid inventory transaction $requestId: " . $e->getMessage()); + + return false; + }catch(TransactionCancelledException){ + $this->session->getLogger()->debug("Inventory transaction $requestId cancelled by a plugin"); return false; }finally{ @@ -526,16 +529,33 @@ class InGamePacketHandler extends PacketHandler{ } private function handleSingleItemStackRequest(ItemStackRequest $request) : ItemStackResponse{ + if(count($request->getActions()) > 20){ + //TODO: we can probably lower this limit, but this will do for now + throw new PacketHandlingException("Too many actions in ItemStackRequest"); + } $executor = new ItemStackRequestExecutor($this->player, $this->inventoryManager, $request); - $transaction = $executor->generateInventoryTransaction(); - $result = $this->executeInventoryTransaction($transaction, $request->getRequestId()); - $this->session->getLogger()->debug("Item stack request " . $request->getRequestId() . " result: " . ($result ? "success" : "failure")); + try{ + $transaction = $executor->generateInventoryTransaction(); + $result = $this->executeInventoryTransaction($transaction, $request->getRequestId()); + }catch(ItemStackRequestProcessException $e){ + $result = false; + $this->session->getLogger()->debug("ItemStackRequest #" . $request->getRequestId() . " failed: " . $e->getMessage()); + $this->session->getLogger()->debug(implode("\n", Utils::printableExceptionInfo($e))); + $this->inventoryManager->requestSyncAll(); + } - return $executor->buildItemStackResponse($result); + if(!$result){ + return new ItemStackResponse(ItemStackResponse::RESULT_ERROR, $request->getRequestId()); + } + return $executor->buildItemStackResponse(); } public function handleItemStackRequest(ItemStackRequestPacket $packet) : bool{ $responses = []; + if(count($packet->getRequests()) > 80){ + //TODO: we can probably lower this limit, but this will do for now + throw new PacketHandlingException("Too many requests in ItemStackRequestPacket"); + } foreach($packet->getRequests() as $request){ $responses[] = $this->handleSingleItemStackRequest($request); } diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index be7e472ac..15f1776f0 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -25,6 +25,7 @@ namespace pocketmine\network\mcpe\handler; use pocketmine\crafting\CraftingGrid; use pocketmine\inventory\CreativeInventory; +use pocketmine\inventory\Inventory; use pocketmine\inventory\transaction\action\CreateItemAction; use pocketmine\inventory\transaction\action\DestroyItemAction; use pocketmine\inventory\transaction\action\DropItemAction; @@ -51,12 +52,11 @@ use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\SwapStackReque use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\TakeStackRequestAction; use pocketmine\network\mcpe\protocol\types\inventory\stackresponse\ItemStackResponse; use pocketmine\network\mcpe\protocol\types\inventory\UIInventorySlotOffset; -use pocketmine\network\PacketHandlingException; use pocketmine\player\Player; use pocketmine\utils\AssumptionFailedError; use function array_key_first; use function count; -use function get_class; +use function spl_object_id; final class ItemStackRequestExecutor{ private TransactionBuilder $builder; @@ -81,25 +81,49 @@ final class ItemStackRequestExecutor{ $this->builder = new TransactionBuilder(); } + private function prettyInventoryAndSlot(Inventory $inventory, int $slot) : string{ + if($inventory instanceof TransactionBuilderInventory){ + $inventory = $inventory->getActualInventory(); + } + return (new \ReflectionClass($inventory))->getShortName() . "#" . spl_object_id($inventory) . ", slot: $slot"; + } + + /** + * @throws ItemStackRequestProcessException + */ + private function matchItemStack(Inventory $inventory, int $slotId, int $clientItemStackId) : void{ + $info = $this->inventoryManager->getItemStackInfo($inventory, $slotId); + if($info === null){ + throw new AssumptionFailedError("The inventory is tracked and the slot is valid, so this should not be null"); + } + + if(!($clientItemStackId < 0 ? $info->getRequestId() === $clientItemStackId : $info->getStackId() === $clientItemStackId)){ + throw new ItemStackRequestProcessException( + $this->prettyInventoryAndSlot($inventory, $slotId) . ": " . + "Mismatched expected itemstack, " . + "client expected: $clientItemStackId, server actual: " . $info->getStackId() . ", last modified by request: " . ($info->getRequestId() ?? "none") + ); + } + } + /** * @phpstan-return array{TransactionBuilderInventory, int} + * + * @throws ItemStackRequestProcessException */ private function getBuilderInventoryAndSlot(ItemStackRequestSlotInfo $info) : array{ $windowId = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId()); $windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $info->getSlotId()); if($windowAndSlot === null){ - throw new PacketHandlingException("Stack request action cannot target an inventory that is not open"); + throw new ItemStackRequestProcessException("No open inventory matches container UI ID: " . $info->getContainerId() . ", slot ID: " . $info->getSlotId()); } [$inventory, $slot] = $windowAndSlot; if(!$inventory->slotExists($slot)){ - throw new PacketHandlingException("Stack request action cannot target an inventory slot that does not exist"); + throw new ItemStackRequestProcessException("No such inventory slot :" . $this->prettyInventoryAndSlot($inventory, $slot)); } - if( - $info->getStackId() !== $this->request->getRequestId() && //using TransactionBuilderInventory enables this to work - !$this->inventoryManager->matchItemStack($inventory, $slot, $info->getStackId()) - ){ - throw new PacketHandlingException("Inventory " . $info->getContainerId() . ", slot " . $slot . ": server-side item does not match expected"); + if($info->getStackId() !== $this->request->getRequestId()){ //the itemstack may have been modified by the current request + $this->matchItemStack($inventory, $slot, $info->getStackId()); } return [$this->builder->getInventory($inventory), $slot]; @@ -112,6 +136,7 @@ final class ItemStackRequestExecutor{ /** * Deducts items from an inventory slot, returning a stack containing the removed items. + * @throws ItemStackRequestProcessException */ private function removeItemFromSlot(ItemStackRequestSlotInfo $slotInfo, int $count) : Item{ $this->requestSlotInfos[] = $slotInfo; @@ -119,7 +144,7 @@ final class ItemStackRequestExecutor{ $existingItem = $inventory->getItem($slot); if($existingItem->getCount() < $count){ - throw new PacketHandlingException("Cannot take $count items from a stack of " . $existingItem->getCount()); + throw new ItemStackRequestProcessException($this->prettyInventoryAndSlot($inventory, $slot) . ": Cannot take $count items from a stack of " . $existingItem->getCount()); } $removed = $existingItem->pop($count); @@ -137,7 +162,7 @@ final class ItemStackRequestExecutor{ $existingItem = $inventory->getItem($slot); if(!$existingItem->isNull() && !$existingItem->canStackWith($item)){ - throw new PacketHandlingException("Can only add items to an empty slot, or a slot containing the same item"); + throw new ItemStackRequestProcessException($this->prettyInventoryAndSlot($inventory, $slot) . ": Can only add items to an empty slot, or a slot containing the same item"); } //we can't use the existing item here; it may be an empty stack @@ -146,6 +171,9 @@ final class ItemStackRequestExecutor{ $inventory->setItem($slot, $newItem); } + /** + * @throws ItemStackRequestProcessException + */ private function setNextCreatedItem(?Item $item, bool $creative = false) : void{ if($item !== null && $item->isNull()){ $item = null; @@ -157,7 +185,7 @@ final class ItemStackRequestExecutor{ $this->nextCreatedItem->setCount($this->createdItemsTakenCount); $this->builder->addAction(new CreateItemAction($this->nextCreatedItem)); }elseif($this->createdItemsTakenCount < $this->nextCreatedItem->getCount()){ - throw new PacketHandlingException("Not all of the previous created item was taken"); + throw new ItemStackRequestProcessException("Not all of the previous created item was taken"); } } $this->nextCreatedItem = $item; @@ -165,24 +193,27 @@ final class ItemStackRequestExecutor{ $this->createdItemsTakenCount = 0; } + /** + * @throws ItemStackRequestProcessException + */ private function beginCrafting(int $recipeId, int $repetitions) : void{ if($this->specialTransaction !== null){ - throw new PacketHandlingException("Cannot perform more than 1 special action per request"); + throw new ItemStackRequestProcessException("Another special transaction is already in progress"); } if($repetitions < 1){ //TODO: upper bound? - throw new PacketHandlingException("Cannot craft a recipe less than 1 time"); + throw new ItemStackRequestProcessException("Cannot craft a recipe less than 1 time"); } $craftingManager = $this->player->getServer()->getCraftingManager(); - $recipe = $craftingManager->getCraftingRecipeIndex()[$recipeId] ?? null; + $recipe = $craftingManager->getCraftingRecipeFromIndex($recipeId); if($recipe === null){ - throw new PacketHandlingException("Unknown crafting recipe ID $recipeId"); + throw new ItemStackRequestProcessException("No such crafting recipe index: $recipeId"); } $this->specialTransaction = new CraftingTransaction($this->player, $craftingManager, [], $recipe, $repetitions); $currentWindow = $this->player->getCurrentWindow(); if($currentWindow !== null && !($currentWindow instanceof CraftingGrid)){ - throw new PacketHandlingException("Cannot complete crafting when the player's current window is not a crafting grid"); + throw new ItemStackRequestProcessException("Player's current window is not a crafting grid"); } $craftingGrid = $currentWindow ?? $this->player->getCraftingGrid(); @@ -200,13 +231,13 @@ final class ItemStackRequestExecutor{ private function takeCreatedItem(ItemStackRequestSlotInfo $destination, int $count) : void{ $createdItem = $this->nextCreatedItem; if($createdItem === null){ - throw new PacketHandlingException("No created item is waiting to be taken"); + throw new ItemStackRequestProcessException("No created item is waiting to be taken"); } if(!$this->createdItemFromCreativeInventory){ $availableCount = $createdItem->getCount() - $this->createdItemsTakenCount; if($count > $availableCount){ - throw new PacketHandlingException("Not enough created items available to be taken (have $availableCount, tried to take $count)"); + throw new ItemStackRequestProcessException("Not enough created items available to be taken (have $availableCount, tried to take $count)"); } } @@ -217,6 +248,22 @@ final class ItemStackRequestExecutor{ } } + /** + * @throws ItemStackRequestProcessException + */ + private function assertDoingCrafting() : void{ + if(!$this->specialTransaction instanceof CraftingTransaction){ + if($this->specialTransaction === null){ + throw new ItemStackRequestProcessException("Expected CraftRecipe or CraftRecipeAuto action to precede this action"); + }else{ + throw new ItemStackRequestProcessException("A different special transaction is already in progress"); + } + } + } + + /** + * @throws ItemStackRequestProcessException + */ private function processItemStackRequestAction(ItemStackRequestAction $action) : void{ if( $action instanceof TakeStackRequestAction || @@ -253,10 +300,7 @@ final class ItemStackRequestExecutor{ }elseif($action instanceof CreativeCreateStackRequestAction){ $item = CreativeInventory::getInstance()->getItem($action->getCreativeItemId()); if($item === null){ - //TODO: the item may have been unregistered after the client was sent the creative contents, leaving a - //gap in the creative item list. This probably shouldn't be a violation, but I'm not sure how else to - //handle it right now. - throw new PacketHandlingException("Tried to create nonexisting creative item " . $action->getCreativeItemId()); + throw new ItemStackRequestProcessException("No such creative item index: " . $action->getCreativeItemId()); } $this->setNextCreatedItem($item, true); @@ -265,31 +309,34 @@ final class ItemStackRequestExecutor{ }elseif($action instanceof CraftRecipeAutoStackRequestAction){ $this->beginCrafting($action->getRecipeId(), $action->getRepetitions()); }elseif($action instanceof CraftingConsumeInputStackRequestAction){ - if(!$this->specialTransaction instanceof CraftingTransaction){ - throw new PacketHandlingException("Cannot consume crafting input when no crafting transaction is in progress"); - } + $this->assertDoingCrafting(); $this->removeItemFromSlot($action->getSource(), $action->getCount()); //output discarded - we allow CraftingTransaction to verify the balance }elseif($action instanceof CraftingCreateSpecificResultStackRequestAction){ - if(!$this->specialTransaction instanceof CraftingTransaction){ - throw new AssumptionFailedError("Cannot mark crafting result index when no crafting transaction is in progress"); - } + $this->assertDoingCrafting(); $nextResultItem = $this->craftingResults[$action->getResultIndex()] ?? null; if($nextResultItem === null){ - throw new PacketHandlingException("No such crafting result index " . $action->getResultIndex()); + throw new ItemStackRequestProcessException("No such crafting result index: " . $action->getResultIndex()); } $this->setNextCreatedItem($nextResultItem); }elseif($action instanceof DeprecatedCraftingResultsStackRequestAction){ //no obvious use }else{ - throw new PacketHandlingException("Unhandled item stack request action: " . get_class($action)); + throw new ItemStackRequestProcessException("Unhandled item stack request action"); } } + /** + * @throws ItemStackRequestProcessException + */ public function generateInventoryTransaction() : InventoryTransaction{ - foreach($this->request->getActions() as $action){ - $this->processItemStackRequestAction($action); + foreach($this->request->getActions() as $k => $action){ + try{ + $this->processItemStackRequestAction($action); + }catch(ItemStackRequestProcessException $e){ + throw new ItemStackRequestProcessException("Error processing action $k (" . (new \ReflectionClass($action))->getShortName() . "): " . $e->getMessage(), 0, $e); + } } $this->setNextCreatedItem(null); $inventoryActions = $this->builder->generateActions(); @@ -302,12 +349,12 @@ final class ItemStackRequestExecutor{ return $transaction; } - public function buildItemStackResponse(bool $success) : ItemStackResponse{ + public function buildItemStackResponse() : ItemStackResponse{ $builder = new ItemStackResponseBuilder($this->request->getRequestId(), $this->inventoryManager); foreach($this->requestSlotInfos as $requestInfo){ $builder->addSlot($requestInfo->getContainerId(), $requestInfo->getSlotId()); } - return $builder->build($success); + return $builder->build(); } } diff --git a/src/network/mcpe/handler/ItemStackRequestProcessException.php b/src/network/mcpe/handler/ItemStackRequestProcessException.php new file mode 100644 index 000000000..7d5c667cf --- /dev/null +++ b/src/network/mcpe/handler/ItemStackRequestProcessException.php @@ -0,0 +1,31 @@ +inventoryManager->getCurrentWindowId()); $windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slotId); if($windowAndSlot === null){ - throw new PacketHandlingException("Stack request action cannot target an inventory that is not open"); + return null; } [$inventory, $slot] = $windowAndSlot; if(!$inventory->slotExists($slot)){ - throw new PacketHandlingException("Stack request action cannot target an inventory slot that does not exist"); + return null; } return [$inventory, $slot]; } - public function build(bool $success) : ItemStackResponse{ + public function build() : ItemStackResponse{ $responseInfosByContainer = []; foreach($this->changedSlots as $containerInterfaceId => $slotIds){ if($containerInterfaceId === ContainerUIIds::CREATED_OUTPUT){ continue; } foreach($slotIds as $slotId){ - [$inventory, $slot] = $this->getInventoryAndSlot($containerInterfaceId, $slotId); + $inventoryAndSlot = $this->getInventoryAndSlot($containerInterfaceId, $slotId); + if($inventoryAndSlot === null){ + //a plugin may have closed the inventory during an event, or the slot may have been invalid + continue; + } + [$inventory, $slot] = $inventoryAndSlot; $itemStackInfo = $this->inventoryManager->getItemStackInfo($inventory, $slot); if($itemStackInfo === null){ - //TODO: what if a plugin closes the inventory while the transaction is ongoing? - throw new \LogicException("ItemStackInfo should never be null for an open inventory"); + 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); @@ -102,6 +107,6 @@ final class ItemStackResponseBuilder{ $responseContainerInfos[] = new ItemStackResponseContainerInfo($containerInterfaceId, $responseInfos); } - return new ItemStackResponse($success ? ItemStackResponse::RESULT_OK : ItemStackResponse::RESULT_ERROR, $this->requestId, $responseContainerInfos); + return new ItemStackResponse(ItemStackResponse::RESULT_OK, $this->requestId, $responseContainerInfos); } }