Improved placement logic

src/item/HangingSign.php

@@ -24,13 +24,10 @@ declare(strict_types=1);
 namespace pocketmine\item;
 
 use pocketmine\block\Block;
-use pocketmine\block\CeilingCenterHangingSign;
-use pocketmine\block\CeilingEdgesHangingSign;
-use pocketmine\block\utils\SupportType;
-use pocketmine\block\WallHangingSign;
 use pocketmine\math\Facing;
 use pocketmine\math\Vector3;
 use pocketmine\player\Player;
+use pocketmine\world\BlockTransaction;
 
 final class HangingSign extends Item{
 
@@ -44,27 +41,15 @@ final class HangingSign extends Item{
 		parent::__construct($identifier, $name);
 	}
 
-	public function getPlacementBlock(?Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : Block{
+	public function getPlacementTransaction(Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : ?BlockTransaction{
-		//we don't verify valid placement conditions here, only decide which block to return
+		if($face !== Facing::DOWN){
-		if($face === Facing::DOWN){
+			return $this->tryPlacementTransaction(clone $this->wallVariant, $blockReplace, $blockClicked, $face, $clickVector, $player);
-			if($player !== null && $player->isSneaking()){
-				return clone $this->centerPointCeilingVariant;
-			}
-
-			//we select the center variant when support is edge/wall sign with perpendicular player facing,
-			//support is a center sign itself, or support provides center support.
-			//otherwise use the edge variant.
-			$support = $blockReplace->getSide(Facing::UP);
-			$result =
-				(($support instanceof CeilingEdgesHangingSign || $support instanceof WallHangingSign) && ($player === null || Facing::axis($player->getHorizontalFacing()) !== Facing::axis($support->getFacing()))) ||
-				$support instanceof CeilingCenterHangingSign ||
-				$support->getSupportType(Facing::DOWN) === SupportType::CENTER ?
-					$this->centerPointCeilingVariant :
-					$this->edgePointCeilingVariant;
-		}else{
-			$result = $this->wallVariant;
 		}
-		return clone $result;
+		//ceiling edges sign has stricter placement conditions than ceiling center sign, so try that first
+		$ceilingEdgeTx = $player === null || !$player->isSneaking() ?
+			$this->tryPlacementTransaction(clone $this->edgePointCeilingVariant, $blockReplace, $blockClicked, $face, $clickVector, $player) :
+			null;
+		return $ceilingEdgeTx ?? $this->tryPlacementTransaction(clone $this->centerPointCeilingVariant, $blockReplace, $blockClicked, $face, $clickVector, $player);
 	}
 
 	public function getBlock(?int $clickedFace = null) : Block{

src/item/Item.php

@@ -48,6 +48,7 @@ use pocketmine\nbt\tag\StringTag;
 use pocketmine\nbt\TreeRoot;
 use pocketmine\player\Player;
 use pocketmine\utils\Utils;
+use pocketmine\world\BlockTransaction;
 use pocketmine\world\format\io\GlobalItemDataHandlers;
 use function base64_decode;
 use function base64_encode;
@@ -485,8 +486,18 @@ class Item implements \JsonSerializable{
 		return $this->getBlock()->canBePlaced();
 	}
 
-	public function getPlacementBlock(?Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : Block{
+	protected final function tryPlacementTransaction(Block $blockPlace, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player) : ?BlockTransaction{
-		return $this->getBlock($face);
+		$position = $blockReplace->getPosition();
+		$blockPlace->position($position->getWorld(), $position->getFloorX(), $position->getFloorY(), $position->getFloorZ());
+		if(!$blockPlace->canBePlacedAt($blockReplace, $clickVector, $face, $blockReplace->getPosition()->equals($blockClicked->getPosition()))){
+			return null;
+		}
+		$transaction = new BlockTransaction($position->getWorld());
+		return $blockPlace->place($transaction, $this, $blockReplace, $blockClicked, $face, $clickVector, $player) ? $transaction : null;
+	}
+
+	public function getPlacementTransaction(Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : ?BlockTransaction{
+		return $this->tryPlacementTransaction($this->getBlock($face), $blockReplace, $blockClicked, $face, $clickVector, $player);
 	}
 
 	/**

src/world/World.php

@@ -2298,22 +2298,15 @@ class World implements ChunkManager{
 		if($item->isNull() || !$item->canBePlaced()){
 			return false;
 		}
-		$hand = $item->getPlacementBlock($player, $blockReplace, $blockClicked, $face, $clickVector);
-		$hand->position($this, $blockReplace->getPosition()->x, $blockReplace->getPosition()->y, $blockReplace->getPosition()->z);
 
-		if($hand->canBePlacedAt($blockClicked, $clickVector, $face, true)){
+		//TODO: while passing Facing::UP mimics the vanilla behaviour with replaceable blocks, we should really pass
-			$blockReplace = $blockClicked;
+		//some other value like NULL and let place() deal with it. This will look like a bug to anyone who doesn't know
-			//TODO: while this mimics the vanilla behaviour with replaceable blocks, we should really pass some other
+		//about the vanilla behaviour.
-			//value like NULL and let place() deal with it. This will look like a bug to anyone who doesn't know about
+		$tx =
-			//the vanilla behaviour.
+			$item->getPlacementTransaction($blockClicked, $blockClicked, Facing::UP, $clickVector, $player) ??
-			$face = Facing::UP;
+			$item->getPlacementTransaction($blockReplace, $blockClicked, $face, $clickVector, $player);
-			$hand->position($this, $blockReplace->getPosition()->x, $blockReplace->getPosition()->y, $blockReplace->getPosition()->z);
+		if($tx === null){
-		}elseif(!$hand->canBePlacedAt($blockReplace, $clickVector, $face, false)){
+			//no placement options available
-			return false;
-		}
-
-		$tx = new BlockTransaction($this);
-		if(!$hand->place($tx, $item, $blockReplace, $blockClicked, $face, $clickVector, $player)){
 			return false;
 		}
 
@@ -2357,6 +2350,7 @@ class World implements ChunkManager{
 		if(!$tx->apply()){
 			return false;
 		}
+		$first = true;
 		foreach($tx->getBlocks() as [$x, $y, $z, $_]){
 			$tile = $this->getTileAt($x, $y, $z);
 			if($tile !== null){
@@ -2364,11 +2358,12 @@ class World implements ChunkManager{
 				$tile->copyDataFromItem($item);
 			}
 
-			$this->getBlockAt($x, $y, $z)->onPostPlace();
+			$placed = $this->getBlockAt($x, $y, $z);
-		}
+			$placed->onPostPlace();
-
+			if($first && $playSound){
-		if($playSound){
+				$this->addSound($placed->getPosition(), new BlockPlaceSound($placed));
-			$this->addSound($hand->getPosition(), new BlockPlaceSound($hand));
+			}
+			$first = false;
 		}
 
 		$item->pop();