Unwrap more code from packet handlers

src/pocketmine/Player.php

@@ -55,6 +55,7 @@ use pocketmine\event\player\PlayerExhaustEvent;
 use pocketmine\event\player\PlayerGameModeChangeEvent;
 use pocketmine\event\player\PlayerInteractEvent;
 use pocketmine\event\player\PlayerItemConsumeEvent;
+use pocketmine\event\player\PlayerItemHeldEvent;
 use pocketmine\event\player\PlayerJoinEvent;
 use pocketmine\event\player\PlayerJumpEvent;
 use pocketmine\event\player\PlayerKickEvent;
@@ -100,7 +101,6 @@ use pocketmine\network\mcpe\protocol\AdventureSettingsPacket;
 use pocketmine\network\mcpe\protocol\AnimatePacket;
 use pocketmine\network\mcpe\protocol\AvailableCommandsPacket;
 use pocketmine\network\mcpe\protocol\BlockEntityDataPacket;
-use pocketmine\network\mcpe\protocol\BlockPickRequestPacket;
 use pocketmine\network\mcpe\protocol\BookEditPacket;
 use pocketmine\network\mcpe\protocol\ChunkRadiusUpdatedPacket;
 use pocketmine\network\mcpe\protocol\DataPacket;
@@ -111,7 +111,6 @@ use pocketmine\network\mcpe\protocol\LevelEventPacket;
 use pocketmine\network\mcpe\protocol\LevelSoundEventPacket;
 use pocketmine\network\mcpe\protocol\LoginPacket;
 use pocketmine\network\mcpe\protocol\MobEffectPacket;
-use pocketmine\network\mcpe\protocol\MobEquipmentPacket;
 use pocketmine\network\mcpe\protocol\MovePlayerPacket;
 use pocketmine\network\mcpe\protocol\SetPlayerGameTypePacket;
 use pocketmine\network\mcpe\protocol\SetSpawnPositionPacket;
@@ -2413,27 +2412,29 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 		return false; //TODO
 	}
 
