NetworkSession: Fixed InventoryManager nullability disaster

fixes #4277
fixes #4275
fixes #3139
This commit is contained in:
Dylan K. Taylor 2021-06-26 17:44:42 +01:00
parent e43bca95bf
commit 0910054c41
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
8 changed files with 74 additions and 48 deletions

View File

@ -34,7 +34,9 @@ use pocketmine\item\Item;
use pocketmine\math\Vector3; use pocketmine\math\Vector3;
use pocketmine\nbt\tag\CompoundTag; use pocketmine\nbt\tag\CompoundTag;
use pocketmine\network\mcpe\protocol\ContainerSetDataPacket; use pocketmine\network\mcpe\protocol\ContainerSetDataPacket;
use pocketmine\player\Player;
use pocketmine\world\World; use pocketmine\world\World;
use function array_map;
use function max; use function max;
class Furnace extends Spawnable implements Container, Nameable{ 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; $this->remainingFuelTime = $this->cookTime = $this->maxFuelTime = 0;
} }
if($prevCookTime !== $this->cookTime){ $viewers = array_map(fn(Player $p) => $p->getNetworkSession()->getInvManager(), $this->inventory->getViewers());
foreach($this->inventory->getViewers() as $v){ foreach($viewers as $v){
$v->getNetworkSession()->getInvManager()->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_SMELT_PROGRESS, $this->cookTime); if($v === null){
continue;
} }
} if($prevCookTime !== $this->cookTime){
if($prevRemainingFuelTime !== $this->remainingFuelTime){ $v->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_SMELT_PROGRESS, $this->cookTime);
foreach($this->inventory->getViewers() as $v){
$v->getNetworkSession()->getInvManager()->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_REMAINING_FUEL_TIME, $this->remainingFuelTime);
} }
} if($prevRemainingFuelTime !== $this->remainingFuelTime){
if($prevMaxFuelTime !== $this->maxFuelTime){ $v->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_REMAINING_FUEL_TIME, $this->remainingFuelTime);
foreach($this->inventory->getViewers() as $v){ }
$v->getNetworkSession()->getInvManager()->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_MAX_FUEL_TIME, $this->maxFuelTime); if($prevMaxFuelTime !== $this->maxFuelTime){
$v->syncData($this->inventory, ContainerSetDataPacket::PROPERTY_FURNACE_MAX_FUEL_TIME, $this->maxFuelTime);
} }
} }

View File

@ -136,7 +136,11 @@ abstract class BaseInventory implements Inventory{
$listener->onSlotChange($this, $index, $before); $listener->onSlotChange($this, $index, $before);
} }
foreach($this->viewers as $viewer){ 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){ foreach($this->getViewers() as $viewer){
$viewer->getNetworkSession()->getInvManager()->syncContents($this); $invManager = $viewer->getNetworkSession()->getInvManager();
if($invManager === null){
continue;
}
$invManager->syncContents($this);
} }
} }

View File

@ -672,7 +672,7 @@ class NetworkSession{
} }
private function beginSpawnSequence() : void{ 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->player->setImmobile(); //TODO: HACK: fix client-side falling pre-spawn
$this->logger->debug("Waiting for chunk radius request"); $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->setImmobile(false); //TODO: HACK: we set this during the spawn sequence to prevent the client sending junk movements
$this->player->doFirstSpawn(); $this->player->doFirstSpawn();
$this->forceAsyncCompression = false; $this->forceAsyncCompression = false;
$this->setHandler(new InGamePacketHandler($this->player, $this)); $this->setHandler(new InGamePacketHandler($this->player, $this, $this->invManager));
} }
public function onServerDeath() : void{ public function onServerDeath() : void{
@ -705,7 +705,7 @@ class NetworkSession{
$this->syncAdventureSettings($this->player); $this->syncAdventureSettings($this->player);
$this->invManager->syncAll(); $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{ 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)); $this->sendDataPacket(SetDifficultyPacket::create($worldDifficulty));
} }
public function getInvManager() : InventoryManager{ public function getInvManager() : ?InventoryManager{
return $this->invManager; return $this->invManager;
} }

View File

