diff --git a/src/block/tile/Furnace.php b/src/block/tile/Furnace.php index 5f31c8dfb..756e23ff9 100644 --- a/src/block/tile/Furnace.php +++ b/src/block/tile/Furnace.php @@ -34,7 +34,9 @@ use pocketmine\item\Item; use pocketmine\math\Vector3; use pocketmine\nbt\tag\CompoundTag; use pocketmine\network\mcpe\protocol\ContainerSetDataPacket; +use pocketmine\player\Player; use pocketmine\world\World; +use function array_map; use function max; class Furnace extends Spawnable implements Container, Nameable{ @@ -202,19 +204,19 @@ class Furnace extends Spawnable implements Container, Nameable{ $this->remainingFuelTime = $this->cookTime = $this->maxFuelTime = 0; } - if($prevCookTime !== $this->cookTime){ - foreach($this->inventory->getViewers() as $v){ - $v->getNetworkSession()->getInvManager()->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_SMELT_PROGRESS, $this->cookTime); + $viewers = array_map(fn(Player $p) => $p->getNetworkSession()->getInvManager(), $this->inventory->getViewers()); + foreach($viewers as $v){ + if($v === null){ + continue; } - } - if($prevRemainingFuelTime !== $this->remainingFuelTime){ - foreach($this->inventory->getViewers() as $v){ - $v->getNetworkSession()->getInvManager()->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_REMAINING_FUEL_TIME, $this->remainingFuelTime); + if($prevCookTime !== $this->cookTime){ + $v->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_SMELT_PROGRESS, $this->cookTime); } - } - if($prevMaxFuelTime !== $this->maxFuelTime){ - foreach($this->inventory->getViewers() as $v){ - $v->getNetworkSession()->getInvManager()->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_MAX_FUEL_TIME, $this->maxFuelTime); + if($prevRemainingFuelTime !== $this->remainingFuelTime){ + $v->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_REMAINING_FUEL_TIME, $this->remainingFuelTime); + } + if($prevMaxFuelTime !== $this->maxFuelTime){ + $v->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_MAX_FUEL_TIME, $this->maxFuelTime); } } diff --git a/src/inventory/BaseInventory.php b/src/inventory/BaseInventory.php index 03bbea73c..425c6a809 100644 --- a/src/inventory/BaseInventory.php +++ b/src/inventory/BaseInventory.php @@ -136,7 +136,11 @@ abstract class BaseInventory implements Inventory{ $listener->onSlotChange($this, $index, $before); } foreach($this->viewers as $viewer){ - $viewer->getNetworkSession()->getInvManager()->syncSlot($this, $index); + $invManager = $viewer->getNetworkSession()->getInvManager(); + if($invManager === null){ + continue; + } + $invManager->syncSlot($this, $index); } } @@ -150,7 +154,11 @@ abstract class BaseInventory implements Inventory{ } foreach($this->getViewers() as $viewer){ - $viewer->getNetworkSession()->getInvManager()->syncContents($this); + $invManager = $viewer->getNetworkSession()->getInvManager(); + if($invManager === null){ + continue; + } + $invManager->syncContents($this); } } diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 9ba761abd..45926887a 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -672,7 +672,7 @@ class NetworkSession{ } private function beginSpawnSequence() : void{ - $this->setHandler(new PreSpawnPacketHandler($this->server, $this->player, $this)); + $this->setHandler(new PreSpawnPacketHandler($this->server, $this->player, $this, $this->invManager)); $this->player->setImmobile(); //TODO: HACK: fix client-side falling pre-spawn $this->logger->debug("Waiting for chunk radius request"); @@ -691,7 +691,7 @@ class NetworkSession{ $this->player->setImmobile(false); //TODO: HACK: we set this during the spawn sequence to prevent the client sending junk movements $this->player->doFirstSpawn(); $this->forceAsyncCompression = false; - $this->setHandler(new InGamePacketHandler($this->player, $this)); + $this->setHandler(new InGamePacketHandler($this->player, $this, $this->invManager)); } public function onServerDeath() : void{ @@ -705,7 +705,7 @@ class NetworkSession{ $this->syncAdventureSettings($this->player); $this->invManager->syncAll(); - $this->setHandler(new InGamePacketHandler($this->player, $this)); + $this->setHandler(new InGamePacketHandler($this->player, $this, $this->invManager)); } public function syncMovement(Vector3 $pos, ?float $yaw = null, ?float $pitch = null, int $mode = MovePlayerPacket::MODE_NORMAL) : void{ @@ -937,7 +937,7 @@ class NetworkSession{ $this->sendDataPacket(SetDifficultyPacket::create($worldDifficulty)); } - public function getInvManager() : InventoryManager{ + public function getInvManager() : ?InventoryManager{ return $this->invManager; } diff --git a/src/network/mcpe/convert/TypeConverter.php b/src/network/mcpe/convert/TypeConverter.php index a1adb20e6..c523eac10 100644 --- a/src/network/mcpe/convert/TypeConverter.php +++ b/src/network/mcpe/convert/TypeConverter.php @@ -39,6 +39,7 @@ use pocketmine\item\ItemFactory; use pocketmine\item\ItemIds; use pocketmine\nbt\tag\CompoundTag; use pocketmine\nbt\tag\IntTag; +use pocketmine\network\mcpe\InventoryManager; use pocketmine\network\mcpe\protocol\types\GameMode as ProtocolGameMode; use pocketmine\network\mcpe\protocol\types\inventory\ContainerIds; use pocketmine\network\mcpe\protocol\types\inventory\ItemStack; @@ -240,7 +241,7 @@ class TypeConverter{ /** * @throws \UnexpectedValueException */ - public function createInventoryAction(NetworkInventoryAction $action, Player $player) : ?InventoryAction{ + public function createInventoryAction(NetworkInventoryAction $action, Player $player, InventoryManager $inventoryManager) : ?InventoryAction{ if($action->oldItem->getItemStack()->equals($action->newItem->getItemStack())){ //filter out useless noise in 1.13 return null; @@ -276,7 +277,7 @@ class TypeConverter{ } [$slot, $window] = $mapped; }else{ - $window = $player->getNetworkSession()->getInvManager()->getWindow($action->windowId); + $window = $inventoryManager->getWindow($action->windowId); $slot = $action->inventorySlot; } if($window !== null){ diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 376fa5361..21509067b 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -143,9 +143,12 @@ class InGamePacketHandler extends PacketHandler{ */ protected $openHardcodedWindows = []; - public function __construct(Player $player, NetworkSession $session){ + private InventoryManager $inventoryManager; + + public function __construct(Player $player, NetworkSession $session, InventoryManager $inventoryManager){ $this->player = $player; $this->session = $session; + $this->inventoryManager = $inventoryManager; } public function handleText(TextPacket $packet) : bool{ @@ -224,7 +227,7 @@ class InGamePacketHandler extends PacketHandler{ $result = $this->handleNormalTransaction($packet->trData); }elseif($packet->trData instanceof MismatchTransactionData){ $this->session->getLogger()->debug("Mismatch transaction received"); - $this->session->getInvManager()->syncAll(); + $this->inventoryManager->syncAll(); $result = true; }elseif($packet->trData instanceof UseItemTransactionData){ $result = $this->handleUseItemTransaction($packet->trData); @@ -235,7 +238,7 @@ class InGamePacketHandler extends PacketHandler{ } if(!$result){ - $this->session->getInvManager()->syncAll(); + $this->inventoryManager->syncAll(); } return $result; } @@ -265,7 +268,7 @@ class InGamePacketHandler extends PacketHandler{ } try{ - $action = $converter->createInventoryAction($networkInventoryAction, $this->player); + $action = $converter->createInventoryAction($networkInventoryAction, $this->player, $this->inventoryManager); if($action !== null){ $actions[] = $action; } @@ -293,14 +296,14 @@ class InGamePacketHandler extends PacketHandler{ return true; } try{ - $this->session->getInvManager()->onTransactionStart($this->craftingTransaction); + $this->inventoryManager->onTransactionStart($this->craftingTransaction); $this->craftingTransaction->execute(); }catch(TransactionException $e){ $this->session->getLogger()->debug("Failed to execute crafting transaction: " . $e->getMessage()); //TODO: only sync slots that the client tried to change foreach($this->craftingTransaction->getInventories() as $inventory){ - $this->session->getInvManager()->syncContents($inventory); + $this->inventoryManager->syncContents($inventory); } /* * TODO: HACK! @@ -328,7 +331,7 @@ class InGamePacketHandler extends PacketHandler{ } $transaction = new InventoryTransaction($this->player, $actions); - $this->session->getInvManager()->onTransactionStart($transaction); + $this->inventoryManager->onTransactionStart($transaction); try{ $transaction->execute(); }catch(TransactionException $e){ @@ -337,7 +340,7 @@ class InGamePacketHandler extends PacketHandler{ $logger->debug("Actions: " . json_encode($data->getActions())); foreach($transaction->getInventories() as $inventory){ - $this->session->getInvManager()->syncContents($inventory); + $this->inventoryManager->syncContents($inventory); } return false; @@ -392,12 +395,12 @@ class InGamePacketHandler extends PacketHandler{ case UseItemTransactionData::ACTION_CLICK_AIR: if($this->player->isUsingItem()){ if(!$this->player->consumeHeldItem()){ - $this->session->getInvManager()->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); + $this->inventoryManager->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); } return true; } if(!$this->player->useHeldItem()){ - $this->session->getInvManager()->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); + $this->inventoryManager->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); } return true; } @@ -409,7 +412,7 @@ class InGamePacketHandler extends PacketHandler{ * Internal function used to execute rollbacks when an action fails on a block. */ private function onFailedBlockAction(Vector3 $blockPos, ?int $face) : void{ - $this->session->getInvManager()->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); + $this->inventoryManager->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); if($blockPos->distanceSquared($this->player->getLocation()) < 10000){ $blocks = $blockPos->sidesArray(); if($face !== null){ @@ -436,12 +439,12 @@ class InGamePacketHandler extends PacketHandler{ switch($data->getActionType()){ case UseItemOnEntityTransactionData::ACTION_INTERACT: if(!$this->player->interactEntity($target, $data->getClickPos())){ - $this->session->getInvManager()->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); + $this->inventoryManager->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); } return true; case UseItemOnEntityTransactionData::ACTION_ATTACK: if(!$this->player->attackEntity($target)){ - $this->session->getInvManager()->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); + $this->inventoryManager->syncSlot($this->player->getInventory(), $this->player->getInventory()->getHeldItemIndex()); } return true; } @@ -454,7 +457,7 @@ class InGamePacketHandler extends PacketHandler{ switch($data->getActionType()){ case ReleaseItemTransactionData::ACTION_RELEASE: if(!$this->player->releaseHeldItem()){ - $this->session->getInvManager()->syncContents($this->player->getInventory()); + $this->inventoryManager->syncContents($this->player->getInventory()); } return true; } @@ -467,9 +470,9 @@ class InGamePacketHandler extends PacketHandler{ return true; //this happens when we put an item into the offhand } if($packet->windowId === ContainerIds::INVENTORY){ - $this->session->getInvManager()->onClientSelectHotbarSlot($packet->hotbarSlot); + $this->inventoryManager->onClientSelectHotbarSlot($packet->hotbarSlot); if(!$this->player->selectHotbarSlot($packet->hotbarSlot)){ - $this->session->getInvManager()->syncSelectedHotbarSlot(); + $this->inventoryManager->syncSelectedHotbarSlot(); } return true; } @@ -603,7 +606,7 @@ class InGamePacketHandler extends PacketHandler{ if(array_key_exists($packet->windowId, $this->openHardcodedWindows)){ unset($this->openHardcodedWindows[$packet->windowId]); }else{ - $this->session->getInvManager()->onClientRemoveWindow($packet->windowId); + $this->inventoryManager->onClientRemoveWindow($packet->windowId); } $this->session->sendDataPacket(ContainerClosePacket::create($packet->windowId, false)); diff --git a/src/network/mcpe/handler/PreSpawnPacketHandler.php b/src/network/mcpe/handler/PreSpawnPacketHandler.php index a6f52af3e..82996ac66 100644 --- a/src/network/mcpe/handler/PreSpawnPacketHandler.php +++ b/src/network/mcpe/handler/PreSpawnPacketHandler.php @@ -27,6 +27,7 @@ use pocketmine\network\mcpe\cache\CraftingDataCache; use pocketmine\network\mcpe\cache\StaticPacketCache; use pocketmine\network\mcpe\convert\GlobalItemTypeDictionary; use pocketmine\network\mcpe\convert\TypeConverter; +use pocketmine\network\mcpe\InventoryManager; use pocketmine\network\mcpe\NetworkSession; use pocketmine\network\mcpe\protocol\RequestChunkRadiusPacket; use pocketmine\network\mcpe\protocol\StartGamePacket; @@ -53,10 +54,13 @@ class PreSpawnPacketHandler extends PacketHandler{ /** @var NetworkSession */ private $session; - public function __construct(Server $server, Player $player, NetworkSession $session){ + private InventoryManager $inventoryManager; + + public function __construct(Server $server, Player $player, NetworkSession $session, InventoryManager $inventoryManager){ $this->player = $player; $this->server = $server; $this->session = $session; + $this->inventoryManager = $inventoryManager; } public function setUp() : void{ @@ -104,9 +108,9 @@ class PreSpawnPacketHandler extends PacketHandler{ } $this->player->sendData([$this->player]); - $this->session->getInvManager()->syncAll(); - $this->session->getInvManager()->syncCreative(); - $this->session->getInvManager()->syncSelectedHotbarSlot(); + $this->inventoryManager->syncAll(); + $this->inventoryManager->syncCreative(); + $this->inventoryManager->syncSelectedHotbarSlot(); $this->session->sendDataPacket(CraftingDataCache::getInstance()->getCache($this->server->getCraftingManager())); $this->session->syncPlayerList($this->server->getOnlinePlayers()); diff --git a/src/player/Player.php b/src/player/Player.php index d62362f43..4dedf5118 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -2340,8 +2340,11 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ //TODO: client side race condition here makes the opening work incorrectly $this->removeCurrentWindow(); + if(($inventoryManager = $this->getNetworkSession()->getInvManager()) === null){ + throw new \InvalidArgumentException("Player cannot open inventories in this state"); + } $this->logger->debug("Opening inventory " . get_class($inventory) . "#" . spl_object_id($inventory)); - $this->getNetworkSession()->getInvManager()->onCurrentWindowChange($inventory); + $inventoryManager->onCurrentWindowChange($inventory); $inventory->onOpen($this); $this->currentWindow = $inventory; return true; @@ -2353,8 +2356,8 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ $this->logger->debug("Closing inventory " . get_class($this->currentWindow) . "#" . spl_object_id($this->currentWindow)); $this->currentWindow->onClose($this); - if($this->isConnected()){ - $this->getNetworkSession()->getInvManager()->onCurrentWindowRemove(); + if(($inventoryManager = $this->getNetworkSession()->getInvManager()) !== null){ + $inventoryManager->onCurrentWindowRemove(); } $this->currentWindow = null; } diff --git a/tests/phpstan/configs/l8-baseline.neon b/tests/phpstan/configs/l8-baseline.neon index c8742c269..9af2e2e80 100644 --- a/tests/phpstan/configs/l8-baseline.neon +++ b/tests/phpstan/configs/l8-baseline.neon @@ -135,11 +135,6 @@ parameters: count: 1 path: ../../../src/network/mcpe/NetworkSession.php - - - message: "#^Method pocketmine\\\\network\\\\mcpe\\\\NetworkSession\\:\\:getInvManager\\(\\) should return pocketmine\\\\network\\\\mcpe\\\\InventoryManager but returns pocketmine\\\\network\\\\mcpe\\\\InventoryManager\\|null\\.$#" - count: 1 - path: ../../../src/network/mcpe/NetworkSession.php - - message: "#^Parameter \\#1 \\$clientPub of class pocketmine\\\\network\\\\mcpe\\\\encryption\\\\PrepareEncryptionTask constructor expects Mdanter\\\\Ecc\\\\Crypto\\\\Key\\\\PublicKeyInterface, Mdanter\\\\Ecc\\\\Crypto\\\\Key\\\\PublicKeyInterface\\|null given\\.$#" count: 1 @@ -185,6 +180,16 @@ parameters: count: 1 path: ../../../src/network/mcpe/NetworkSession.php + - + message: "#^Parameter \\#3 \\$inventoryManager of class pocketmine\\\\network\\\\mcpe\\\\handler\\\\InGamePacketHandler constructor expects pocketmine\\\\network\\\\mcpe\\\\InventoryManager, pocketmine\\\\network\\\\mcpe\\\\InventoryManager\\|null given\\.$#" + count: 2 + path: ../../../src/network/mcpe/NetworkSession.php + + - + message: "#^Parameter \\#4 \\$inventoryManager of class pocketmine\\\\network\\\\mcpe\\\\handler\\\\PreSpawnPacketHandler constructor expects pocketmine\\\\network\\\\mcpe\\\\InventoryManager, pocketmine\\\\network\\\\mcpe\\\\InventoryManager\\|null given\\.$#" + count: 1 + path: ../../../src/network/mcpe/NetworkSession.php + - message: "#^Property pocketmine\\\\network\\\\mcpe\\\\protocol\\\\LevelSoundEventPacket\\:\\:\\$position \\(pocketmine\\\\math\\\\Vector3\\) does not accept pocketmine\\\\math\\\\Vector3\\|null\\.$#" count: 1