From a79be994de43610a0404d239ca474a828c214f9d Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Thu, 12 Jan 2023 21:11:48 +0000
Subject: [PATCH] World: fixed block placement when clicking on replaceable
 blocks in vanilla, it appears to behave as if the player always clicked on
 the up face if a block was replaced.

In PM, we were still using the original face, which caused bugs when, for example, placing a button next to a wall by clicking on the side of tallgrass. The button would replace the tallgrass, but stick to the wall, instead of placing itself on the ground like vanilla expects.

This may appear unusual to anyone who also happens to implement canBePlacedAt(), since the facing behaviour will be different. However, this behaviour appears to match vanilla, and even slabs (which I feared might break because of this change) work perfectly.

In the future, it may be desirable to pass some other value here, such as null, to indicate that the clicked block is being replaced. However, that's a BC break and therefore outside of the scope of a stable bug fix.
---
 src/block/Torch.php | 5 +----
 src/world/World.php | 5 +++++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/block/Torch.php b/src/block/Torch.php
index 99975d4f6..77046867a 100644
--- a/src/block/Torch.php
+++ b/src/block/Torch.php
@@ -74,10 +74,7 @@ class Torch extends Flowable{
 	}
 
 	public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{
-		if($blockClicked->canBeReplaced() && $this->canBeSupportedBy($blockClicked->getSide(Facing::DOWN), Facing::UP)){
-			$this->facing = Facing::UP;
-			return parent::place($tx, $item, $blockReplace, $blockClicked, $face, $clickVector, $player);
-		}elseif($face !== Facing::DOWN && $this->canBeSupportedBy($blockReplace->getSide(Facing::opposite($face)), $face)){
+		if($face !== Facing::DOWN && $this->canBeSupportedBy($blockReplace->getSide(Facing::opposite($face)), $face)){
 			$this->facing = $face;
 			return parent::place($tx, $item, $blockReplace, $blockClicked, $face, $clickVector, $player);
 		}else{
diff --git a/src/world/World.php b/src/world/World.php
index 09795fd2f..c9521d11d 100644
--- a/src/world/World.php
+++ b/src/world/World.php
@@ -58,6 +58,7 @@ use pocketmine\item\StringToItemParser;
 use pocketmine\item\VanillaItems;
 use pocketmine\lang\KnownTranslationFactory;
 use pocketmine\math\AxisAlignedBB;
+use pocketmine\math\Facing;
 use pocketmine\math\Vector3;
 use pocketmine\nbt\tag\IntTag;
 use pocketmine\nbt\tag\StringTag;
@@ -1955,6 +1956,10 @@ class World implements ChunkManager{
 
 		if($hand->canBePlacedAt($blockClicked, $clickVector, $face, true)){
 			$blockReplace = $blockClicked;
+			//TODO: while this mimics the vanilla behaviour with replaceable blocks, we should really pass some other
+			//value like NULL and let place() deal with it. This will look like a bug to anyone who doesn't know about
+			//the vanilla behaviour.
+			$face = Facing::UP;
 			$hand->position($this, $blockReplace->getPosition()->x, $blockReplace->getPosition()->y, $blockReplace->getPosition()->z);
 		}elseif(!$hand->canBePlacedAt($blockReplace, $clickVector, $face, false)){
 			return false;