Block: Cleaned up double-block break handling, close #1862, close #1525

This now removes the need for recursing around for structures comprised of multiple blocks. Instead, override getAffectedBlocks() to return all blocks that need to be deleted when the current block is deleted, and make sure that only one half of the block drops something. When a player breaks one of the blocks, all the blocks affected by that block will also be destroyed, creating particles and sounds where appropriate.

This fixes creative drops for double plants and beds.

src/pocketmine/block/Bed.php

@@ -190,15 +190,6 @@ class Bed extends Transparent{
 		return false;
 	}
 
-	public function onBreak(Item $item, Player $player = null) : bool{
-		$this->getLevel()->setBlock($this, BlockFactory::get(Block::AIR), true, true);
-		if(($other = $this->getOtherHalf()) !== null){
-			$this->getLevel()->useBreakOn($other, $item, null, $player !== null); //make sure tiles get removed
-		}
-
-		return true;
-	}
-
 	public function getDropsForCompatibleTool(Item $item) : array{
 		if($this->isHeadPart()){
 			$tile = $this->getLevel()->getTile($this);
@@ -216,4 +207,11 @@ class Bed extends Transparent{
 		return [];
 	}
 
+	public function getAffectedBlocks() : array{
+		if(($other = $this->getOtherHalf()) !== null){
+			return [$this, $other];
+		}
+
+		return parent::getAffectedBlocks();
+	}
 }

src/pocketmine/block/Block.php

@@ -513,6 +513,16 @@ class Block extends Position implements BlockIds, Metadatable{
 		);
 	}
 
+	/**
+	 * Returns a list of blocks that this block is part of. In most cases, only contains the block itself, but in cases
+	 * such as double plants, beds and doors, will contain both halves.
+	 *
+	 * @return Block[]
+	 */
+	public function getAffectedBlocks() : array{
+		return [$this];
+	}
+
 	/**
 	 * @return string
 	 */

src/pocketmine/block/Door.php

@@ -246,23 +246,6 @@ abstract class Door extends Transparent{
 		return false;
 	}
 
