From 657c07b1a31a41a8a91dca6756730c4535c809d1 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 30 Aug 2025 23:58:44 +0100 Subject: [PATCH] Remove dependencies on hotbar and offhand inventory to get equipment It's always grated at me that the call chain to get the held item is so long --- src/command/defaults/EnchantCommand.php | 4 +-- src/entity/ExperienceManager.php | 8 ++--- src/entity/Human.php | 34 ++++++++++++++----- src/entity/object/ItemEntity.php | 2 +- src/entity/projectile/Arrow.php | 2 +- src/inventory/Hotbar.php | 28 --------------- src/item/Armor.php | 2 +- .../mcpe/StandardEntityEventBroadcaster.php | 13 ++++--- .../mcpe/handler/InGamePacketHandler.php | 2 +- src/player/Player.php | 32 ++++++++--------- src/player/SurvivalBlockBreakHandler.php | 2 +- 11 files changed, 58 insertions(+), 71 deletions(-) diff --git a/src/command/defaults/EnchantCommand.php b/src/command/defaults/EnchantCommand.php index 6b73aad6c..9160eef14 100644 --- a/src/command/defaults/EnchantCommand.php +++ b/src/command/defaults/EnchantCommand.php @@ -56,7 +56,7 @@ class EnchantCommand extends VanillaCommand{ return true; } - $item = $player->getHotbar()->getHeldItem(); + $item = $player->getMainHandItem(); if($item->isNull()){ $sender->sendMessage(KnownTranslationFactory::commands_enchant_noItem()); @@ -79,7 +79,7 @@ class EnchantCommand extends VanillaCommand{ //this is necessary to deal with enchanted books, which are a different item type than regular books $enchantedItem = EnchantingHelper::enchantItem($item, [new EnchantmentInstance($enchantment, $level)]); - $player->getHotbar()->setHeldItem($enchantedItem); + $player->setMainHandItem($enchantedItem); self::broadcastCommandMessage($sender, KnownTranslationFactory::commands_enchant_success($player->getName())); return true; diff --git a/src/entity/ExperienceManager.php b/src/entity/ExperienceManager.php index 5e04728f8..534a88cd2 100644 --- a/src/entity/ExperienceManager.php +++ b/src/entity/ExperienceManager.php @@ -243,10 +243,10 @@ class ExperienceManager{ //TODO: replace this with a more generic equipment getting/setting interface $equipment = []; - if(($item = $this->entity->getHotbar()->getHeldItem()) instanceof Durable && $item->hasEnchantment(VanillaEnchantments::MENDING())){ + if(($item = $this->entity->getMainHandItem()) instanceof Durable && $item->hasEnchantment(VanillaEnchantments::MENDING())){ $equipment[$mainHandIndex] = $item; } - if(($item = $this->entity->getOffHandInventory()->getItem(0)) instanceof Durable && $item->hasEnchantment(VanillaEnchantments::MENDING())){ + if(($item = $this->entity->getOffHandItem()) instanceof Durable && $item->hasEnchantment(VanillaEnchantments::MENDING())){ $equipment[$offHandIndex] = $item; } foreach($this->entity->getArmorInventory()->getContents() as $k => $armorItem){ @@ -263,9 +263,9 @@ class ExperienceManager{ $xpValue -= (int) ceil($repairAmount / 2); if($k === $mainHandIndex){ - $this->entity->getHotbar()->setHeldItem($repairItem); + $this->entity->setMainHandItem($repairItem); }elseif($k === $offHandIndex){ - $this->entity->getOffHandInventory()->setItem(0, $repairItem); + $this->entity->setOffHandItem($repairItem); }else{ $this->entity->getArmorInventory()->setItem($k, $repairItem); } diff --git a/src/entity/Human.php b/src/entity/Human.php index 279156d7d..591a6c473 100644 --- a/src/entity/Human.php +++ b/src/entity/Human.php @@ -245,6 +245,22 @@ class Human extends Living implements ProjectileSource, InventoryHolder{ return $this->inventory; } + public function getMainHandItem() : Item{ + return $this->inventory->getItem($this->hotbar->getSelectedIndex()); + } + + public function setMainHandItem(Item $item) : void{ + $this->inventory->setItem($this->hotbar->getSelectedIndex(), $item); + } + + public function getOffHandItem() : Item{ + return $this->offHandInventory->getItem(0); + } + + public function setOffHandItem(Item $item) : void{ + $this->offHandInventory->setItem(0, $item); + } + public function getOffHandInventory() : Inventory{ return $this->offHandInventory; } public function getEnderInventory() : Inventory{ @@ -279,7 +295,7 @@ class Human extends Living implements ProjectileSource, InventoryHolder{ $this->xpManager = new ExperienceManager($this); $this->inventory = new SimpleInventory(36); - $this->hotbar = new Hotbar($this->inventory); + $this->hotbar = new Hotbar(); $syncHeldItem = fn() => NetworkBroadcastUtils::broadcastEntityEvent( $this->getViewers(), @@ -323,7 +339,7 @@ class Human extends Living implements ProjectileSource, InventoryHolder{ } $offHand = $nbt->getCompoundTag(self::TAG_OFF_HAND_ITEM); if($offHand !== null){ - $this->offHandInventory->setItem(0, Item::nbtDeserialize($offHand)); + $this->setOffHandItem(Item::nbtDeserialize($offHand)); } $this->offHandInventory->getListeners()->add(CallbackInventoryListener::onAnyChange(fn() => NetworkBroadcastUtils::broadcastEntityEvent( $this->getViewers(), @@ -383,7 +399,7 @@ class Human extends Living implements ProjectileSource, InventoryHolder{ $type = $source->getCause(); if($type !== EntityDamageEvent::CAUSE_SUICIDE && $type !== EntityDamageEvent::CAUSE_VOID - && ($this->hotbar->getHeldItem() instanceof Totem || $this->offHandInventory->getItem(0) instanceof Totem)){ + && ($this->getMainHandItem() instanceof Totem || $this->getOffHandItem() instanceof Totem)){ $compensation = $this->getHealth() - $source->getFinalDamage() - 1; if($compensation <= -1){ @@ -405,13 +421,13 @@ class Human extends Living implements ProjectileSource, InventoryHolder{ $this->broadcastAnimation(new TotemUseAnimation($this)); $this->broadcastSound(new TotemUseSound()); - $hand = $this->hotbar->getHeldItem(); + $hand = $this->getMainHandItem(); if($hand instanceof Totem){ $hand->pop(); //Plugins could alter max stack size - $this->hotbar->setHeldItem($hand); - }elseif(($offHand = $this->offHandInventory->getItem(0)) instanceof Totem){ + $this->setMainHandItem($hand); + }elseif(($offHand = $this->getOffHandItem()) instanceof Totem){ $offHand->pop(); - $this->offHandInventory->setItem(0, $offHand); + $this->setOffHandItem($offHand); } } } @@ -459,7 +475,7 @@ class Human extends Living implements ProjectileSource, InventoryHolder{ $nbt->setInt(self::TAG_SELECTED_INVENTORY_SLOT, $this->hotbar->getSelectedIndex()); - $offHandItem = $this->offHandInventory->getItem(0); + $offHandItem = $this->getOffHandItem(); if(!$offHandItem->isNull()){ $nbt->setTag(self::TAG_OFF_HAND_ITEM, $offHandItem->nbtSerialize()); } @@ -511,7 +527,7 @@ class Human extends Living implements ProjectileSource, InventoryHolder{ $this->location->pitch, $this->location->yaw, $this->location->yaw, //TODO: head yaw - ItemStackWrapper::legacy($typeConverter->coreItemStackToNet($this->hotbar->getHeldItem())), + ItemStackWrapper::legacy($typeConverter->coreItemStackToNet($this->getMainHandItem())), GameMode::SURVIVAL, $this->getAllNetworkData(), new PropertySyncData([], []), diff --git a/src/entity/object/ItemEntity.php b/src/entity/object/ItemEntity.php index 43c495b6d..abae1fae4 100644 --- a/src/entity/object/ItemEntity.php +++ b/src/entity/object/ItemEntity.php @@ -322,7 +322,7 @@ class ItemEntity extends Entity{ $item = $this->getItem(); $playerInventory = match(true){ - $player->getOffHandInventory()->getItem(0)->canStackWith($item) && $player->getOffHandInventory()->getAddableItemQuantity($item) > 0 => $player->getOffHandInventory(), + $player->getOffHandItem()->canStackWith($item) && $player->getOffHandInventory()->getAddableItemQuantity($item) > 0 => $player->getOffHandInventory(), $player->getInventory()->getAddableItemQuantity($item) > 0 => $player->getInventory(), default => null }; diff --git a/src/entity/projectile/Arrow.php b/src/entity/projectile/Arrow.php index 81cd07663..07bf3c34d 100644 --- a/src/entity/projectile/Arrow.php +++ b/src/entity/projectile/Arrow.php @@ -172,7 +172,7 @@ class Arrow extends Projectile{ $item = VanillaItems::ARROW(); $playerInventory = match(true){ !$player->hasFiniteResources() => null, //arrows are not picked up in creative - $player->getOffHandInventory()->getItem(0)->canStackWith($item) && $player->getOffHandInventory()->canAddItem($item) => $player->getOffHandInventory(), + $player->getOffHandItem()->canStackWith($item) && $player->getOffHandInventory()->canAddItem($item) => $player->getOffHandInventory(), $player->getInventory()->canAddItem($item) => $player->getInventory(), default => null }; diff --git a/src/inventory/Hotbar.php b/src/inventory/Hotbar.php index a98a868d4..f9c1384dc 100644 --- a/src/inventory/Hotbar.php +++ b/src/inventory/Hotbar.php @@ -36,12 +36,8 @@ final class Hotbar{ protected ObjectSet $selectedIndexChangeListeners; public function __construct( - private Inventory $inventory, private int $size = 9 ){ - if($this->inventory->getSize() < $this->size){ - throw new \InvalidArgumentException("Inventory size must be at least $this->size"); - } $this->selectedIndexChangeListeners = new ObjectSet(); } @@ -58,16 +54,6 @@ final class Hotbar{ } } - /** - * Returns the item in the specified hotbar slot. - * - * @throws \InvalidArgumentException if the hotbar slot index is out of range - */ - public function getHotbarSlotItem(int $hotbarSlot) : Item{ - $this->throwIfNotHotbarSlot($hotbarSlot); - return $this->inventory->getItem($hotbarSlot); - } - /** * Returns the hotbar slot number the holder is currently holding. */ @@ -99,20 +85,6 @@ final class Hotbar{ */ public function getSelectedIndexChangeListeners() : ObjectSet{ return $this->selectedIndexChangeListeners; } - /** - * Returns the currently-held item. - */ - public function getHeldItem() : Item{ - return $this->getHotbarSlotItem($this->selectedIndex); - } - - /** - * Sets the item in the currently-held slot to the specified item. - */ - public function setHeldItem(Item $item) : void{ - $this->inventory->setItem($this->getSelectedIndex(), $item); - } - /** * Returns the number of slots in the hotbar. */ diff --git a/src/item/Armor.php b/src/item/Armor.php index b50d151d1..1bf322451 100644 --- a/src/item/Armor.php +++ b/src/item/Armor.php @@ -145,7 +145,7 @@ class Armor extends Durable{ $thisCopy = clone $this; $new = $thisCopy->pop(); $player->getArmorInventory()->setItem($this->getArmorSlot(), $new); - $player->getHotbar()->setHeldItem($existing); + $player->setMainHandItem($existing); $sound = $new->getMaterial()->getEquipSound(); if($sound !== null){ $player->broadcastSound($sound); diff --git a/src/network/mcpe/StandardEntityEventBroadcaster.php b/src/network/mcpe/StandardEntityEventBroadcaster.php index b19a631dc..6e1e461a1 100644 --- a/src/network/mcpe/StandardEntityEventBroadcaster.php +++ b/src/network/mcpe/StandardEntityEventBroadcaster.php @@ -102,22 +102,21 @@ final class StandardEntityEventBroadcaster implements EntityEventBroadcaster{ } public function onMobMainHandItemChange(array $recipients, Human $mob) : void{ - //TODO: we could send zero for slot here because remote players don't need to know which slot was selected - $inv = $mob->getHotbar(); + $item = $mob->getMainHandItem(); $this->sendDataPacket($recipients, MobEquipmentPacket::create( $mob->getId(), - ItemStackWrapper::legacy($this->typeConverter->coreItemStackToNet($inv->getHeldItem())), - $inv->getSelectedIndex(), - $inv->getSelectedIndex(), + ItemStackWrapper::legacy($this->typeConverter->coreItemStackToNet($item)), + 0, + 0, ContainerIds::INVENTORY )); } public function onMobOffHandItemChange(array $recipients, Human $mob) : void{ - $inv = $mob->getOffHandInventory(); + $item = $mob->getOffHandItem(); $this->sendDataPacket($recipients, MobEquipmentPacket::create( $mob->getId(), - ItemStackWrapper::legacy($this->typeConverter->coreItemStackToNet($inv->getItem(0))), + ItemStackWrapper::legacy($this->typeConverter->coreItemStackToNet($item)), 0, 0, ContainerIds::OFFHAND diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 28fd61f70..2153d64e4 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -307,7 +307,7 @@ class InGamePacketHandler extends PacketHandler{ switch($packet->eventId){ case ActorEvent::EATING_ITEM: //TODO: ignore this and handle it server-side - $item = $this->player->getHotbar()->getHeldItem(); + $item = $this->player->getMainHandItem(); if($item->isNull()){ return false; } diff --git a/src/player/Player.php b/src/player/Player.php index 5f67414f5..32b5ab6a5 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -1649,7 +1649,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ private function returnItemsFromAction(Item $oldHeldItem, Item $newHeldItem, array $extraReturnedItems) : void{ $heldItemChanged = false; - if(!$newHeldItem->equalsExact($oldHeldItem) && $oldHeldItem->equalsExact($this->hotbar->getHeldItem())){ + if(!$newHeldItem->equalsExact($oldHeldItem) && $oldHeldItem->equalsExact($this->getMainHandItem())){ //determine if the item was changed in some meaningful way, or just damaged/changed count //if it was really changed we always need to set it, whether we have finite resources or not $newReplica = clone $oldHeldItem; @@ -1666,7 +1666,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ if($newHeldItem instanceof Durable && $newHeldItem->isBroken()){ $this->broadcastSound(new ItemBreakSound()); } - $this->hotbar->setHeldItem($newHeldItem); + $this->setMainHandItem($newHeldItem); $heldItemChanged = true; } } @@ -1676,7 +1676,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ } if($heldItemChanged && count($extraReturnedItems) > 0 && $newHeldItem->isNull()){ - $this->hotbar->setHeldItem(array_shift($extraReturnedItems)); + $this->setMainHandItem(array_shift($extraReturnedItems)); } foreach($this->inventory->addItem(...$extraReturnedItems) as $drop){ //TODO: we can't generate a transaction for this since the items aren't coming from an inventory :( @@ -1698,7 +1698,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ */ public function useHeldItem() : bool{ $directionVector = $this->getDirectionVector(); - $item = $this->hotbar->getHeldItem(); + $item = $this->getMainHandItem(); $oldItem = clone $item; $ev = new PlayerItemUseEvent($this, $item, $directionVector); @@ -1732,7 +1732,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ * @return bool if the consumption succeeded. */ public function consumeHeldItem() : bool{ - $slot = $this->hotbar->getHeldItem(); + $slot = $this->getMainHandItem(); if($slot instanceof ConsumableItem){ $oldItem = clone $slot; @@ -1765,7 +1765,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ */ public function releaseHeldItem() : bool{ try{ - $item = $this->hotbar->getHeldItem(); + $item = $this->getMainHandItem(); if(!$this->isUsingItem() || $this->hasItemCooldown($item)){ return false; } @@ -1843,13 +1843,13 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ }else{ $firstEmpty = $this->inventory->firstEmpty(); if($firstEmpty === -1){ //full inventory - $this->hotbar->setHeldItem($item); + $this->setMainHandItem($item); }elseif($firstEmpty < $this->hotbar->getSize()){ $this->inventory->setItem($firstEmpty, $item); $this->hotbar->setSelectedIndex($firstEmpty); }else{ $this->inventory->swap($this->hotbar->getSelectedIndex(), $firstEmpty); - $this->hotbar->setHeldItem($item); + $this->setMainHandItem($item); } } } @@ -1866,7 +1866,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ $target = $this->getWorld()->getBlock($pos); - $ev = new PlayerInteractEvent($this, $this->hotbar->getHeldItem(), $target, null, $face, PlayerInteractEvent::LEFT_CLICK_BLOCK); + $ev = new PlayerInteractEvent($this, $this->getMainHandItem(), $target, null, $face, PlayerInteractEvent::LEFT_CLICK_BLOCK); if($this->isSpectator()){ $ev->cancel(); } @@ -1875,7 +1875,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ return false; } $this->broadcastAnimation(new ArmSwingAnimation($this), $this->getViewers()); - if($target->onAttack($this->hotbar->getHeldItem(), $face, $this)){ + if($target->onAttack($this->getMainHandItem(), $face, $this)){ return true; } @@ -1916,7 +1916,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ if($this->canInteract($pos->add(0.5, 0.5, 0.5), $this->isCreative() ? self::MAX_REACH_DISTANCE_CREATIVE : self::MAX_REACH_DISTANCE_SURVIVAL)){ $this->broadcastAnimation(new ArmSwingAnimation($this), $this->getViewers()); $this->stopBreakBlock($pos); - $item = $this->hotbar->getHeldItem(); + $item = $this->getMainHandItem(); $oldItem = clone $item; $returnedItems = []; if($this->getWorld()->useBreakOn($pos, $item, $this, true, $returnedItems)){ @@ -1941,7 +1941,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ if($this->canInteract($pos->add(0.5, 0.5, 0.5), $this->isCreative() ? self::MAX_REACH_DISTANCE_CREATIVE : self::MAX_REACH_DISTANCE_SURVIVAL)){ $this->broadcastAnimation(new ArmSwingAnimation($this), $this->getViewers()); - $item = $this->hotbar->getHeldItem(); //this is a copy of the real item + $item = $this->getMainHandItem(); //this is a copy of the real item $oldItem = clone $item; $returnedItems = []; if($this->getWorld()->useItemOn($pos, $item, $face, $clickOffset, $this, true, $returnedItems)){ @@ -1970,7 +1970,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ return false; } - $heldItem = $this->hotbar->getHeldItem(); + $heldItem = $this->getMainHandItem(); $oldItem = clone $heldItem; $ev = new EntityDamageByEntityEvent($this, $entity, EntityDamageEvent::CAUSE_ENTITY_ATTACK, $heldItem->getAttackPoints()); @@ -2056,15 +2056,15 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ $ev->call(); - $item = $this->hotbar->getHeldItem(); + $item = $this->getMainHandItem(); $oldItem = clone $item; if(!$ev->isCancelled()){ if($item->onInteractEntity($this, $entity, $clickPos)){ - if($this->hasFiniteResources() && !$item->equalsExact($oldItem) && $oldItem->equalsExact($this->hotbar->getHeldItem())){ + if($this->hasFiniteResources() && !$item->equalsExact($oldItem) && $oldItem->equalsExact($this->getMainHandItem())){ if($item instanceof Durable && $item->isBroken()){ $this->broadcastSound(new ItemBreakSound()); } - $this->hotbar->setHeldItem($item); + $this->setMainHandItem($item); } } return $entity->onInteract($this, $clickPos); diff --git a/src/player/SurvivalBlockBreakHandler.php b/src/player/SurvivalBlockBreakHandler.php index 5c012bccf..786b7042e 100644 --- a/src/player/SurvivalBlockBreakHandler.php +++ b/src/player/SurvivalBlockBreakHandler.php @@ -67,7 +67,7 @@ final class SurvivalBlockBreakHandler{ if(!$this->block->getBreakInfo()->isBreakable()){ return 0.0; } - $breakTimePerTick = $this->block->getBreakInfo()->getBreakTime($this->player->getHotbar()->getHeldItem()) * 20; + $breakTimePerTick = $this->block->getBreakInfo()->getBreakTime($this->player->getMainHandItem()) * 20; if(!$this->player->isOnGround() && !$this->player->isFlying()){ $breakTimePerTick *= 5; }