Player: Clean up item frame drop-item hack

This is now re-routed through a newly-created attack-block handler.
Closes #339

src/pocketmine/Player.php

@@ -26,7 +26,6 @@ namespace pocketmine;
 use pocketmine\block\Bed;
 use pocketmine\block\Block;
 use pocketmine\block\BlockFactory;
-use pocketmine\block\ItemFrame;
 use pocketmine\block\UnknownBlock;
 use pocketmine\command\Command;
 use pocketmine\command\CommandSender;
@@ -112,7 +111,6 @@ use pocketmine\network\mcpe\protocol\BookEditPacket;
 use pocketmine\network\mcpe\protocol\ChunkRadiusUpdatedPacket;
 use pocketmine\network\mcpe\protocol\ClientboundPacket;
 use pocketmine\network\mcpe\protocol\EntityEventPacket;
-use pocketmine\network\mcpe\protocol\ItemFrameDropItemPacket;
 use pocketmine\network\mcpe\protocol\LevelEventPacket;
 use pocketmine\network\mcpe\protocol\LevelSoundEventPacket;
 use pocketmine\network\mcpe\protocol\LoginPacket;
@@ -154,7 +152,6 @@ use function in_array;
 use function is_int;
 use function json_encode;
 use function json_last_error_msg;
-use function lcg_value;
 use function max;
 use function microtime;
 use function min;
@@ -2174,7 +2171,15 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		return true;
 	}
 
-	public function startBreakBlock(Vector3 $pos, int $face) : bool{
+	/**
+	 * Performs a left-click (attack) action on the block.
+	 *
+	 * @param Vector3 $pos
+	 * @param int     $face
+	 *
+	 * @return bool
+	 */
+	public function attackBlock(Vector3 $pos, int $face) : bool{
 		if($pos->distanceSquared($this) > 10000){
 			return false; //TODO: maybe this should throw an exception instead?
 		}
@@ -2184,9 +2189,13 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		$ev = new PlayerInteractEvent($this, $this->inventory->getItemInHand(), $target, null, $face, PlayerInteractEvent::LEFT_CLICK_BLOCK);
 		$ev->call();
 		if($ev->isCancelled()){
+			$this->level->sendBlocks([$this], [$target]);
 			$this->inventory->sendHeldItem($this);
 			return true;
 		}
+		if($target->onAttack($this->inventory->getItemInHand(), $face, $this)){
+			return true;
+		}
 
 		$block = $target->getSide($face);
 		if($block->getId() === Block::FIRE){
@@ -2490,30 +2499,6 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		return true;
 	}
 
-	public function handleItemFrameDropItem(ItemFrameDropItemPacket $packet) : bool{
-		$block = $this->level->getBlockAt($packet->x, $packet->y, $packet->z);
-		if($block instanceof ItemFrame){
-			$ev = new PlayerInteractEvent($this, $this->inventory->getItemInHand(), $block, null, $block->getFacing(), PlayerInteractEvent::LEFT_CLICK_BLOCK);
-			if($this->isSpectator()){
-				$ev->setCancelled();
-			}
-
-			$ev->call();
-			if($ev->isCancelled()){
-				$this->level->sendBlocks([$this], [$block]);
-				return true;
-			}
-
-			if(lcg_value() <= $block->getItemDropChance()){
-				$this->level->dropItem($block->add(0.5, 0.5, 0.5), $block->getFramedItem());
-			}
-			$block->setFramedItem(null);
-			$this->level->setBlock($block, $block);
-		}
-
-		return true;
-	}
-
 	public function handleBookEdit(BookEditPacket $packet) : bool{
 		/** @var WritableBook $oldBook */
 		$oldBook = $this->inventory->getItem($packet->inventorySlot);

src/pocketmine/block/Block.php

@@ -380,6 +380,20 @@ class Block extends Position implements BlockIds, Metadatable{
 		return false;
 	}
 
+	/**
+	 * Called when this block is attacked (left-clicked). This is called when a player left-clicks the block to try and
+	 * start to break it in survival mode.
+	 *
+	 * @param Item        $item
+	 * @param int         $face
+	 * @param Player|null $player
+	 *
+	 * @return bool if an action took place, prevents starting to break the block if true.
+	 */
+	public function onAttack(Item $item, int $face, ?Player $player = null) : bool{
+		return false;
+	}
+
 	/**
 	 * Returns a base value used to compute block break times.
 	 * @return float

src/pocketmine/block/ItemFrame.php

@@ -156,6 +156,18 @@ class ItemFrame extends Flowable{
 		return true;
 	}
 
+	public function onAttack(Item $item, int $face, ?Player $player = null) : bool{
+		if($this->framedItem === null){
+			return false;
+		}
+		if(lcg_value() <= $this->itemDropChance){
+			$this->level->dropItem($this->add(0.5, 0.5, 0.5), $this->getFramedItem());
+		}
+		$this->setFramedItem(null);
+		$this->level->setBlock($this, $this);
+		return true;
+	}
+
 	public function onNearbyBlockChange() : void{
 		if(!$this->getSide(Facing::opposite($this->facing))->isSolid()){
 			$this->level->useBreakOn($this);

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

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe\handler;
 
+use pocketmine\block\ItemFrame;
 use pocketmine\inventory\transaction\action\InventoryAction;
 use pocketmine\inventory\transaction\CraftingTransaction;
 use pocketmine\inventory\transaction\InventoryTransaction;
@@ -309,7 +310,7 @@ class SimpleSessionHandler extends SessionHandler{
 
 		switch($packet->action){
 			case PlayerActionPacket::ACTION_START_BREAK:
-				$this->player->startBreakBlock($pos, $packet->face);
+				$this->player->attackBlock($pos, $packet->face);
 
 				break;
 
@@ -415,7 +416,11 @@ class SimpleSessionHandler extends SessionHandler{
 	}
 
 	public function handleItemFrameDropItem(ItemFrameDropItemPacket $packet) : bool{
-		return $this->player->handleItemFrameDropItem($packet);
+		$block = $this->player->getLevel()->getBlockAt($packet->x, $packet->y, $packet->z);
+		if($block instanceof ItemFrame and $block->getFramedItem() !== null){
+			return $this->player->attackBlock(new Vector3($packet->x, $packet->y, $packet->z), $block->getFacing());
+		}
+		return false;
 	}
 
 	public function handleBossEvent(BossEventPacket $packet) : bool{