-	public function handleMobEquipment(MobEquipmentPacket $packet) : bool{
+	public function equipItem(int $hotbarSlot) : bool{
-		$item = $this->inventory->getItem($packet->hotbarSlot);
+		if(!$this->inventory->isHotbarSlot($hotbarSlot)){
-
-		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);
+		$this->server->getPluginManager()->callEvent($ev = new PlayerItemHeldEvent($this, $this->inventory->getItem($hotbarSlot), $hotbarSlot));
+		if($ev->isCancelled()){
+			$this->inventory->sendHeldItem($this);
+			return false;
+		}
 
+		$this->inventory->setHeldItemIndex($hotbarSlot, false);
 		$this->setUsingItem(false);
 
 		return true;
 	}
 
-	public function handleBlockPickRequest(BlockPickRequestPacket $packet) : bool{
+	public function pickBlock(Vector3 $pos, bool $addTileNBT) : bool{
-		$block = $this->level->getBlockAt($packet->blockX, $packet->blockY, $packet->blockZ);
+		$block = $this->level->getBlock($pos);
 
 		$item = $block->getPickedItem();
-		if($packet->addUserData){
+		if($addTileNBT){
 			$tile = $this->getLevel()->getTile($block);
 			if($tile instanceof Tile){
 				$nbt = $tile->getCleanedNBT();
@@ -2456,7 +2457,6 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 		}
 
 		return true;
-
 	}
 
 	public function startBreakBlock(Vector3 $pos, int $face) : bool{
@@ -2529,8 +2529,8 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 		}
 	}
 
-	public function handleAnimate(AnimatePacket $packet) : bool{
+	public function animate(int $action) : bool{
-		$this->server->getPluginManager()->callEvent($ev = new PlayerAnimationEvent($this, $packet->action));
+		$this->server->getPluginManager()->callEvent($ev = new PlayerAnimationEvent($this, $action));
 		if($ev->isCancelled()){
 			return true;
 		}
@@ -2618,15 +2618,6 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 		return true;
 	}
 
-	public function handleSetPlayerGameType(SetPlayerGameTypePacket $packet) : bool{
-		if($packet->gamemode !== $this->gamemode){
-			//Set this back to default. TODO: handle this properly
-			$this->sendGamemode();
-			$this->sendSettings();
-		}
-		return true;
-	}
-
 	public function handleItemFrameDropItem(ItemFrameDropItemPacket $packet) : bool{
 		$tile = $this->level->getTileAt($packet->x, $packet->y, $packet->z);
 		if($tile instanceof ItemFrame){

src/pocketmine/inventory/PlayerInventory.php

@@ -24,7 +24,6 @@ declare(strict_types=1);
 namespace pocketmine\inventory;
 
 use pocketmine\entity\Human;
-use pocketmine\event\player\PlayerItemHeldEvent;
 use pocketmine\item\Item;
 use pocketmine\network\mcpe\protocol\InventoryContentPacket;
 use pocketmine\network\mcpe\protocol\MobEquipmentPacket;
@@ -55,33 +54,7 @@ class PlayerInventory extends BaseInventory{
 		return 36;
 	}
 
-	/**
+	public function isHotbarSlot(int $slot) : bool{
-	 * Called when a client equips a hotbar slot. This method should not be used by plugins.
-	 * This method will call PlayerItemHeldEvent.
-	 *
-	 * @param int $hotbarSlot Number of the hotbar slot to equip.
-	 *
-	 * @return bool if the equipment change was successful, false if not.
-	 */
-	public function equipItem(int $hotbarSlot) : bool{
-		if(!$this->isHotbarSlot($hotbarSlot)){
-			$this->sendContents($this->getHolder());
-			return false;
-		}
-
-		$this->getHolder()->getLevel()->getServer()->getPluginManager()->callEvent($ev = new PlayerItemHeldEvent($this->getHolder(), $this->getItem($hotbarSlot), $hotbarSlot));
-
-		if($ev->isCancelled()){
-			$this->sendHeldItem($this->getHolder());
-			return false;
-		}
-
-		$this->setHeldItemIndex($hotbarSlot, false);
-
-		return true;
-	}
-
-	private function isHotbarSlot(int $slot) : bool{
 		return $slot >= 0 and $slot <= $this->getHotbarSize();
 	}
 

src/pocketmine/network/mcpe/handler/SimpleSessionHandler.php

@@ -104,7 +104,7 @@ class SimpleSessionHandler extends SessionHandler{
 	}
 
 	public function handleMobEquipment(MobEquipmentPacket $packet) : bool{
-		return $this->player->handleMobEquipment($packet);
+		return $this->player->equipItem($packet->hotbarSlot);
 	}
 
 	public function handleMobArmorEquipment(MobArmorEquipmentPacket $packet) : bool{
@@ -116,7 +116,7 @@ class SimpleSessionHandler extends SessionHandler{
 	}
 
 	public function handleBlockPickRequest(BlockPickRequestPacket $packet) : bool{
-		return $this->player->handleBlockPickRequest($packet);
+		return $this->player->pickBlock(new Vector3($packet->blockX, $packet->blockY, $packet->blockZ), $packet->addUserData);
 	}
 
 	public function handleEntityPickRequest(EntityPickRequestPacket $packet) : bool{
@@ -183,7 +183,7 @@ class SimpleSessionHandler extends SessionHandler{
 	}
 
 	public function handleAnimate(AnimatePacket $packet) : bool{
-		return $this->player->handleAnimate($packet);
+		return $this->player->animate($packet->action);
 	}
 
 	public function handleContainerClose(ContainerClosePacket $packet) : bool{
@@ -211,7 +211,12 @@ class SimpleSessionHandler extends SessionHandler{
 	}
 
 	public function handleSetPlayerGameType(SetPlayerGameTypePacket $packet) : bool{
-		return $this->player->handleSetPlayerGameType($packet);
+		if($packet->gamemode !== $this->player->getGamemode()){
+			//Set this back to default. TODO: handle this properly
+			$this->player->sendGamemode();
+			$this->player->sendSettings();
+		}
+		return true;
 	}
 
 	public function handleSpawnExperienceOrb(SpawnExperienceOrbPacket $packet) : bool{