-	public function onBreak(Item $item, Player $player = null) : bool{
-		if(($this->getDamage() & 0x08) === 0x08){
-			$down = $this->getSide(Vector3::SIDE_DOWN);
-			if($down->getId() === $this->getId()){
-				$this->getLevel()->setBlock($down, BlockFactory::get(Block::AIR), true);
-			}
-		}else{
-			$up = $this->getSide(Vector3::SIDE_UP);
-			if($up->getId() === $this->getId()){
-				$this->getLevel()->setBlock($up, BlockFactory::get(Block::AIR), true);
-			}
-		}
-		$this->getLevel()->setBlock($this, BlockFactory::get(Block::AIR), true);
-
-		return true;
-	}
-
 	public function onActivate(Item $item, Player $player = null) : bool{
 		if(($this->getDamage() & 0x08) === 0x08){ //Top
 			$down = $this->getSide(Vector3::SIDE_DOWN);
@@ -286,4 +269,28 @@ abstract class Door extends Transparent{
 	public function getVariantBitmask() : int{
 		return 0;
 	}
+
+	public function getDropsForCompatibleTool(Item $item) : array{
+		if(($this->meta & 0x08) === 0){ //bottom half only
+			return parent::getDropsForCompatibleTool($item);
+		}
+
+		return [];
+	}
+
+	public function getAffectedBlocks() : array{
+		if(($this->getDamage() & 0x08) === 0x08){
+			$down = $this->getSide(Vector3::SIDE_DOWN);
+			if($down->getId() === $this->getId()){
+				return [$this, $down];
+			}
+		}else{
+			$up = $this->getSide(Vector3::SIDE_UP);
+			if($up->getId() === $this->getId()){
+				return [$this, $up];
+			}
+		}
+
+		return parent::getAffectedBlocks();
+	}
 }

src/pocketmine/block/DoublePlant.php

@@ -97,14 +97,6 @@ class DoublePlant extends Flowable{
 		return false;
 	}
 
-	public function onBreak(Item $item, Player $player = null) : bool{
-		if(parent::onBreak($item, $player) and $this->isValidHalfPlant()){
-			$this->getLevel()->useBreakOn($this->getSide(($this->meta & self::BITFLAG_TOP) !== 0 ? Vector3::SIDE_DOWN : Vector3::SIDE_UP), $item, $player, $player !== null);
-		}
-
-		return false;
-	}
-
 	public function getVariantBitmask() : int{
 		return 0x07;
 	}
@@ -132,4 +124,12 @@ class DoublePlant extends Flowable{
 
 		return [];
 	}
+
+	public function getAffectedBlocks() : array{
+		if($this->isValidHalfPlant()){
+			return [$this, $this->getSide(($this->meta & self::BITFLAG_TOP) !== 0 ? Vector3::SIDE_DOWN : Vector3::SIDE_UP)];
+		}
+
+		return parent::getAffectedBlocks();
+	}
 }

src/pocketmine/event/block/BlockBreakEvent.php

@@ -45,16 +45,20 @@ class BlockBreakEvent extends BlockEvent implements Cancellable{
 	/** @var Item[] */
 	protected $blockDrops = [];
 
-	public function __construct(Player $player, Block $block, Item $item, bool $instaBreak = false){
+	/**
+	 * @param Player $player
+	 * @param Block  $block
+	 * @param Item   $item
+	 * @param bool   $instaBreak
+	 * @param Item[] $drops
+	 */
+	public function __construct(Player $player, Block $block, Item $item, bool $instaBreak = false, array $drops){
 		parent::__construct($block);
 		$this->item = $item;
 		$this->player = $player;
 
 		$this->instaBreak = $instaBreak;
-
-		if($player->isSurvival()){
-			$this->setDrops($block->getDrops($item));
-		}
+		$this->setDrops($drops);
 	}
 
 	/**

src/pocketmine/level/Level.php

@@ -1735,13 +1735,16 @@ class Level implements ChunkManager, Metadatable{
 	 */
 	public function useBreakOn(Vector3 $vector, Item &$item = null, Player $player = null, bool $createParticles = false) : bool{
 		$target = $this->getBlock($vector);
+		$affectedBlocks = $target->getAffectedBlocks();
 
 		if($item === null){
 			$item = ItemFactory::get(Item::AIR, 0, 0);
 		}
 
+		$drops = ($player !== null and $player->isCreative()) ? [] : array_merge(...array_map(function(Block $block) use ($item) : array{ return $block->getDrops($item); }, $affectedBlocks));
+
 		if($player !== null){
-			$ev = new BlockBreakEvent($player, $target, $item, $player->isCreative() or $player->allowInstaBreak());
+			$ev = new BlockBreakEvent($player, $target, $item, $player->isCreative() or $player->allowInstaBreak(), $drops);
 
 			if(($player->isSurvival() and !$target->isBreakable($item)) or $player->isSpectator()){
 				$ev->setCancelled();
@@ -1798,10 +1801,27 @@ class Level implements ChunkManager, Metadatable{
 
 		}elseif(!$target->isBreakable($item)){
 			return false;
-		}else{
-			$drops = $target->getDrops($item); //Fixes tile entities being deleted before getting drops
 		}
 
+		foreach($affectedBlocks as $t){
+			$this->destroyBlockInternal($t, $item, $player, $createParticles);
+		}
+
+		$item->useOn($target);
+
+		if(!empty($drops)){
+			$dropPos = $target->add(0.5, 0.5, 0.5);
+			foreach($drops as $drop){
+				if(!$drop->isNull()){
+					$this->dropItem($dropPos, $drop);
+				}
+			}
+		}
+
+		return true;
+	}
+
+	private function destroyBlockInternal(Block $target, Item $item, ?Player $player = null, bool $createParticles = false) : void{
 		$above = $this->getBlockAt($target->x, $target->y + 1, $target->z);
 		if($above->getId() === Block::FIRE){ //TODO: this should be done in Fire's onUpdate(), not with this hack
 			$this->setBlock($above, BlockFactory::get(Block::AIR), true);
@@ -1825,18 +1845,6 @@ class Level implements ChunkManager, Metadatable{
 
 			$tile->close();
 		}
-
-		$item->useOn($target);
-
-		if($player === null or $player->isSurvival()){
-			foreach($drops as $drop){
-				if(!$drop->isNull()){
-					$this->dropItem($vector->add(0.5, 0.5, 0.5), $drop);
-				}
-			}
-		}
-
-		return true;
 	}
 
 	/**