diff --git a/src/inventory/CombinedInventory.php b/src/inventory/CombinedInventory.php index 922f1bc0d..aee5170cc 100644 --- a/src/inventory/CombinedInventory.php +++ b/src/inventory/CombinedInventory.php @@ -24,8 +24,11 @@ declare(strict_types=1); namespace pocketmine\inventory; use pocketmine\item\Item; +use pocketmine\item\VanillaItems; +use pocketmine\utils\AssumptionFailedError; use function array_fill_keys; use function array_keys; +use function count; use function spl_object_id; /** @@ -52,6 +55,9 @@ final class CombinedInventory extends BaseInventory{ */ private array $inventoryToOffsetMap = []; + private InventoryListener $backingInventoryListener; + private bool $modifyingBackingInventory = false; + /** * @phpstan-param Inventory[] $backingInventories */ @@ -74,6 +80,45 @@ final class CombinedInventory extends BaseInventory{ $combinedSize += $size; } $this->size = $combinedSize; + + $weakThis = \WeakReference::create($this); + $getThis = static fn() => $weakThis->get() ?? throw new AssumptionFailedError("Listener should've been unregistered in __destruct()"); + + $this->backingInventoryListener = new CallbackInventoryListener( + onSlotChange: static function(Inventory $inventory, int $slot, Item $oldItem) use ($getThis) : void{ + $strongThis = $getThis(); + if($strongThis->modifyingBackingInventory){ + return; + } + + $offset = $strongThis->inventoryToOffsetMap[spl_object_id($inventory)]; + $strongThis->onSlotChange($offset + $slot, $oldItem); + }, + onContentChange: static function(Inventory $inventory, array $oldContents) use ($getThis) : void{ + $strongThis = $getThis(); + if($strongThis->modifyingBackingInventory){ + return; + } + + if(count($strongThis->backingInventories) === 1){ + $strongThis->onContentChange($oldContents); + }else{ + $offset = $strongThis->inventoryToOffsetMap[spl_object_id($inventory)]; + for($slot = 0, $limit = $inventory->getSize(); $slot < $limit; $slot++){ + $strongThis->onSlotChange($offset + $slot, $oldContents[$slot] ?? VanillaItems::AIR()); + } + } + } + ); + foreach($this->backingInventories as $inventory){ + $inventory->getListeners()->add($this->backingInventoryListener); + } + } + + public function __destruct(){ + foreach($this->backingInventories as $inventory){ + $inventory->getListeners()->remove($this->backingInventoryListener); + } } /** @@ -87,7 +132,14 @@ final class CombinedInventory extends BaseInventory{ protected function internalSetItem(int $index, Item $item) : void{ [$inventory, $actualSlot] = $this->getInventory($index); - $inventory->setItem($actualSlot, $item); + + //Make sure our backing listener doesn't dispatch double updates to our own listeners + $this->modifyingBackingInventory = true; + try{ + $inventory->setItem($actualSlot, $item); + }finally{ + $this->modifyingBackingInventory = false; + } } protected function internalSetContents(array $items) : void{ @@ -98,7 +150,14 @@ final class CombinedInventory extends BaseInventory{ } foreach($contentsByInventory as $splObjectId => $backingInventoryContents){ $backingInventory = $this->backingInventories[$splObjectId]; - $backingInventory->setContents($backingInventoryContents); + + //Make sure our backing listener doesn't dispatch double updates to our own listeners + $this->modifyingBackingInventory = true; + try{ + $backingInventory->setContents($backingInventoryContents); + }finally{ + $this->modifyingBackingInventory = false; + } } } diff --git a/tests/phpunit/inventory/CombinedInventoryTest.php b/tests/phpunit/inventory/CombinedInventoryTest.php index 9bd1bed65..b62786ffe 100644 --- a/tests/phpunit/inventory/CombinedInventoryTest.php +++ b/tests/phpunit/inventory/CombinedInventoryTest.php @@ -177,4 +177,92 @@ final class CombinedInventoryTest extends \PHPUnit\Framework\TestCase{ self::assertFalse($inventory->isSlotEmpty(1)); self::assertFalse($inventory->isSlotEmpty(3)); } + + public function testListenersOnProxySlotUpdate() : void{ + $inventory = new CombinedInventory($this->createInventories()); + + $numChanges = 0; + $inventory->getListeners()->add(new CallbackInventoryListener( + onSlotChange: function(Inventory $inventory, int $slot, Item $before) use (&$numChanges) : void{ + $numChanges++; + }, + onContentChange: null + )); + $inventory->setItem(0, VanillaItems::DIAMOND_SWORD()); + self::assertSame(1, $numChanges, "Inventory listener detected wrong number of changes"); + } + + public function testListenersOnProxyContentUpdate() : void{ + $inventory = new CombinedInventory($this->createInventories()); + + $numChanges = 0; + $inventory->getListeners()->add(new CallbackInventoryListener( + onSlotChange: null, + onContentChange: function(Inventory $inventory, array $oldItems) use (&$numChanges) : void{ + $numChanges++; + } + )); + $inventory->setContents(self::getAltItems()); + self::assertSame(1, $numChanges, "Expected onContentChange to be called exactly 1 time"); + } + + public function testListenersOnBackingSlotUpdate() : void{ + $backing = $this->createInventories(); + $inventory = new CombinedInventory($backing); + + $slotChangeDetected = null; + $numChanges = 0; + $inventory->getListeners()->add(new CallbackInventoryListener( + onSlotChange: function(Inventory $inventory, int $slot, Item $before) use (&$slotChangeDetected, &$numChanges) : void{ + $slotChangeDetected = $slot; + $numChanges++; + }, + onContentChange: null + )); + $backing[2]->setItem(0, VanillaItems::DIAMOND_SWORD()); + self::assertNotNull($slotChangeDetected, "Inventory listener didn't hear about backing inventory update"); + self::assertSame(2, $slotChangeDetected, "Inventory listener detected unexpected slot change"); + self::assertSame(1, $numChanges, "Inventory listener detected wrong number of changes"); + } + + /** + * When a combined inventory has multiple backing inventories, content updates of the backing inventories must be + * turned into slot updates on the proxy, to avoid syncing the entire proxy inventory. + */ + public function testListenersOnBackingContentUpdate() : void{ + $backing = $this->createInventories(); + $inventory = new CombinedInventory($backing); + + $slotChanges = []; + $inventory->getListeners()->add(new CallbackInventoryListener( + onSlotChange: function(Inventory $inventory, int $slot, Item $before) use (&$slotChanges) : void{ + $slotChanges[] = $slot; + }, + onContentChange: null + )); + $backing[2]->setContents([VanillaItems::DIAMOND_SWORD(), VanillaItems::DIAMOND()]); + self::assertCount(2, $slotChanges, "Inventory listener detected wrong number of changes"); + self::assertSame([2, 3], $slotChanges, "Incorrect slots updated"); + } + + /** + * If a combined inventory has only 1 backing inventory, content updates on the backing inventory can be directly + * processed as content updates on the proxy inventory without modification. This allows optimizations when only 1 + * backing inventory is used. + * This test verifies that this special case works as expected. + */ + public function testListenersOnSingleBackingContentUpdate() : void{ + $backing = new SimpleInventory(2); + $inventory = new CombinedInventory([$backing]); + + $numChanges = 0; + $inventory->getListeners()->add(new CallbackInventoryListener( + onSlotChange: null, + onContentChange: function(Inventory $inventory, array $oldItems) use (&$numChanges) : void{ + $numChanges++; + } + )); + $inventory->setContents([VanillaItems::DIAMOND_SWORD(), VanillaItems::DIAMOND()]); + self::assertSame(1, $numChanges, "Expected onContentChange to be called exactly 1 time"); + } }