CombinedInventoryProxy for double chests is now ephemeral

previously we had to make sure that the two chests shared the same instance, so that the viewer
lists would be properly updated. Now, instead, we can just combine the viewer lists of the individual
chests, which is a lot cleaner.

src/block/Chest.php

@@ -172,26 +172,17 @@ class Chest extends Transparent implements AnimatedContainerLike, Container, Hor
 	}
 
 	public function getInventory() : ?Inventory{
-		$thisTile = $this->getTile();
-		if($thisTile === null){
+		$thisInventory = $this->getTile()?->getRealInventory();
+		if($thisInventory === null){
 			return null;
 		}
-		$pairTile = $this->getOtherHalf()?->getTile();
-		$thisInventory = $thisTile->getRealInventory();
-		if($pairTile === null){
-			$thisTile->setDoubleInventory(null);
+		$pairInventory = $this->getOtherHalf()?->getTile()?->getRealInventory();
+		if($pairInventory === null){
 			return $thisInventory;
 		}
-		$doubleInventory = $thisTile->getDoubleInventory() ?? $pairTile->getDoubleInventory() ?? null;
-		if($doubleInventory === null){
-			$pairInventory = $pairTile->getRealInventory();
-			[$left, $right] = $this->pairHalf === ChestPairHalf::LEFT ? [$thisInventory, $pairInventory] : [$pairInventory, $thisInventory];
-			$doubleInventory = new CombinedInventoryProxy([$left, $right]);
-			$thisTile->setDoubleInventory($doubleInventory);
-			$pairTile->setDoubleInventory($doubleInventory);
-		}
 
-		return $doubleInventory;
+		[$left, $right] = $this->pairHalf === ChestPairHalf::LEFT ? [$thisInventory, $pairInventory] : [$pairInventory, $thisInventory];
+		return new CombinedInventoryProxy([$left, $right]);
 	}
 
 	protected function newMenu(Player $player, Inventory $inventory, Position $position) : InventoryWindow{

src/block/tile/Barrel.php

@@ -52,7 +52,7 @@ class Barrel extends Spawnable implements ContainerTile, Nameable{
 
 	public function close() : void{
 		if(!$this->closed){
-			$this->inventory->removeAllViewers();
+			$this->inventory->removeAllWindows();
 			parent::close();
 		}
 	}

src/block/tile/BrewingStand.php

@@ -107,7 +107,7 @@ class BrewingStand extends Spawnable implements ContainerTile, Nameable{
 
 	public function close() : void{
 		if(!$this->closed){
-			$this->inventory->removeAllViewers();
+			$this->inventory->removeAllWindows();
 
 			parent::close();
 		}

src/block/tile/Chest.php

@@ -94,25 +94,13 @@ class Chest extends Spawnable implements ContainerTile, Nameable{
 
 	public function close() : void{
 		if(!$this->closed){
-			$this->inventory->removeAllViewers();
-
-			if($this->doubleInventory !== null){
-				$this->doubleInventory->removeAllViewers();
-				$this->doubleInventory = null;
-			}
-
+			$this->inventory->removeAllWindows();
 			parent::close();
 		}
 	}
 
-	public function getDoubleInventory() : ?CombinedInventoryProxy{ return $this->doubleInventory; }
-
-	public function setDoubleInventory(?CombinedInventoryProxy $inventory) : void{
-		$this->doubleInventory = $inventory;
-	}
-
-	public function getInventory() : Inventory|CombinedInventoryProxy{
-		return $this->doubleInventory ?? $this->inventory;
+	public function getInventory() : Inventory{
+		return $this->inventory;
 	}
 
 	public function getRealInventory() : Inventory{

src/block/tile/Furnace.php

@@ -99,7 +99,7 @@ abstract class Furnace extends Spawnable implements ContainerTile, Nameable{
 
 	public function close() : void{
 		if(!$this->closed){
-			$this->inventory->removeAllViewers();
+			$this->inventory->removeAllWindows();
 
 			parent::close();
 		}

src/block/tile/Hopper.php

@@ -60,7 +60,7 @@ class Hopper extends Spawnable implements ContainerTile, Nameable{
 
 	public function close() : void{
 		if(!$this->closed){
-			$this->inventory->removeAllViewers();
+			$this->inventory->removeAllWindows();
 
 			parent::close();
 		}

src/block/tile/ShulkerBox.php

@@ -85,7 +85,7 @@ class ShulkerBox extends Spawnable implements ContainerTile, Nameable{
 
 	public function close() : void{
 		if(!$this->closed){
-			$this->inventory->removeAllViewers();
+			$this->inventory->removeAllWindows();
 			parent::close();
 		}
 	}

src/entity/Human.php

@@ -563,9 +563,9 @@ class Human extends Living implements ProjectileSource, InventoryHolder{
 
 	protected function onDispose() : void{
 		$this->hotbar->getSelectedIndexChangeListeners()->clear();
-		$this->inventory->removeAllViewers();
-		$this->offHandInventory->removeAllViewers();
-		$this->enderInventory->removeAllViewers();
+		$this->inventory->removeAllWindows();
+		$this->offHandInventory->removeAllWindows();
+		$this->enderInventory->removeAllWindows();
 		parent::onDispose();
 	}
 

src/entity/Living.php

@@ -987,7 +987,7 @@ abstract class Living extends Entity{
 	}
 
 	protected function onDispose() : void{
-		$this->armorInventory->removeAllViewers();
+		$this->armorInventory->removeAllWindows();
 		$this->effectManager->getEffectAddHooks()->clear();
 		$this->effectManager->getEffectRemoveHooks()->clear();
 		parent::onDispose();

src/inventory/BaseInventory.php

@@ -25,14 +25,12 @@ namespace pocketmine\inventory;
 
 use pocketmine\item\Item;
 use pocketmine\item\VanillaItems;
-use pocketmine\player\Player;
 use pocketmine\utils\ObjectSet;
 use pocketmine\utils\Utils;
 use function array_slice;
 use function count;
 use function max;
 use function min;
-use function spl_object_id;
 
 /**
  * This class provides everything needed to implement an inventory, minus the underlying storage system.
@@ -41,11 +39,6 @@ use function spl_object_id;
  */
 abstract class BaseInventory implements Inventory, SlotValidatedInventory{
 	protected int $maxStackSize = Inventory::MAX_STACK;
-	/**
-	 * @var Player[]
-	 * @phpstan-var array<int, Player>
-	 */
-	protected array $viewers = [];
 	/**
 	 * @var InventoryListener[]|ObjectSet
 	 * @phpstan-var ObjectSet<InventoryListener>
@@ -325,33 +318,6 @@ abstract class BaseInventory implements Inventory, SlotValidatedInventory{
 		$this->setItem($slot2, $i1);
 	}
 
-	/**
-	 * @return Player[]
-	 */
-	public function getViewers() : array{
-		return $this->viewers;
-	}
-
-	/**
-	 * Removes the inventory window from all players currently viewing it.
-	 */
-	public function removeAllViewers() : void{
-		foreach($this->viewers as $hash => $viewer){
-			if($viewer->getCurrentWindow()?->getInventory() === $this){ //this might not be the case for the player's own inventory
-				$viewer->removeCurrentWindow();
-			}
-			unset($this->viewers[$hash]);
-		}
-	}
-
-	public function onOpen(Player $who) : void{
-		$this->viewers[spl_object_id($who)] = $who;
-	}
-
-	public function onClose(Player $who) : void{
-		unset($this->viewers[spl_object_id($who)]);
-	}
-
 	protected function onSlotChange(int $index, Item $before) : void{
 		foreach($this->listeners as $listener){
 			$listener->onSlotChange($this, $index, $before);

src/inventory/CombinedInventoryProxy.php

@@ -25,9 +25,12 @@ namespace pocketmine\inventory;
 
 use pocketmine\item\Item;
 use pocketmine\item\VanillaItems;
+use pocketmine\player\InventoryWindow;
 use pocketmine\utils\AssumptionFailedError;
 use function array_fill_keys;
 use function array_keys;
+use function array_map;
+use function array_merge;
 use function count;
 use function spl_object_id;
 
@@ -191,4 +194,26 @@ final class CombinedInventoryProxy extends BaseInventory{
 		[$inventory, $actualSlot] = $this->getInventory($index);
 		return $inventory->isSlotEmpty($actualSlot);
 	}
+
+	public function onOpen(InventoryWindow $window) : void{
+		foreach($this->backingInventories as $inventory){
+			$inventory->onOpen($window);
+		}
+	}
+
+	public function onClose(InventoryWindow $window) : void{
+		foreach($this->backingInventories as $inventory){
+			$inventory->onClose($window);
+		}
+	}
+
+	public function removeAllWindows() : void{
+		foreach($this->backingInventories as $inventory){
+			$inventory->removeAllWindows();
+		}
+	}
+
+	public function getViewers() : array{
+		return array_merge(...array_map(fn(Inventory $inventory) => $inventory->getViewers(), $this->backingInventories));
+	}
 }

src/inventory/Inventory.php

@@ -27,6 +27,7 @@ declare(strict_types=1);
 namespace pocketmine\inventory;
 
 use pocketmine\item\Item;
+use pocketmine\player\InventoryWindow;
 use pocketmine\player\Player;
 use pocketmine\utils\ObjectSet;
 
@@ -189,14 +190,14 @@ interface Inventory{
 	/**
 	 * Tells all Players viewing this inventory to stop viewing it and discard associated windows.
 	 */
-	public function removeAllViewers() : void;
+	public function removeAllWindows() : void;
 
 	/**
 	 * Called when a player opens this inventory.
 	 */
-	public function onOpen(Player $who) : void;
+	public function onOpen(InventoryWindow $window) : void;
 
-	public function onClose(Player $who) : void;
+	public function onClose(InventoryWindow $window) : void;
 
 	/**
 	 * Returns whether the specified slot exists in the inventory.

src/inventory/SimpleInventory.php

@@ -25,6 +25,10 @@ namespace pocketmine\inventory;
 
 use pocketmine\item\Item;
 use pocketmine\item\VanillaItems;
+use pocketmine\player\InventoryWindow;
+use pocketmine\player\Player;
+use function array_map;
+use function spl_object_id;
 
 /**
  * This class provides a complete implementation of a regular inventory.
@@ -36,6 +40,12 @@ class SimpleInventory extends BaseInventory{
 	 */
 	protected \SplFixedArray $slots;
 
+	/**
+	 * @var InventoryWindow[]
+	 * @phpstan-var array<int, InventoryWindow>
+	 */
+	protected array $windows = [];
+
 	public function __construct(int $size){
 		$this->slots = new \SplFixedArray($size);
 		parent::__construct();
@@ -92,4 +102,38 @@ class SimpleInventory extends BaseInventory{
 	public function isSlotEmpty(int $index) : bool{
 		return $this->slots[$index] === null || $this->slots[$index]->isNull();
 	}
+
+	/**
+	 * @return Player[]
+	 */
+	public function getViewers() : array{
+		$result = [];
+		//this can't use array_map - the result needs to be keyed by spl_object_id(player), not spl_object_id(window)
+		foreach($this->windows as $window){
+			$player = $window->getViewer();
+			$result[spl_object_id($player)] = $player;
+		}
+		return $result;
+	}
+
+	/**
+	 * Gets rid of any inventory windows known to be referencing this inventory
+	 */
+	public function removeAllWindows() : void{
+		foreach($this->windows as $hash => $window){
+			$viewer = $window->getViewer();
+			if($window->getViewer()->getCurrentWindow() === $window){
+				$viewer->removeCurrentWindow();
+			}
+			unset($this->windows[$hash]);
+		}
+	}
+
+	public function onOpen(InventoryWindow $window) : void{
+		$this->windows[spl_object_id($window)] = $window;
+	}
+
+	public function onClose(InventoryWindow $window) : void{
+		unset($this->windows[spl_object_id($window)]);
+	}
 }

src/inventory/transaction/SlotChangeActionBuilder.php

@@ -117,4 +117,20 @@ final class SlotChangeActionBuilder extends BaseInventory{
 		}
 		return $result;
 	}
+
+	public function getViewers() : array{
+		return [];
+	}
+
+	public function removeAllWindows() : void{
+		//NOOP
+	}
+
+	public function onOpen(InventoryWindow $window) : void{
+		//NOOP
+	}
+
+	public function onClose(InventoryWindow $window) : void{
+		//NOOP
+	}
 }

src/player/InventoryWindow.php

@@ -41,10 +41,10 @@ abstract class InventoryWindow{
 	}
 
 	public function onOpen() : void{
-		$this->inventory->onOpen($this->viewer);
+		$this->inventory->onOpen($this);
 	}
 
 	public function onClose() : void{
-		$this->inventory->onClose($this->viewer);
+		$this->inventory->onClose($this);
 	}
 }

src/player/Player.php

@@ -2414,8 +2414,8 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
 
 	protected function onDispose() : void{
 		$this->disconnect("Player destroyed");
-		$this->cursorInventory->removeAllViewers();
-		$this->craftingGrid->removeAllViewers();
+		$this->cursorInventory->removeAllWindows();
+		$this->craftingGrid->removeAllWindows();
 		parent::onDispose();
 	}