From 93b83b41893e3d351ba136c6d46e0291b4473499 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 15 Jun 2019 18:19:09 +0100 Subject: [PATCH] Player: Window system now only allows 1 window at a time --- src/pocketmine/Player.php | 182 ++++++++---------- src/pocketmine/block/Anvil.php | 2 +- src/pocketmine/block/BrewingStand.php | 2 +- src/pocketmine/block/Chest.php | 2 +- src/pocketmine/block/EnchantingTable.php | 2 +- src/pocketmine/block/EnderChest.php | 2 +- src/pocketmine/block/Furnace.php | 2 +- src/pocketmine/block/Hopper.php | 2 +- src/pocketmine/inventory/BaseInventory.php | 4 +- .../network/mcpe/NetworkSession.php | 6 +- 10 files changed, 96 insertions(+), 110 deletions(-) diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php index 8ef03daee..38ef47229 100644 --- a/src/pocketmine/Player.php +++ b/src/pocketmine/Player.php @@ -115,6 +115,7 @@ use pocketmine\world\particle\PunchBlockParticle; use pocketmine\world\Position; use pocketmine\world\World; use function abs; +use function array_search; use function assert; use function ceil; use function count; @@ -182,12 +183,13 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener, /** @var PlayerInfo */ protected $playerInfo; + /** @var int */ protected $lastInventoryNetworkId = 2; - /** @var int[] object ID => network ID */ - protected $inventoryNetworkIdMap = []; + /** @var Inventory[] network ID => inventory */ + protected $networkIdToInventoryMap = []; + /** @var Inventory|null */ + protected $currentWindow = null; /** @var Inventory[] */ - protected $windowIndex = []; - /** @var bool[] */ protected $permanentWindows = []; /** @var PlayerCursorInventory */ protected $cursorInventory; @@ -2424,13 +2426,11 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener, $this->usedChunks = []; $this->loadQueue = []; - $this->removeAllWindows(); - foreach($this->permanentWindows as $networkId => $bool){ - $this->closeInventoryInternal($this->windowIndex[$networkId], true); + $this->removeCurrentWindow(); + foreach($this->permanentWindows as $objectId => $inventory){ + $this->closeInventoryInternal($inventory, true); } - assert(empty($this->windowIndex) && empty($this->inventoryNetworkIdMap)); - $this->inventoryNetworkIdMap = []; - $this->windowIndex = []; + assert(empty($this->networkIdToInventoryMap)); $this->perm->clearPermissions(); @@ -2657,7 +2657,7 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener, public function teleport(Vector3 $pos, ?float $yaw = null, ?float $pitch = null) : bool{ if(parent::teleport($pos, $yaw, $pitch)){ - $this->removeAllWindows(); + $this->removeCurrentWindow(); $this->sendPosition($this, $this->yaw, $this->pitch, MovePlayerPacket::MODE_TELEPORT); $this->broadcastMovement(true); @@ -2741,14 +2741,10 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener, * @return bool */ public function doCloseWindow(int $windowId) : bool{ - if($windowId === 0){ - return false; - } - $this->doCloseInventory(); - if(isset($this->windowIndex[$windowId])){ - $this->removeWindow($this->windowIndex[$windowId]); + if($windowId === $this->lastInventoryNetworkId and $this->currentWindow !== null){ + $this->removeCurrentWindow(); return true; } if($windowId === 255){ @@ -2756,18 +2752,62 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener, return true; } + $this->logger->debug("Attempted to close inventory with network ID $windowId, but current is $this->lastInventoryNetworkId"); return false; } + /** + * Returns the inventory the player is currently viewing. This might be a chest, furnace, or any other container. + * + * @return Inventory|null + */ + public function getCurrentWindow() : ?Inventory{ + return $this->currentWindow; + } + + /** + * Opens an inventory window to the player. Returns if it was successful. + * + * @param Inventory $inventory + * + * @return bool + */ + public function setCurrentWindow(Inventory $inventory) : bool{ + if($inventory === $this->currentWindow){ + return true; + } + $ev = new InventoryOpenEvent($inventory, $this); + $ev->call(); + if($ev->isCancelled()){ + return false; + } + + //TODO: client side race condition here makes the opening work incorrectly + $this->removeCurrentWindow(); + + $networkId = $this->lastInventoryNetworkId = max(ContainerIds::FIRST, ($this->lastInventoryNetworkId + 1) % ContainerIds::LAST); + + $this->openInventoryInternal($inventory, $networkId, false); + $this->currentWindow = $inventory; + return true; + } + + public function removeCurrentWindow() : void{ + if($this->currentWindow !== null){ + (new InventoryCloseEvent($this->craftingGrid, $this))->call(); + $this->closeInventoryInternal($this->currentWindow, false); + } + } + /** * Returns the window ID which the inventory has for this player, or -1 if the window is not open to the player. * * @param Inventory $inventory * - * @return int + * @return int|null */ - public function getWindowId(Inventory $inventory) : int{ - return $this->inventoryNetworkIdMap[spl_object_id($inventory)] ?? ContainerIds::NONE; + public function getWindowId(Inventory $inventory) : ?int{ + return ($id = array_search($inventory, $this->networkIdToInventoryMap, true)) !== false ? $id : null; } /** @@ -2779,104 +2819,48 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener, * @return Inventory|null */ public function getWindow(int $windowId) : ?Inventory{ - return $this->windowIndex[$windowId] ?? null; + return $this->networkIdToInventoryMap[$windowId] ?? null; } protected function openInventoryInternal(Inventory $inventory, int $networkId, bool $permanent) : void{ - $this->windowIndex[$networkId] = $inventory; - $this->inventoryNetworkIdMap[spl_object_id($inventory)] = $networkId; + $this->logger->debug("Opening inventory " . get_class($inventory) . "#" . spl_object_id($inventory) . " with network ID $networkId"); + $this->networkIdToInventoryMap[$networkId] = $inventory; $inventory->onOpen($this); if($permanent){ - $this->logger->debug("Opening permanent inventory " . get_class($inventory) . " with network ID $networkId"); - $this->permanentWindows[$networkId] = true; + $this->permanentWindows[spl_object_id($inventory)] = $inventory; } } protected function closeInventoryInternal(Inventory $inventory, bool $force) : bool{ + $this->logger->debug("Closing inventory " . get_class($inventory) . "#" . spl_object_id($inventory)); $objectId = spl_object_id($inventory); - $networkId = $this->inventoryNetworkIdMap[$objectId] ?? null; - if($networkId !== null){ - if(isset($this->permanentWindows[$networkId])){ - if(!$force){ - throw new \InvalidArgumentException("Cannot remove fixed window " . get_class($inventory) . " from " . $this->getName()); - } - $this->logger->debug("Closing permanent inventory " . get_class($inventory) . " with network ID $networkId"); + if($inventory === $this->currentWindow){ + $this->currentWindow = null; + }elseif(isset($this->permanentWindows[$objectId])){ + if(!$force){ + throw new \InvalidArgumentException("Cannot remove fixed window " . get_class($inventory) . " from " . $this->getName()); } - - $inventory->onClose($this); - unset($this->inventoryNetworkIdMap[$objectId], $this->windowIndex[$networkId], $this->permanentWindows[$networkId]); - return true; - } - return false; - } - - /** - * Opens an inventory window to the player. Returns the ID of the created window, or the existing window ID if the - * player is already viewing the specified inventory. - * - * @param Inventory $inventory - * - * @return int - * - * @throws \InvalidArgumentException if a forceID which is already in use is specified - * @throws \InvalidStateException if trying to add a window without forceID when no slots are free - */ - public function addWindow(Inventory $inventory) : int{ - if(($id = $this->getWindowId($inventory)) !== ContainerIds::NONE){ - return $id; + unset($this->permanentWindows[$objectId]); + }else{ + return false; } - $networkId = $this->lastInventoryNetworkId; - do{ - $networkId = max(ContainerIds::FIRST, ($networkId + 1) % ContainerIds::LAST); - if($networkId === $this->lastInventoryNetworkId){ //wraparound, no free slots - throw new \InvalidStateException("No free window IDs found"); - } - }while(isset($this->windowIndex[$networkId])); - $this->lastInventoryNetworkId = $networkId; - - $ev = new InventoryOpenEvent($inventory, $this); - $ev->call(); - if($ev->isCancelled()){ - return -1; - } - - $this->openInventoryInternal($inventory, $networkId, false); - return $networkId; - } - - /** - * Removes an inventory window from the player. - * - * @param Inventory $inventory - * - * @throws \InvalidArgumentException if trying to remove a fixed inventory window - */ - public function removeWindow(Inventory $inventory){ - if($this->closeInventoryInternal($inventory, false)){ - (new InventoryCloseEvent($inventory, $this))->call(); - } - } - - /** - * Removes all inventory windows from the player. This WILL NOT remove permanent inventories such as the player's - * own inventory. - */ - public function removeAllWindows(){ - foreach($this->windowIndex as $networkId => $window){ - if(isset($this->permanentWindows[$networkId])){ - continue; - } - - $this->removeWindow($window); - } + $inventory->onClose($this); + $networkId = $this->getWindowId($inventory); + assert($networkId !== null); + unset($this->networkIdToInventoryMap[$networkId]); + return true; } /** * @return Inventory[] */ public function getAllWindows() : array{ - return $this->windowIndex; + $windows = $this->permanentWindows; + if($this->currentWindow !== null){ + $windows[] = $this->currentWindow; + } + return $windows; } public function setMetadata(string $metadataKey, MetadataValue $newMetadataValue) : void{ diff --git a/src/pocketmine/block/Anvil.php b/src/pocketmine/block/Anvil.php index e70506057..f03c9174c 100644 --- a/src/pocketmine/block/Anvil.php +++ b/src/pocketmine/block/Anvil.php @@ -64,7 +64,7 @@ class Anvil extends Transparent implements Fallable{ public function onInteract(Item $item, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ if($player instanceof Player){ - $player->addWindow(new AnvilInventory($this)); + $player->setCurrentWindow(new AnvilInventory($this)); } return true; diff --git a/src/pocketmine/block/BrewingStand.php b/src/pocketmine/block/BrewingStand.php index 67408f0b6..465934fab 100644 --- a/src/pocketmine/block/BrewingStand.php +++ b/src/pocketmine/block/BrewingStand.php @@ -62,7 +62,7 @@ class BrewingStand extends Transparent{ if($player instanceof Player){ $stand = $this->getWorld()->getTile($this); if($stand instanceof TileBrewingStand and $stand->canOpenWith($item->getCustomName())){ - $player->addWindow($stand->getInventory()); + $player->setCurrentWindow($stand->getInventory()); } } diff --git a/src/pocketmine/block/Chest.php b/src/pocketmine/block/Chest.php index 1ef27fa6b..c56eeafac 100644 --- a/src/pocketmine/block/Chest.php +++ b/src/pocketmine/block/Chest.php @@ -99,7 +99,7 @@ class Chest extends Transparent{ return true; } - $player->addWindow($chest->getInventory()); + $player->setCurrentWindow($chest->getInventory()); } } diff --git a/src/pocketmine/block/EnchantingTable.php b/src/pocketmine/block/EnchantingTable.php index 2e3db817e..66ebae89d 100644 --- a/src/pocketmine/block/EnchantingTable.php +++ b/src/pocketmine/block/EnchantingTable.php @@ -45,7 +45,7 @@ class EnchantingTable extends Transparent{ if($player instanceof Player){ //TODO lock - $player->addWindow(new EnchantInventory($this)); + $player->setCurrentWindow(new EnchantInventory($this)); } return true; diff --git a/src/pocketmine/block/EnderChest.php b/src/pocketmine/block/EnderChest.php index d0078625e..d750b56db 100644 --- a/src/pocketmine/block/EnderChest.php +++ b/src/pocketmine/block/EnderChest.php @@ -76,7 +76,7 @@ class EnderChest extends Transparent{ $enderChest = $this->getWorld()->getTile($this); if($enderChest instanceof TileEnderChest and $this->getSide(Facing::UP)->isTransparent()){ $player->getEnderChestInventory()->setHolderPosition($enderChest); - $player->addWindow($player->getEnderChestInventory()); + $player->setCurrentWindow($player->getEnderChestInventory()); } } diff --git a/src/pocketmine/block/Furnace.php b/src/pocketmine/block/Furnace.php index 1a847f19a..c2248ebf7 100644 --- a/src/pocketmine/block/Furnace.php +++ b/src/pocketmine/block/Furnace.php @@ -92,7 +92,7 @@ class Furnace extends Solid{ if($player instanceof Player){ $furnace = $this->getWorld()->getTile($this); if($furnace instanceof TileFurnace and $furnace->canOpenWith($item->getCustomName())){ - $player->addWindow($furnace->getInventory()); + $player->setCurrentWindow($furnace->getInventory()); } } diff --git a/src/pocketmine/block/Hopper.php b/src/pocketmine/block/Hopper.php index 7f8f0a71a..6c997f350 100644 --- a/src/pocketmine/block/Hopper.php +++ b/src/pocketmine/block/Hopper.php @@ -78,7 +78,7 @@ class Hopper extends Transparent{ if($player !== null){ $tile = $this->world->getTile($this); if($tile instanceof TileHopper){ //TODO: find a way to have inventories open on click without this boilerplate in every block - $player->addWindow($tile->getInventory()); + $player->setCurrentWindow($tile->getInventory()); } return true; } diff --git a/src/pocketmine/inventory/BaseInventory.php b/src/pocketmine/inventory/BaseInventory.php index 879712d56..b71ac0453 100644 --- a/src/pocketmine/inventory/BaseInventory.php +++ b/src/pocketmine/inventory/BaseInventory.php @@ -342,7 +342,9 @@ abstract class BaseInventory implements Inventory{ */ public function removeAllViewers() : void{ foreach($this->viewers as $hash => $viewer){ - $viewer->removeWindow($this); + if($viewer->getCurrentWindow() === $this){ //this might not be the case for the player's own inventory + $viewer->removeCurrentWindow(); + } unset($this->viewers[$hash]); } } diff --git a/src/pocketmine/network/mcpe/NetworkSession.php b/src/pocketmine/network/mcpe/NetworkSession.php index 391944094..1c731fd2e 100644 --- a/src/pocketmine/network/mcpe/NetworkSession.php +++ b/src/pocketmine/network/mcpe/NetworkSession.php @@ -753,14 +753,14 @@ class NetworkSession{ public function syncInventorySlot(Inventory $inventory, int $slot) : void{ $windowId = $this->player->getWindowId($inventory); - if($windowId !== ContainerIds::NONE){ + if($windowId !== null){ $this->sendDataPacket(InventorySlotPacket::create($windowId, $slot, $inventory->getItem($slot))); } } public function syncInventoryContents(Inventory $inventory) : void{ $windowId = $this->player->getWindowId($inventory); - if($windowId !== ContainerIds::NONE){ + if($windowId !== null){ $this->sendDataPacket(InventoryContentPacket::create($windowId, $inventory->getContents(true))); } } @@ -773,7 +773,7 @@ class NetworkSession{ public function syncInventoryData(Inventory $inventory, int $propertyId, int $value) : void{ $windowId = $this->player->getWindowId($inventory); - if($windowId !== ContainerIds::NONE){ + if($windowId !== null){ $this->sendDataPacket(ContainerSetDataPacket::create($windowId, $propertyId, $value)); } }