Clean up EnderChestInventory implementation

now, EnderChestInventory is just a temporary window, much like anvil/enchanting windows. It provides a gateway to the player's PlayerEnderInventory.

This removes one of the remaining obstacles to disallowing null World in Position constructor.

src/block/EnderChest.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\block;
 
+use pocketmine\block\inventory\EnderChestInventory;
 use pocketmine\block\tile\EnderChest as TileEnderChest;
 use pocketmine\block\utils\FacesOppositePlacingPlayerTrait;
 use pocketmine\block\utils\NormalHorizontalFacingInMetadataTrait;
@@ -57,8 +58,7 @@ class EnderChest extends Transparent{
 		if($player instanceof Player){
 			$enderChest = $this->pos->getWorld()->getTile($this->pos);
 			if($enderChest instanceof TileEnderChest and $this->getSide(Facing::UP)->isTransparent()){
-				$player->getEnderChestInventory()->setHolderPosition($this->pos);
+				$player->setCurrentWindow(new EnderChestInventory($this->pos, $player->getEnderInventory()));
-				$player->setCurrentWindow($player->getEnderChestInventory());
 			}
 		}
 

src/block/inventory/EnderChestInventory.php

@@ -23,20 +23,57 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\CallbackInventoryListener;
+use pocketmine\inventory\Inventory;
+use pocketmine\inventory\InventoryListener;
+use pocketmine\inventory\PlayerEnderInventory;
+use pocketmine\item\Item;
 use pocketmine\network\mcpe\protocol\BlockEventPacket;
+use pocketmine\player\Player;
 use pocketmine\world\Position;
 use pocketmine\world\sound\EnderChestCloseSound;
 use pocketmine\world\sound\EnderChestOpenSound;
 use pocketmine\world\sound\Sound;
 
+/**
+ * EnderChestInventory is not a real inventory; it's just a gateway to the player's ender inventory.
+ */
 class EnderChestInventory extends AnimatedBlockInventory{
 
-	public function __construct(){
+	private PlayerEnderInventory $inventory;
-		parent::__construct(new Position(0, 0, 0, null), 27);
+	private InventoryListener $inventoryListener;
+
+	public function __construct(Position $holder, PlayerEnderInventory $inventory){
+		parent::__construct($holder, $inventory->getSize());
+		$this->inventory = $inventory;
+		$this->inventory->getListeners()->add($this->inventoryListener = new CallbackInventoryListener(
+			function(Inventory $unused, int $slot, Item $oldItem) : void{
+				$this->onSlotChange($slot, $oldItem);
+			},
+			function(Inventory $unused, array $oldContents) : void{
+				$this->onContentChange($oldContents);
+			}
+		));
 	}
 
-	public function setHolderPosition(Position $pos) : void{
+	public function getEnderInventory() : PlayerEnderInventory{
-		$this->holder = $pos->asPosition();
+		return $this->inventory;
+	}
+
+	public function getItem(int $index) : Item{
+		return $this->inventory->getItem($index);
+	}
+
+	public function setItem(int $index, Item $item) : void{
+		$this->inventory->setItem($index, $item);
+	}
+
+	public function getContents(bool $includeEmpty = false) : array{
+		return $this->inventory->getContents($includeEmpty);
+	}
+
+	public function setContents(array $items) : void{
+		$this->inventory->setContents($items);
 	}
 
 	protected function getOpenSound() : Sound{
@@ -53,4 +90,12 @@ class EnderChestInventory extends AnimatedBlockInventory{
 		//event ID is always 1 for a chest
 		$holder->getWorld()->broadcastPacketToViewers($holder, BlockEventPacket::create(1, $isOpen ? 1 : 0, $holder->asVector3()));
 	}
+
+	public function onClose(Player $who) : void{
+		parent::onClose($who);
+		if($who === $this->inventory->getHolder()){
+			$this->inventory->getListeners()->remove($this->inventoryListener);
+			$this->inventoryListener = CallbackInventoryListener::onAnyChange(static function() : void{}); //break cyclic reference
+		}
+	}
 }

src/entity/Human.php

@@ -33,6 +33,7 @@ use pocketmine\event\player\PlayerExhaustEvent;
 use pocketmine\inventory\CallbackInventoryListener;
 use pocketmine\inventory\Inventory;
 use pocketmine\inventory\InventoryHolder;
+use pocketmine\inventory\PlayerEnderInventory;
 use pocketmine\inventory\PlayerInventory;
 use pocketmine\item\enchantment\VanillaEnchantments;
 use pocketmine\item\Item;
@@ -73,8 +74,8 @@ class Human extends Living implements ProjectileSource, InventoryHolder{
 	/** @var PlayerInventory */
 	protected $inventory;
 
-	/** @var EnderChestInventory */
+	/** @var PlayerEnderInventory */
-	protected $enderChestInventory;
+	protected $enderInventory;
 
 	/** @var UuidInterface */
 	protected $uuid;
@@ -193,8 +194,8 @@ class Human extends Living implements ProjectileSource, InventoryHolder{
 		return $this->inventory;
 	}
 
-	public function getEnderChestInventory() : EnderChestInventory{
+	public function getEnderInventory() : PlayerEnderInventory{
-		return $this->enderChestInventory;
+		return $this->enderInventory;
 	}
 
 	/**
@@ -233,7 +234,7 @@ class Human extends Living implements ProjectileSource, InventoryHolder{
 				}
 			}
 		));
-		$this->enderChestInventory = new EnderChestInventory();
+		$this->enderInventory = new PlayerEnderInventory($this);
 		$this->initHumanData($nbt);
 
 		$inventoryTag = $nbt->getListTag("Inventory");
@@ -263,7 +264,7 @@ class Human extends Living implements ProjectileSource, InventoryHolder{
 		if($enderChestInventoryTag !== null){
 			/** @var CompoundTag $item */
 			foreach($enderChestInventoryTag as $i => $item){
-				$this->enderChestInventory->setItem($item->getByte("Slot"), Item::nbtDeserialize($item));
+				$this->enderInventory->setItem($item->getByte("Slot"), Item::nbtDeserialize($item));
 			}
 		}
 
@@ -382,13 +383,13 @@ class Human extends Living implements ProjectileSource, InventoryHolder{
 			$nbt->setInt("SelectedInventorySlot", $this->inventory->getHeldItemIndex());
 		}
 
-		if($this->enderChestInventory !== null){
+		if($this->enderInventory !== null){
 			/** @var CompoundTag[] $items */
 			$items = [];
 
-			$slotCount = $this->enderChestInventory->getSize();
+			$slotCount = $this->enderInventory->getSize();
 			for($slot = 0; $slot < $slotCount; ++$slot){
-				$item = $this->enderChestInventory->getItem($slot);
+				$item = $this->enderInventory->getItem($slot);
 				if(!$item->isNull()){
 					$items[] = $item->nbtSerialize($slot);
 				}
@@ -466,13 +467,13 @@ class Human extends Living implements ProjectileSource, InventoryHolder{
 	protected function onDispose() : void{
 		$this->inventory->removeAllViewers();
 		$this->inventory->getHeldItemIndexChangeListeners()->clear();
-		$this->enderChestInventory->removeAllViewers();
+		$this->enderInventory->removeAllViewers();
 		parent::onDispose();
 	}
 
 	protected function destroyCycles() : void{
 		$this->inventory = null;
-		$this->enderChestInventory = null;
+		$this->enderInventory = null;
 		$this->hungerManager = null;
 		$this->xpManager = null;
 		parent::destroyCycles();

src/inventory/BaseInventory.php

@@ -117,13 +117,7 @@ abstract class BaseInventory implements Inventory{
 			$this->viewers[$id] = $viewer;
 		}
 
-		foreach($this->listeners as $listener){
+		$this->onContentChange($oldContents);
-			$listener->onContentChange($this, $oldContents);
-		}
-
-		foreach($this->getViewers() as $viewer){
-			$viewer->getNetworkSession()->getInvManager()->syncContents($this);
-		}
 	}
 
 	public function setItem(int $index, Item $item) : void{
@@ -179,6 +173,20 @@ abstract class BaseInventory implements Inventory{
 		}
 	}
 
+	/**
+	 * @param Item[] $itemsBefore
+	 * @phpstan-param array<int, Item> $itemsBefore
+	 */
+	protected function onContentChange(array $itemsBefore) : void{
+		foreach($this->listeners as $listener){
+			$listener->onContentChange($this, $itemsBefore);
+		}
+
+		foreach($this->getViewers() as $viewer){
+			$viewer->getNetworkSession()->getInvManager()->syncContents($this);
+		}
+	}
+
 	public function slotExists(int $slot) : bool{
 		return $slot >= 0 and $slot < $this->slots->getSize();
 	}

src/inventory/PlayerEnderInventory.php

@@ -0,0 +1,38 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\inventory;
+
+use pocketmine\entity\Human;
+
+final class PlayerEnderInventory extends BaseInventory{
+
+	private Human $holder;
+
+	public function __construct(Human $holder, int $size = 27){
+		$this->holder = $holder;
+		parent::__construct($size);
+	}
+
+	public function getHolder() : Human{ return $this->holder; }
+}

tests/phpstan/configs/gc-hacks.neon

@@ -36,7 +36,7 @@ parameters:
 			path: ../../../src/entity/Entity.php
 
 		-
-			message: "#^Property pocketmine\\\\entity\\\\Human\\:\\:\\$enderChestInventory \\(pocketmine\\\\block\\\\inventory\\\\EnderChestInventory\\) does not accept null\\.$#"
+			message: "#^Property pocketmine\\\\entity\\\\Human\\:\\:\\$enderInventory \\(pocketmine\\\\inventory\\\\PlayerEnderInventory\\) does not accept null\\.$#"
 			count: 1
 			path: ../../../src/entity/Human.php