Removed hotbar slot linking (works like PC now)

src/pocketmine/Player.php

@@ -2158,7 +2158,6 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 			$inventory->sendContents($this);
 			if($inventory instanceof PlayerInventory){
 				$inventory->sendArmorContents($this);
-				$inventory->sendHotbar();
 			}
 		}
 	}
@@ -2480,26 +2479,15 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 			return true;
 		}
 
-		if($packet->inventorySlot === 255){
+		$item = $this->inventory->getItem($packet->hotbarSlot);
-			$packet->inventorySlot = -1; //Cleared slot
-		}else{
-			if($packet->inventorySlot < 9){
-				$this->server->getLogger()->debug("Tried to equip a slot that does not exist (index " . $packet->inventorySlot . ")");
-				$this->inventory->sendContents($this);
-				return false;
-			}
-			$packet->inventorySlot -= 9; //Get real inventory slot
-
-			$item = $this->inventory->getItem($packet->inventorySlot);
 
 		if(!$item->equals($packet->item)){
 			$this->server->getLogger()->debug("Tried to equip " . $packet->item . " but have " . $item . " in target slot");
 			$this->inventory->sendContents($this);
 			return false;
 		}
-		}
 
-		$this->inventory->equipItem($packet->hotbarSlot, $packet->inventorySlot);
+		$this->inventory->equipItem($packet->hotbarSlot);
 
 		$this->setGenericFlag(self::DATA_FLAG_ACTION, false);
 
@@ -2775,10 +2763,6 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 			return false; //In PE this should never happen
 		}
 
-		foreach($packet->slots as $hotbarSlot => $slotLink){
-			$this->inventory->setHotbarSlotIndex($hotbarSlot, $slotLink === -1 ? $slotLink : $slotLink - 9);
-		}
-
 		$this->inventory->equipItem($packet->selectedHotbarSlot);
 
 		return true;
@@ -3608,9 +3592,8 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 			}
 
 			if($this->inventory !== null){
+				$this->inventory->setHeldItemIndex(0, false); //This is already handled when sending contents, don't send it twice
 				$this->inventory->clearAll();
-				$this->inventory->setHeldItemIndex(0);
-				$this->inventory->resetHotbar(true);
 			}
 		}
 

src/pocketmine/entity/Human.php

