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

@@ -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;

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);
 				}

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);
+				$this->setMainHandItem($hand);
-			}elseif(($offHand = $this->offHandInventory->getItem(0)) instanceof Totem){
+			}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([], []),

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
 		};

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
 		};

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.
 	 */

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);

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
+		$item = $mob->getMainHandItem();
-		$inv = $mob->getHotbar();
 		$this->sendDataPacket($recipients, MobEquipmentPacket::create(
 			$mob->getId(),
-			ItemStackWrapper::legacy($this->typeConverter->coreItemStackToNet($inv->getHeldItem())),
+			ItemStackWrapper::legacy($this->typeConverter->coreItemStackToNet($item)),
-			$inv->getSelectedIndex(),
+			0,
-			$inv->getSelectedIndex(),
+			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

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;
 				}

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);

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;
 		}