@ -39,6 +39,7 @@ use pocketmine\item\ItemFactory;
use pocketmine\item\ItemIds; use pocketmine\item\ItemIds;
use pocketmine\nbt\tag\CompoundTag; use pocketmine\nbt\tag\CompoundTag;
use pocketmine\nbt\tag\IntTag; 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\GameMode as ProtocolGameMode;
use pocketmine\network\mcpe\protocol\types\inventory\ContainerIds; use pocketmine\network\mcpe\protocol\types\inventory\ContainerIds;
use pocketmine\network\mcpe\protocol\types\inventory\ItemStack; use pocketmine\network\mcpe\protocol\types\inventory\ItemStack;
@ -240,7 +241,7 @@ class TypeConverter{
/** /**
* @throws \UnexpectedValueException * @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())){ if($action->oldItem->getItemStack()->equals($action->newItem->getItemStack())){
//filter out useless noise in 1.13 //filter out useless noise in 1.13
return null; return null;
@ -276,7 +277,7 @@ class TypeConverter{
} }
[$slot, $window] = $mapped; [$slot, $window] = $mapped;
}else{ }else{
$window = $player->getNetworkSession()->getInvManager()->getWindow($action->windowId); $window = $inventoryManager->getWindow($action->windowId);
$slot = $action->inventorySlot; $slot = $action->inventorySlot;
} }
if($window !== null){ if($window !== null){

View File

@ -143,9 +143,12 @@ class InGamePacketHandler extends PacketHandler{
*/ */
protected $openHardcodedWindows = []; 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->player = $player;
$this->session = $session; $this->session = $session;
$this->inventoryManager = $inventoryManager;
} }
public function handleText(TextPacket $packet) : bool{ public function handleText(TextPacket $packet) : bool{
@ -224,7 +227,7 @@ class InGamePacketHandler extends PacketHandler{
$result = $this->handleNormalTransaction($packet->trData); $result = $this->handleNormalTransaction($packet->trData);
}elseif($packet->trData instanceof MismatchTransactionData){ }elseif($packet->trData instanceof MismatchTransactionData){
$this->session->getLogger()->debug("Mismatch transaction received"); $this->session->getLogger()->debug("Mismatch transaction received");
$this->session->getInvManager()->syncAll(); $this->inventoryManager->syncAll();
$result = true; $result = true;
}elseif($packet->trData instanceof UseItemTransactionData){ }elseif($packet->trData instanceof UseItemTransactionData){
$result = $this->handleUseItemTransaction($packet->trData); $result = $this->handleUseItemTransaction($packet->trData);
@ -235,7 +238,7 @@ class InGamePacketHandler extends PacketHandler{
} }
if(!$result){ if(!$result){
$this->session->getInvManager()->syncAll(); $this->inventoryManager->syncAll();
} }
return $result; return $result;
} }
@ -265,7 +268,7 @@ class InGamePacketHandler extends PacketHandler{
} }
try{ try{
$action = $converter->createInventoryAction($networkInventoryAction, $this->player); $action = $converter->createInventoryAction($networkInventoryAction, $this->player, $this->inventoryManager);
if($action !== null){ if($action !== null){
$actions[] = $action; $actions[] = $action;
} }
@ -293,14 +296,14 @@ class InGamePacketHandler extends PacketHandler{
return true; return true;
} }
try{ try{
$this->session->getInvManager()->onTransactionStart($this->craftingTransaction); $this->inventoryManager->onTransactionStart($this->craftingTransaction);
$this->craftingTransaction->execute(); $this->craftingTransaction->execute();
}catch(TransactionException $e){ }catch(TransactionException $e){
$this->session->getLogger()->debug("Failed to execute crafting transaction: " . $e->getMessage()); $this->session->getLogger()->debug("Failed to execute crafting transaction: " . $e->getMessage());
//TODO: only sync slots that the client tried to change //TODO: only sync slots that the client tried to change
foreach($this->craftingTransaction->getInventories() as $inventory){ foreach($this->craftingTransaction->getInventories() as $inventory){
$this->session->getInvManager()->syncContents($inventory); $this->inventoryManager->syncContents($inventory);
} }
/* /*
* TODO: HACK! * TODO: HACK!
@ -328,7 +331,7 @@ class InGamePacketHandler extends PacketHandler{
} }
$transaction = new InventoryTransaction($this->player, $actions); $transaction = new InventoryTransaction($this->player, $actions);
$this->session->getInvManager()->onTransactionStart($transaction); $this->inventoryManager->onTransactionStart($transaction);
try{ try{
$transaction->execute(); $transaction->execute();
}catch(TransactionException $e){ }catch(TransactionException $e){
@ -337,7 +340,7 @@ class InGamePacketHandler extends PacketHandler{
$logger->debug("Actions: " . json_encode($data->getActions())); $logger->debug("Actions: " . json_encode($data->getActions()));
foreach($transaction->getInventories() as $inventory){ foreach($transaction->getInventories() as $inventory){
$this->session->getInvManager()->syncContents($inventory); $this->inventoryManager->syncContents($inventory);
} }
return false; return false;
@ -392,12 +395,12 @@ class InGamePacketHandler extends PacketHandler{
case UseItemTransactionData::ACTION_CLICK_AIR: case UseItemTransactionData::ACTION_CLICK_AIR:
if($this->player->isUsingItem()){ if($this->player->isUsingItem()){
if(!$this->player->consumeHeldItem()){ 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; return true;
} }
if(!$this->player->useHeldItem()){ 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; return true;
} }
@ -409,7 +412,7 @@ class InGamePacketHandler extends PacketHandler{
* Internal function used to execute rollbacks when an action fails on a block. * Internal function used to execute rollbacks when an action fails on a block.
*/ */
private function onFailedBlockAction(Vector3 $blockPos, ?int $face) : void{ 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){ if($blockPos->distanceSquared($this->player->getLocation()) < 10000){
$blocks = $blockPos->sidesArray(); $blocks = $blockPos->sidesArray();
if($face !== null){ if($face !== null){
@ -436,12 +439,12 @@ class InGamePacketHandler extends PacketHandler{
switch($data->getActionType()){ switch($data->getActionType()){
case UseItemOnEntityTransactionData::ACTION_INTERACT: case UseItemOnEntityTransactionData::ACTION_INTERACT:
if(!$this->player->interactEntity($target, $data->getClickPos())){ 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; return true;
case UseItemOnEntityTransactionData::ACTION_ATTACK: case UseItemOnEntityTransactionData::ACTION_ATTACK:
if(!$this->player->attackEntity($target)){ 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; return true;
} }
@ -454,7 +457,7 @@ class InGamePacketHandler extends PacketHandler{
switch($data->getActionType()){ switch($data->getActionType()){
case ReleaseItemTransactionData::ACTION_RELEASE: case ReleaseItemTransactionData::ACTION_RELEASE:
if(!$this->player->releaseHeldItem()){ if(!$this->player->releaseHeldItem()){
$this->session->getInvManager()->syncContents($this->player->getInventory()); $this->inventoryManager->syncContents($this->player->getInventory());
} }
return true; return true;
} }
@ -467,9 +470,9 @@ class InGamePacketHandler extends PacketHandler{
return true; //this happens when we put an item into the offhand return true; //this happens when we put an item into the offhand
} }
if($packet->windowId === ContainerIds::INVENTORY){ if($packet->windowId === ContainerIds::INVENTORY){
$this->session->getInvManager()->onClientSelectHotbarSlot($packet->hotbarSlot); $this->inventoryManager->onClientSelectHotbarSlot($packet->hotbarSlot);
if(!$this->player->selectHotbarSlot($packet->hotbarSlot)){ if(!$this->player->selectHotbarSlot($packet->hotbarSlot)){
$this->session->getInvManager()->syncSelectedHotbarSlot(); $this->inventoryManager->syncSelectedHotbarSlot();
} }
return true; return true;
} }
@ -603,7 +606,7 @@ class InGamePacketHandler extends PacketHandler{
if(array_key_exists($packet->windowId, $this->openHardcodedWindows)){ if(array_key_exists($packet->windowId, $this->openHardcodedWindows)){
unset($this->openHardcodedWindows[$packet->windowId]); unset($this->openHardcodedWindows[$packet->windowId]);
}else{ }else{
$this->session->getInvManager()->onClientRemoveWindow($packet->windowId); $this->inventoryManager->onClientRemoveWindow($packet->windowId);
} }
$this->session->sendDataPacket(ContainerClosePacket::create($packet->windowId, false)); $this->session->sendDataPacket(ContainerClosePacket::create($packet->windowId, false));

View File

@ -27,6 +27,7 @@ use pocketmine\network\mcpe\cache\CraftingDataCache;
use pocketmine\network\mcpe\cache\StaticPacketCache; use pocketmine\network\mcpe\cache\StaticPacketCache;
use pocketmine\network\mcpe\convert\GlobalItemTypeDictionary; use pocketmine\network\mcpe\convert\GlobalItemTypeDictionary;
use pocketmine\network\mcpe\convert\TypeConverter; use pocketmine\network\mcpe\convert\TypeConverter;
use pocketmine\network\mcpe\InventoryManager;
use pocketmine\network\mcpe\NetworkSession; use pocketmine\network\mcpe\NetworkSession;
use pocketmine\network\mcpe\protocol\RequestChunkRadiusPacket; use pocketmine\network\mcpe\protocol\RequestChunkRadiusPacket;
use pocketmine\network\mcpe\protocol\StartGamePacket; use pocketmine\network\mcpe\protocol\StartGamePacket;
@ -53,10 +54,13 @@ class PreSpawnPacketHandler extends PacketHandler{
/** @var NetworkSession */ /** @var NetworkSession */
private $session; 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->player = $player;
$this->server = $server; $this->server = $server;
$this->session = $session; $this->session = $session;
$this->inventoryManager = $inventoryManager;
} }
public function setUp() : void{ public function setUp() : void{
@ -104,9 +108,9 @@ class PreSpawnPacketHandler extends PacketHandler{
} }
$this->player->sendData([$this->player]); $this->player->sendData([$this->player]);
$this->session->getInvManager()->syncAll(); $this->inventoryManager->syncAll();
$this->session->getInvManager()->syncCreative(); $this->inventoryManager->syncCreative();
$this->session->getInvManager()->syncSelectedHotbarSlot(); $this->inventoryManager->syncSelectedHotbarSlot();
$this->session->sendDataPacket(CraftingDataCache::getInstance()->getCache($this->server->getCraftingManager())); $this->session->sendDataPacket(CraftingDataCache::getInstance()->getCache($this->server->getCraftingManager()));
$this->session->syncPlayerList($this->server->getOnlinePlayers()); $this->session->syncPlayerList($this->server->getOnlinePlayers());

View File

@ -2340,8 +2340,11 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
//TODO: client side race condition here makes the opening work incorrectly //TODO: client side race condition here makes the opening work incorrectly
$this->removeCurrentWindow(); $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->logger->debug("Opening inventory " . get_class($inventory) . "#" . spl_object_id($inventory));
$this->getNetworkSession()->getInvManager()->onCurrentWindowChange($inventory); $inventoryManager->onCurrentWindowChange($inventory);
$inventory->onOpen($this); $inventory->onOpen($this);
$this->currentWindow = $inventory; $this->currentWindow = $inventory;
return true; 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->logger->debug("Closing inventory " . get_class($this->currentWindow) . "#" . spl_object_id($this->currentWindow));
$this->currentWindow->onClose($this); $this->currentWindow->onClose($this);
if($this->isConnected()){ if(($inventoryManager = $this->getNetworkSession()->getInvManager()) !== null){
$this->getNetworkSession()->getInvManager()->onCurrentWindowRemove(); $inventoryManager->onCurrentWindowRemove();
} }
$this->currentWindow = null; $this->currentWindow = null;
} }

View File

@ -135,11 +135,6 @@ parameters:
count: 1 count: 1
path: ../../../src/network/mcpe/NetworkSession.php 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\\.$#" 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 count: 1
@ -185,6 +180,16 @@ parameters:
count: 1 count: 1
path: ../../../src/network/mcpe/NetworkSession.php 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\\.$#" message: "#^Property pocketmine\\\\network\\\\mcpe\\\\protocol\\\\LevelSoundEventPacket\\:\\:\\$position \\(pocketmine\\\\math\\\\Vector3\\) does not accept pocketmine\\\\math\\\\Vector3\\|null\\.$#"
count: 1 count: 1