mirrors/PocketMine-MP
1
0
Fork 0
mirror of https://github.com/pmmp/PocketMine-MP.git synced 2025-06-20 10:24:07 +00:00
Code Issues Projects Releases Wiki Activity

CombinedInventory now propagates updates if its backing inventories were directly modified

Browse Source 
this was always lacking with DoubleChestInventory and is a major factor in it being basically useless for custom use cases.
This commit is contained in:
Dylan K. Taylor 2024-12-08 16:15:43 +00:00
parent 3ee78e20a5
commit 9949e3815f
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
2 changed files with 149 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
src/inventory/CombinedInventory.php
View File

@ -24,8 +24,11 @@ declare(strict_types=1);
namespace pocketmine\inventory; namespace pocketmine\inventory;
use pocketmine\item\Item; use pocketmine\item\Item;
use pocketmine\item\VanillaItems;
use pocketmine\utils\AssumptionFailedError;
use function array_fill_keys; use function array_fill_keys;
use function array_keys; use function array_keys;
use function count;
use function spl_object_id; use function spl_object_id;
/** /**
@ -52,6 +55,9 @@ final class CombinedInventory extends BaseInventory{
*/ */
private array $inventoryToOffsetMap = []; private array $inventoryToOffsetMap = [];
private InventoryListener $backingInventoryListener;
private bool $modifyingBackingInventory = false;
/** /**
* @phpstan-param Inventory[] $backingInventories * @phpstan-param Inventory[] $backingInventories
*/ */
@ -74,6 +80,45 @@ final class CombinedInventory extends BaseInventory{
$combinedSize += $size; $combinedSize += $size;
} }
$this->size = $combinedSize; $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{ protected function internalSetItem(int $index, Item $item) : void{
[$inventory, $actualSlot] = $this->getInventory($index); [$inventory, $actualSlot] = $this->getInventory($index);
//Make sure our backing listener doesn't dispatch double updates to our own listeners
$this->modifyingBackingInventory = true;
try{
$inventory->setItem($actualSlot, $item); $inventory->setItem($actualSlot, $item);
}finally{
$this->modifyingBackingInventory = false;
}
} }
protected function internalSetContents(array $items) : void{ protected function internalSetContents(array $items) : void{
@ -98,7 +150,14 @@ final class CombinedInventory extends BaseInventory{
} }
foreach($contentsByInventory as $splObjectId => $backingInventoryContents){ foreach($contentsByInventory as $splObjectId => $backingInventoryContents){
$backingInventory = $this->backingInventories[$splObjectId]; $backingInventory = $this->backingInventories[$splObjectId];
//Make sure our backing listener doesn't dispatch double updates to our own listeners
$this->modifyingBackingInventory = true;
try{
$backingInventory->setContents($backingInventoryContents); $backingInventory->setContents($backingInventoryContents);
}finally{
$this->modifyingBackingInventory = false;
}
} }
} }

88
tests/phpunit/inventory/CombinedInventoryTest.php
View File

@ -177,4 +177,92 @@ final class CombinedInventoryTest extends \PHPUnit\Framework\TestCase{
self::assertFalse($inventory->isSlotEmpty(1)); self::assertFalse($inventory->isSlotEmpty(1));
self::assertFalse($inventory->isSlotEmpty(3)); 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");
}
} }