From 6610a19640aaa280abfce4f1c32dfd39b2217d49 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Wed, 3 Sep 2025 17:34:37 +0100
Subject: [PATCH] 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                           | 21 +++------
 src/block/tile/Barrel.php                     |  2 +-
 src/block/tile/BrewingStand.php               |  2 +-
 src/block/tile/Chest.php                      | 18 ++------
 src/block/tile/Furnace.php                    |  2 +-
 src/block/tile/Hopper.php                     |  2 +-
 src/block/tile/ShulkerBox.php                 |  2 +-
 src/entity/Human.php                          |  6 +--
 src/entity/Living.php                         |  2 +-
 src/inventory/BaseInventory.php               | 34 --------------
 src/inventory/CombinedInventoryProxy.php      | 25 +++++++++++
 src/inventory/Inventory.php                   |  7 +--
 src/inventory/SimpleInventory.php             | 44 +++++++++++++++++++
 .../transaction/SlotChangeActionBuilder.php   | 16 +++++++
 src/player/InventoryWindow.php                |  4 +-
 src/player/Player.php                         |  4 +-
 16 files changed, 111 insertions(+), 80 deletions(-)

diff --git a/src/block/Chest.php b/src/block/Chest.php
index 7abe42eb8..a912c3821 100644
--- a/src/block/Chest.php
+++ b/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{
diff --git a/src/block/tile/Barrel.php b/src/block/tile/Barrel.php
index c5914c648..f526e43f8 100644
--- a/src/block/tile/Barrel.php
+++ b/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();
 		}
 	}
diff --git a/src/block/tile/BrewingStand.php b/src/block/tile/BrewingStand.php
index cda7599f6..a86b9c3de 100644
--- a/src/block/tile/BrewingStand.php
+++ b/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();
 		}
diff --git a/src/block/tile/Chest.php b/src/block/tile/Chest.php
index 1bcd4293f..6f16b1d03 100644
--- a/src/block/tile/Chest.php
+++ b/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{
diff --git a/src/block/tile/Furnace.php b/src/block/tile/Furnace.php
index 0da17bba4..1f2ad2beb 100644
--- a/src/block/tile/Furnace.php
+++ b/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();
 		}
diff --git a/src/block/tile/Hopper.php b/src/block/tile/Hopper.php
index ff58b68e9..86487efdc 100644
--- a/src/block/tile/Hopper.php
+++ b/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();
 		}
diff --git a/src/block/tile/ShulkerBox.php b/src/block/tile/ShulkerBox.php
index 858bf4477..252bb00db 100644
--- a/src/block/tile/ShulkerBox.php
+++ b/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();
 		}
 	}
diff --git a/src/entity/Human.php b/src/entity/Human.php
index 591a6c473..834fd31aa 100644
--- a/src/entity/Human.php
+++ b/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();
 	}
 
diff --git a/src/entity/Living.php b/src/entity/Living.php
index e26ae1e2e..9155a93a4 100644
--- a/src/entity/Living.php
+++ b/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();
diff --git a/src/inventory/BaseInventory.php b/src/inventory/BaseInventory.php
index 3afb3eb2a..26c13dec5 100644
--- a/src/inventory/BaseInventory.php
+++ b/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);
diff --git a/src/inventory/CombinedInventoryProxy.php b/src/inventory/CombinedInventoryProxy.php
index 5b6d1a668..6fd0570b7 100644
--- a/src/inventory/CombinedInventoryProxy.php
+++ b/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));
+	}
 }
diff --git a/src/inventory/Inventory.php b/src/inventory/Inventory.php
index de460ca29..8df033664 100644
--- a/src/inventory/Inventory.php
+++ b/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.
diff --git a/src/inventory/SimpleInventory.php b/src/inventory/SimpleInventory.php
index a6a339322..90925488e 100644
--- a/src/inventory/SimpleInventory.php
+++ b/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)]);
+	}
 }
diff --git a/src/inventory/transaction/SlotChangeActionBuilder.php b/src/inventory/transaction/SlotChangeActionBuilder.php
index 7fe2490d9..02d089029 100644
--- a/src/inventory/transaction/SlotChangeActionBuilder.php
+++ b/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
+	}
 }
diff --git a/src/player/InventoryWindow.php b/src/player/InventoryWindow.php
index 78dcef64a..119b57049 100644
--- a/src/player/InventoryWindow.php
+++ b/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);
 	}
 }
diff --git a/src/player/Player.php b/src/player/Player.php
index 12da3a059..e014dc008 100644
--- a/src/player/Player.php
+++ b/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();
 	}