From 48625db2b22d9b2ca76a5f476f2288aa5b37ffff Mon Sep 17 00:00:00 2001 From: ColinHDev Date: Sat, 15 Aug 2026 19:09:55 +0300 Subject: [PATCH 01/12] feat: implement hopper pushing, pulling and picking up items --- src/block/Hopper.php | 243 +++++++++++++++++++++++++++++++++++++- src/block/tile/Hopper.php | 10 ++ 2 files changed, 251 insertions(+), 2 deletions(-) diff --git a/src/block/Hopper.php b/src/block/Hopper.php index be180f6d1..7581a87db 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -25,17 +25,27 @@ namespace pocketmine\block; +use pocketmine\block\inventory\FurnaceInventory; +use pocketmine\block\inventory\HopperInventory; +use pocketmine\block\tile\Container; +use pocketmine\block\tile\Furnace as TileFurnace; use pocketmine\block\tile\Hopper as TileHopper; +use pocketmine\block\tile\Jukebox as TileJukebox; use pocketmine\block\utils\PoweredByRedstone; use pocketmine\block\utils\PoweredByRedstoneTrait; use pocketmine\block\utils\SupportType; use pocketmine\data\runtime\RuntimeDataDescriber; +use pocketmine\entity\object\ItemEntity; +use pocketmine\inventory\Inventory; +use pocketmine\item\Bucket; use pocketmine\item\Item; +use pocketmine\item\Record; use pocketmine\math\AxisAlignedBB; use pocketmine\math\Facing; use pocketmine\math\Vector3; use pocketmine\player\Player; use pocketmine\world\BlockTransaction; +use function count; class Hopper extends Transparent implements PoweredByRedstone{ use PoweredByRedstoneTrait; @@ -95,8 +105,237 @@ public function onInteract(Item $item, int $face, Vector3 $clickVector, ?Player } public function onScheduledUpdate() : void{ - //TODO + $this->position->getWorld()->scheduleDelayedBlockUpdate($this->position, 1); + + $tile = $this->position->getWorld()->getTile($this->position); + if(!$tile instanceof TileHopper){ + return; + } + + $transferCooldown = $tile->getTransferCooldown(); + if($transferCooldown > 0){ + $transferCooldown--; + $tile->setTransferCooldown($transferCooldown); + } + + if($this->isPowered() || $transferCooldown > 0){ + return; + } + + $inventory = $tile->getInventory(); + $success = $this->push($inventory); + // Hoppers that have a container above them, won't try to pick up items. + $origin = $this->position->getWorld()->getTile($this->position->getSide(Facing::UP)); + //TODO: Not all blocks a hopper can pull from have an inventory (for example: Jukebox). + if($origin instanceof Container){ + $success = $this->pull($inventory, $origin->getInventory()) || $success; + }else{ + $success = $this->pickup($inventory) || $success; + } + // The cooldown is only set back to the default amount of ticks if the hopper has done anything. + if($success){ + $tile->setTransferCooldown(TileHopper::DEFAULT_TRANSFER_COOLDOWN); + } } - //TODO: redstone logic, sucking logic + /** + * This function handles pushing items from the hopper to a tile in the direction the hopper is facing. + * Returns true if an item was successfully pushed or false on failure. + */ + private function push(HopperInventory $inventory) : bool{ + if(count($inventory->getContents()) === 0){ + return false; + } + $destination = $this->position->getWorld()->getTile($this->position->getSide($this->facing)); + if($destination === null){ + return false; + } + + for($slot = 0; $slot < $inventory->getSize(); $slot++){ + $item = $inventory->getItem($slot); + if($item->isNull()){ + continue; + } + + // Hoppers interact differently when pushing into different kinds of tiles. + //TODO: Composter + //TODO: Brewing Stand + //TODO: Jukebox (improve) + if($destination instanceof TileFurnace){ + // If the hopper is facing down, it will push every item to the furnace's input slot, even items that aren't smeltable. + // If the hopper is facing in any other direction, it will only push items that can be used as fuel to the furnace's fuel slot. + if($this->facing === Facing::DOWN){ + $slotInFurnace = FurnaceInventory::SLOT_INPUT; + $itemInFurnace = $destination->getInventory()->getSmelting(); + }else{ + if($item->getFuelTime() === 0){ + continue; + } + $slotInFurnace = FurnaceInventory::SLOT_FUEL; + $itemInFurnace = $destination->getInventory()->getFuel(); + } + if(!$itemInFurnace->isNull()){ + if($itemInFurnace->getCount() >= $itemInFurnace->getMaxStackSize()){ + return false; + } + if(!$itemInFurnace->canStackWith($item)){ + continue; + } + $item->pop(); + $itemInFurnace->setCount($itemInFurnace->getCount() + 1); + }else{ + $itemInFurnace = $item->pop(); + } + + //TODO: event on item inventory switch + + $destination->getInventory()->setItem($slotInFurnace, $itemInFurnace); + $inventory->setItem($slot, $item); + return true; + + }elseif($destination instanceof TileHopper){ + $itemToPush = $item->pop(); + if(!$destination->getInventory()->canAddItem($itemToPush)){ + continue; + } + // Hoppers pushing into empty hoppers set the empty hoppers transfer cooldown back to the default amount of ticks. + if(count($destination->getInventory()->getContents()) === 0){ + $destination->setTransferCooldown(TileHopper::DEFAULT_TRANSFER_COOLDOWN); + } + + }elseif($destination instanceof TileJukebox){ + if(!($item instanceof Record)){ + continue; + } + //TODO: + // Jukeboxes actually emit a redstone signal when playing a record so nearby hoppers are blocked and + // prevented from inserting another disk. Because neither does redstone work properly nor can we check if + // a jukebox is still playing a record or has already finished it, we can just check if it has already a + // record inserted. + if($destination->getRecord() !== null){ + return false; + } + + // The Jukebox block is handling the playing of records, so we need to get it here and can't use TileJukebox::setRecord(). + $jukeboxBlock = $destination->getBlock(); + if($jukeboxBlock instanceof Jukebox){ + $record = $item->pop(); + if($record instanceof Record){ + $jukeboxBlock->insertRecord($record); + $jukeboxBlock->getPosition()->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); + $inventory->setItem($slot, $item); + return true; + } + } + return false; + + }elseif($destination instanceof Container){ + $itemToPush = $item->pop(); + if(!$destination->getInventory()->canAddItem($itemToPush)){ + continue; + } + + }else{ + return false; + } + + //TODO: event on item inventory switch + + $inventory->setItem($slot, $item); + $destination->getInventory()->addItem($itemToPush); + return true; + } + return false; + } + + /** + * This function handles pulling items by the hopper from a container above. + * Returns true if an item was successfully pulled or false on failure. + */ + private function pull(HopperInventory $inventory, Inventory $origin) : bool{ + // Hoppers interact differently when pulling from different kinds of tiles. + //TODO: Composter + //TODO: Brewing Stand + //TODO: Jukebox + if($origin instanceof FurnaceInventory){ + // Hoppers either pull empty buckets from the furnace's fuel slot or pull from its result slot. + // They prioritise pulling from the fuel slot over the result slot. + $item = $origin->getFuel(); + if($item instanceof Bucket){ + $slot = FurnaceInventory::SLOT_FUEL; + }else{ + $slot = FurnaceInventory::SLOT_RESULT; + $item = $origin->getResult(); + if($item->isNull()){ + return false; + } + } + $itemToPull = $item->pop(); + if(!$inventory->canAddItem($itemToPull)){ + return false; + } + + //TODO: event on item inventory switch + + $origin->setItem($slot, $item); + $inventory->addItem($itemToPull); + return true; + + }else{ + for($slot = 0; $slot < $origin->getSize(); $slot++){ + $item = $origin->getItem($slot); + if($item->isNull()){ + continue; + } + $itemToPull = $item->pop(); + if(!$inventory->canAddItem($itemToPull)){ + continue; + } + + //TODO: event on item inventory switch + + $origin->setItem($slot, $item); + $inventory->addItem($itemToPull); + return true; + } + } + return false; + } + + /** + * This function handles picking up items by the hopper. + * Returns true if an item was successfully picked up or false on failure. + */ + private function pickup(HopperInventory $inventory) : bool{ + // In Bedrock Edition hoppers collect from the lower 3/4 of the block space above them. + $pickupCollisionBox = new AxisAlignedBB( + $this->position->getX(), + $this->position->getY() + 1, + $this->position->getZ(), + $this->position->getX() + 1, + $this->position->getY() + 1.75, + $this->position->getZ() + 1 + ); + + foreach($this->position->getWorld()->getNearbyEntities($pickupCollisionBox) as $entity){ + if($entity->isClosed() || $entity->isFlaggedForDespawn() || !$entity instanceof ItemEntity){ + continue; + } + // Unlike Java Edition, Bedrock Edition's hoppers don't save in which order item entities landed on top of them to collect them in that order. + // In Bedrock Edition hoppers collect item entities in the order in which they entered the chunk. + // Because of how entities are saved by PocketMine-MP the first entities of this loop are also the first ones who were saved. + // That's why we don't need to implement any sorting mechanism. + $item = $entity->getItem(); + if(!$inventory->canAddItem($item)){ + continue; + } + + //TODO: event on block picking up an item + + $inventory->addItem($item); + $entity->flagForDespawn(); + return true; + } + return false; + } } diff --git a/src/block/tile/Hopper.php b/src/block/tile/Hopper.php index 2f17f7987..796fead2b 100644 --- a/src/block/tile/Hopper.php +++ b/src/block/tile/Hopper.php @@ -36,6 +36,7 @@ class Hopper extends Spawnable implements Container, Nameable{ use NameableTrait; private const TAG_TRANSFER_COOLDOWN = "TransferCooldown"; + public const DEFAULT_TRANSFER_COOLDOWN = 8; private HopperInventory $inventory; private int $transferCooldown = 0; @@ -43,6 +44,7 @@ class Hopper extends Spawnable implements Container, Nameable{ public function __construct(World $world, Vector3 $pos){ parent::__construct($world, $pos); $this->inventory = new HopperInventory($this->position); + $this->position->getWorld()->scheduleDelayedBlockUpdate($this->position, 1); } public function readSaveData(CompoundTag $nbt) : void{ @@ -78,4 +80,12 @@ public function getInventory() : HopperInventory{ public function getRealInventory() : HopperInventory{ return $this->inventory; } + + public function getTransferCooldown() : int{ + return $this->transferCooldown; + } + + public function setTransferCooldown(int $transferCooldown) : void{ + $this->transferCooldown = $transferCooldown; + } } From 8165572478e4ba6242d299cf7d8d32ec4a1ded72 Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 19:41:42 +0300 Subject: [PATCH 02/12] feat: implement hopper brewing stand transfer, jukebox pulling and transfer events --- src/block/Hopper.php | 177 ++++++++++++++++-- src/block/Jukebox.php | 15 +- .../inventory/InventoryMoveItemEvent.php | 68 +++++++ 3 files changed, 238 insertions(+), 22 deletions(-) create mode 100644 src/event/inventory/InventoryMoveItemEvent.php diff --git a/src/block/Hopper.php b/src/block/Hopper.php index 7581a87db..74643ca07 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -25,8 +25,10 @@ namespace pocketmine\block; +use pocketmine\block\inventory\BrewingStandInventory; use pocketmine\block\inventory\FurnaceInventory; use pocketmine\block\inventory\HopperInventory; +use pocketmine\block\tile\BrewingStand as TileBrewingStand; use pocketmine\block\tile\Container; use pocketmine\block\tile\Furnace as TileFurnace; use pocketmine\block\tile\Hopper as TileHopper; @@ -36,10 +38,16 @@ use pocketmine\block\utils\SupportType; use pocketmine\data\runtime\RuntimeDataDescriber; use pocketmine\entity\object\ItemEntity; +use pocketmine\event\block\BlockItemPickupEvent; +use pocketmine\event\inventory\InventoryMoveItemEvent; use pocketmine\inventory\Inventory; use pocketmine\item\Bucket; +use pocketmine\item\GlassBottle; use pocketmine\item\Item; +use pocketmine\item\Potion; use pocketmine\item\Record; +use pocketmine\item\SplashPotion; +use pocketmine\item\VanillaItems; use pocketmine\math\AxisAlignedBB; use pocketmine\math\Facing; use pocketmine\math\Vector3; @@ -126,9 +134,10 @@ public function onScheduledUpdate() : void{ $success = $this->push($inventory); // Hoppers that have a container above them, won't try to pick up items. $origin = $this->position->getWorld()->getTile($this->position->getSide(Facing::UP)); - //TODO: Not all blocks a hopper can pull from have an inventory (for example: Jukebox). if($origin instanceof Container){ $success = $this->pull($inventory, $origin->getInventory()) || $success; + }elseif($origin instanceof TileJukebox){ + $success = $this->pullFromJukebox($inventory, $origin) || $success; }else{ $success = $this->pickup($inventory) || $success; } @@ -157,10 +166,10 @@ private function push(HopperInventory $inventory) : bool{ continue; } + $resetDestinationCooldown = false; + // Hoppers interact differently when pushing into different kinds of tiles. //TODO: Composter - //TODO: Brewing Stand - //TODO: Jukebox (improve) if($destination instanceof TileFurnace){ // If the hopper is facing down, it will push every item to the furnace's input slot, even items that aren't smeltable. // If the hopper is facing in any other direction, it will only push items that can be used as fuel to the furnace's fuel slot. @@ -181,27 +190,60 @@ private function push(HopperInventory $inventory) : bool{ if(!$itemInFurnace->canStackWith($item)){ continue; } - $item->pop(); - $itemInFurnace->setCount($itemInFurnace->getCount() + 1); - }else{ - $itemInFurnace = $item->pop(); } - //TODO: event on item inventory switch + $itemToPush = $this->callMoveItemEvent($inventory, $destination->getInventory(), $item->pop()); + if($itemToPush === null){ + continue; + } + if(!$itemInFurnace->isNull()){ + if(!$itemInFurnace->canStackWith($itemToPush)){ + continue; + } + $itemInFurnace->setCount($itemInFurnace->getCount() + $itemToPush->getCount()); + }else{ + $itemInFurnace = $itemToPush; + } $destination->getInventory()->setItem($slotInFurnace, $itemInFurnace); $inventory->setItem($slot, $item); return true; + }elseif($destination instanceof TileBrewingStand){ + $brewingInventory = $destination->getInventory(); + $slotInStand = $this->getBrewingStandSlot($brewingInventory, $item); + if($slotInStand === null){ + continue; + } + $itemInStand = $brewingInventory->getItem($slotInStand); + if(!$itemInStand->isNull() && (!$itemInStand->canStackWith($item) || $itemInStand->getCount() >= $itemInStand->getMaxStackSize())){ + continue; + } + + $itemToPush = $this->callMoveItemEvent($inventory, $brewingInventory, $item->pop()); + if($itemToPush === null){ + continue; + } + if(!$itemInStand->isNull()){ + if(!$itemInStand->canStackWith($itemToPush)){ + continue; + } + $itemInStand->setCount($itemInStand->getCount() + $itemToPush->getCount()); + }else{ + $itemInStand = $itemToPush; + } + + $brewingInventory->setItem($slotInStand, $itemInStand); + $inventory->setItem($slot, $item); + return true; + }elseif($destination instanceof TileHopper){ $itemToPush = $item->pop(); if(!$destination->getInventory()->canAddItem($itemToPush)){ continue; } // Hoppers pushing into empty hoppers set the empty hoppers transfer cooldown back to the default amount of ticks. - if(count($destination->getInventory()->getContents()) === 0){ - $destination->setTransferCooldown(TileHopper::DEFAULT_TRANSFER_COOLDOWN); - } + $resetDestinationCooldown = count($destination->getInventory()->getContents()) === 0; }elseif($destination instanceof TileJukebox){ if(!($item instanceof Record)){ @@ -239,7 +281,13 @@ private function push(HopperInventory $inventory) : bool{ return false; } - //TODO: event on item inventory switch + $itemToPush = $this->callMoveItemEvent($inventory, $destination->getInventory(), $itemToPush); + if($itemToPush === null){ + continue; + } + if($resetDestinationCooldown && $destination instanceof TileHopper){ + $destination->setTransferCooldown(TileHopper::DEFAULT_TRANSFER_COOLDOWN); + } $inventory->setItem($slot, $item); $destination->getInventory()->addItem($itemToPush); @@ -255,8 +303,6 @@ private function push(HopperInventory $inventory) : bool{ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ // Hoppers interact differently when pulling from different kinds of tiles. //TODO: Composter - //TODO: Brewing Stand - //TODO: Jukebox if($origin instanceof FurnaceInventory){ // Hoppers either pull empty buckets from the furnace's fuel slot or pull from its result slot. // They prioritise pulling from the fuel slot over the result slot. @@ -274,13 +320,36 @@ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ if(!$inventory->canAddItem($itemToPull)){ return false; } - - //TODO: event on item inventory switch + $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); + if($itemToPull === null){ + return false; + } $origin->setItem($slot, $item); $inventory->addItem($itemToPull); return true; + }elseif($origin instanceof BrewingStandInventory){ + // Hoppers only pull the brewed potions out of a brewing stand's bottle slots. + foreach([BrewingStandInventory::SLOT_BOTTLE_LEFT, BrewingStandInventory::SLOT_BOTTLE_MIDDLE, BrewingStandInventory::SLOT_BOTTLE_RIGHT] as $slot){ + $item = $origin->getItem($slot); + if($item->isNull()){ + continue; + } + $itemToPull = $item->pop(); + if(!$inventory->canAddItem($itemToPull)){ + continue; + } + $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); + if($itemToPull === null){ + continue; + } + + $origin->setItem($slot, $item); + $inventory->addItem($itemToPull); + return true; + } + }else{ for($slot = 0; $slot < $origin->getSize(); $slot++){ $item = $origin->getItem($slot); @@ -291,8 +360,10 @@ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ if(!$inventory->canAddItem($itemToPull)){ continue; } - - //TODO: event on item inventory switch + $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); + if($itemToPull === null){ + continue; + } $origin->setItem($slot, $item); $inventory->addItem($itemToPull); @@ -302,6 +373,27 @@ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ return false; } + /** + * This function handles pulling the inserted record out of a jukebox above the hopper. + * Returns true if the record was successfully pulled or false on failure. + */ + private function pullFromJukebox(HopperInventory $inventory, TileJukebox $jukebox) : bool{ + // The Jukebox block is handling the playing of records, so we need to get it here and can't use TileJukebox::setRecord(). + $jukeboxBlock = $jukebox->getBlock(); + if(!$jukeboxBlock instanceof Jukebox){ + return false; + } + $record = $jukeboxBlock->getRecord(); + if($record === null || !$inventory->canAddItem($record)){ + return false; + } + + $jukeboxBlock->extractRecord(); + $this->position->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); + $inventory->addItem($record); + return true; + } + /** * This function handles picking up items by the hopper. * Returns true if an item was successfully picked up or false on failure. @@ -330,12 +422,57 @@ private function pickup(HopperInventory $inventory) : bool{ continue; } - //TODO: event on block picking up an item + $ev = new BlockItemPickupEvent($this, $entity, $item, $inventory); + $ev->call(); + if($ev->isCancelled()){ + continue; + } + $destination = $ev->getInventory(); + if($destination === null){ + continue; + } + $pickedUpItem = $ev->getItem(); + if(!$destination->canAddItem($pickedUpItem)){ + continue; + } - $inventory->addItem($item); + $destination->addItem($pickedUpItem); $entity->flagForDespawn(); return true; } return false; } + + /** + * Returns the item to move after the event has been called, or null if the move was cancelled. + */ + private function callMoveItemEvent(Inventory $source, Inventory $destination, Item $item) : ?Item{ + $ev = new InventoryMoveItemEvent($source, $destination, $item); + $ev->call(); + return $ev->isCancelled() ? null : $ev->getItem(); + } + + /** + * Returns the brewing stand slot the given item would be pushed into, or null if the item cannot be pushed. + */ + private function getBrewingStandSlot(BrewingStandInventory $inventory, Item $item) : ?int{ + // Hoppers pushing from above fill the ingredient slot, while hoppers pushing from the side fill the fuel and bottle slots. + if($this->facing === Facing::DOWN){ + return BrewingStandInventory::SLOT_INGREDIENT; + } + if($item->equals(VanillaItems::BLAZE_POWDER(), true, false)){ + return BrewingStandInventory::SLOT_FUEL; + } + if(!$item instanceof Potion && !$item instanceof SplashPotion && !$item instanceof GlassBottle){ + return null; + } + + foreach([BrewingStandInventory::SLOT_BOTTLE_LEFT, BrewingStandInventory::SLOT_BOTTLE_MIDDLE, BrewingStandInventory::SLOT_BOTTLE_RIGHT] as $bottleSlot){ + $itemInSlot = $inventory->getItem($bottleSlot); + if($itemInSlot->isNull() || ($itemInSlot->canStackWith($item) && $itemInSlot->getCount() < $itemInSlot->getMaxStackSize())){ + return $bottleSlot; + } + } + return null; + } } diff --git a/src/block/Jukebox.php b/src/block/Jukebox.php index c85aa3754..9aaa7a357 100644 --- a/src/block/Jukebox.php +++ b/src/block/Jukebox.php @@ -62,11 +62,22 @@ public function getRecord() : ?Record{ } public function ejectRecord() : void{ - if($this->record !== null){ - $this->position->getWorld()->dropItem($this->position->add(0.5, 1, 0.5), $this->record); + $record = $this->extractRecord(); + if($record !== null){ + $this->position->getWorld()->dropItem($this->position->add(0.5, 1, 0.5), $record); + } + } + + /** + * Removes the record from the jukebox without dropping it and returns it, or null if there was no record inside. + */ + public function extractRecord() : ?Record{ + $record = $this->record; + if($record !== null){ $this->record = null; $this->stopSound(); } + return $record; } public function insertRecord(Record $record) : void{ diff --git a/src/event/inventory/InventoryMoveItemEvent.php b/src/event/inventory/InventoryMoveItemEvent.php new file mode 100644 index 000000000..646a571a4 --- /dev/null +++ b/src/event/inventory/InventoryMoveItemEvent.php @@ -0,0 +1,68 @@ +inventory; + } + + public function getDestination() : Inventory{ + return $this->destination; + } + + /** + * Returns the item which is being moved. + */ + public function getItem() : Item{ + return clone $this->item; + } + + /** + * Changes the item which is moved to the destination inventory. + */ + public function setItem(Item $item) : void{ + $this->item = clone $item; + } +} From 3c810a32845e2837089cde97a1947318570e6fe6 Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 19:49:30 +0300 Subject: [PATCH 03/12] fix: harden hopper item transfers against event-modified items --- src/block/Hopper.php | 63 +++++++++++-------- src/block/tile/Hopper.php | 6 +- .../inventory/InventoryMoveItemEvent.php | 20 +++--- 3 files changed, 55 insertions(+), 34 deletions(-) diff --git a/src/block/Hopper.php b/src/block/Hopper.php index 74643ca07..c37337542 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -135,7 +135,8 @@ public function onScheduledUpdate() : void{ // Hoppers that have a container above them, won't try to pick up items. $origin = $this->position->getWorld()->getTile($this->position->getSide(Facing::UP)); if($origin instanceof Container){ - $success = $this->pull($inventory, $origin->getInventory()) || $success; + // Hoppers only pull from the container part directly above them, not from the other half of a double chest. + $success = $this->pull($inventory, $origin->getRealInventory()) || $success; }elseif($origin instanceof TileJukebox){ $success = $this->pullFromJukebox($inventory, $origin) || $success; }else{ @@ -183,23 +184,15 @@ private function push(HopperInventory $inventory) : bool{ $slotInFurnace = FurnaceInventory::SLOT_FUEL; $itemInFurnace = $destination->getInventory()->getFuel(); } - if(!$itemInFurnace->isNull()){ - if($itemInFurnace->getCount() >= $itemInFurnace->getMaxStackSize()){ - return false; - } - if(!$itemInFurnace->canStackWith($item)){ - continue; - } + if(!$itemInFurnace->isNull() && (!$itemInFurnace->canStackWith($item) || $itemInFurnace->getCount() >= $itemInFurnace->getMaxStackSize())){ + continue; } $itemToPush = $this->callMoveItemEvent($inventory, $destination->getInventory(), $item->pop()); - if($itemToPush === null){ + if($itemToPush === null || !$this->canMergeInto($itemInFurnace, $itemToPush)){ continue; } if(!$itemInFurnace->isNull()){ - if(!$itemInFurnace->canStackWith($itemToPush)){ - continue; - } $itemInFurnace->setCount($itemInFurnace->getCount() + $itemToPush->getCount()); }else{ $itemInFurnace = $itemToPush; @@ -221,13 +214,10 @@ private function push(HopperInventory $inventory) : bool{ } $itemToPush = $this->callMoveItemEvent($inventory, $brewingInventory, $item->pop()); - if($itemToPush === null){ + if($itemToPush === null || !$this->canMergeInto($itemInStand, $itemToPush)){ continue; } if(!$itemInStand->isNull()){ - if(!$itemInStand->canStackWith($itemToPush)){ - continue; - } $itemInStand->setCount($itemInStand->getCount() + $itemToPush->getCount()); }else{ $itemInStand = $itemToPush; @@ -282,7 +272,7 @@ private function push(HopperInventory $inventory) : bool{ } $itemToPush = $this->callMoveItemEvent($inventory, $destination->getInventory(), $itemToPush); - if($itemToPush === null){ + if($itemToPush === null || !$destination->getInventory()->canAddItem($itemToPush)){ continue; } if($resetDestinationCooldown && $destination instanceof TileHopper){ @@ -321,7 +311,7 @@ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ return false; } $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); - if($itemToPull === null){ + if($itemToPull === null || !$inventory->canAddItem($itemToPull)){ return false; } @@ -341,7 +331,7 @@ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ continue; } $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); - if($itemToPull === null){ + if($itemToPull === null || !$inventory->canAddItem($itemToPull)){ continue; } @@ -361,7 +351,7 @@ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ continue; } $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); - if($itemToPull === null){ + if($itemToPull === null || !$inventory->canAddItem($itemToPull)){ continue; } @@ -387,10 +377,14 @@ private function pullFromJukebox(HopperInventory $inventory, TileJukebox $jukebo if($record === null || !$inventory->canAddItem($record)){ return false; } + $recordToPull = $this->callMoveItemEvent(null, $inventory, $record); + if($recordToPull === null || !$inventory->canAddItem($recordToPull)){ + return false; + } $jukeboxBlock->extractRecord(); $this->position->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); - $inventory->addItem($record); + $inventory->addItem($recordToPull); return true; } @@ -418,7 +412,7 @@ private function pickup(HopperInventory $inventory) : bool{ // Because of how entities are saved by PocketMine-MP the first entities of this loop are also the first ones who were saved. // That's why we don't need to implement any sorting mechanism. $item = $entity->getItem(); - if(!$inventory->canAddItem($item)){ + if($inventory->getAddableItemQuantity($item) <= 0){ continue; } @@ -432,21 +426,38 @@ private function pickup(HopperInventory $inventory) : bool{ continue; } $pickedUpItem = $ev->getItem(); - if(!$destination->canAddItem($pickedUpItem)){ + // Hoppers pick up as much of the item entity's stack as they can hold and leave the rest on the ground. + $addableQuantity = $destination->getAddableItemQuantity($pickedUpItem); + if($addableQuantity <= 0){ continue; } - $destination->addItem($pickedUpItem); - $entity->flagForDespawn(); + $destination->addItem((clone $pickedUpItem)->setCount($addableQuantity)); + $remainingCount = $entity->getItem()->getCount() - $addableQuantity; + if($remainingCount > 0){ + $entity->setStackSize($remainingCount); + }else{ + $entity->flagForDespawn(); + } return true; } return false; } + /** + * Returns whether the given item can be merged into the item currently occupying a slot. + */ + private function canMergeInto(Item $existing, Item $incoming) : bool{ + if($existing->isNull()){ + return $incoming->getCount() <= $incoming->getMaxStackSize(); + } + return $existing->canStackWith($incoming) && $existing->getCount() + $incoming->getCount() <= $existing->getMaxStackSize(); + } + /** * Returns the item to move after the event has been called, or null if the move was cancelled. */ - private function callMoveItemEvent(Inventory $source, Inventory $destination, Item $item) : ?Item{ + private function callMoveItemEvent(?Inventory $source, Inventory $destination, Item $item) : ?Item{ $ev = new InventoryMoveItemEvent($source, $destination, $item); $ev->call(); return $ev->isCancelled() ? null : $ev->getItem(); diff --git a/src/block/tile/Hopper.php b/src/block/tile/Hopper.php index 796fead2b..f87c63d7f 100644 --- a/src/block/tile/Hopper.php +++ b/src/block/tile/Hopper.php @@ -29,6 +29,7 @@ use pocketmine\math\Vector3; use pocketmine\nbt\tag\CompoundTag; use pocketmine\world\World; +use function max; class Hopper extends Spawnable implements Container, Nameable{ @@ -51,7 +52,7 @@ public function readSaveData(CompoundTag $nbt) : void{ $this->loadItems($nbt); $this->loadName($nbt); - $this->transferCooldown = $nbt->getInt(self::TAG_TRANSFER_COOLDOWN, 0); + $this->transferCooldown = max(0, $nbt->getInt(self::TAG_TRANSFER_COOLDOWN, 0)); } protected function writeSaveData(CompoundTag $nbt) : void{ @@ -86,6 +87,9 @@ public function getTransferCooldown() : int{ } public function setTransferCooldown(int $transferCooldown) : void{ + if($transferCooldown < 0){ + throw new \InvalidArgumentException("Transfer cooldown must not be negative"); + } $this->transferCooldown = $transferCooldown; } } diff --git a/src/event/inventory/InventoryMoveItemEvent.php b/src/event/inventory/InventoryMoveItemEvent.php index 646a571a4..817192197 100644 --- a/src/event/inventory/InventoryMoveItemEvent.php +++ b/src/event/inventory/InventoryMoveItemEvent.php @@ -31,25 +31,31 @@ use pocketmine\item\Item; /** - * Called when an item is moved from one inventory to another by a block, such as a hopper. + * Called when an item is moved into an inventory by a block, such as a hopper. + * + * The inventory of this event is the one receiving the item. Some sources, such as jukeboxes, don't have an inventory + * at all, in which case the source is null. */ class InventoryMoveItemEvent extends InventoryEvent implements Cancellable{ use CancellableTrait; public function __construct( - Inventory $source, - private Inventory $destination, + private ?Inventory $source, + Inventory $destination, private Item $item ){ - parent::__construct($source); + parent::__construct($destination); } - public function getSource() : Inventory{ - return $this->inventory; + /** + * Returns the inventory the item is taken from, or null if the item doesn't come from an inventory. + */ + public function getSource() : ?Inventory{ + return $this->source; } public function getDestination() : Inventory{ - return $this->destination; + return $this->inventory; } /** From f9e48de4594e8f74203db4cf7e9e8c0ad5b044b5 Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 19:56:40 +0300 Subject: [PATCH 04/12] fix: address hopper transfer review findings --- src/block/Hopper.php | 42 ++++++++++++------- src/block/tile/Hopper.php | 3 +- .../inventory/InventoryMoveItemEvent.php | 22 +++++----- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/block/Hopper.php b/src/block/Hopper.php index c37337542..45abedbb1 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -53,7 +53,7 @@ use pocketmine\math\Vector3; use pocketmine\player\Player; use pocketmine\world\BlockTransaction; -use function count; +use function min; class Hopper extends Transparent implements PoweredByRedstone{ use PoweredByRedstoneTrait; @@ -153,7 +153,7 @@ public function onScheduledUpdate() : void{ * Returns true if an item was successfully pushed or false on failure. */ private function push(HopperInventory $inventory) : bool{ - if(count($inventory->getContents()) === 0){ + if($this->isInventoryEmpty($inventory)){ return false; } $destination = $this->position->getWorld()->getTile($this->position->getSide($this->facing)); @@ -189,7 +189,7 @@ private function push(HopperInventory $inventory) : bool{ } $itemToPush = $this->callMoveItemEvent($inventory, $destination->getInventory(), $item->pop()); - if($itemToPush === null || !$this->canMergeInto($itemInFurnace, $itemToPush)){ + if($itemToPush === null || !$this->canMergeInto($destination->getInventory(), $itemInFurnace, $itemToPush)){ continue; } if(!$itemInFurnace->isNull()){ @@ -214,7 +214,7 @@ private function push(HopperInventory $inventory) : bool{ } $itemToPush = $this->callMoveItemEvent($inventory, $brewingInventory, $item->pop()); - if($itemToPush === null || !$this->canMergeInto($itemInStand, $itemToPush)){ + if($itemToPush === null || !$this->canMergeInto($brewingInventory, $itemInStand, $itemToPush)){ continue; } if(!$itemInStand->isNull()){ @@ -233,7 +233,7 @@ private function push(HopperInventory $inventory) : bool{ continue; } // Hoppers pushing into empty hoppers set the empty hoppers transfer cooldown back to the default amount of ticks. - $resetDestinationCooldown = count($destination->getInventory()->getContents()) === 0; + $resetDestinationCooldown = $this->isInventoryEmpty($destination->getInventory()); }elseif($destination instanceof TileJukebox){ if(!($item instanceof Record)){ @@ -251,9 +251,9 @@ private function push(HopperInventory $inventory) : bool{ // The Jukebox block is handling the playing of records, so we need to get it here and can't use TileJukebox::setRecord(). $jukeboxBlock = $destination->getBlock(); if($jukeboxBlock instanceof Jukebox){ - $record = $item->pop(); - if($record instanceof Record){ - $jukeboxBlock->insertRecord($record); + $recordToPush = $this->callMoveItemEvent($inventory, null, $item->pop()); + if($recordToPush instanceof Record){ + $jukeboxBlock->insertRecord($recordToPush); $jukeboxBlock->getPosition()->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); $inventory->setItem($slot, $item); return true; @@ -427,13 +427,14 @@ private function pickup(HopperInventory $inventory) : bool{ } $pickedUpItem = $ev->getItem(); // Hoppers pick up as much of the item entity's stack as they can hold and leave the rest on the ground. - $addableQuantity = $destination->getAddableItemQuantity($pickedUpItem); + $entityCount = $entity->getItem()->getCount(); + $addableQuantity = min($destination->getAddableItemQuantity($pickedUpItem), $entityCount); if($addableQuantity <= 0){ continue; } $destination->addItem((clone $pickedUpItem)->setCount($addableQuantity)); - $remainingCount = $entity->getItem()->getCount() - $addableQuantity; + $remainingCount = $entityCount - $addableQuantity; if($remainingCount > 0){ $entity->setStackSize($remainingCount); }else{ @@ -447,17 +448,30 @@ private function pickup(HopperInventory $inventory) : bool{ /** * Returns whether the given item can be merged into the item currently occupying a slot. */ - private function canMergeInto(Item $existing, Item $incoming) : bool{ + private function canMergeInto(Inventory $inventory, Item $existing, Item $incoming) : bool{ + $maxStackSize = min($inventory->getMaxStackSize(), $incoming->getMaxStackSize()); if($existing->isNull()){ - return $incoming->getCount() <= $incoming->getMaxStackSize(); + return $incoming->getCount() <= $maxStackSize; } - return $existing->canStackWith($incoming) && $existing->getCount() + $incoming->getCount() <= $existing->getMaxStackSize(); + return $existing->canStackWith($incoming) && $existing->getCount() + $incoming->getCount() <= $maxStackSize; + } + + /** + * Returns whether the given inventory holds no items. + */ + private function isInventoryEmpty(Inventory $inventory) : bool{ + for($slot = 0, $size = $inventory->getSize(); $slot < $size; $slot++){ + if(!$inventory->isSlotEmpty($slot)){ + return false; + } + } + return true; } /** * Returns the item to move after the event has been called, or null if the move was cancelled. */ - private function callMoveItemEvent(?Inventory $source, Inventory $destination, Item $item) : ?Item{ + private function callMoveItemEvent(?Inventory $source, ?Inventory $destination, Item $item) : ?Item{ $ev = new InventoryMoveItemEvent($source, $destination, $item); $ev->call(); return $ev->isCancelled() ? null : $ev->getItem(); diff --git a/src/block/tile/Hopper.php b/src/block/tile/Hopper.php index f87c63d7f..b568f4561 100644 --- a/src/block/tile/Hopper.php +++ b/src/block/tile/Hopper.php @@ -30,6 +30,7 @@ use pocketmine\nbt\tag\CompoundTag; use pocketmine\world\World; use function max; +use function min; class Hopper extends Spawnable implements Container, Nameable{ @@ -52,7 +53,7 @@ public function readSaveData(CompoundTag $nbt) : void{ $this->loadItems($nbt); $this->loadName($nbt); - $this->transferCooldown = max(0, $nbt->getInt(self::TAG_TRANSFER_COOLDOWN, 0)); + $this->transferCooldown = max(0, min(self::DEFAULT_TRANSFER_COOLDOWN, $nbt->getInt(self::TAG_TRANSFER_COOLDOWN, 0))); } protected function writeSaveData(CompoundTag $nbt) : void{ diff --git a/src/event/inventory/InventoryMoveItemEvent.php b/src/event/inventory/InventoryMoveItemEvent.php index 817192197..ffcadc919 100644 --- a/src/event/inventory/InventoryMoveItemEvent.php +++ b/src/event/inventory/InventoryMoveItemEvent.php @@ -27,25 +27,24 @@ use pocketmine\event\Cancellable; use pocketmine\event\CancellableTrait; +use pocketmine\event\Event; use pocketmine\inventory\Inventory; use pocketmine\item\Item; /** - * Called when an item is moved into an inventory by a block, such as a hopper. + * Called when an item is moved from one inventory to another by a block, such as a hopper. * - * The inventory of this event is the one receiving the item. Some sources, such as jukeboxes, don't have an inventory - * at all, in which case the source is null. + * Some sources and destinations, such as jukeboxes, don't have an inventory at all, in which case the respective side + * of the move is null. */ -class InventoryMoveItemEvent extends InventoryEvent implements Cancellable{ +class InventoryMoveItemEvent extends Event implements Cancellable{ use CancellableTrait; public function __construct( private ?Inventory $source, - Inventory $destination, + private ?Inventory $destination, private Item $item - ){ - parent::__construct($destination); - } + ){} /** * Returns the inventory the item is taken from, or null if the item doesn't come from an inventory. @@ -54,8 +53,11 @@ public function getSource() : ?Inventory{ return $this->source; } - public function getDestination() : Inventory{ - return $this->inventory; + /** + * Returns the inventory the item is moved into, or null if the item isn't moved into an inventory. + */ + public function getDestination() : ?Inventory{ + return $this->destination; } /** From 986023bb2a56a863a0829ed4961fcdc7e89cf443 Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 20:05:52 +0300 Subject: [PATCH 05/12] fix: prevent hopper item duplication through InventoryMoveItemEvent --- src/block/Hopper.php | 180 ++++++++++++++++++-------------------- src/block/tile/Hopper.php | 4 +- 2 files changed, 86 insertions(+), 98 deletions(-) diff --git a/src/block/Hopper.php b/src/block/Hopper.php index 45abedbb1..ea5bbb116 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -113,12 +113,11 @@ public function onInteract(Item $item, int $face, Vector3 $clickVector, ?Player } public function onScheduledUpdate() : void{ - $this->position->getWorld()->scheduleDelayedBlockUpdate($this->position, 1); - $tile = $this->position->getWorld()->getTile($this->position); if(!$tile instanceof TileHopper){ return; } + $this->position->getWorld()->scheduleDelayedBlockUpdate($this->position, 1); $transferCooldown = $tile->getTransferCooldown(); if($transferCooldown > 0){ @@ -167,8 +166,6 @@ private function push(HopperInventory $inventory) : bool{ continue; } - $resetDestinationCooldown = false; - // Hoppers interact differently when pushing into different kinds of tiles. //TODO: Composter if($destination instanceof TileFurnace){ @@ -176,64 +173,36 @@ private function push(HopperInventory $inventory) : bool{ // If the hopper is facing in any other direction, it will only push items that can be used as fuel to the furnace's fuel slot. if($this->facing === Facing::DOWN){ $slotInFurnace = FurnaceInventory::SLOT_INPUT; - $itemInFurnace = $destination->getInventory()->getSmelting(); }else{ if($item->getFuelTime() === 0){ continue; } $slotInFurnace = FurnaceInventory::SLOT_FUEL; - $itemInFurnace = $destination->getInventory()->getFuel(); - } - if(!$itemInFurnace->isNull() && (!$itemInFurnace->canStackWith($item) || $itemInFurnace->getCount() >= $itemInFurnace->getMaxStackSize())){ - continue; } - - $itemToPush = $this->callMoveItemEvent($inventory, $destination->getInventory(), $item->pop()); - if($itemToPush === null || !$this->canMergeInto($destination->getInventory(), $itemInFurnace, $itemToPush)){ + if(!$this->transferToSlot($inventory, $slot, $item, $destination->getInventory(), $slotInFurnace)){ continue; } - if(!$itemInFurnace->isNull()){ - $itemInFurnace->setCount($itemInFurnace->getCount() + $itemToPush->getCount()); - }else{ - $itemInFurnace = $itemToPush; - } - - $destination->getInventory()->setItem($slotInFurnace, $itemInFurnace); - $inventory->setItem($slot, $item); return true; }elseif($destination instanceof TileBrewingStand){ $brewingInventory = $destination->getInventory(); $slotInStand = $this->getBrewingStandSlot($brewingInventory, $item); - if($slotInStand === null){ + if($slotInStand === null || !$this->transferToSlot($inventory, $slot, $item, $brewingInventory, $slotInStand)){ continue; } - $itemInStand = $brewingInventory->getItem($slotInStand); - if(!$itemInStand->isNull() && (!$itemInStand->canStackWith($item) || $itemInStand->getCount() >= $itemInStand->getMaxStackSize())){ - continue; - } - - $itemToPush = $this->callMoveItemEvent($inventory, $brewingInventory, $item->pop()); - if($itemToPush === null || !$this->canMergeInto($brewingInventory, $itemInStand, $itemToPush)){ - continue; - } - if(!$itemInStand->isNull()){ - $itemInStand->setCount($itemInStand->getCount() + $itemToPush->getCount()); - }else{ - $itemInStand = $itemToPush; - } - - $brewingInventory->setItem($slotInStand, $itemInStand); - $inventory->setItem($slot, $item); return true; }elseif($destination instanceof TileHopper){ - $itemToPush = $item->pop(); - if(!$destination->getInventory()->canAddItem($itemToPush)){ + $destinationInventory = $destination->getInventory(); + // Hoppers pushing into empty hoppers set the empty hoppers transfer cooldown back to the default amount of ticks. + $resetDestinationCooldown = $this->isInventoryEmpty($destinationInventory); + if(!$this->transferToInventory($inventory, $slot, $item, $destinationInventory)){ continue; } - // Hoppers pushing into empty hoppers set the empty hoppers transfer cooldown back to the default amount of ticks. - $resetDestinationCooldown = $this->isInventoryEmpty($destination->getInventory()); + if($resetDestinationCooldown){ + $destination->setTransferCooldown(TileHopper::DEFAULT_TRANSFER_COOLDOWN); + } + return true; }elseif($destination instanceof TileJukebox){ if(!($item instanceof Record)){ @@ -262,26 +231,14 @@ private function push(HopperInventory $inventory) : bool{ return false; }elseif($destination instanceof Container){ - $itemToPush = $item->pop(); - if(!$destination->getInventory()->canAddItem($itemToPush)){ + if(!$this->transferToInventory($inventory, $slot, $item, $destination->getInventory())){ continue; } + return true; }else{ return false; } - - $itemToPush = $this->callMoveItemEvent($inventory, $destination->getInventory(), $itemToPush); - if($itemToPush === null || !$destination->getInventory()->canAddItem($itemToPush)){ - continue; - } - if($resetDestinationCooldown && $destination instanceof TileHopper){ - $destination->setTransferCooldown(TileHopper::DEFAULT_TRANSFER_COOLDOWN); - } - - $inventory->setItem($slot, $item); - $destination->getInventory()->addItem($itemToPush); - return true; } return false; } @@ -306,57 +263,24 @@ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ return false; } } - $itemToPull = $item->pop(); - if(!$inventory->canAddItem($itemToPull)){ - return false; - } - $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); - if($itemToPull === null || !$inventory->canAddItem($itemToPull)){ - return false; - } - - $origin->setItem($slot, $item); - $inventory->addItem($itemToPull); - return true; + return $this->transferToInventory($origin, $slot, $item, $inventory); }elseif($origin instanceof BrewingStandInventory){ // Hoppers only pull the brewed potions out of a brewing stand's bottle slots. foreach([BrewingStandInventory::SLOT_BOTTLE_LEFT, BrewingStandInventory::SLOT_BOTTLE_MIDDLE, BrewingStandInventory::SLOT_BOTTLE_RIGHT] as $slot){ $item = $origin->getItem($slot); - if($item->isNull()){ - continue; - } - $itemToPull = $item->pop(); - if(!$inventory->canAddItem($itemToPull)){ + if($item->isNull() || !$this->transferToInventory($origin, $slot, $item, $inventory)){ continue; } - $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); - if($itemToPull === null || !$inventory->canAddItem($itemToPull)){ - continue; - } - - $origin->setItem($slot, $item); - $inventory->addItem($itemToPull); return true; } }else{ - for($slot = 0; $slot < $origin->getSize(); $slot++){ + for($slot = 0, $size = $origin->getSize(); $slot < $size; $slot++){ $item = $origin->getItem($slot); - if($item->isNull()){ - continue; - } - $itemToPull = $item->pop(); - if(!$inventory->canAddItem($itemToPull)){ + if($item->isNull() || !$this->transferToInventory($origin, $slot, $item, $inventory)){ continue; } - $itemToPull = $this->callMoveItemEvent($origin, $inventory, $itemToPull); - if($itemToPull === null || !$inventory->canAddItem($itemToPull)){ - continue; - } - - $origin->setItem($slot, $item); - $inventory->addItem($itemToPull); return true; } } @@ -368,6 +292,11 @@ private function pull(HopperInventory $inventory, Inventory $origin) : bool{ * Returns true if the record was successfully pulled or false on failure. */ private function pullFromJukebox(HopperInventory $inventory, TileJukebox $jukebox) : bool{ + //TODO: + // Just like inserting a record, pulling one out should be blocked while the jukebox is playing, since a playing + // jukebox emits a redstone signal which powers the hopper. We can neither rely on redstone nor tell whether the + // record has finished playing, so the record is pulled out immediately for now. + // The Jukebox block is handling the playing of records, so we need to get it here and can't use TileJukebox::setRecord(). $jukeboxBlock = $jukebox->getBlock(); if(!$jukeboxBlock instanceof Jukebox){ @@ -426,8 +355,13 @@ private function pickup(HopperInventory $inventory) : bool{ continue; } $pickedUpItem = $ev->getItem(); + // The item left on the ground is the one the entity holds, so a different item may not be picked up in its + // place - otherwise the leftover stack would no longer match what was actually taken. + if(!$pickedUpItem->canStackWith($item)){ + continue; + } // Hoppers pick up as much of the item entity's stack as they can hold and leave the rest on the ground. - $entityCount = $entity->getItem()->getCount(); + $entityCount = $item->getCount(); $addableQuantity = min($destination->getAddableItemQuantity($pickedUpItem), $entityCount); if($addableQuantity <= 0){ continue; @@ -445,6 +379,50 @@ private function pickup(HopperInventory $inventory) : bool{ return false; } + /** + * Moves a single item out of the given source slot into a fixed slot of the destination inventory, merging it with + * the item already occupying that slot. Returns true if the item was moved. + */ + private function transferToSlot(Inventory $source, int $sourceSlot, Item $sourceItem, Inventory $destination, int $destinationSlot) : bool{ + $itemInSlot = $destination->getItem($destinationSlot); + if(!$itemInSlot->isNull() && (!$itemInSlot->canStackWith($sourceItem) || $itemInSlot->getCount() >= $itemInSlot->getMaxStackSize())){ + return false; + } + + $itemToMove = $this->callMoveItemEvent($source, $destination, $sourceItem->pop()); + if($itemToMove === null || !$this->canMergeInto($destination, $itemInSlot, $itemToMove)){ + return false; + } + if(!$itemInSlot->isNull()){ + $itemInSlot->setCount($itemInSlot->getCount() + $itemToMove->getCount()); + }else{ + $itemInSlot = $itemToMove; + } + + $destination->setItem($destinationSlot, $itemInSlot); + $source->setItem($sourceSlot, $sourceItem); + return true; + } + + /** + * Moves a single item out of the given source slot into the first slot of the destination inventory that can hold + * it. Returns true if the item was moved. + */ + private function transferToInventory(Inventory $source, int $sourceSlot, Item $sourceItem, Inventory $destination) : bool{ + $itemToMove = $sourceItem->pop(); + if(!$destination->canAddItem($itemToMove)){ + return false; + } + $itemToMove = $this->callMoveItemEvent($source, $destination, $itemToMove); + if($itemToMove === null || !$destination->canAddItem($itemToMove)){ + return false; + } + + $source->setItem($sourceSlot, $sourceItem); + $destination->addItem($itemToMove); + return true; + } + /** * Returns whether the given item can be merged into the item currently occupying a slot. */ @@ -469,12 +447,22 @@ private function isInventoryEmpty(Inventory $inventory) : bool{ } /** - * Returns the item to move after the event has been called, or null if the move was cancelled. + * Returns the item to move after the event has been called, or null if the move was cancelled or the event left + * nothing to move. */ private function callMoveItemEvent(?Inventory $source, ?Inventory $destination, Item $item) : ?Item{ $ev = new InventoryMoveItemEvent($source, $destination, $item); $ev->call(); - return $ev->isCancelled() ? null : $ev->getItem(); + if($ev->isCancelled()){ + return null; + } + $itemToMove = $ev->getItem(); + if($itemToMove->isNull()){ + return null; + } + // Only ever a single item is taken out of the source inventory, so the count has to be clamped here - otherwise + // an event handler raising it would create items out of thin air. + return $itemToMove->setCount(1); } /** diff --git a/src/block/tile/Hopper.php b/src/block/tile/Hopper.php index b568f4561..5c8f87490 100644 --- a/src/block/tile/Hopper.php +++ b/src/block/tile/Hopper.php @@ -30,7 +30,6 @@ use pocketmine\nbt\tag\CompoundTag; use pocketmine\world\World; use function max; -use function min; class Hopper extends Spawnable implements Container, Nameable{ @@ -53,7 +52,8 @@ public function readSaveData(CompoundTag $nbt) : void{ $this->loadItems($nbt); $this->loadName($nbt); - $this->transferCooldown = max(0, min(self::DEFAULT_TRANSFER_COOLDOWN, $nbt->getInt(self::TAG_TRANSFER_COOLDOWN, 0))); + // Only negative values are rejected here, to stay consistent with what setTransferCooldown() accepts. + $this->transferCooldown = max(0, $nbt->getInt(self::TAG_TRANSFER_COOLDOWN, 0)); } protected function writeSaveData(CompoundTag $nbt) : void{ From 3a9043a6cca9d18e00a76b9dab49da989b988eac Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 20:17:47 +0300 Subject: [PATCH 06/12] fix: revalidate hopper inventories after transfer events --- src/block/Hopper.php | 64 ++++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 17 deletions(-) diff --git a/src/block/Hopper.php b/src/block/Hopper.php index ea5bbb116..fc6770646 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -217,18 +217,26 @@ private function push(HopperInventory $inventory) : bool{ return false; } + $originalItem = clone $item; + $recordToPush = $this->callMoveItemEvent($inventory, null, $item->pop()); + if(!$recordToPush instanceof Record || !$this->isSlotUnchanged($inventory, $slot, $originalItem)){ + return false; + } + // A handler of the event above may have inserted a record itself, in which case the push has to be + // aborted - the record would otherwise be taken out of the hopper without ending up anywhere. + if($destination->getRecord() !== null){ + return false; + } + // The Jukebox block is handling the playing of records, so we need to get it here and can't use TileJukebox::setRecord(). $jukeboxBlock = $destination->getBlock(); - if($jukeboxBlock instanceof Jukebox){ - $recordToPush = $this->callMoveItemEvent($inventory, null, $item->pop()); - if($recordToPush instanceof Record){ - $jukeboxBlock->insertRecord($recordToPush); - $jukeboxBlock->getPosition()->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); - $inventory->setItem($slot, $item); - return true; - } + if(!$jukeboxBlock instanceof Jukebox){ + return false; } - return false; + $jukeboxBlock->insertRecord($recordToPush); + $this->position->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); + $inventory->setItem($slot, $item); + return true; }elseif($destination instanceof Container){ if(!$this->transferToInventory($inventory, $slot, $item, $destination->getInventory())){ @@ -303,7 +311,7 @@ private function pullFromJukebox(HopperInventory $inventory, TileJukebox $jukebo return false; } $record = $jukeboxBlock->getRecord(); - if($record === null || !$inventory->canAddItem($record)){ + if($record === null){ return false; } $recordToPull = $this->callMoveItemEvent(null, $inventory, $record); @@ -311,7 +319,12 @@ private function pullFromJukebox(HopperInventory $inventory, TileJukebox $jukebo return false; } - $jukeboxBlock->extractRecord(); + // A handler of the event above may have ejected the record itself, so the block has to be read again before + // the record is taken out of it. + $jukeboxBlock = $jukebox->getBlock(); + if(!$jukeboxBlock instanceof Jukebox || $jukeboxBlock->extractRecord() === null){ + return false; + } $this->position->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); $inventory->addItem($recordToPull); return true; @@ -341,10 +354,6 @@ private function pickup(HopperInventory $inventory) : bool{ // Because of how entities are saved by PocketMine-MP the first entities of this loop are also the first ones who were saved. // That's why we don't need to implement any sorting mechanism. $item = $entity->getItem(); - if($inventory->getAddableItemQuantity($item) <= 0){ - continue; - } - $ev = new BlockItemPickupEvent($this, $entity, $item, $inventory); $ev->call(); if($ev->isCancelled()){ @@ -385,14 +394,22 @@ private function pickup(HopperInventory $inventory) : bool{ */ private function transferToSlot(Inventory $source, int $sourceSlot, Item $sourceItem, Inventory $destination, int $destinationSlot) : bool{ $itemInSlot = $destination->getItem($destinationSlot); - if(!$itemInSlot->isNull() && (!$itemInSlot->canStackWith($sourceItem) || $itemInSlot->getCount() >= $itemInSlot->getMaxStackSize())){ + if(!$this->canMergeInto($destination, $itemInSlot, (clone $sourceItem)->setCount(1))){ return false; } + $originalSourceItem = clone $sourceItem; $itemToMove = $this->callMoveItemEvent($source, $destination, $sourceItem->pop()); - if($itemToMove === null || !$this->canMergeInto($destination, $itemInSlot, $itemToMove)){ + if($itemToMove === null || !$this->isSlotUnchanged($source, $sourceSlot, $originalSourceItem)){ return false; } + // The destination slot was read before the event, so it has to be read again - merging into a stale copy + // would overwrite whatever a handler put there. + $itemInSlot = $destination->getItem($destinationSlot); + if(!$this->canMergeInto($destination, $itemInSlot, $itemToMove)){ + return false; + } + if(!$itemInSlot->isNull()){ $itemInSlot->setCount($itemInSlot->getCount() + $itemToMove->getCount()); }else{ @@ -409,6 +426,7 @@ private function transferToSlot(Inventory $source, int $sourceSlot, Item $source * it. Returns true if the item was moved. */ private function transferToInventory(Inventory $source, int $sourceSlot, Item $sourceItem, Inventory $destination) : bool{ + $originalSourceItem = clone $sourceItem; $itemToMove = $sourceItem->pop(); if(!$destination->canAddItem($itemToMove)){ return false; @@ -417,6 +435,9 @@ private function transferToInventory(Inventory $source, int $sourceSlot, Item $s if($itemToMove === null || !$destination->canAddItem($itemToMove)){ return false; } + if(!$this->isSlotUnchanged($source, $sourceSlot, $originalSourceItem)){ + return false; + } $source->setItem($sourceSlot, $sourceItem); $destination->addItem($itemToMove); @@ -434,6 +455,15 @@ private function canMergeInto(Inventory $inventory, Item $existing, Item $incomi return $existing->canStackWith($incoming) && $existing->getCount() + $incoming->getCount() <= $maxStackSize; } + /** + * Returns whether the given slot still holds the item it held before InventoryMoveItemEvent was called. Handlers of + * that event are free to modify the inventories involved, and writing a snapshot taken beforehand back into the slot + * would duplicate or destroy whatever they put there. + */ + private function isSlotUnchanged(Inventory $inventory, int $slot, Item $expected) : bool{ + return $inventory->getItem($slot)->equalsExact($expected); + } + /** * Returns whether the given inventory holds no items. */ From 75842781e6318dc2c913cfc7289fbc86f53d8b4a Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 20:17:47 +0300 Subject: [PATCH 07/12] refactor: drop redundant hopper tile block update schedule --- src/block/tile/Hopper.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/block/tile/Hopper.php b/src/block/tile/Hopper.php index 5c8f87490..23be70e4f 100644 --- a/src/block/tile/Hopper.php +++ b/src/block/tile/Hopper.php @@ -45,7 +45,6 @@ class Hopper extends Spawnable implements Container, Nameable{ public function __construct(World $world, Vector3 $pos){ parent::__construct($world, $pos); $this->inventory = new HopperInventory($this->position); - $this->position->getWorld()->scheduleDelayedBlockUpdate($this->position, 1); } public function readSaveData(CompoundTag $nbt) : void{ From d263fcc8ffd9bbe907b02b7100803ee4867aa3ea Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 20:28:07 +0300 Subject: [PATCH 08/12] fix: address hopper review findings --- src/block/Hopper.php | 56 ++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/block/Hopper.php b/src/block/Hopper.php index fc6770646..044ac2df6 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -33,6 +33,7 @@ use pocketmine\block\tile\Furnace as TileFurnace; use pocketmine\block\tile\Hopper as TileHopper; use pocketmine\block\tile\Jukebox as TileJukebox; +use pocketmine\block\tile\Tile; use pocketmine\block\utils\PoweredByRedstone; use pocketmine\block\utils\PoweredByRedstoneTrait; use pocketmine\block\utils\SupportType; @@ -113,26 +114,31 @@ public function onInteract(Item $item, int $face, Vector3 $clickVector, ?Player } public function onScheduledUpdate() : void{ - $tile = $this->position->getWorld()->getTile($this->position); + $world = $this->position->getWorld(); + $tile = $world->getTile($this->position); if(!$tile instanceof TileHopper){ return; } - $this->position->getWorld()->scheduleDelayedBlockUpdate($this->position, 1); + $world->scheduleDelayedBlockUpdate($this->position, 1); + + // A powered hopper is locked, which freezes its cooldown instead of letting it tick down. + if($this->isPowered()){ + return; + } $transferCooldown = $tile->getTransferCooldown(); if($transferCooldown > 0){ $transferCooldown--; $tile->setTransferCooldown($transferCooldown); - } - - if($this->isPowered() || $transferCooldown > 0){ - return; + if($transferCooldown > 0){ + return; + } } $inventory = $tile->getInventory(); $success = $this->push($inventory); // Hoppers that have a container above them, won't try to pick up items. - $origin = $this->position->getWorld()->getTile($this->position->getSide(Facing::UP)); + $origin = $this->getLoadedTile($this->position->getSide(Facing::UP)); if($origin instanceof Container){ // Hoppers only pull from the container part directly above them, not from the other half of a double chest. $success = $this->pull($inventory, $origin->getRealInventory()) || $success; @@ -152,19 +158,18 @@ public function onScheduledUpdate() : void{ * Returns true if an item was successfully pushed or false on failure. */ private function push(HopperInventory $inventory) : bool{ - if($this->isInventoryEmpty($inventory)){ - return false; - } - $destination = $this->position->getWorld()->getTile($this->position->getSide($this->facing)); - if($destination === null){ - return false; - } + $destination = null; - for($slot = 0; $slot < $inventory->getSize(); $slot++){ - $item = $inventory->getItem($slot); - if($item->isNull()){ + for($slot = 0, $size = $inventory->getSize(); $slot < $size; $slot++){ + if($inventory->isSlotEmpty($slot)){ continue; } + // The destination is only looked up once the hopper is known to hold something, so idle hoppers don't probe + // the world every tick. + if($destination === null && ($destination = $this->getLoadedTile($this->position->getSide($this->facing))) === null){ + return false; + } + $item = $inventory->getItem($slot); // Hoppers interact differently when pushing into different kinds of tiles. //TODO: Composter @@ -349,6 +354,10 @@ private function pickup(HopperInventory $inventory) : bool{ if($entity->isClosed() || $entity->isFlaggedForDespawn() || !$entity instanceof ItemEntity){ continue; } + // Just like players, hoppers can't collect an item entity before its pickup delay has run out. + if($entity->getPickupDelay() !== 0){ + continue; + } // Unlike Java Edition, Bedrock Edition's hoppers don't save in which order item entities landed on top of them to collect them in that order. // In Bedrock Edition hoppers collect item entities in the order in which they entered the chunk. // Because of how entities are saved by PocketMine-MP the first entities of this loop are also the first ones who were saved. @@ -416,8 +425,10 @@ private function transferToSlot(Inventory $source, int $sourceSlot, Item $source $itemInSlot = $itemToMove; } - $destination->setItem($destinationSlot, $itemInSlot); + // The source is always written first, so a listener reacting to either write can never observe the same item in + // both inventories at once. $source->setItem($sourceSlot, $sourceItem); + $destination->setItem($destinationSlot, $itemInSlot); return true; } @@ -444,6 +455,15 @@ private function transferToInventory(Inventory $source, int $sourceSlot, Item $s return true; } + /** + * Returns the tile at the given position, or null if there is none or the position isn't in loaded terrain. Hoppers + * tick every tick, so probing a neighbour with World::getTile() would keep loading the chunk it sits in. + */ + private function getLoadedTile(Vector3 $pos) : ?Tile{ + $world = $this->position->getWorld(); + return $world->isInLoadedTerrain($pos) ? $world->getTile($pos) : null; + } + /** * Returns whether the given item can be merged into the item currently occupying a slot. */ From e0de95786fbba03ad138313618c26e62e45701ba Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 20:38:00 +0300 Subject: [PATCH 09/12] fix: restore hopper transfer tick bootstrap --- src/block/tile/Hopper.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/block/tile/Hopper.php b/src/block/tile/Hopper.php index 23be70e4f..63d4efdee 100644 --- a/src/block/tile/Hopper.php +++ b/src/block/tile/Hopper.php @@ -45,6 +45,9 @@ class Hopper extends Spawnable implements Container, Nameable{ public function __construct(World $world, Vector3 $pos){ parent::__construct($world, $pos); $this->inventory = new HopperInventory($this->position); + // Hopper::onScheduledUpdate() keeps rescheduling itself, but something has to start that chain off - both for + // newly placed hoppers and for the ones read back from disk. + $world->scheduleDelayedBlockUpdate($pos, 1); } public function readSaveData(CompoundTag $nbt) : void{ From 6b3b781c82688b2ce09683394521e5cffbd1a9fd Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sat, 15 Aug 2026 20:38:00 +0300 Subject: [PATCH 10/12] fix: harden hopper jukebox transfers against event handlers --- src/block/Hopper.php | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/block/Hopper.php b/src/block/Hopper.php index 044ac2df6..8b08dfc0b 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -238,9 +238,11 @@ private function push(HopperInventory $inventory) : bool{ if(!$jukeboxBlock instanceof Jukebox){ return false; } + // The source is always written first, so a listener reacting to either write can never observe the record + // in both the hopper and the jukebox at once. + $inventory->setItem($slot, $item); $jukeboxBlock->insertRecord($recordToPush); $this->position->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); - $inventory->setItem($slot, $item); return true; }elseif($destination instanceof Container){ @@ -320,16 +322,24 @@ private function pullFromJukebox(HopperInventory $inventory, TileJukebox $jukebo return false; } $recordToPull = $this->callMoveItemEvent(null, $inventory, $record); - if($recordToPull === null || !$inventory->canAddItem($recordToPull)){ + // Only a record can leave a jukebox, so a handler replacing the item with something else would create an item + // out of thin air and destroy the record in the process. + if(!$recordToPull instanceof Record || !$inventory->canAddItem($recordToPull)){ return false; } - // A handler of the event above may have ejected the record itself, so the block has to be read again before - // the record is taken out of it. + // A handler of the event above may have ejected or swapped the record itself, so the block has to be read again + // - taking a record out that the jukebox no longer holds would duplicate it. $jukeboxBlock = $jukebox->getBlock(); - if(!$jukeboxBlock instanceof Jukebox || $jukeboxBlock->extractRecord() === null){ + if(!$jukeboxBlock instanceof Jukebox){ return false; } + $currentRecord = $jukeboxBlock->getRecord(); + if($currentRecord === null || !$currentRecord->equalsExact($record)){ + return false; + } + + $jukeboxBlock->extractRecord(); $this->position->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); $inventory->addItem($recordToPull); return true; From 406d6b95a1f2b814a97f55f959cdd55e651e1d79 Mon Sep 17 00:00:00 2001 From: xRookieFight Date: Sun, 16 Aug 2026 16:22:10 +0300 Subject: [PATCH 11/12] Finish work --- src/block/Hopper.php | 14 +- tests/plugins/TesterPlugin/src/Main.php | 16 +- .../src/hopper/HopperChainStressTest.php | 120 +++++++++ .../src/hopper/HopperCooldownTest.php | 113 ++++++++ .../src/hopper/HopperJukeboxDupeTest.php | 162 ++++++++++++ .../src/hopper/HopperMoveEventDupeTest.php | 145 ++++++++++ .../src/hopper/HopperPickupBoundsTest.php | 114 ++++++++ .../src/hopper/HopperPickupDupeTest.php | 155 +++++++++++ .../src/hopper/HopperPickupRangeTest.php | 136 ++++++++++ .../src/hopper/HopperTestBase.php | 247 ++++++++++++++++++ .../src/hopper/HostileItemPickupListener.php | 81 ++++++ .../src/hopper/HostileJukeboxListener.php | 151 +++++++++++ .../src/hopper/HostileMoveItemListener.php | 114 ++++++++ 13 files changed, 1562 insertions(+), 6 deletions(-) create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperChainStressTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperCooldownTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperJukeboxDupeTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperMoveEventDupeTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperPickupBoundsTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperPickupDupeTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperPickupRangeTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperTestBase.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HostileItemPickupListener.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HostileJukeboxListener.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HostileMoveItemListener.php diff --git a/src/block/Hopper.php b/src/block/Hopper.php index 8b08dfc0b..3712469f1 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -59,6 +59,8 @@ class Hopper extends Transparent implements PoweredByRedstone{ use PoweredByRedstoneTrait; + private const BOWL_DEPTH = 6 / 16; + private int $facing = Facing::DOWN; protected function describeBlockOnlyState(RuntimeDataDescriber $w) : void{ @@ -79,7 +81,7 @@ public function setFacing(int $facing) : self{ protected function recalculateCollisionBoxes() : array{ $result = [ - AxisAlignedBB::one()->trim(Facing::UP, 6 / 16) //the empty area around the bottom is currently considered solid + AxisAlignedBB::one()->trim(Facing::UP, self::BOWL_DEPTH) //the empty area around the bottom is currently considered solid ]; foreach(Facing::HORIZONTAL as $f){ //add the frame parts around the bowl @@ -350,10 +352,11 @@ private function pullFromJukebox(HopperInventory $inventory, TileJukebox $jukebo * Returns true if an item was successfully picked up or false on failure. */ private function pickup(HopperInventory $inventory) : bool{ - // In Bedrock Edition hoppers collect from the lower 3/4 of the block space above them. + // In Bedrock Edition hoppers collect from the lower 3/4 of the block space above them, down to the floor of + // their own bowl - that's where items dropped onto a hopper come to rest. $pickupCollisionBox = new AxisAlignedBB( $this->position->getX(), - $this->position->getY() + 1, + $this->position->getY() + 1 - self::BOWL_DEPTH, $this->position->getZ(), $this->position->getX() + 1, $this->position->getY() + 1.75, @@ -364,8 +367,9 @@ private function pickup(HopperInventory $inventory) : bool{ if($entity->isClosed() || $entity->isFlaggedForDespawn() || !$entity instanceof ItemEntity){ continue; } - // Just like players, hoppers can't collect an item entity before its pickup delay has run out. - if($entity->getPickupDelay() !== 0){ + // The pickup delay only holds players off from collecting an item they have just thrown, so hoppers ignore + // it. An item whose delay never runs out is meant to be uncollectable though. + if($entity->getPickupDelay() === ItemEntity::NEVER_DESPAWN){ continue; } // Unlike Java Edition, Bedrock Edition's hoppers don't save in which order item entities landed on top of them to collect them in that order. diff --git a/tests/plugins/TesterPlugin/src/Main.php b/tests/plugins/TesterPlugin/src/Main.php index d4cb55574..167ec8041 100644 --- a/tests/plugins/TesterPlugin/src/Main.php +++ b/tests/plugins/TesterPlugin/src/Main.php @@ -25,6 +25,13 @@ namespace pmmp\TesterPlugin; +use pmmp\TesterPlugin\hopper\HopperChainStressTest; +use pmmp\TesterPlugin\hopper\HopperCooldownTest; +use pmmp\TesterPlugin\hopper\HopperJukeboxDupeTest; +use pmmp\TesterPlugin\hopper\HopperMoveEventDupeTest; +use pmmp\TesterPlugin\hopper\HopperPickupBoundsTest; +use pmmp\TesterPlugin\hopper\HopperPickupDupeTest; +use pmmp\TesterPlugin\hopper\HopperPickupRangeTest; use pocketmine\event\Listener; use pocketmine\event\server\CommandEvent; use pocketmine\plugin\PluginBase; @@ -74,7 +81,14 @@ function() : void{ throw new TestFailedException(); } } - ) + ), + new HopperChainStressTest($this->getLogger(), $this), + new HopperCooldownTest($this->getLogger(), $this), + new HopperMoveEventDupeTest($this->getLogger(), $this), + new HopperPickupDupeTest($this->getLogger(), $this), + new HopperPickupRangeTest($this->getLogger(), $this), + new HopperPickupBoundsTest($this->getLogger(), $this), + new HopperJukeboxDupeTest($this->getLogger(), $this) ]; } diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperChainStressTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperChainStressTest.php new file mode 100644 index 000000000..1745e2e2b --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperChainStressTest.php @@ -0,0 +1,120 @@ +> + */ + private array $laneInventories = []; + /** + * @var int[] + * @phpstan-var list + */ + private array $laneTotals = []; + /** + * @var Inventory[] + * @phpstan-var list + */ + private array $laneDestinations = []; + + public function __construct(\Logger $logger, Main $plugin){ + parent::__construct( + $logger, + $plugin, + "Hopper chain stress test", + "Runs " . self::LANES . " chains of " . self::CHAIN_LENGTH . " hoppers moving full chests and checks that no items are duplicated or lost" + ); + } + + protected function setUpArea() : void{ + $hopper = VanillaBlocks::HOPPER()->setFacing(Facing::DOWN); + $chest = VanillaBlocks::CHEST(); + + for($lane = 0; $lane < self::LANES; $lane++){ + // the lanes are two blocks apart so the chests don't pair up into double chests + $x = $lane * 2; + + $destinationPos = $this->areaPos($x, 0, 0); + $this->world->setBlock($destinationPos, $chest, false); + for($y = 1; $y <= self::CHAIN_LENGTH; $y++){ + $this->world->setBlock($this->areaPos($x, $y, 0), $hopper, false); + } + $sourcePos = $this->areaPos($x, self::CHAIN_LENGTH + 1, 0); + $this->world->setBlock($sourcePos, $chest, false); + + $source = $this->getContainerInventory($sourcePos); + for($slot = 0, $size = $source->getSize(); $slot < $size; $slot++){ + // two item types are used so the hoppers have to merge into partially filled slots as well + $item = $slot % 2 === 0 ? VanillaBlocks::COBBLESTONE()->asItem() : VanillaBlocks::DIRT()->asItem(); + $source->setItem($slot, $item->setCount($item->getMaxStackSize())); + } + + $destination = $this->getContainerInventory($destinationPos); + $inventories = [$source]; + for($y = 1; $y <= self::CHAIN_LENGTH; $y++){ + $inventories[] = $this->getContainerInventory($this->areaPos($x, $y, 0)); + } + $inventories[] = $destination; + + $this->laneInventories[] = $inventories; + $this->laneTotals[] = $this->countItems($inventories); + $this->laneDestinations[] = $destination; + } + } + + protected function checkInvariants(int $tick) : void{ + foreach($this->laneInventories as $lane => $inventories){ + $total = $this->countItems($inventories); + if($total !== $this->laneTotals[$lane]){ + throw new TestFailedException("Lane $lane held " . $this->laneTotals[$lane] . " items but holds $total after $tick ticks"); + } + } + } + + protected function checkOutcome() : void{ + foreach($this->laneDestinations as $lane => $destination){ + if($this->countItems([$destination]) === 0){ + throw new TestFailedException("Lane $lane didn't deliver a single item within " . self::DURATION_TICKS . " ticks"); + } + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperCooldownTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperCooldownTest.php new file mode 100644 index 000000000..0ca1ff0e5 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperCooldownTest.php @@ -0,0 +1,113 @@ +source = $this->buildLane(0, false); + $this->initialSourceCount = $this->countItems([$this->source]); + + $this->lockedSource = $this->buildLane(3, true); + $this->initialLockedSourceCount = $this->countItems([$this->lockedSource]); + $this->lockedDestination = $this->getContainerInventory($this->areaPos(3, 0, 0)); + + $tile = $this->world->getTile($this->areaPos(3, 1, 0)); + if(!$tile instanceof TileHopper){ + throw new TestFailedException("Expected a hopper tile at " . $this->areaPos(3, 1, 0)->__toString()); + } + $this->lockedTile = $tile; + $this->lockedTile->setTransferCooldown(self::LOCKED_COOLDOWN); + } + + private function buildLane(int $x, bool $powered) : Inventory{ + $this->world->setBlock($this->areaPos($x, 0, 0), VanillaBlocks::CHEST(), false); + $this->world->setBlock($this->areaPos($x, 1, 0), VanillaBlocks::HOPPER()->setFacing(Facing::DOWN)->setPowered($powered), false); + + $sourcePos = $this->areaPos($x, 2, 0); + $this->world->setBlock($sourcePos, VanillaBlocks::CHEST(), false); + $source = $this->getContainerInventory($sourcePos); + $cobblestone = VanillaBlocks::COBBLESTONE()->asItem(); + $source->setItem(0, $cobblestone->setCount($cobblestone->getMaxStackSize())); + + return $source; + } + + protected function checkInvariants(int $tick) : void{ + $moved = $this->initialSourceCount - $this->countItems([$this->source]); + $maximum = intdiv($tick, TileHopper::DEFAULT_TRANSFER_COOLDOWN) + self::TOLERANCE_TICKS; + if($moved > $maximum){ + throw new TestFailedException("The hopper moved $moved items within $tick ticks, which is more than the $maximum items its transfer cooldown allows"); + } + + if($this->countItems([$this->lockedSource]) !== $this->initialLockedSourceCount || $this->countItems([$this->lockedDestination]) !== 0){ + throw new TestFailedException("The powered hopper moved items after $tick ticks even though it is locked"); + } + if($this->lockedTile->getTransferCooldown() !== self::LOCKED_COOLDOWN){ + throw new TestFailedException("The powered hopper ticked its transfer cooldown down to " . $this->lockedTile->getTransferCooldown() . " after $tick ticks even though it is locked"); + } + } + + protected function checkOutcome() : void{ + $moved = $this->initialSourceCount - $this->countItems([$this->source]); + $expected = intdiv(self::DURATION_TICKS, TileHopper::DEFAULT_TRANSFER_COOLDOWN); + if(abs($moved - $expected) > self::TOLERANCE_TICKS){ + throw new TestFailedException("The hopper moved $moved items within " . self::DURATION_TICKS . " ticks, but around $expected were expected"); + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperJukeboxDupeTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperJukeboxDupeTest.php new file mode 100644 index 000000000..92655c1bb --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperJukeboxDupeTest.php @@ -0,0 +1,162 @@ + + */ + private array $inventories = []; + /** + * @var Vector3[] + * @phpstan-var list + */ + private array $jukeboxPositions = []; + private int $initialTotal = 0; + private Inventory $pushSource; + + public function __construct(\Logger $logger, Main $plugin){ + parent::__construct( + $logger, + $plugin, + "Hopper jukebox dupe test", + "Checks that hoppers neither duplicate nor destroy records when a plugin moves records in and out of jukeboxes from within InventoryMoveItemEvent" + ); + } + + protected function setUpArea() : void{ + $this->inventories = []; + $this->jukeboxPositions = []; + + $this->buildPushLane(); + $this->buildPullLane(); + + $this->initialTotal = $this->countTotal(); + + $this->listener = new HostileJukeboxListener($this->world, $this->jukeboxPositions); + $this->plugin->getServer()->getPluginManager()->registerEvents($this->listener, $this->plugin); + } + + private function buildPushLane() : void{ + $jukeboxPos = $this->areaPos(0, 1, 0); + $this->world->setBlock($jukeboxPos, VanillaBlocks::JUKEBOX(), false); + $this->world->setBlock($this->areaPos(0, 2, 0), VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $sourcePos = $this->areaPos(0, 3, 0); + $this->world->setBlock($sourcePos, VanillaBlocks::CHEST(), false); + $source = $this->getContainerInventory($sourcePos); + for($slot = 0; $slot < self::PUSHED_RECORDS; $slot++){ + // records don't stack, so every record needs a slot of its own + $source->setItem($slot, VanillaItems::RECORD_CAT()); + } + + $this->jukeboxPositions[] = $jukeboxPos; + $this->inventories[] = $source; + $this->inventories[] = $this->getContainerInventory($this->areaPos(0, 2, 0)); + $this->pushSource = $source; + } + + private function buildPullLane() : void{ + $destinationPos = $this->areaPos(4, 1, 0); + $this->world->setBlock($destinationPos, VanillaBlocks::CHEST(), false); + $this->world->setBlock($this->areaPos(4, 2, 0), VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $jukeboxPos = $this->areaPos(4, 3, 0); + $this->world->setBlock($jukeboxPos, VanillaBlocks::JUKEBOX(), false); + $jukebox = $this->world->getBlock($jukeboxPos); + if(!$jukebox instanceof Jukebox){ + throw new TestFailedException("Expected a jukebox at " . $jukeboxPos->__toString()); + } + $jukebox->insertRecord(VanillaItems::RECORD_CAT()); + $this->world->setBlock($jukeboxPos, $jukebox, false); + + $this->jukeboxPositions[] = $jukeboxPos; + $this->inventories[] = $this->getContainerInventory($this->areaPos(4, 2, 0)); + $this->inventories[] = $this->getContainerInventory($destinationPos); + } + + private function countTotal() : int{ + $total = $this->countItems($this->inventories) + $this->countDroppedItems(); + foreach($this->jukeboxPositions as $position){ + $jukebox = $this->world->getBlock($position); + if($jukebox instanceof Jukebox && $jukebox->getRecord() !== null){ + $total++; + } + } + return $total; + } + + protected function checkInvariants(int $tick) : void{ + $expected = $this->initialTotal + ($this->listener?->getLedger() ?? 0); + $total = $this->countTotal(); + if($total !== $expected){ + throw new TestFailedException("Expected $expected items after $tick ticks, but found $total"); + } + } + + protected function checkOutcome() : void{ + if($this->listener === null || $this->listener->getCalls() === 0){ + throw new TestFailedException("InventoryMoveItemEvent was never called, so nothing was actually tested"); + } + + $remaining = 0; + for($slot = 0, $size = $this->pushSource->getSize(); $slot < $size; $slot++){ + if($this->pushSource->getItem($slot) instanceof Record){ + $remaining++; + } + } + if($remaining >= self::PUSHED_RECORDS){ + throw new TestFailedException("The hopper didn't push a single record into the jukebox within " . self::DURATION_TICKS . " ticks"); + } + } + + protected function tearDownArea() : void{ + if($this->listener !== null){ + HandlerListManager::global()->unregisterAll($this->listener); + $this->listener = null; + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperMoveEventDupeTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperMoveEventDupeTest.php new file mode 100644 index 000000000..b1e8ffdb6 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperMoveEventDupeTest.php @@ -0,0 +1,145 @@ + + */ + private array $inventories = []; + private int $initialTotal = 0; + private Inventory $chestDestination; + + public function __construct(\Logger $logger, Main $plugin){ + parent::__construct( + $logger, + $plugin, + "Hopper InventoryMoveItemEvent abuse test", + "Checks that hoppers neither duplicate nor destroy items when a plugin mutates the inventories from within InventoryMoveItemEvent" + ); + } + + protected function setUpArea() : void{ + $this->inventories = []; + + $this->chestDestination = $this->buildChestLane(); + $this->buildFurnaceLane(); + $this->buildBrewingStandLane(); + + $this->initialTotal = $this->countItems($this->inventories); + + $this->listener = new HostileMoveItemListener(); + $this->plugin->getServer()->getPluginManager()->registerEvents($this->listener, $this->plugin); + } + + private function buildChestLane() : Inventory{ + $destinationPos = $this->areaPos(0, 0, 0); + $this->world->setBlock($destinationPos, VanillaBlocks::CHEST(), false); + $this->world->setBlock($this->areaPos(0, 1, 0), VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $sourcePos = $this->areaPos(0, 2, 0); + $this->world->setBlock($sourcePos, VanillaBlocks::CHEST(), false); + $source = $this->getContainerInventory($sourcePos); + $cobblestone = VanillaBlocks::COBBLESTONE()->asItem(); + $source->setItem(0, $cobblestone->setCount($cobblestone->getMaxStackSize())); + + $destination = $this->getContainerInventory($destinationPos); + $this->inventories[] = $source; + $this->inventories[] = $this->getContainerInventory($this->areaPos(0, 1, 0)); + $this->inventories[] = $destination; + return $destination; + } + + private function buildFurnaceLane() : void{ + $this->world->setBlock($this->areaPos(3, 0, 0), VanillaBlocks::FURNACE(), false); + $this->world->setBlock($this->areaPos(3, 1, 0), VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $sourcePos = $this->areaPos(3, 2, 0); + $this->world->setBlock($sourcePos, VanillaBlocks::CHEST(), false); + $source = $this->getContainerInventory($sourcePos); + $cobblestone = VanillaBlocks::COBBLESTONE()->asItem(); + $source->setItem(0, $cobblestone->setCount($cobblestone->getMaxStackSize())); + + $this->inventories[] = $source; + $this->inventories[] = $this->getContainerInventory($this->areaPos(3, 1, 0)); + $this->inventories[] = $this->getContainerInventory($this->areaPos(3, 0, 0)); + } + + private function buildBrewingStandLane() : void{ + $this->world->setBlock($this->areaPos(7, 1, 0), VanillaBlocks::BREWING_STAND(), false); + $this->world->setBlock($this->areaPos(6, 1, 0), VanillaBlocks::HOPPER()->setFacing(Facing::EAST), false); + + $sourcePos = $this->areaPos(6, 2, 0); + $this->world->setBlock($sourcePos, VanillaBlocks::CHEST(), false); + $source = $this->getContainerInventory($sourcePos); + $bottle = VanillaItems::GLASS_BOTTLE(); + $source->setItem(0, $bottle->setCount($bottle->getMaxStackSize())); + + $this->inventories[] = $source; + $this->inventories[] = $this->getContainerInventory($this->areaPos(6, 1, 0)); + $this->inventories[] = $this->getContainerInventory($this->areaPos(7, 1, 0)); + } + + protected function checkInvariants(int $tick) : void{ + $expected = $this->initialTotal + ($this->listener?->getLedger() ?? 0); + $total = $this->countItems($this->inventories); + if($total !== $expected){ + throw new TestFailedException("Expected $expected items after $tick ticks, but found $total"); + } + } + + protected function checkOutcome() : void{ + if($this->listener === null || $this->listener->getCalls() === 0){ + throw new TestFailedException("InventoryMoveItemEvent was never called, so nothing was actually tested"); + } + if($this->countItems([$this->chestDestination]) === 0){ + throw new TestFailedException("The hopper didn't move a single item into the destination chest within " . self::DURATION_TICKS . " ticks"); + } + } + + protected function tearDownArea() : void{ + if($this->listener !== null){ + HandlerListManager::global()->unregisterAll($this->listener); + $this->listener = null; + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperPickupBoundsTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperPickupBoundsTest.php new file mode 100644 index 000000000..53cabe0d9 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperPickupBoundsTest.php @@ -0,0 +1,114 @@ + + */ + private array $inventories = []; + private int $outOfRangeCount = 0; + + public function __construct(\Logger $logger, Main $plugin){ + parent::__construct( + $logger, + $plugin, + "Hopper pickup bounds test", + "Checks that a hopper doesn't reach items lying on the floor next to it instead of inside its own column" + ); + } + + protected function setUpArea() : void{ + $this->inventories = []; + $this->outOfRangeCount = 0; + + $hopperPos = $this->areaPos(8, 1, 8); + $this->world->setBlock($this->areaPos(8, 0, 8), VanillaBlocks::CHEST(), false); + $this->world->setBlock($hopperPos, VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + $this->inventories[] = $this->getContainerInventory($hopperPos); + $this->inventories[] = $this->getContainerInventory($this->areaPos(8, 0, 8)); + + // a floor around the hopper for the surrounding items to lie on, at the same height as the hopper itself. + $stone = VanillaBlocks::STONE(); + foreach(Facing::HORIZONTAL as $facing){ + $offset = Facing::OFFSET[$facing]; + foreach(self::DISTANCES as $distance){ + $this->world->setBlock($this->areaPos(8 + $offset[0] * $distance, 1, 8 + $offset[2] * $distance), $stone, false); + } + } + + // the items rest on top of that floor, which reaches into the height range the hopper collects from - only + // their distance keeps them out of its reach. + foreach(Facing::HORIZONTAL as $facing){ + $offset = Facing::OFFSET[$facing]; + foreach(self::DISTANCES as $distance){ + $position = $this->areaPos(8 + $offset[0] * $distance, 2, 8 + $offset[2] * $distance); + $this->dropStack($position); + $this->outOfRangeCount += self::STACK_SIZE; + } + } + + // a control item inside the hopper itself, so a hopper which collects nothing at all can't pass this test. + $this->dropStack($hopperPos->add(0, 1, 0)); + } + + private function dropStack(Vector3 $position) : void{ + $item = VanillaBlocks::COBBLESTONE()->asItem()->setCount(self::STACK_SIZE); + $this->world->dropItem($position->add(0.5, 0, 0.5), $item, new Vector3(0, 0, 0), 0); + } + + protected function checkInvariants(int $tick) : void{ + $collected = $this->countItems($this->inventories); + if($collected > self::STACK_SIZE){ + throw new TestFailedException("The hopper collected $collected items after $tick ticks, which is more than the " . self::STACK_SIZE . " items lying inside it"); + } + if($this->countDroppedItems() < $this->outOfRangeCount){ + throw new TestFailedException("Items lying next to the hopper went missing after $tick ticks"); + } + } + + protected function checkOutcome() : void{ + if($this->countItems($this->inventories) !== self::STACK_SIZE){ + throw new TestFailedException("The hopper didn't collect the " . self::STACK_SIZE . " items lying inside it within " . self::DURATION_TICKS . " ticks"); + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperPickupDupeTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperPickupDupeTest.php new file mode 100644 index 000000000..9cb51c525 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperPickupDupeTest.php @@ -0,0 +1,155 @@ + + */ + private array $inventories = []; + /** + * @var Inventory[] + * @phpstan-var list + */ + private array $chestInventories = []; + /** + * @var Vector3[] + * @phpstan-var list + */ + private array $hopperPositions = []; + private int $spawnedTotal = 0; + + public function __construct(\Logger $logger, Main $plugin){ + parent::__construct( + $logger, + $plugin, + "Hopper item pickup dupe test", + "Checks that hoppers collect exactly as many items as the item entities hold, even when a plugin rewrites BlockItemPickupEvent" + ); + } + + protected function setUpArea() : void{ + $this->inventories = []; + $this->chestInventories = []; + $this->hopperPositions = []; + $this->spawnedTotal = 0; + + $hopperInventories = []; + for($index = 0; $index < self::HOPPERS; $index++){ + // the hoppers are two blocks apart so the chests below them don't pair up into double chests. + $x = $index * 2; + + $chestPos = $this->areaPos($x, 0, 0); + $this->world->setBlock($chestPos, VanillaBlocks::CHEST(), false); + $hopperPos = $this->areaPos($x, 1, 0); + $this->world->setBlock($hopperPos, VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $chest = $this->getContainerInventory($chestPos); + $hopper = $this->getContainerInventory($hopperPos); + $this->hopperPositions[] = $hopperPos; + $this->chestInventories[] = $chest; + $hopperInventories[] = $hopper; + $this->inventories[] = $hopper; + $this->inventories[] = $chest; + } + + $this->refillEntities(); + + $this->listener = new HostileItemPickupListener($hopperInventories); + $this->plugin->getServer()->getPluginManager()->registerEvents($this->listener, $this->plugin); + } + + private function refillEntities() : void{ + foreach($this->hopperPositions as $position){ + // the hoppers only collect from the lower part of the block space above them, so that's where the entities + // have to sit. + $box = new AxisAlignedBB($position->x, $position->y + 1, $position->z, $position->x + 1, $position->y + 2, $position->z + 1); + $occupied = false; + foreach($this->world->getNearbyEntities($box) as $entity){ + if($entity instanceof ItemEntity && !$entity->isFlaggedForDespawn()){ + $occupied = true; + break; + } + } + if($occupied){ + continue; + } + + $item = VanillaBlocks::COBBLESTONE()->asItem()->setCount(self::STACK_SIZE); + $entity = $this->world->dropItem($position->add(0.5, 1.3, 0.5), $item, new Vector3(0, 0, 0), 0); + if($entity !== null){ + // without gravity the entity stays inside the hopper's collection area instead of dropping out of it + // again after a few ticks. + $entity->setHasGravity(false); + $this->spawnedTotal += self::STACK_SIZE; + } + } + } + + protected function checkInvariants(int $tick) : void{ + $total = $this->countItems($this->inventories) + $this->countDroppedItems(); + if($total !== $this->spawnedTotal){ + throw new TestFailedException("Dropped " . $this->spawnedTotal . " items but found $total after $tick ticks"); + } + $this->refillEntities(); + } + + protected function checkOutcome() : void{ + if($this->listener === null || $this->listener->getCalls() === 0){ + throw new TestFailedException("BlockItemPickupEvent was never called, so nothing was actually tested"); + } + if($this->countItems($this->chestInventories) === 0){ + throw new TestFailedException("The hoppers didn't collect a single item within " . self::DURATION_TICKS . " ticks"); + } + } + + protected function tearDownArea() : void{ + if($this->listener !== null){ + HandlerListManager::global()->unregisterAll($this->listener); + $this->listener = null; + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperPickupRangeTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperPickupRangeTest.php new file mode 100644 index 000000000..7015ce82d --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperPickupRangeTest.php @@ -0,0 +1,136 @@ +> + */ + private array $lanes = []; + private ?int $thrownCollectedTick = null; + + public function __construct(\Logger $logger, Main $plugin){ + parent::__construct( + $logger, + $plugin, + "Hopper pickup range test", + "Checks that a hopper collects items which came to rest on top of it, without waiting out their pickup delay" + ); + } + + protected function shouldFreezeDroppedItems() : bool{ + return false; + } + + protected function setUpArea() : void{ + $this->lanes = []; + $this->thrownCollectedTick = null; + + $this->lanes["resting"] = $this->buildLane(2, 0); + $this->lanes["thrown"] = $this->buildLane(6, self::THROWN_PICKUP_DELAY); + $this->lanes["uncollectable"] = $this->buildLane(10, ItemEntity::NEVER_DESPAWN); + } + + /** + * @return Inventory[] + * @phpstan-return list + */ + private function buildLane(int $x, int $pickupDelay) : array{ + $chestPos = $this->areaPos($x, 0, 4); + $this->world->setBlock($chestPos, VanillaBlocks::CHEST(), false); + $hopperPos = $this->areaPos($x, 1, 4); + $this->world->setBlock($hopperPos, VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + // the hopper is held back until the item has come to rest, otherwise it would collect the item while it is + // still falling past it and the test would say nothing about where items actually end up. + $tile = $this->world->getTile($hopperPos); + if(!$tile instanceof TileHopper){ + throw new TestFailedException("Expected a hopper tile at " . $hopperPos->__toString()); + } + $tile->setTransferCooldown(self::SETTLE_TICKS); + + // the item is dropped from above the hopper without any motion of its own, so it falls straight down onto it + // instead of being placed inside the collection area to begin with. + $item = VanillaBlocks::COBBLESTONE()->asItem()->setCount(self::STACK_SIZE); + $this->world->dropItem($hopperPos->add(0.5, 2, 0.5), $item, new Vector3(0, 0, 0), $pickupDelay); + + return [$this->getContainerInventory($hopperPos), $this->getContainerInventory($chestPos)]; + } + + protected function checkInvariants(int $tick) : void{ + $expected = self::STACK_SIZE * 3; + $total = $this->countDroppedItems(); + foreach($this->lanes as $inventories){ + $total += $this->countItems($inventories); + } + if($total !== $expected){ + throw new TestFailedException("Dropped $expected items but found $total after $tick ticks"); + } + + $uncollectable = $this->countItems($this->lanes["uncollectable"]); + if($uncollectable !== 0){ + throw new TestFailedException("The hopper collected $uncollectable items which are meant to be uncollectable after $tick ticks"); + } + + if($this->thrownCollectedTick === null && $this->countItems($this->lanes["thrown"]) > 0){ + $this->thrownCollectedTick = $tick; + } + } + + protected function checkOutcome() : void{ + $resting = $this->countItems($this->lanes["resting"]); + if($resting !== self::STACK_SIZE){ + throw new TestFailedException("The hopper only collected $resting of the " . self::STACK_SIZE . " items resting on it within " . self::DURATION_TICKS . " ticks"); + } + + $thrown = $this->countItems($this->lanes["thrown"]); + if($thrown !== self::STACK_SIZE){ + throw new TestFailedException("The hopper only collected $thrown of the " . self::STACK_SIZE . " items thrown onto it within " . self::DURATION_TICKS . " ticks"); + } + if($this->thrownCollectedTick === null || $this->thrownCollectedTick >= self::THROWN_PICKUP_DELAY){ + throw new TestFailedException("The hopper waited until tick " . ($this->thrownCollectedTick ?? self::DURATION_TICKS) . " to collect a thrown item, which means it sat out its " . self::THROWN_PICKUP_DELAY . " tick pickup delay"); + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperTestBase.php b/tests/plugins/TesterPlugin/src/hopper/HopperTestBase.php new file mode 100644 index 000000000..f59dee2f8 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperTestBase.php @@ -0,0 +1,247 @@ +|null */ + private ?TaskHandler $tickHandler = null; + private int $elapsedTicks = 0; + + public function __construct(\Logger $logger, protected Main $plugin, string $name, string $description){ + parent::__construct($logger, $name, $description); + $this->testLogger = $logger; + } + + final public function run() : void{ + $world = $this->plugin->getServer()->getWorldManager()->getDefaultWorld(); + if($world === null){ + throw new TestFailedException("The server has no default world to build the test setup in"); + } + $this->world = $world; + + $spawn = $world->getSpawnLocation(); + $chunkX = $spawn->getFloorX() >> Chunk::COORD_BIT_SIZE; + $chunkZ = $spawn->getFloorZ() >> Chunk::COORD_BIT_SIZE; + $this->areaX = $chunkX << Chunk::COORD_BIT_SIZE; + $this->areaZ = $chunkZ << Chunk::COORD_BIT_SIZE; + + $world->registerChunkLoader($this, $chunkX, $chunkZ); + $world->orderChunkPopulation($chunkX, $chunkZ, $this)->onCompletion( + function() : void{ + $this->startTicking(); + }, + function() : void{ + $this->testLogger->error("Failed to generate the chunk the test setup is built in"); + $this->finish(Test::RESULT_ERROR); + } + ); + } + + private function startTicking() : void{ + try{ + $this->clearArea(); + $this->setUpArea(); + }catch(TestFailedException $e){ + $this->testLogger->error($e->getMessage()); + $this->finish(Test::RESULT_FAILED); + return; + }catch(\Throwable $e){ + $this->testLogger->logException($e); + $this->finish(Test::RESULT_ERROR); + return; + } + + $this->tickHandler = $this->plugin->getScheduler()->scheduleRepeatingTask(new ClosureTask(function() : void{ + $this->tickTest(); + }), 1); + } + + private function tickTest() : void{ + if($this->isTimedOut()){ + $this->cleanUp(); + return; + } + + $this->elapsedTicks++; + try{ + if($this->shouldFreezeDroppedItems()){ + $this->freezeDroppedItems(); + } + $this->checkInvariants($this->elapsedTicks); + if($this->elapsedTicks >= $this->getDurationTicks()){ + $this->checkOutcome(); + $this->finish(Test::RESULT_OK); + } + }catch(TestFailedException $e){ + $this->testLogger->error($e->getMessage()); + $this->finish(Test::RESULT_FAILED); + }catch(\Throwable $e){ + $this->testLogger->logException($e); + $this->finish(Test::RESULT_ERROR); + } + } + + private function finish(int $result) : void{ + $this->cleanUp(); + $this->setResult($result); + } + + private function cleanUp() : void{ + $this->tickHandler?->cancel(); + $this->tickHandler = null; + try{ + $this->tearDownArea(); + $this->clearArea(); + }catch(\Throwable $e){ + $this->testLogger->logException($e); + } + $this->world->unregisterChunkLoader($this, $this->areaX >> Chunk::COORD_BIT_SIZE, $this->areaZ >> Chunk::COORD_BIT_SIZE); + } + + abstract protected function setUpArea() : void; + + abstract protected function checkInvariants(int $tick) : void; + + abstract protected function checkOutcome() : void; + + abstract protected function getDurationTicks() : int; + + protected function tearDownArea() : void{ + + } + + protected function areaPos(int $x, int $y, int $z) : Vector3{ + return new Vector3($this->areaX + $x, self::AREA_Y + $y, $this->areaZ + $z); + } + + protected function entityBoundingBox() : AxisAlignedBB{ + return new AxisAlignedBB( + $this->areaX - self::ENTITY_SEARCH_PADDING, + World::Y_MIN, + $this->areaZ - self::ENTITY_SEARCH_PADDING, + $this->areaX + self::AREA_SIZE + self::ENTITY_SEARCH_PADDING, + self::AREA_Y + self::AREA_HEIGHT, + $this->areaZ + self::AREA_SIZE + self::ENTITY_SEARCH_PADDING + ); + } + + protected function shouldFreezeDroppedItems() : bool{ + return true; + } + + private function freezeDroppedItems() : void{ + foreach($this->world->getNearbyEntities($this->entityBoundingBox()) as $entity){ + if($entity instanceof ItemEntity && $entity->hasGravity()){ + $entity->setHasGravity(false); + $entity->setMotion(new Vector3(0, 0, 0)); + } + } + } + + /** + * @throws TestFailedException + */ + protected function getContainerInventory(Vector3 $pos) : Inventory{ + $tile = $this->world->getTile($pos); + if(!$tile instanceof Container){ + throw new TestFailedException("Expected a container tile at " . $pos->__toString() . ", but found " . ($tile === null ? "nothing" : $tile::class)); + } + return $tile->getInventory(); + } + + /** + * @param Inventory[] $inventories + */ + protected function countItems(array $inventories) : int{ + $count = 0; + foreach($inventories as $inventory){ + for($slot = 0, $size = $inventory->getSize(); $slot < $size; $slot++){ + $count += $inventory->getItem($slot)->getCount(); + } + } + return $count; + } + + /** + * @phpstan-param (\Closure(Item) : bool)|null $filter + */ + protected function countDroppedItems(?\Closure $filter = null) : int{ + $count = 0; + foreach($this->world->getNearbyEntities($this->entityBoundingBox()) as $entity){ + if(!$entity instanceof ItemEntity || $entity->isFlaggedForDespawn()){ + continue; + } + $item = $entity->getItem(); + if($filter === null || $filter($item)){ + $count += $item->getCount(); + } + } + return $count; + } + + private function clearArea() : void{ + foreach($this->world->getNearbyEntities($this->entityBoundingBox()) as $entity){ + if($entity instanceof ItemEntity){ + $entity->close(); + } + } + + $air = VanillaBlocks::AIR(); + for($x = 0; $x < self::AREA_SIZE; $x++){ + for($z = 0; $z < self::AREA_SIZE; $z++){ + for($y = 0; $y < self::AREA_HEIGHT; $y++){ + $this->world->setBlockAt($this->areaX + $x, self::AREA_Y + $y, $this->areaZ + $z, $air, false); + } + } + } + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HostileItemPickupListener.php b/tests/plugins/TesterPlugin/src/hopper/HostileItemPickupListener.php new file mode 100644 index 000000000..d56f4fdb5 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HostileItemPickupListener.php @@ -0,0 +1,81 @@ + $foreignInventories + */ + public function __construct(private array $foreignInventories){} + + public function onBlockItemPickup(BlockItemPickupEvent $event) : void{ + switch($this->calls++ % self::BEHAVIOUR_COUNT){ + case 0: + break; + case 1: + $event->cancel(); + break; + case 2: + // The stack left on the ground is what the entity holds, so raising the count must not make the hopper + // collect more than that. + $item = $event->getItem(); + $event->setItem($item->setCount($item->getMaxStackSize())); + break; + case 3: + // Collecting a different item than the entity holds would leave a mismatching stack behind, so the + // hopper has to skip the entity instead. + $event->setItem(VanillaBlocks::DIRT()->asItem()); + break; + case 4: + $event->setInventory(null); + break; + case 5: + if(count($this->foreignInventories) > 0){ + $event->setInventory($this->foreignInventories[$this->calls % count($this->foreignInventories)]); + } + break; + } + } + + public function getCalls() : int{ + return $this->calls; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HostileJukeboxListener.php b/tests/plugins/TesterPlugin/src/hopper/HostileJukeboxListener.php new file mode 100644 index 000000000..a6bdc426d --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HostileJukeboxListener.php @@ -0,0 +1,151 @@ + $jukeboxPositions + */ + public function __construct( + private World $world, + private array $jukeboxPositions + ){} + + public function onInventoryMoveItem(InventoryMoveItemEvent $event) : void{ + $this->calls++; + + // which side of the move is missing tells apart the three kinds of move happening in this setup, so every kind + // gets hit by the behaviour that actually stresses it instead of relying on a single counter to line up. + if($event->getSource() === null){ + $this->abusePullFromJukebox($event); + }elseif($event->getDestination() === null){ + $this->abusePushIntoJukebox($event); + }else{ + $this->abuseInventoryMove($event); + } + } + + private function abusePullFromJukebox(InventoryMoveItemEvent $event) : void{ + switch($this->pullCalls++ % self::BEHAVIOUR_COUNT){ + case 0: + // the record the hopper decided to pull is dropped on the ground, so pulling it out anyway would + // duplicate it. + $this->ejectRecords(); + break; + case 1: + // only records can leave a jukebox, so this move has to be rejected rather than turning the record into + // something else. + $event->setItem(VanillaBlocks::DIRT()->asItem()); + break; + default: + break; + } + } + + private function abusePushIntoJukebox(InventoryMoveItemEvent $event) : void{ + switch($this->pushCalls++ % self::BEHAVIOUR_COUNT){ + case 0: + // the jukebox the hopper decided to push into is filled, so pushing into it anyway would destroy one of + // the two records. + $this->insertRecords(); + break; + case 1: + // hoppers move a single record at a time, so raising the count must not make them move more than that. + $item = $event->getItem(); + $event->setItem($item->setCount($item->getMaxStackSize())); + break; + case 2: + $event->setItem(VanillaBlocks::DIRT()->asItem()); + break; + default: + break; + } + } + + private function abuseInventoryMove(InventoryMoveItemEvent $event) : void{ + switch($this->otherCalls++ % self::BEHAVIOUR_COUNT){ + case 0: + $event->cancel(); + break; + case 1: + $this->ejectRecords(); + break; + case 2: + $this->insertRecords(); + break; + default: + break; + } + } + + private function ejectRecords() : void{ + foreach($this->jukeboxPositions as $position){ + $block = $this->world->getBlock($position); + if(!$block instanceof Jukebox || $block->getRecord() === null){ + continue; + } + $block->ejectRecord(); + $this->world->setBlock($position, $block); + } + } + + private function insertRecords() : void{ + foreach($this->jukeboxPositions as $position){ + $block = $this->world->getBlock($position); + if(!$block instanceof Jukebox || $block->getRecord() !== null){ + continue; + } + $block->insertRecord(VanillaItems::RECORD_CAT()); + $this->world->setBlock($position, $block); + $this->ledger++; + } + } + + public function getLedger() : int{ + return $this->ledger; + } + + public function getCalls() : int{ + return $this->calls; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HostileMoveItemListener.php b/tests/plugins/TesterPlugin/src/hopper/HostileMoveItemListener.php new file mode 100644 index 000000000..b5dffe8a4 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HostileMoveItemListener.php @@ -0,0 +1,114 @@ + + */ + private array $callsPerDestination = []; + + public function onInventoryMoveItem(InventoryMoveItemEvent $event) : void{ + $this->calls++; + + $destination = $event->getDestination(); + $key = $destination === null ? 0 : spl_object_id($destination); + $behaviour = ($this->callsPerDestination[$key] = ($this->callsPerDestination[$key] ?? 0) + 1) - 1; + + switch($behaviour % self::BEHAVIOUR_COUNT){ + case 0: + $event->cancel(); + break; + case 1: + // hoppers move a single item at a time, so raising the count must not make them move more than that. + $item = $event->getItem(); + $event->setItem($item->setCount($item->getMaxStackSize())); + break; + case 2: + // swapping the moved item for a different type is allowed, but must never change how many items exist. + $event->setItem(VanillaBlocks::DIRT()->asItem()); + break; + case 3: + $this->takeFromSource($event); + break; + case 4: + $this->fillDestination($event); + break; + case 5: + $event->setItem(VanillaItems::AIR()); + break; + } + } + + private function takeFromSource(InventoryMoveItemEvent $event) : void{ + $source = $event->getSource(); + if($source === null){ + return; + } + for($slot = 0, $size = $source->getSize(); $slot < $size; $slot++){ + $item = $source->getItem($slot); + if($item->isNull()){ + continue; + } + $item->pop(); + $source->setItem($slot, $item); + $this->ledger--; + return; + } + } + + private function fillDestination(InventoryMoveItemEvent $event) : void{ + $destination = $event->getDestination(); + if($destination === null){ + return; + } + $extra = VanillaBlocks::COBBLESTONE()->asItem()->setCount(1); + $leftover = 0; + foreach($destination->addItem($extra) as $item){ + $leftover += $item->getCount(); + } + $this->ledger += $extra->getCount() - $leftover; + } + + public function getLedger() : int{ + return $this->ledger; + } + + public function getCalls() : int{ + return $this->calls; + } +} From 4108d4998b31a61ba91bfe25a51b1662c1ba65ab Mon Sep 17 00:00:00 2001 From: bonbionTR Date: Wed, 16 Sep 2026 17:01:40 +0300 Subject: [PATCH 12/12] ready? --- src/block/Hopper.php | 78 +++++++++- src/block/inventory/ShulkerBoxInventory.php | 32 ++++- src/block/tile/Hopper.php | 9 ++ .../inventory/ShulkerBoxInventoryTest.php | 61 ++++++++ tests/plugins/TesterPlugin/src/Main.php | 6 + .../HopperInventoryListenerDupeTest.php | 136 ++++++++++++++++++ .../src/hopper/HopperPickupDupeTest.php | 5 +- .../hopper/HopperPickupShulkerNestTest.php | 114 +++++++++++++++ .../src/hopper/HopperStaleUpdateTest.php | 87 +++++++++++ .../src/hopper/HostileItemPickupListener.php | 27 +++- 10 files changed, 542 insertions(+), 13 deletions(-) create mode 100644 tests/phpunit/block/inventory/ShulkerBoxInventoryTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperInventoryListenerDupeTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperPickupShulkerNestTest.php create mode 100644 tests/plugins/TesterPlugin/src/hopper/HopperStaleUpdateTest.php diff --git a/src/block/Hopper.php b/src/block/Hopper.php index 3712469f1..63181ee14 100644 --- a/src/block/Hopper.php +++ b/src/block/Hopper.php @@ -54,6 +54,7 @@ use pocketmine\math\Vector3; use pocketmine\player\Player; use pocketmine\world\BlockTransaction; +use function count; use function min; class Hopper extends Transparent implements PoweredByRedstone{ @@ -121,6 +122,13 @@ public function onScheduledUpdate() : void{ if(!$tile instanceof TileHopper){ return; } + + $currentTick = $world->getServer()->getTick(); + if($tile->getLastScheduledUpdateTick() === $currentTick){ + return; + } + $tile->setLastScheduledUpdateTick($currentTick); + $world->scheduleDelayedBlockUpdate($this->position, 1); // A powered hopper is locked, which freezes its cooldown instead of letting it tick down. @@ -243,6 +251,11 @@ private function push(HopperInventory $inventory) : bool{ // The source is always written first, so a listener reacting to either write can never observe the record // in both the hopper and the jukebox at once. $inventory->setItem($slot, $item); + $jukeboxBlock = $destination->getBlock(); + if(!$jukeboxBlock instanceof Jukebox || $jukeboxBlock->getRecord() !== null){ + $this->returnItemToSource($inventory, $slot, $recordToPush); + return false; + } $jukeboxBlock->insertRecord($recordToPush); $this->position->getWorld()->setBlock($jukeboxBlock->getPosition(), $jukeboxBlock); return true; @@ -382,6 +395,13 @@ private function pickup(HopperInventory $inventory) : bool{ if($ev->isCancelled()){ continue; } + if($entity->isClosed() || $entity->isFlaggedForDespawn()){ + continue; + } + $item = $entity->getItem(); + if($item->isNull()){ + continue; + } $destination = $ev->getInventory(); if($destination === null){ continue; @@ -392,6 +412,10 @@ private function pickup(HopperInventory $inventory) : bool{ if(!$pickedUpItem->canStackWith($item)){ continue; } + $probe = (clone $pickedUpItem)->setCount(1); + if(!$destination->canAddItem($probe)){ + continue; + } // Hoppers pick up as much of the item entity's stack as they can hold and leave the rest on the ground. $entityCount = $item->getCount(); $addableQuantity = min($destination->getAddableItemQuantity($pickedUpItem), $entityCount); @@ -399,8 +423,15 @@ private function pickup(HopperInventory $inventory) : bool{ continue; } - $destination->addItem((clone $pickedUpItem)->setCount($addableQuantity)); - $remainingCount = $entityCount - $addableQuantity; + $leftover = $destination->addItem((clone $pickedUpItem)->setCount($addableQuantity)); + $inserted = $addableQuantity - $this->countItems($leftover); + if($inserted <= 0){ + continue; + } + if($entity->isClosed() || $entity->isFlaggedForDespawn()){ + return true; + } + $remainingCount = $entity->getItem()->getCount() - $inserted; if($remainingCount > 0){ $entity->setStackSize($remainingCount); }else{ @@ -433,15 +464,19 @@ private function transferToSlot(Inventory $source, int $sourceSlot, Item $source return false; } + // The source is always written first, so a listener reacting to either write can never observe the same item in + // both inventories at once. + $source->setItem($sourceSlot, $sourceItem); + $itemInSlot = $destination->getItem($destinationSlot); + if(!$this->canMergeInto($destination, $itemInSlot, $itemToMove)){ + $this->returnItemToSource($source, $sourceSlot, $itemToMove); + return false; + } if(!$itemInSlot->isNull()){ $itemInSlot->setCount($itemInSlot->getCount() + $itemToMove->getCount()); }else{ $itemInSlot = $itemToMove; } - - // The source is always written first, so a listener reacting to either write can never observe the same item in - // both inventories at once. - $source->setItem($sourceSlot, $sourceItem); $destination->setItem($destinationSlot, $itemInSlot); return true; } @@ -465,7 +500,11 @@ private function transferToInventory(Inventory $source, int $sourceSlot, Item $s } $source->setItem($sourceSlot, $sourceItem); - $destination->addItem($itemToMove); + $leftover = $destination->addItem($itemToMove); + if(count($leftover) !== 0){ + $this->returnItemToSource($source, $sourceSlot, $itemToMove); + return false; + } return true; } @@ -510,6 +549,31 @@ private function isInventoryEmpty(Inventory $inventory) : bool{ return true; } + private function returnItemToSource(Inventory $source, int $sourceSlot, Item $item) : void{ + $current = $source->getItem($sourceSlot); + if($this->canMergeInto($source, $current, $item)){ + if($current->isNull()){ + $source->setItem($sourceSlot, clone $item); + return; + } + $current->setCount($current->getCount() + $item->getCount()); + $source->setItem($sourceSlot, $current); + return; + } + $source->addItem(clone $item); + } + + /** + * @param Item[] $items + */ + private function countItems(array $items) : int{ + $total = 0; + foreach($items as $item){ + $total += $item->getCount(); + } + return $total; + } + /** * Returns the item to move after the event has been called, or null if the move was cancelled or the event left * nothing to move. diff --git a/src/block/inventory/ShulkerBoxInventory.php b/src/block/inventory/ShulkerBoxInventory.php index 81f4f35bd..abc1830e2 100644 --- a/src/block/inventory/ShulkerBoxInventory.php +++ b/src/block/inventory/ShulkerBoxInventory.php @@ -53,13 +53,41 @@ protected function getCloseSound() : Sound{ } public function canAddItem(Item $item) : bool{ - $blockTypeId = ItemTypeIds::toBlockTypeId($item->getTypeId()); - if($blockTypeId === BlockTypeIds::SHULKER_BOX || $blockTypeId === BlockTypeIds::DYED_SHULKER_BOX){ + if($this->isNestedShulkerBox($item)){ return false; } return parent::canAddItem($item); } + public function getAddableItemQuantity(Item $item) : int{ + if($this->isNestedShulkerBox($item)){ + return 0; + } + return parent::getAddableItemQuantity($item); + } + + public function addItem(Item ...$slots) : array{ + $accepted = []; + $rejected = []; + foreach($slots as $slot){ + if($this->isNestedShulkerBox($slot)){ + $rejected[] = clone $slot; + continue; + } + $accepted[] = $slot; + } + $leftover = $accepted === [] ? [] : parent::addItem(...$accepted); + foreach($rejected as $item){ + $leftover[] = $item; + } + return $leftover; + } + + private function isNestedShulkerBox(Item $item) : bool{ + $blockTypeId = ItemTypeIds::toBlockTypeId($item->getTypeId()); + return $blockTypeId === BlockTypeIds::SHULKER_BOX || $blockTypeId === BlockTypeIds::DYED_SHULKER_BOX; + } + protected function animateBlock(bool $isOpen) : void{ $holder = $this->getHolder(); diff --git a/src/block/tile/Hopper.php b/src/block/tile/Hopper.php index 63d4efdee..41cb58157 100644 --- a/src/block/tile/Hopper.php +++ b/src/block/tile/Hopper.php @@ -41,6 +41,7 @@ class Hopper extends Spawnable implements Container, Nameable{ private HopperInventory $inventory; private int $transferCooldown = 0; + private int $lastScheduledUpdateTick = -1; public function __construct(World $world, Vector3 $pos){ parent::__construct($world, $pos); @@ -95,4 +96,12 @@ public function setTransferCooldown(int $transferCooldown) : void{ } $this->transferCooldown = $transferCooldown; } + + public function getLastScheduledUpdateTick() : int{ + return $this->lastScheduledUpdateTick; + } + + public function setLastScheduledUpdateTick(int $tick) : void{ + $this->lastScheduledUpdateTick = $tick; + } } diff --git a/tests/phpunit/block/inventory/ShulkerBoxInventoryTest.php b/tests/phpunit/block/inventory/ShulkerBoxInventoryTest.php new file mode 100644 index 000000000..6618c2ef2 --- /dev/null +++ b/tests/phpunit/block/inventory/ShulkerBoxInventoryTest.php @@ -0,0 +1,61 @@ +createInventory(); + $shulker = VanillaBlocks::SHULKER_BOX()->asItem(); + $dyed = VanillaBlocks::DYED_SHULKER_BOX()->asItem(); + + self::assertFalse($inventory->canAddItem($shulker)); + self::assertFalse($inventory->canAddItem($dyed)); + self::assertSame(0, $inventory->getAddableItemQuantity($shulker)); + self::assertSame(0, $inventory->getAddableItemQuantity($dyed)); + self::assertNotEmpty($inventory->addItem($shulker)); + self::assertNotEmpty($inventory->addItem($dyed)); + self::assertTrue($inventory->isSlotEmpty(0)); + } + + public function testStillAcceptsNormalItems() : void{ + $inventory = $this->createInventory(); + $dirt = VanillaBlocks::DIRT()->asItem()->setCount(16); + + self::assertTrue($inventory->canAddItem($dirt)); + self::assertSame(16, $inventory->getAddableItemQuantity($dirt)); + self::assertEmpty($inventory->addItem($dirt)); + self::assertTrue($inventory->getItem(0)->equalsExact($dirt)); + } +} diff --git a/tests/plugins/TesterPlugin/src/Main.php b/tests/plugins/TesterPlugin/src/Main.php index 167ec8041..48ef00134 100644 --- a/tests/plugins/TesterPlugin/src/Main.php +++ b/tests/plugins/TesterPlugin/src/Main.php @@ -27,11 +27,14 @@ use pmmp\TesterPlugin\hopper\HopperChainStressTest; use pmmp\TesterPlugin\hopper\HopperCooldownTest; +use pmmp\TesterPlugin\hopper\HopperInventoryListenerDupeTest; use pmmp\TesterPlugin\hopper\HopperJukeboxDupeTest; use pmmp\TesterPlugin\hopper\HopperMoveEventDupeTest; use pmmp\TesterPlugin\hopper\HopperPickupBoundsTest; use pmmp\TesterPlugin\hopper\HopperPickupDupeTest; use pmmp\TesterPlugin\hopper\HopperPickupRangeTest; +use pmmp\TesterPlugin\hopper\HopperPickupShulkerNestTest; +use pmmp\TesterPlugin\hopper\HopperStaleUpdateTest; use pocketmine\event\Listener; use pocketmine\event\server\CommandEvent; use pocketmine\plugin\PluginBase; @@ -85,9 +88,12 @@ function() : void{ new HopperChainStressTest($this->getLogger(), $this), new HopperCooldownTest($this->getLogger(), $this), new HopperMoveEventDupeTest($this->getLogger(), $this), + new HopperInventoryListenerDupeTest($this->getLogger(), $this), new HopperPickupDupeTest($this->getLogger(), $this), new HopperPickupRangeTest($this->getLogger(), $this), new HopperPickupBoundsTest($this->getLogger(), $this), + new HopperPickupShulkerNestTest($this->getLogger(), $this), + new HopperStaleUpdateTest($this->getLogger(), $this), new HopperJukeboxDupeTest($this->getLogger(), $this) ]; } diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperInventoryListenerDupeTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperInventoryListenerDupeTest.php new file mode 100644 index 000000000..97476038a --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperInventoryListenerDupeTest.php @@ -0,0 +1,136 @@ + + */ + private array $inventories = []; + private int $initialTotal = 0; + private int $ledger = 0; + + public function __construct(\Logger $logger, Main $plugin){ + parent::__construct( + $logger, + $plugin, + "Hopper InventoryListener destination fill test", + "Checks that hoppers neither duplicate nor destroy items when a source InventoryListener fills the destination during setItem()" + ); + } + + protected function setUpArea() : void{ + $this->inventories = []; + $this->ledger = 0; + + $this->buildChestLane(); + $this->buildFurnaceLane(); + $this->initialTotal = $this->countItems($this->inventories); + } + + private function buildChestLane() : void{ + $destinationPos = $this->areaPos(0, 0, 0); + $this->world->setBlock($destinationPos, VanillaBlocks::CHEST(), false); + $this->world->setBlock($this->areaPos(0, 1, 0), VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $sourcePos = $this->areaPos(0, 2, 0); + $this->world->setBlock($sourcePos, VanillaBlocks::CHEST(), false); + $source = $this->getContainerInventory($sourcePos); + $cobblestone = VanillaBlocks::COBBLESTONE()->asItem(); + $source->setItem(0, $cobblestone->setCount($cobblestone->getMaxStackSize())); + + $hopper = $this->getContainerInventory($this->areaPos(0, 1, 0)); + $destination = $this->getContainerInventory($destinationPos); + $this->attachFiller($source, $hopper); + $this->attachFiller($hopper, $destination); + $this->inventories[] = $source; + $this->inventories[] = $hopper; + $this->inventories[] = $destination; + } + + private function buildFurnaceLane() : void{ + $destinationPos = $this->areaPos(3, 0, 0); + $this->world->setBlock($destinationPos, VanillaBlocks::FURNACE(), false); + $this->world->setBlock($this->areaPos(3, 1, 0), VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $sourcePos = $this->areaPos(3, 2, 0); + $this->world->setBlock($sourcePos, VanillaBlocks::CHEST(), false); + $source = $this->getContainerInventory($sourcePos); + $cobblestone = VanillaBlocks::COBBLESTONE()->asItem(); + $source->setItem(0, $cobblestone->setCount($cobblestone->getMaxStackSize())); + + $hopper = $this->getContainerInventory($this->areaPos(3, 1, 0)); + $destination = $this->getContainerInventory($destinationPos); + $this->attachFiller($source, $hopper); + $this->attachFiller($hopper, $destination); + $this->inventories[] = $source; + $this->inventories[] = $hopper; + $this->inventories[] = $destination; + } + + private function attachFiller(Inventory $source, Inventory $destination) : void{ + $source->getListeners()->add(new CallbackInventoryListener( + function(Inventory $inventory, int $slot, Item $oldItem) use ($destination) : void{ + $extra = VanillaBlocks::COBBLESTONE()->asItem()->setCount(1); + $leftover = 0; + foreach($destination->addItem($extra) as $item){ + $leftover += $item->getCount(); + } + $this->ledger += $extra->getCount() - $leftover; + }, + null + )); + } + + protected function checkInvariants(int $tick) : void{ + $expected = $this->initialTotal + $this->ledger; + $total = $this->countItems($this->inventories); + if($total !== $expected){ + throw new TestFailedException("Expected $expected items after $tick ticks, but found $total"); + } + } + + protected function checkOutcome() : void{ + if($this->ledger === 0){ + throw new TestFailedException("The source InventoryListener never filled the destination, so nothing was actually tested"); + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperPickupDupeTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperPickupDupeTest.php index 9cb51c525..c195c14ff 100644 --- a/tests/plugins/TesterPlugin/src/hopper/HopperPickupDupeTest.php +++ b/tests/plugins/TesterPlugin/src/hopper/HopperPickupDupeTest.php @@ -126,9 +126,10 @@ private function refillEntities() : void{ } protected function checkInvariants(int $tick) : void{ + $expected = $this->spawnedTotal + ($this->listener?->getLedger() ?? 0); $total = $this->countItems($this->inventories) + $this->countDroppedItems(); - if($total !== $this->spawnedTotal){ - throw new TestFailedException("Dropped " . $this->spawnedTotal . " items but found $total after $tick ticks"); + if($total !== $expected){ + throw new TestFailedException("Expected $expected items after $tick ticks, but found $total"); } $this->refillEntities(); } diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperPickupShulkerNestTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperPickupShulkerNestTest.php new file mode 100644 index 000000000..7537ddf22 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperPickupShulkerNestTest.php @@ -0,0 +1,114 @@ +calls = 0; + + $shulkerPos = $this->areaPos(0, 0, 0); + $this->world->setBlock($shulkerPos, VanillaBlocks::SHULKER_BOX(), false); + $this->shulkerInventory = $this->getContainerInventory($shulkerPos); + + $hopperPos = $this->areaPos(2, 1, 0); + $this->world->setBlock($hopperPos, VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $this->dropAboveHopper($hopperPos, VanillaBlocks::SHULKER_BOX()->asItem()); + $this->dropAboveHopper($hopperPos, VanillaBlocks::DIRT()->asItem()->setCount(8)); + + $this->plugin->getServer()->getPluginManager()->registerEvents($this, $this->plugin); + } + + private function dropAboveHopper(Vector3 $hopperPos, Item $item) : void{ + $entity = $this->world->dropItem($hopperPos->add(0.5, 1.3, 0.5), $item, new Vector3(0, 0, 0), 0); + $entity?->setHasGravity(false); + } + + public function onBlockItemPickup(BlockItemPickupEvent $event) : void{ + if($this->shulkerInventory === null){ + return; + } + $this->calls++; + $event->setInventory($this->shulkerInventory); + } + + protected function checkInvariants(int $tick) : void{ + if($this->shulkerInventory === null){ + return; + } + for($slot = 0, $size = $this->shulkerInventory->getSize(); $slot < $size; $slot++){ + $item = $this->shulkerInventory->getItem($slot); + if($item->equals(VanillaBlocks::SHULKER_BOX()->asItem(), false, false)){ + throw new TestFailedException("A shulker box was inserted into another shulker box after $tick ticks"); + } + } + } + + protected function checkOutcome() : void{ + if($this->calls === 0){ + throw new TestFailedException("BlockItemPickupEvent was never called, so nothing was actually tested"); + } + if($this->shulkerInventory === null || $this->countItems([$this->shulkerInventory]) === 0){ + throw new TestFailedException("The hopper never moved the dirt into the shulker box within " . self::DURATION_TICKS . " ticks"); + } + if($this->countDroppedItems(static fn(Item $item) : bool => $item->equals(VanillaBlocks::SHULKER_BOX()->asItem(), false, false)) !== 1){ + throw new TestFailedException("The dropped shulker box should have stayed on the ground"); + } + } + + protected function tearDownArea() : void{ + HandlerListManager::global()->unregisterAll($this); + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HopperStaleUpdateTest.php b/tests/plugins/TesterPlugin/src/hopper/HopperStaleUpdateTest.php new file mode 100644 index 000000000..baa0990e2 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/hopper/HopperStaleUpdateTest.php @@ -0,0 +1,87 @@ +areaPos(0, 1, 0); + $this->world->setBlock($this->areaPos(0, 0, 0), VanillaBlocks::CHEST(), false); + $this->world->scheduleDelayedBlockUpdate($hopperPos, self::STALE_DELAY_TICKS); + $this->world->setBlock($hopperPos, VanillaBlocks::HOPPER()->setFacing(Facing::DOWN), false); + + $sourcePos = $this->areaPos(0, 2, 0); + $this->world->setBlock($sourcePos, VanillaBlocks::CHEST(), false); + $this->source = $this->getContainerInventory($sourcePos); + $cobblestone = VanillaBlocks::COBBLESTONE()->asItem(); + $this->source->setItem(0, $cobblestone->setCount($cobblestone->getMaxStackSize())); + $this->initialSourceCount = $this->countItems([$this->source]); + } + + protected function checkInvariants(int $tick) : void{ + $moved = $this->initialSourceCount - $this->countItems([$this->source]); + $maximum = intdiv($tick, TileHopper::DEFAULT_TRANSFER_COOLDOWN) + self::TOLERANCE_TICKS; + if($moved > $maximum){ + throw new TestFailedException("The hopper moved $moved items within $tick ticks, which is more than the $maximum items its transfer cooldown allows"); + } + } + + protected function checkOutcome() : void{ + $moved = $this->initialSourceCount - $this->countItems([$this->source]); + $expected = intdiv(self::DURATION_TICKS, TileHopper::DEFAULT_TRANSFER_COOLDOWN); + if(abs($moved - $expected) > self::TOLERANCE_TICKS){ + throw new TestFailedException("The hopper moved $moved items within " . self::DURATION_TICKS . " ticks, but around $expected were expected"); + } + } + + protected function getDurationTicks() : int{ + return self::DURATION_TICKS; + } +} diff --git a/tests/plugins/TesterPlugin/src/hopper/HostileItemPickupListener.php b/tests/plugins/TesterPlugin/src/hopper/HostileItemPickupListener.php index d56f4fdb5..68d7f3360 100644 --- a/tests/plugins/TesterPlugin/src/hopper/HostileItemPickupListener.php +++ b/tests/plugins/TesterPlugin/src/hopper/HostileItemPickupListener.php @@ -26,6 +26,7 @@ namespace pmmp\TesterPlugin\hopper; use pocketmine\block\VanillaBlocks; +use pocketmine\entity\object\ItemEntity; use pocketmine\event\block\BlockItemPickupEvent; use pocketmine\event\Listener; use pocketmine\inventory\Inventory; @@ -33,12 +34,13 @@ /** * Abuses BlockItemPickupEvent while a hopper is collecting an item entity, by cancelling the pickup, replacing the - * collected item and redirecting the pickup into a completely different inventory. + * collected item, mutating the origin entity and redirecting the pickup into a completely different inventory. */ final class HostileItemPickupListener implements Listener{ - private const BEHAVIOUR_COUNT = 6; + private const BEHAVIOUR_COUNT = 8; private int $calls = 0; + private int $ledger = 0; /** * @param Inventory[] $foreignInventories inventories the pickup may be redirected into @@ -72,9 +74,30 @@ public function onBlockItemPickup(BlockItemPickupEvent $event) : void{ $event->setInventory($this->foreignInventories[$this->calls % count($this->foreignInventories)]); } break; + case 6: + $origin = $event->getOrigin(); + if($origin instanceof ItemEntity && !$origin->isClosed()){ + $current = $origin->getItem()->getCount(); + if($current > 1){ + $origin->setStackSize(1); + $this->ledger -= $current - 1; + } + } + break; + case 7: + $origin = $event->getOrigin(); + if($origin instanceof ItemEntity && !$origin->isClosed() && !$origin->isFlaggedForDespawn()){ + $this->ledger -= $origin->getItem()->getCount(); + $origin->flagForDespawn(); + } + break; } } + public function getLedger() : int{ + return $this->ledger; + } + public function getCalls() : int{ return $this->calls; }