From 5ebbcd5d33286d37f311a1c9f8a69100dcaf87e9 Mon Sep 17 00:00:00 2001 From: "Dylan T." Date: Mon, 2 Jun 2025 15:24:25 +0100 Subject: [PATCH] Move to newer systems for movement and block break handling (#6718) MS is due to remove the non-server-auth versions of all of this stuff. Fortunately v3 server auth movement works just fine without any changes, although we will need to start sending player tick in some packets if someone wants to actually use the rewind stuff. --- src/network/mcpe/InventoryManager.php | 19 +++-- .../mcpe/handler/InGamePacketHandler.php | 85 +++++++++++-------- .../mcpe/handler/ItemStackRequestExecutor.php | 26 +++++- .../mcpe/handler/PreSpawnPacketHandler.php | 2 +- 4 files changed, 89 insertions(+), 43 deletions(-) diff --git a/src/network/mcpe/InventoryManager.php b/src/network/mcpe/InventoryManager.php index 2ff23a73a..19bd94fce 100644 --- a/src/network/mcpe/InventoryManager.php +++ b/src/network/mcpe/InventoryManager.php @@ -41,6 +41,7 @@ use pocketmine\inventory\transaction\action\SlotChangeAction; use pocketmine\inventory\transaction\InventoryTransaction; use pocketmine\item\enchantment\EnchantingOption; use pocketmine\item\enchantment\EnchantmentInstance; +use pocketmine\item\Item; use pocketmine\network\mcpe\cache\CreativeInventoryCache; use pocketmine\network\mcpe\protocol\ClientboundPacket; use pocketmine\network\mcpe\protocol\ContainerClosePacket; @@ -228,17 +229,25 @@ class InventoryManager{ return null; } - private function addPredictedSlotChange(Inventory $inventory, int $slot, ItemStack $item) : void{ + private function addPredictedSlotChangeInternal(Inventory $inventory, int $slot, ItemStack $item) : void{ $this->inventories[spl_object_id($inventory)]->predictions[$slot] = $item; } - public function addTransactionPredictedSlotChanges(InventoryTransaction $tx) : void{ + public function addPredictedSlotChange(Inventory $inventory, int $slot, Item $item) : void{ $typeConverter = $this->session->getTypeConverter(); + $itemStack = $typeConverter->coreItemStackToNet($item); + $this->addPredictedSlotChangeInternal($inventory, $slot, $itemStack); + } + + public function addTransactionPredictedSlotChanges(InventoryTransaction $tx) : void{ foreach($tx->getActions() as $action){ if($action instanceof SlotChangeAction){ //TODO: ItemStackRequestExecutor can probably build these predictions with much lower overhead - $itemStack = $typeConverter->coreItemStackToNet($action->getTargetItem()); - $this->addPredictedSlotChange($action->getInventory(), $action->getSlot(), $itemStack); + $this->addPredictedSlotChange( + $action->getInventory(), + $action->getSlot(), + $action->getTargetItem() + ); } } } @@ -267,7 +276,7 @@ class InventoryManager{ } [$inventory, $slot] = $info; - $this->addPredictedSlotChange($inventory, $slot, $action->newItem->getItemStack()); + $this->addPredictedSlotChangeInternal($inventory, $slot, $action->newItem->getItemStack()); } } diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index eec200e4b..c2ca3cb5c 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -136,6 +136,8 @@ class InGamePacketHandler extends PacketHandler{ protected ?float $lastPlayerAuthInputPitch = null; protected ?BitSet $lastPlayerAuthInputFlags = null; + protected ?BlockPosition $lastBlockAttacked = null; + public bool $forceMoveSync = false; protected ?string $lastRequestedFullSkinId = null; @@ -248,6 +250,28 @@ class InGamePacketHandler extends PacketHandler{ $packetHandled = true; + $useItemTransaction = $packet->getItemInteractionData(); + if($useItemTransaction !== null){ + if(count($useItemTransaction->getTransactionData()->getActions()) > 100){ + throw new PacketHandlingException("Too many actions in item use transaction"); + } + + $this->inventoryManager->setCurrentItemStackRequestId($useItemTransaction->getRequestId()); + $this->inventoryManager->addRawPredictedSlotChanges($useItemTransaction->getTransactionData()->getActions()); + if(!$this->handleUseItemTransaction($useItemTransaction->getTransactionData())){ + $packetHandled = false; + $this->session->getLogger()->debug("Unhandled transaction in PlayerAuthInputPacket (type " . $useItemTransaction->getTransactionData()->getActionType() . ")"); + }else{ + $this->inventoryManager->syncMismatchedPredictedSlotChanges(); + } + $this->inventoryManager->setCurrentItemStackRequestId(null); + } + + $itemStackRequest = $packet->getItemStackRequest(); + $itemStackResponseBuilder = $itemStackRequest !== null ? $this->handleSingleItemStackRequest($itemStackRequest) : null; + + //itemstack request or transaction may set predictions for the outcome of these actions, so these need to be + //processed last $blockActions = $packet->getBlockActions(); if($blockActions !== null){ if(count($blockActions) > 100){ @@ -268,27 +292,9 @@ class InGamePacketHandler extends PacketHandler{ } } - $useItemTransaction = $packet->getItemInteractionData(); - if($useItemTransaction !== null){ - if(count($useItemTransaction->getTransactionData()->getActions()) > 100){ - throw new PacketHandlingException("Too many actions in item use transaction"); - } - - $this->inventoryManager->setCurrentItemStackRequestId($useItemTransaction->getRequestId()); - $this->inventoryManager->addRawPredictedSlotChanges($useItemTransaction->getTransactionData()->getActions()); - if(!$this->handleUseItemTransaction($useItemTransaction->getTransactionData())){ - $packetHandled = false; - $this->session->getLogger()->debug("Unhandled transaction in PlayerAuthInputPacket (type " . $useItemTransaction->getTransactionData()->getActionType() . ")"); - }else{ - $this->inventoryManager->syncMismatchedPredictedSlotChanges(); - } - $this->inventoryManager->setCurrentItemStackRequestId(null); - } - - $itemStackRequest = $packet->getItemStackRequest(); if($itemStackRequest !== null){ - $result = $this->handleSingleItemStackRequest($itemStackRequest); - $this->session->sendDataPacket(ItemStackResponsePacket::create([$result])); + $itemStackResponse = $itemStackResponseBuilder?->build() ?? new ItemStackResponse(ItemStackResponse::RESULT_ERROR, $itemStackRequest->getRequestId()); + $this->session->sendDataPacket(ItemStackResponsePacket::create([$itemStackResponse])); } return $packetHandled; @@ -498,13 +504,6 @@ class InGamePacketHandler extends PacketHandler{ //if only the client would tell us what blocks it thinks changed... $this->syncBlocksNearby($vBlockPos, $data->getFace()); return true; - case UseItemTransactionData::ACTION_BREAK_BLOCK: - $blockPos = $data->getBlockPosition(); - $vBlockPos = new Vector3($blockPos->getX(), $blockPos->getY(), $blockPos->getZ()); - if(!$this->player->breakBlock($vBlockPos)){ - $this->syncBlocksNearby($vBlockPos, null); - } - return true; case UseItemTransactionData::ACTION_CLICK_AIR: if($this->player->isUsingItem()){ if(!$this->player->consumeHeldItem()){ @@ -580,7 +579,7 @@ class InGamePacketHandler extends PacketHandler{ return false; } - private function handleSingleItemStackRequest(ItemStackRequest $request) : ItemStackResponse{ + private function handleSingleItemStackRequest(ItemStackRequest $request) : ?ItemStackResponseBuilder{ if(count($request->getActions()) > 60){ //recipe book auto crafting can affect all slots of the inventory when consuming inputs or producing outputs //this means there could be as many as 50 CraftingConsumeInput actions or Place (taking the result) actions @@ -597,7 +596,11 @@ class InGamePacketHandler extends PacketHandler{ $executor = new ItemStackRequestExecutor($this->player, $this->inventoryManager, $request); try{ $transaction = $executor->generateInventoryTransaction(); - $result = $this->executeInventoryTransaction($transaction, $request->getRequestId()); + if($transaction !== null){ + $result = $this->executeInventoryTransaction($transaction, $request->getRequestId()); + }else{ + $result = true; //predictions only, just send responses + } }catch(ItemStackRequestProcessException $e){ $result = false; $this->session->getLogger()->debug("ItemStackRequest #" . $request->getRequestId() . " failed: " . $e->getMessage()); @@ -605,10 +608,7 @@ class InGamePacketHandler extends PacketHandler{ $this->inventoryManager->requestSyncAll(); } - if(!$result){ - return new ItemStackResponse(ItemStackResponse::RESULT_ERROR, $request->getRequestId()); - } - return $executor->buildItemStackResponse(); + return $result ? $executor->getItemStackResponseBuilder() : null; } public function handleItemStackRequest(ItemStackRequestPacket $packet) : bool{ @@ -618,7 +618,7 @@ class InGamePacketHandler extends PacketHandler{ throw new PacketHandlingException("Too many requests in ItemStackRequestPacket"); } foreach($packet->getRequests() as $request){ - $responses[] = $this->handleSingleItemStackRequest($request); + $responses[] = $this->handleSingleItemStackRequest($request)?->build() ?? new ItemStackResponse(ItemStackResponse::RESULT_ERROR, $request->getRequestId()); } $this->session->sendDataPacket(ItemStackResponsePacket::create($responses)); @@ -681,16 +681,27 @@ class InGamePacketHandler extends PacketHandler{ switch($action){ case PlayerAction::START_BREAK: + case PlayerAction::CONTINUE_DESTROY_BLOCK: //destroy the next block while holding down left click self::validateFacing($face); + if($this->lastBlockAttacked !== null && $blockPosition->equals($this->lastBlockAttacked)){ + //the client will send CONTINUE_DESTROY_BLOCK for the currently targeted block directly before it + //sends PREDICT_DESTROY_BLOCK, but also when it starts to break the block + //this seems like a bug in the client and would cause spurious left-click events if we allowed it to + //be delivered to the player + $this->session->getLogger()->debug("Ignoring PlayerAction $action on $pos because we were already destroying this block"); + break; + } if(!$this->player->attackBlock($pos, $face)){ $this->syncBlocksNearby($pos, $face); } + $this->lastBlockAttacked = $blockPosition; break; case PlayerAction::ABORT_BREAK: case PlayerAction::STOP_BREAK: $this->player->stopBreakBlock($pos); + $this->lastBlockAttacked = null; break; case PlayerAction::START_SLEEPING: //unused @@ -701,11 +712,17 @@ class InGamePacketHandler extends PacketHandler{ case PlayerAction::CRACK_BREAK: self::validateFacing($face); $this->player->continueBreakBlock($pos, $face); + $this->lastBlockAttacked = $blockPosition; break; case PlayerAction::INTERACT_BLOCK: //TODO: ignored (for now) break; case PlayerAction::CREATIVE_PLAYER_DESTROY_BLOCK: //TODO: do we need to handle this? + case PlayerAction::PREDICT_DESTROY_BLOCK: + if(!$this->player->breakBlock($pos)){ + $this->syncBlocksNearby($pos, $face); + } + $this->lastBlockAttacked = null; break; case PlayerAction::START_ITEM_USE_ON: case PlayerAction::STOP_ITEM_USE_ON: diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index 6db8f1e12..d71a1c6bf 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -33,9 +33,11 @@ use pocketmine\inventory\transaction\EnchantingTransaction; use pocketmine\inventory\transaction\InventoryTransaction; use pocketmine\inventory\transaction\TransactionBuilder; use pocketmine\inventory\transaction\TransactionBuilderInventory; +use pocketmine\item\Durable; use pocketmine\item\Item; use pocketmine\network\mcpe\InventoryManager; use pocketmine\network\mcpe\protocol\types\inventory\ContainerUIIds; +use pocketmine\network\mcpe\protocol\types\inventory\FullContainerName; use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\CraftingConsumeInputStackRequestAction; use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\CraftingCreateSpecificResultStackRequestAction; use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\CraftRecipeAutoStackRequestAction; @@ -47,6 +49,7 @@ use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\DropStackReque use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\ItemStackRequest; use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\ItemStackRequestAction; use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\ItemStackRequestSlotInfo; +use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\MineBlockStackRequestAction; use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\PlaceStackRequestAction; use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\SwapStackRequestAction; use pocketmine\network\mcpe\protocol\types\inventory\stackrequest\TakeStackRequestAction; @@ -362,6 +365,16 @@ class ItemStackRequestExecutor{ $this->setNextCreatedItem($nextResultItem); }elseif($action instanceof DeprecatedCraftingResultsStackRequestAction){ //no obvious use + }elseif($action instanceof MineBlockStackRequestAction){ + $slot = $action->getHotbarSlot(); + $this->requestSlotInfos[] = new ItemStackRequestSlotInfo(new FullContainerName(ContainerUIIds::HOTBAR), $slot, $action->getStackId()); + $inventory = $this->player->getInventory(); + $usedItem = $inventory->slotExists($slot) ? $inventory->getItem($slot) : null; + $predictedDamage = $action->getPredictedDurability(); + if($usedItem instanceof Durable && $predictedDamage >= 0 && $predictedDamage <= $usedItem->getMaxDurability()){ + $usedItem->setDamage($predictedDamage); + $this->inventoryManager->addPredictedSlotChange($inventory, $slot, $usedItem); + } }else{ throw new ItemStackRequestProcessException("Unhandled item stack request action"); } @@ -370,7 +383,7 @@ class ItemStackRequestExecutor{ /** * @throws ItemStackRequestProcessException */ - public function generateInventoryTransaction() : InventoryTransaction{ + public function generateInventoryTransaction() : ?InventoryTransaction{ foreach(Utils::promoteKeys($this->request->getActions()) as $k => $action){ try{ $this->processItemStackRequestAction($action); @@ -380,6 +393,9 @@ class ItemStackRequestExecutor{ } $this->setNextCreatedItem(null); $inventoryActions = $this->builder->generateActions(); + if(count($inventoryActions) === 0){ + return null; + } $transaction = $this->specialTransaction ?? new InventoryTransaction($this->player); foreach($inventoryActions as $action){ @@ -389,12 +405,16 @@ class ItemStackRequestExecutor{ return $transaction; } - public function buildItemStackResponse() : ItemStackResponse{ + public function getItemStackResponseBuilder() : ItemStackResponseBuilder{ $builder = new ItemStackResponseBuilder($this->request->getRequestId(), $this->inventoryManager); foreach($this->requestSlotInfos as $requestInfo){ $builder->addSlot($requestInfo->getContainerName()->getContainerId(), $requestInfo->getSlotId()); } - return $builder->build(); + return $builder; + } + + public function buildItemStackResponse() : ItemStackResponse{ + return $this->getItemStackResponseBuilder()->build(); } } diff --git a/src/network/mcpe/handler/PreSpawnPacketHandler.php b/src/network/mcpe/handler/PreSpawnPacketHandler.php index 9aa302c0c..161a679d6 100644 --- a/src/network/mcpe/handler/PreSpawnPacketHandler.php +++ b/src/network/mcpe/handler/PreSpawnPacketHandler.php @@ -99,7 +99,7 @@ class PreSpawnPacketHandler extends PacketHandler{ $this->server->getMotd(), "", false, - new PlayerMovementSettings(ServerAuthMovementMode::SERVER_AUTHORITATIVE_V2, 0, false), + new PlayerMovementSettings(ServerAuthMovementMode::SERVER_AUTHORITATIVE_V3, 0, true), 0, 0, "",