Cleaned up implementations for EnderChestInventory/DoubleChestInventory

previously, these were forced to extend BaseInventory because of the amount of crap in Inventory's interface.
This meant that these inventories had their own slots storage, which would be _mostly_ unused because these inventories aren't real inventories, but rather just delegates.
This lead to a variety of bugs in the past, such as certain API methods on BaseInventory not working correctly for DoubleChestInventory in particular.

Now, BaseInventory just implements the functional part of the inventory implementation, leaving the storage system up to the implementation.
A SimpleInventory class is provided with a simple SplFixedArray storage backing, which is used by most inventories.
EnderChestInventory and DoubleChestInventory now extend BaseInventory directly, and implement custom methods for dealing with their delegates.

src/block/inventory/AnimatedBlockInventoryTrait.php

@@ -27,7 +27,14 @@ use pocketmine\player\Player;
 use pocketmine\world\sound\Sound;
 use function count;
 
-abstract class AnimatedBlockInventory extends BlockInventory{
+trait AnimatedBlockInventoryTrait{
+	use BlockInventoryTrait;
+
+	/**
+	 * @return Player[]
+	 * @phpstan-return array<int, Player>
+	 */
+	abstract public function getViewers() : array;
 
 	abstract protected function getOpenSound() : Sound;
 

src/block/inventory/AnvilInventory.php

@@ -23,13 +23,16 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\player\Player;
 use pocketmine\world\Position;
 
-class AnvilInventory extends BlockInventory{
+class AnvilInventory extends SimpleInventory implements BlockInventory{
+	use BlockInventoryTrait;
 
 	public function __construct(Position $holder){
-		parent::__construct($holder, 2);
+		$this->holder = $holder;
+		parent::__construct(2);
 	}
 
 	public function onClose(Player $who) : void{

src/block/inventory/BarrelInventory.php

@@ -24,15 +24,18 @@ declare(strict_types=1);
 namespace pocketmine\block\inventory;
 
 use pocketmine\block\Barrel;
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\world\Position;
 use pocketmine\world\sound\BarrelCloseSound;
 use pocketmine\world\sound\BarrelOpenSound;
 use pocketmine\world\sound\Sound;
 
-class BarrelInventory extends AnimatedBlockInventory{
+class BarrelInventory extends SimpleInventory implements BlockInventory{
+	use AnimatedBlockInventoryTrait;
 
 	public function __construct(Position $holder){
-		parent::__construct($holder, 27);
+		$this->holder = $holder;
+		parent::__construct(27);
 	}
 
 	protected function getOpenSound() : Sound{

src/block/inventory/BlockInventory.php

@@ -23,22 +23,8 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
-use pocketmine\inventory\BaseInventory;
 use pocketmine\world\Position;
 
-class BlockInventory extends BaseInventory{
-	/** @var Position */
-	protected $holder;
-
-	public function __construct(Position $holder, int $size){
-		$this->holder = $holder->asPosition();
-		parent::__construct($size);
-	}
-
-	/**
-	 * @return Position
-	 */
-	public function getHolder(){
-		return $this->holder;
-	}
+interface BlockInventory{
+	public function getHolder() : Position;
 }

src/block/inventory/BlockInventoryTrait.php

@@ -0,0 +1,34 @@
+<?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\block\inventory;
+
+use pocketmine\world\Position;
+
+trait BlockInventoryTrait{
+	protected Position $holder;
+
+	public function getHolder() : Position{
+		return $this->holder;
+	}
+}

src/block/inventory/BrewingStandInventory.php

@@ -23,11 +23,14 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\world\Position;
 
-class BrewingStandInventory extends BlockInventory{
+class BrewingStandInventory extends SimpleInventory implements BlockInventory{
+	use BlockInventoryTrait;
 
 	public function __construct(Position $holder, int $size = 5){
-		parent::__construct($holder, $size);
+		$this->holder = $holder;
+		parent::__construct($size);
 	}
 }

src/block/inventory/ChestInventory.php

@@ -23,16 +23,19 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\network\mcpe\protocol\BlockEventPacket;
 use pocketmine\world\Position;
 use pocketmine\world\sound\ChestCloseSound;
 use pocketmine\world\sound\ChestOpenSound;
 use pocketmine\world\sound\Sound;
 
-class ChestInventory extends AnimatedBlockInventory{
+class ChestInventory extends SimpleInventory implements BlockInventory{
+	use AnimatedBlockInventoryTrait;
 
 	public function __construct(Position $holder){
-		parent::__construct($holder, 27);
+		$this->holder = $holder;
+		parent::__construct(27);
 	}
 
 	protected function getOpenSound() : Sound{
@@ -43,7 +46,7 @@ class ChestInventory extends AnimatedBlockInventory{
 		return new ChestCloseSound();
 	}
 
-	protected function animateBlock(bool $isOpen) : void{
+	public function animateBlock(bool $isOpen) : void{
 		$holder = $this->getHolder();
 
 		//event ID is always 1 for a chest

src/block/inventory/DoubleChestInventory.php

@@ -23,11 +23,16 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\BaseInventory;
 use pocketmine\inventory\InventoryHolder;
 use pocketmine\item\Item;
+use pocketmine\world\sound\ChestCloseSound;
+use pocketmine\world\sound\ChestOpenSound;
 use pocketmine\world\sound\Sound;
 
-class DoubleChestInventory extends AnimatedBlockInventory implements InventoryHolder{
+class DoubleChestInventory extends BaseInventory implements BlockInventory, InventoryHolder{
+	use AnimatedBlockInventoryTrait;
+
 	/** @var ChestInventory */
 	private $left;
 	/** @var ChestInventory */
@@ -36,21 +41,24 @@ class DoubleChestInventory extends AnimatedBlockInventory implements InventoryHo
 	public function __construct(ChestInventory $left, ChestInventory $right){
 		$this->left = $left;
 		$this->right = $right;
-		parent::__construct($this->left->getHolder(), $this->left->getSize() + $this->right->getSize());
+		$this->holder = $this->left->getHolder();
+		parent::__construct();
 	}
 
 	public function getInventory(){
 		return $this;
 	}
 
+	public function getSize() : int{
+		return $this->left->getSize() + $this->right->getSize();
+	}
+
 	public function getItem(int $index) : Item{
 		return $index < $this->left->getSize() ? $this->left->getItem($index) : $this->right->getItem($index - $this->left->getSize());
 	}
 
-	public function setItem(int $index, Item $item) : void{
-		$old = $this->getItem($index);
+	protected function internalSetItem(int $index, Item $item) : void{
 		$index < $this->left->getSize() ? $this->left->setItem($index, $item) : $this->right->setItem($index - $this->left->getSize(), $item);
-		$this->onSlotChange($index, $old);
 	}
 
 	public function getContents(bool $includeEmpty = false) : array{
@@ -64,9 +72,26 @@ class DoubleChestInventory extends AnimatedBlockInventory implements InventoryHo
 		return $result;
 	}
 
-	protected function getOpenSound() : Sound{ return $this->left->getOpenSound(); }
+	protected function internalSetContents(array $items) : void{
+		$leftSize = $this->left->getSize();
 
-	protected function getCloseSound() : Sound{ return $this->left->getCloseSound(); }
+		$leftContents = [];
+		$rightContents = [];
+
+		foreach($items as $i => $item){
+			if($i < $this->left->getSize()){
+				$leftContents[$i] = $item;
+			}else{
+				$rightContents[$i - $leftSize] = $item;
+			}
+		}
+		$this->left->setContents($leftContents);
+		$this->right->setContents($rightContents);
+	}
+
+	protected function getOpenSound() : Sound{ return new ChestOpenSound(); }
+
+	protected function getCloseSound() : Sound{ return new ChestCloseSound(); }
 
 	protected function animateBlock(bool $isOpen) : void{
 		$this->left->animateBlock($isOpen);

src/block/inventory/EnchantInventory.php

@@ -23,13 +23,16 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\player\Player;
 use pocketmine\world\Position;
 
-class EnchantInventory extends BlockInventory{
+class EnchantInventory extends SimpleInventory implements BlockInventory{
+	use BlockInventoryTrait;
 
 	public function __construct(Position $holder){
-		parent::__construct($holder, 2);
+		$this->holder = $holder;
+		parent::__construct(2);
 	}
 
 	public function onClose(Player $who) : void{

src/block/inventory/EnderChestInventory.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\BaseInventory;
 use pocketmine\inventory\CallbackInventoryListener;
 use pocketmine\inventory\Inventory;
 use pocketmine\inventory\InventoryListener;
@@ -38,13 +39,17 @@ 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{
+class EnderChestInventory extends BaseInventory implements BlockInventory{
+	use AnimatedBlockInventoryTrait {
+		onClose as animatedBlockInventoryTrait_onClose;
+	}
 
 	private PlayerEnderInventory $inventory;
 	private InventoryListener $inventoryListener;
 
 	public function __construct(Position $holder, PlayerEnderInventory $inventory){
-		parent::__construct($holder, $inventory->getSize());
+		parent::__construct();
+		$this->holder = $holder;
 		$this->inventory = $inventory;
 		$this->inventory->getListeners()->add($this->inventoryListener = new CallbackInventoryListener(
 			function(Inventory $unused, int $slot, Item $oldItem) : void{
@@ -60,11 +65,15 @@ class EnderChestInventory extends AnimatedBlockInventory{
 		return $this->inventory;
 	}
 
+	public function getSize() : int{
+		return $this->inventory->getSize();
+	}
+
 	public function getItem(int $index) : Item{
 		return $this->inventory->getItem($index);
 	}
 
-	public function setItem(int $index, Item $item) : void{
+	protected function internalSetItem(int $index, Item $item) : void{
 		$this->inventory->setItem($index, $item);
 	}
 
@@ -72,7 +81,7 @@ class EnderChestInventory extends AnimatedBlockInventory{
 		return $this->inventory->getContents($includeEmpty);
 	}
 
-	public function setContents(array $items) : void{
+	protected function internalSetContents(array $items) : void{
 		$this->inventory->setContents($items);
 	}
 
@@ -92,7 +101,7 @@ class EnderChestInventory extends AnimatedBlockInventory{
 	}
 
 	public function onClose(Player $who) : void{
-		parent::onClose($who);
+		$this->animatedBlockInventoryTrait_onClose($who);
 		if($who === $this->inventory->getHolder()){
 			$this->inventory->getListeners()->remove($this->inventoryListener);
 			$this->inventoryListener = CallbackInventoryListener::onAnyChange(static function() : void{}); //break cyclic reference

src/block/inventory/FurnaceInventory.php

@@ -23,13 +23,16 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\item\Item;
 use pocketmine\world\Position;
 
-class FurnaceInventory extends BlockInventory{
+class FurnaceInventory extends SimpleInventory implements BlockInventory{
+	use BlockInventoryTrait;
 
 	public function __construct(Position $holder){
-		parent::__construct($holder, 3);
+		$this->holder = $holder;
+		parent::__construct(3);
 	}
 
 	public function getResult() : Item{

src/block/inventory/HopperInventory.php

@@ -23,11 +23,14 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\world\Position;
 
-class HopperInventory extends BlockInventory{
+class HopperInventory extends SimpleInventory implements BlockInventory{
+	use BlockInventoryTrait;
 
 	public function __construct(Position $holder, int $size = 5){
-		parent::__construct($holder, $size);
+		$this->holder = $holder;
+		parent::__construct($size);
 	}
 }

src/block/inventory/LoomInventory.php

@@ -23,16 +23,20 @@ declare(strict_types=1);
 
 namespace pocketmine\block\inventory;
 
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\player\Player;
 use pocketmine\world\Position;
 
-final class LoomInventory extends BlockInventory{
+final class LoomInventory extends SimpleInventory implements BlockInventory{
+	use BlockInventoryTrait;
+
 	public const SLOT_BANNER = 0;
 	public const SLOT_DYE = 1;
 	public const SLOT_PATTERN = 2;
 
 	public function __construct(Position $holder, int $size = 3){
-		parent::__construct($holder, $size);
+		$this->holder = $holder;
+		parent::__construct($size);
 	}
 
 	public function onClose(Player $who) : void{

src/crafting/CraftingGrid.php

@@ -23,14 +23,14 @@ declare(strict_types=1);
 
 namespace pocketmine\crafting;
 
-use pocketmine\inventory\BaseInventory;
+use pocketmine\inventory\SimpleInventory;
 use pocketmine\item\Item;
 use pocketmine\player\Player;
 use function max;
 use function min;
 use const PHP_INT_MAX;
 
-class CraftingGrid extends BaseInventory{
+class CraftingGrid extends SimpleInventory{
 	public const SIZE_SMALL = 2;
 	public const SIZE_BIG = 3;
 

src/inventory/ArmorInventory.php

@@ -26,7 +26,7 @@ namespace pocketmine\inventory;
 use pocketmine\entity\Living;
 use pocketmine\item\Item;
 
-class ArmorInventory extends BaseInventory{
+class ArmorInventory extends SimpleInventory{
 	public const SLOT_HEAD = 0;
 	public const SLOT_CHEST = 1;
 	public const SLOT_LEGS = 2;

src/inventory/BaseInventory.php

@@ -27,21 +27,18 @@ use pocketmine\item\Item;
 use pocketmine\item\ItemFactory;
 use pocketmine\player\Player;
 use pocketmine\utils\ObjectSet;
-use function array_map;
 use function array_slice;
 use function count;
 use function spl_object_id;
 
+/**
+ * This class provides everything needed to implement an inventory, minus the underlying storage system.
+ */
 abstract class BaseInventory implements Inventory{
 	use InventoryHelpersTrait;
 
 	/** @var int */
 	protected $maxStackSize = Inventory::MAX_STACK;
-	/**
-	 * @var \SplFixedArray|(Item|null)[]
-	 * @phpstan-var \SplFixedArray<Item|null>
-	 */
-	protected $slots;
 	/** @var Player[] */
 	protected $viewers = [];
 	/**
@@ -50,42 +47,18 @@ abstract class BaseInventory implements Inventory{
 	 */
 	protected $listeners;
 
-	public function __construct(int $size){
-		$this->slots = new \SplFixedArray($size);
+	public function __construct(){
 		$this->listeners = new ObjectSet();
 	}
 
-	/**
-	 * Returns the size of the inventory.
-	 */
-	public function getSize() : int{
-		return $this->slots->getSize();
-	}
-
 	public function getMaxStackSize() : int{
 		return $this->maxStackSize;
 	}
 
-	public function getItem(int $index) : Item{
-		return $this->slots[$index] !== null ? clone $this->slots[$index] : ItemFactory::air();
-	}
-
 	/**
-	 * @return Item[]
+	 * @param Item[] $items
 	 */
-	public function getContents(bool $includeEmpty = false) : array{
-		$contents = [];
-
-		foreach($this->slots as $i => $slot){
-			if($slot !== null){
-				$contents[$i] = clone $slot;
-			}elseif($includeEmpty){
-				$contents[$i] = ItemFactory::air();
-			}
-		}
-
-		return $contents;
-	}
+	abstract protected function internalSetContents(array $items) : void;
 
 	/**
 	 * @param Item[] $items
@@ -95,22 +68,14 @@ abstract class BaseInventory implements Inventory{
 			$items = array_slice($items, 0, $this->getSize(), true);
 		}
 
-		$oldContents = array_map(function(?Item $item) : Item{
-			return $item ?? ItemFactory::air();
-		}, $this->slots->toArray());
+		$oldContents = $this->getContents(true);
 
 		$listeners = $this->listeners->toArray();
 		$this->listeners->clear();
 		$viewers = $this->viewers;
 		$this->viewers = [];
 
-		for($i = 0, $size = $this->getSize(); $i < $size; ++$i){
-			if(!isset($items[$i])){
-				$this->clear($i);
-			}else{
-				$this->setItem($i, $items[$i]);
-			}
-		}
+		$this->internalSetContents($items);
 
 		$this->listeners->add(...$listeners); //don't directly write, in case listeners were added while operation was in progress
 		foreach($viewers as $id => $viewer){
@@ -120,6 +85,8 @@ abstract class BaseInventory implements Inventory{
 		$this->onContentChange($oldContents);
 	}
 
+	abstract protected function internalSetItem(int $index, Item $item) : void;
+
 	public function setItem(int $index, Item $item) : void{
 		if($item->isNull()){
 			$item = ItemFactory::air();
@@ -129,7 +96,7 @@ abstract class BaseInventory implements Inventory{
 
 		$oldItem = $this->getItem($index);
 
-		$this->slots[$index] = $item->isNull() ? null : $item;
+		$this->internalSetItem($index, $item);
 		$this->onSlotChange($index, $oldItem);
 	}
 
@@ -188,7 +155,7 @@ abstract class BaseInventory implements Inventory{
 	}
 
 	public function slotExists(int $slot) : bool{
-		return $slot >= 0 and $slot < $this->slots->getSize();
+		return $slot >= 0 and $slot < $this->getSize();
 	}
 
 	public function getListeners() : ObjectSet{

src/inventory/PlayerCursorInventory.php

@@ -25,7 +25,7 @@ namespace pocketmine\inventory;
 
 use pocketmine\player\Player;
 
-class PlayerCursorInventory extends BaseInventory{
+class PlayerCursorInventory extends SimpleInventory{
 	/** @var Player */
 	protected $holder;
 

src/inventory/PlayerEnderInventory.php

@@ -25,7 +25,7 @@ namespace pocketmine\inventory;
 
 use pocketmine\entity\Human;
 
-final class PlayerEnderInventory extends BaseInventory{
+final class PlayerEnderInventory extends SimpleInventory{
 
 	private Human $holder;
 

src/inventory/PlayerInventory.php

@@ -28,7 +28,7 @@ use pocketmine\item\Item;
 use pocketmine\player\Player;
 use pocketmine\utils\ObjectSet;
 
-class PlayerInventory extends BaseInventory{
+class PlayerInventory extends SimpleInventory{
 
 	/** @var Human */
 	protected $holder;

src/inventory/PlayerOffHandInventory.php

@@ -25,7 +25,7 @@ namespace pocketmine\inventory;
 
 use pocketmine\entity\Human;
 
-final class PlayerOffHandInventory extends BaseInventory{
+final class PlayerOffHandInventory extends SimpleInventory{
 	/** @var Human */
 	private $holder;
 

src/inventory/SimpleInventory.php

@@ -0,0 +1,85 @@
+<?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\item\Item;
+use pocketmine\item\ItemFactory;
+
+/**
+ * This class provides a complete implementation of a regular inventory.
+ */
+class SimpleInventory extends BaseInventory{
+	/**
+	 * @var \SplFixedArray|(Item|null)[]
+	 * @phpstan-var \SplFixedArray<Item|null>
+	 */
+	protected \SplFixedArray $slots;
+
+	public function __construct(int $size){
+		$this->slots = new \SplFixedArray($size);
+		parent::__construct();
+	}
+
+	/**
+	 * Returns the size of the inventory.
+	 */
+	public function getSize() : int{
+		return $this->slots->getSize();
+	}
+
+	public function getItem(int $index) : Item{
+		return $this->slots[$index] !== null ? clone $this->slots[$index] : ItemFactory::air();
+	}
+
+	/**
+	 * @return Item[]
+	 */
+	public function getContents(bool $includeEmpty = false) : array{
+		$contents = [];
+
+		foreach($this->slots as $i => $slot){
+			if($slot !== null){
+				$contents[$i] = clone $slot;
+			}elseif($includeEmpty){
+				$contents[$i] = ItemFactory::air();
+			}
+		}
+
+		return $contents;
+	}
+
+	protected function internalSetContents(array $items) : void{
+		for($i = 0, $size = $this->getSize(); $i < $size; ++$i){
+			if(!isset($items[$i])){
+				$this->clear($i);
+			}else{
+				$this->setItem($i, $items[$i]);
+			}
+		}
+	}
+
+	protected function internalSetItem(int $index, Item $item) : void{
+		$this->slots[$index] = $item->isNull() ? null : $item;
+	}
+}

tests/phpunit/inventory/BaseInventoryTest.php

@@ -30,7 +30,7 @@ use pocketmine\item\ItemIds;
 class BaseInventoryTest extends TestCase{
 
 	public function testAddItemDifferentUserData() : void{
-		$inv = new class(1) extends BaseInventory{
+		$inv = new class(1) extends SimpleInventory{
 
 		};
 		$item1 = ItemFactory::getInstance()->get(ItemIds::ARROW, 0, 1);