@@ -304,9 +304,10 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{
 		$this->initHumanData();
 
 		if(isset($this->namedtag->Inventory) and $this->namedtag->Inventory instanceof ListTag){
-			foreach($this->namedtag->Inventory as $item){
+			foreach($this->namedtag->Inventory as $i => $item){
 				if($item["Slot"] >= 0 and $item["Slot"] < 9){ //Hotbar
-					$this->inventory->setHotbarSlotIndex($item["Slot"], isset($item["TrueSlot"]) ? $item["TrueSlot"] : -1);
+					//Old hotbar saving stuff, remove it (useless now)
+					unset($this->namedtag->Inventory->{$i});
 				}elseif($item["Slot"] >= 100 and $item["Slot"] < 104){ //Armor
 					$this->inventory->setItem($this->inventory->getSize() + $item["Slot"] - 100, ItemItem::nbtDeserialize($item));
 				}else{
@@ -448,28 +449,6 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{
 		$this->namedtag->Inventory = new ListTag("Inventory", []);
 		$this->namedtag->Inventory->setTagType(NBT::TAG_Compound);
 		if($this->inventory !== null){
-			for($slot = 0; $slot < 9; ++$slot){
-				$hotbarSlot = $this->inventory->getHotbarSlotIndex($slot);
-				if($hotbarSlot !== -1){
-					$item = $this->inventory->getItem($hotbarSlot);
-					if($item->getId() !== 0 and $item->getCount() > 0){
-						$tag = $item->nbtSerialize($slot);
-						$tag->TrueSlot = new ByteTag("TrueSlot", $hotbarSlot);
-						$this->namedtag->Inventory[$slot] = $tag;
-
-						continue;
-					}
-				}
-
-				$this->namedtag->Inventory[$slot] = new CompoundTag("", [
-					new ByteTag("Count", 0),
-					new ShortTag("Damage", 0),
-					new ByteTag("Slot", $slot),
-					new ByteTag("TrueSlot", -1),
-					new ShortTag("id", 0)
-				]);
-			}
-
 			//Normal inventory
 			$slotCount = $this->inventory->getSize() + $this->inventory->getHotbarSize();
 			for($slot = $this->inventory->getHotbarSize(); $slot < $slotCount; ++$slot){

src/pocketmine/event/player/PlayerItemHeldEvent.php

@@ -34,18 +34,21 @@ class PlayerItemHeldEvent extends PlayerEvent implements Cancellable{
 	private $item;
 	/** @var int */
 	private $hotbarSlot;
-	/** @var int */
-	private $inventorySlot;
 
-	public function __construct(Player $player, Item $item, int $inventorySlot, int $hotbarSlot){
+	public function __construct(Player $player, Item $item, int $hotbarSlot){
 		$this->player = $player;
 		$this->item = $item;
-		$this->inventorySlot = $inventorySlot;
 		$this->hotbarSlot = $hotbarSlot;
 	}
 
 	/**
 	 * Returns the hotbar slot the player is attempting to hold.
+	 *
+	 * NOTE: This event is called BEFORE the slot is equipped server-side. Setting the player's held item during this
+	 * event will result in the **old** slot being changed, not this one.
+	 *
+	 * To change the item in the slot that the player is attempting to hold, set the slot that this function reports.
+	 *
 	 * @return int
 	 */
 	public function getSlot() : int{
@@ -53,14 +56,25 @@ class PlayerItemHeldEvent extends PlayerEvent implements Cancellable{
 	}
 
 	/**
+	 * @deprecated This is currently an alias of {@link getSlot}
+	 *
+	 * Some background for confused future readers: Before MCPE 1.2, hotbar slots and inventory slots were not the same
+	 * thing - a hotbar slot was a link to a certain slot in the inventory.
+	 * As of 1.2, hotbar slots are now always linked to their respective slots in the inventory, meaning that the two
+	 * are now synonymous, rendering the separate methods obsolete.
+	 *
 	 * @return int
 	 */
 	public function getInventorySlot() : int{
-		return $this->inventorySlot;
+		return $this->getSlot();
 	}
 
+	/**
+	 * Returns the item in the slot that the player is trying to equip.
+	 *
+	 * @return Item
+	 */
 	public function getItem() : Item{
 		return $this->item;
 	}
-
 }

src/pocketmine/inventory/PlayerInventory.php

@@ -44,12 +44,13 @@ class PlayerInventory extends BaseInventory{
 	/** @var Human */
 	protected $holder;
 
+	/** @var int */
 	protected $itemInHandIndex = 0;
-	/** @var \SplFixedArray<int> */
-	protected $hotbar;
 
+	/**
+	 * @param Human $player
+	 */
 	public function __construct(Human $player){
-		$this->resetHotbar(false);
 		parent::__construct($player);
 	}
 
@@ -79,117 +80,74 @@ class PlayerInventory extends BaseInventory{
 	 * This method will call PlayerItemHeldEvent.
 	 *
 	 * @param int $hotbarSlot Number of the hotbar slot to equip.
-	 * @param int|null $inventorySlot Inventory slot to map to the specified hotbar slot. Supply null to make no change to the link.
 	 *
 	 * @return bool if the equipment change was successful, false if not.
 	 */
-	public function equipItem(int $hotbarSlot, int $inventorySlot = null) : bool{
+	public function equipItem(int $hotbarSlot) : bool{
-		if($inventorySlot === null){
+		if(!$this->isHotbarSlot($hotbarSlot)){
-			$inventorySlot = $this->getHotbarSlotIndex($hotbarSlot);
-		}
-
-		if($hotbarSlot < 0 or $hotbarSlot >= $this->getHotbarSize() or $inventorySlot < -1 or $inventorySlot >= $this->getSize()){
 			$this->sendContents($this->getHolder());
 			return false;
 		}
 
-		if($inventorySlot === -1){
+		$this->getHolder()->getLevel()->getServer()->getPluginManager()->callEvent($ev = new PlayerItemHeldEvent($this->getHolder(), $this->getItem($hotbarSlot), $hotbarSlot));
-			$item = ItemFactory::get(Item::AIR, 0, 0);
-		}else{
-			$item = $this->getItem($inventorySlot);
-		}
-
-		$this->getHolder()->getLevel()->getServer()->getPluginManager()->callEvent($ev = new PlayerItemHeldEvent($this->getHolder(), $item, $inventorySlot, $hotbarSlot));
 
 		if($ev->isCancelled()){
-			$this->sendContents($this->getHolder());
+			$this->sendHeldItem($this->getHolder());
 			return false;
 		}
 
-		$this->setHotbarSlotIndex($hotbarSlot, $inventorySlot);
 		$this->setHeldItemIndex($hotbarSlot, false);
 
 		return true;
 	}
 
-	/**
+	private function isHotbarSlot(int $slot) : bool{
-	 * Returns the index of the inventory slot mapped to the specified hotbar slot, or -1 if the hotbar slot does not exist.
+		return $slot >= 0 and $slot <= $this->getHotbarSize();
-	 *
-	 * @param int $index
-	 *
-	 * @return int
-	 */
-	public function getHotbarSlotIndex(int $index) : int{
-		return $this->hotbar[$index] ?? -1;
 	}
 
 	/**
-	 * Links a hotbar slot to the specified slot in the main inventory. -1 links to no slot and will clear the hotbar slot.
+	 * @param int $slot
-	 * This method is intended for use in network interaction with clients only.
+	 * @throws \InvalidArgumentException
-	 *
+	 */
-	 * NOTE: Do not change hotbar slot mapping with plugins, this will cause myriad client-sided bugs, especially with desktop GUI clients.
+	private function throwIfNotHotbarSlot(int $slot){
+		if(!$this->isHotbarSlot($slot)){
+			throw new \InvalidArgumentException("$slot is not a valid hotbar slot index (expected 0 - " . ($this->getHotbarSize() - 1) . ")");
+		}
+	}
+
+	/**
+	 * Returns the item in the specified hotbar slot.
 	 *
 	 * @param int $hotbarSlot
-	 * @param int $inventorySlot
-	 *
-	 * @throws \RuntimeException if the hotbar slot is out of range
-	 * @throws \InvalidArgumentException if the inventory slot is out of range
-	 */
-	public function setHotbarSlotIndex(int $hotbarSlot, int $inventorySlot){
-		if($inventorySlot < -1 or $inventorySlot >= $this->getSize()){
-			throw new \InvalidArgumentException("Inventory slot index \"$inventorySlot\" is out of range");
-		}
-
-		if($inventorySlot !== -1 and ($alreadyEquippedIndex = array_search($inventorySlot, $this->getHotbar(), true)) !== false){
-			/* Swap the slots
-			 * This assumes that the equipped slot can only be equipped in one other slot
-			 * it will not account for ancient bugs where the same slot ended up linked to several hotbar slots.
-			 * Such bugs will require a hotbar reset to default.
-			 */
-			$this->hotbar[$alreadyEquippedIndex] = $this->hotbar[$hotbarSlot];
-		}
-
-		$this->hotbar[$hotbarSlot] = $inventorySlot;
-	}
-
-	/**
-	 * Returns the item in the slot linked to the specified hotbar slot, or Air if the slot is not linked to any hotbar slot.
-	 * @param int $hotbarSlotIndex
-	 *
 	 * @return Item
+	 *
+	 * @throws \InvalidArgumentException if the hotbar slot index is out of range
 	 */
-	public function getHotbarSlotItem(int $hotbarSlotIndex) : Item{
+	public function getHotbarSlotItem(int $hotbarSlot) : Item{
-		$inventorySlot = $this->getHotbarSlotIndex($hotbarSlotIndex);
+		$this->throwIfNotHotbarSlot($hotbarSlot);
-		if($inventorySlot !== -1){
+		return $this->getItem($hotbarSlot);
-			return $this->getItem($inventorySlot);
-		}else{
-			return ItemFactory::get(Item::AIR, 0, 0);
-		}
-	}
-
-	public function getHotbar() : array{
-		return $this->hotbar->toArray();
 	}
 
 	/**
-	 * Resets hotbar links to their original defaults.
+	 * This is only used when sending inventory contents. Since we now assume that all hotbar slots are the same as
-	 * @param bool $send Whether to send changes to the holder.
+	 * their respective inventory slots, we simply fill this wil 0-8.
 	 */
-	public function resetHotbar(bool $send = true){
+	private function sendHotbar(){
-		$this->hotbar = \SplFixedArray::fromArray(range(0, $this->getHotbarSize() - 1, 1));
-		if($send){
-			$this->sendHotbar();
-		}
-	}
-
-	public function sendHotbar(){
 		$pk = new PlayerHotbarPacket();
 		$pk->windowId = ContainerIds::INVENTORY;
 		$pk->selectedHotbarSlot = $this->getHeldItemIndex();
-		$pk->slots = array_map(function(int $link){ return $link + $this->getHotbarSize(); }, $this->getHotbar());
+		$pk->slots = range(0, $this->getHotbarSize() - 1, 1);
 		$this->getHolder()->dataPacket($pk);
 	}
 
+	/**
+	 * @deprecated
+	 * @return int
+	 */
+	public function getHeldItemSlot() : int{
+		return $this->getHeldItemIndex();
+	}
+
 	/**
 	 * Returns the hotbar slot number the holder is currently holding.
 	 * @return int
@@ -208,7 +166,8 @@ class PlayerInventory extends BaseInventory{
 	 * @throws \InvalidArgumentException if the hotbar slot is out of range
 	 */
 	public function setHeldItemIndex(int $hotbarSlot, bool $send = true){
-		if($hotbarSlot >= 0 and $hotbarSlot < $this->getHotbarSize()){
+		$this->throwIfNotHotbarSlot($hotbarSlot);
+
 		$this->itemInHandIndex = $hotbarSlot;
 
 		if($this->getHolder() instanceof Player and $send){
@@ -216,9 +175,6 @@ class PlayerInventory extends BaseInventory{
 		}
 
 		$this->sendHeldItem($this->getHolder()->getViewers());
-		}else{
-			throw new \InvalidArgumentException("Hotbar slot index \"$hotbarSlot\" is out of range");
-		}
 	}
 
 	/**
@@ -238,27 +194,7 @@ class PlayerInventory extends BaseInventory{
 	 * @return bool
 	 */
 	public function setItemInHand(Item $item) : bool{
-		return $this->setItem($this->getHeldItemSlot(), $item);
+		return $this->setItem($this->getHeldItemIndex(), $item);
-	}
-
-	/**
-	 * Returns the hotbar slot number currently held.
-	 * @return int
-	 */
-	public function getHeldItemSlot() : int{
-		return $this->getHotbarSlotIndex($this->itemInHandIndex);
-	}
-
-	/**
-	 * Sets the hotbar slot link of the currently-held hotbar slot.
-	 * @deprecated Do not change hotbar slot mapping with plugins, this will cause myriad client-sided bugs, especially with desktop GUI clients.
-	 *
-	 * @param int $slot
-	 */
-	public function setHeldItemSlot(int $slot){
-		if($slot >= -1 and $slot < $this->getSize()){
-			$this->setHotbarSlotIndex($this->getHeldItemIndex(), $slot);
-		}
 	}
 
 	/**
@@ -271,19 +207,18 @@ class PlayerInventory extends BaseInventory{
 		$pk = new MobEquipmentPacket();
 		$pk->entityRuntimeId = $this->getHolder()->getId();
 		$pk->item = $item;
-		$pk->inventorySlot = $this->getHeldItemSlot();
+		$pk->inventorySlot = $pk->hotbarSlot = $this->getHeldItemIndex();
-		$pk->hotbarSlot = $this->getHeldItemIndex();
 		$pk->windowId = ContainerIds::INVENTORY;
 
 		if(!is_array($target)){
 			$target->dataPacket($pk);
-			if($this->getHeldItemSlot() !== -1 and $target === $this->getHolder()){
+			if($target === $this->getHolder()){
-				$this->sendSlot($this->getHeldItemSlot(), $target);
+				$this->sendSlot($this->getHeldItemIndex(), $target);
 			}
 		}else{
 			$this->getHolder()->getLevel()->getServer()->broadcastPacket($target, $pk);
-			if($this->getHeldItemSlot() !== -1 and in_array($this->getHolder(), $target, true)){
+			if(in_array($this->getHolder(), $target, true)){
-				$this->sendSlot($this->getHeldItemSlot(), $this->getHolder());
+				$this->sendSlot($this->getHeldItemIndex(), $this->getHolder());
 			}
 		}
 	}