From 2f3fcef97c4596765873c21bb4e7d2f5c3c47f79 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Jan 2023 19:36:23 +0000 Subject: [PATCH 1/8] Fixed blocks incorrectly using blockClicked for support checks this caused some interesting bugs, such as being able to place floating pressure plates by clicking on the side of a solid block halfway up a wall. --- src/block/ItemFrame.php | 2 +- src/block/PressurePlate.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/block/ItemFrame.php b/src/block/ItemFrame.php index 8df62c72f..7a224c47b 100644 --- a/src/block/ItemFrame.php +++ b/src/block/ItemFrame.php @@ -175,7 +175,7 @@ class ItemFrame extends Flowable{ } public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ - if($face === Facing::DOWN || $face === Facing::UP || !$blockClicked->isSolid()){ + if($face === Facing::DOWN || $face === Facing::UP || !$blockReplace->getSide(Facing::opposite($face))->isSolid()){ return false; } diff --git a/src/block/PressurePlate.php b/src/block/PressurePlate.php index 5610b4e1a..5ad0e7873 100644 --- a/src/block/PressurePlate.php +++ b/src/block/PressurePlate.php @@ -45,7 +45,7 @@ abstract class PressurePlate extends Transparent{ } public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ - if($this->canBeSupportedBy($blockClicked)){ + if($this->canBeSupportedBy($blockReplace->getSide(Facing::DOWN))){ return parent::place($tx, $item, $blockReplace, $blockClicked, $face, $clickVector, $player); } return false; From d295e1be54557c41f2629403f25ebb944d8dd830 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Jan 2023 19:36:47 +0000 Subject: [PATCH 2/8] PressurePlate: destroy self when no support is present --- src/block/PressurePlate.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/block/PressurePlate.php b/src/block/PressurePlate.php index 5ad0e7873..7f9403b74 100644 --- a/src/block/PressurePlate.php +++ b/src/block/PressurePlate.php @@ -55,5 +55,11 @@ abstract class PressurePlate extends Transparent{ return !$block->getSupportType(Facing::UP)->equals(SupportType::NONE()); } + public function onNearbyBlockChange() : void{ + if(!$this->canBeSupportedBy($this->getSide(Facing::DOWN))){ + $this->position->getWorld()->useBreakOn($this->position); + } + } + //TODO } From 4e9c3e101d69fa979915ee820592d8020e0c52fa Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Jan 2023 19:42:33 +0000 Subject: [PATCH 3/8] Bell: fixed blocks not being able to be placed when not ringing the bell --- src/block/Bell.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/block/Bell.php b/src/block/Bell.php index 70c78d027..4af738d59 100644 --- a/src/block/Bell.php +++ b/src/block/Bell.php @@ -161,21 +161,20 @@ final class Bell extends Transparent{ public function onInteract(Item $item, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ if($player !== null){ $faceHit = Facing::opposite($player->getHorizontalFacing()); - if($this->attachmentType->equals(BellAttachmentType::CEILING())){ - $this->ring($faceHit); - } - if($this->attachmentType->equals(BellAttachmentType::FLOOR()) && Facing::axis($faceHit) === Facing::axis($this->facing)){ - $this->ring($faceHit); - } if( - ($this->attachmentType->equals(BellAttachmentType::ONE_WALL()) || $this->attachmentType->equals(BellAttachmentType::TWO_WALLS())) && - ($faceHit === Facing::rotateY($this->facing, false) || $faceHit === Facing::rotateY($this->facing, true)) + $this->attachmentType->equals(BellAttachmentType::CEILING()) || + ($this->attachmentType->equals(BellAttachmentType::FLOOR()) && Facing::axis($faceHit) === Facing::axis($this->facing)) || + ( + ($this->attachmentType->equals(BellAttachmentType::ONE_WALL()) || $this->attachmentType->equals(BellAttachmentType::TWO_WALLS())) && + ($faceHit === Facing::rotateY($this->facing, false) || $faceHit === Facing::rotateY($this->facing, true)) + ) ){ $this->ring($faceHit); + return true; } } - return true; + return false; } public function ring(int $faceHit) : void{ From e26c8b9e9fa3bf902be54d5511d8d9672c77d67b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Jan 2023 20:35:26 +0000 Subject: [PATCH 4/8] block: eliminate suspicious usages of $blockClicked in place() --- src/block/Button.php | 2 +- src/block/Ladder.php | 2 +- src/block/Lever.php | 2 +- src/block/Torch.php | 2 +- src/block/Vine.php | 2 +- src/block/WallCoralFan.php | 2 +- src/block/WaterLily.php | 14 ++------------ 7 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/block/Button.php b/src/block/Button.php index c447ddb8d..f86d8a579 100644 --- a/src/block/Button.php +++ b/src/block/Button.php @@ -61,7 +61,7 @@ abstract class Button extends Flowable{ } public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ - if($this->canBeSupportedBy($blockClicked, $face)){ + if($this->canBeSupportedBy($blockReplace->getSide(Facing::opposite($face)), $face)){ $this->facing = $face; return parent::place($tx, $item, $blockReplace, $blockClicked, $face, $clickVector, $player); } diff --git a/src/block/Ladder.php b/src/block/Ladder.php index fcf2be061..ccedb33bd 100644 --- a/src/block/Ladder.php +++ b/src/block/Ladder.php @@ -70,7 +70,7 @@ class Ladder extends Transparent{ } public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ - if($this->canBeSupportedBy($blockClicked, $face) && Facing::axis($face) !== Axis::Y){ + if($this->canBeSupportedBy($blockReplace->getSide(Facing::opposite($face)), $face) && Facing::axis($face) !== Axis::Y){ $this->facing = $face; return parent::place($tx, $item, $blockReplace, $blockClicked, $face, $clickVector, $player); } diff --git a/src/block/Lever.php b/src/block/Lever.php index beec393b7..01512007b 100644 --- a/src/block/Lever.php +++ b/src/block/Lever.php @@ -95,7 +95,7 @@ class Lever extends Flowable{ } public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ - if(!$this->canBeSupportedBy($blockClicked, $face)){ + if(!$this->canBeSupportedBy($blockReplace->getSide(Facing::opposite($face)), $face)){ return false; } diff --git a/src/block/Torch.php b/src/block/Torch.php index ded081bcc..99975d4f6 100644 --- a/src/block/Torch.php +++ b/src/block/Torch.php @@ -77,7 +77,7 @@ class Torch extends Flowable{ 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($blockClicked, $face)){ + }elseif($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/block/Vine.php b/src/block/Vine.php index 9159315da..80943dc7c 100644 --- a/src/block/Vine.php +++ b/src/block/Vine.php @@ -116,7 +116,7 @@ class Vine extends Flowable{ } public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ - if(!$blockClicked->isFullCube() || Facing::axis($face) === Axis::Y){ + if(!$blockReplace->getSide(Facing::opposite($face))->isFullCube() || Facing::axis($face) === Axis::Y){ return false; } diff --git a/src/block/WallCoralFan.php b/src/block/WallCoralFan.php index ee0dcc35a..fe8dc40a8 100644 --- a/src/block/WallCoralFan.php +++ b/src/block/WallCoralFan.php @@ -113,7 +113,7 @@ final class WallCoralFan extends BaseCoral{ public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ $axis = Facing::axis($face); - if(($axis !== Axis::X && $axis !== Axis::Z) || !$this->canBeSupportedBy($blockClicked, $face)){ + if(($axis !== Axis::X && $axis !== Axis::Z) || !$this->canBeSupportedBy($blockReplace->getSide(Facing::opposite($face)), $face)){ return false; } $this->facing = $face; diff --git a/src/block/WaterLily.php b/src/block/WaterLily.php index 39b3c8cee..25af0a8b0 100644 --- a/src/block/WaterLily.php +++ b/src/block/WaterLily.php @@ -23,12 +23,9 @@ declare(strict_types=1); namespace pocketmine\block; -use pocketmine\item\Item; use pocketmine\math\AxisAlignedBB; use pocketmine\math\Facing; use pocketmine\math\Vector3; -use pocketmine\player\Player; -use pocketmine\world\BlockTransaction; class WaterLily extends Flowable{ @@ -39,15 +36,8 @@ class WaterLily extends Flowable{ return [AxisAlignedBB::one()->contract(1 / 16, 0, 1 / 16)->trim(Facing::UP, 63 / 64)]; } - public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ - if($blockClicked instanceof Water){ - $up = $blockClicked->getSide(Facing::UP); - if($up->canBeReplaced()){ - return parent::place($tx, $item, $up, $blockClicked, $face, $clickVector, $player); - } - } - - return false; + public function canBePlacedAt(Block $blockReplace, Vector3 $clickVector, int $face, bool $isClickedBlock) : bool{ + return !$blockReplace instanceof Water && parent::canBePlacedAt($blockReplace, $clickVector, $face, $isClickedBlock); } public function onNearbyBlockChange() : void{ From a79be994de43610a0404d239ca474a828c214f9d Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Jan 2023 21:11:48 +0000 Subject: [PATCH 5/8] 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; From b25e8e26f0cf5cf16897215e7dbb654f537f561a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Jan 2023 21:31:50 +0000 Subject: [PATCH 6/8] BaseBanner: fixed incorrect support requirements --- src/block/BaseBanner.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/block/BaseBanner.php b/src/block/BaseBanner.php index 5b3f9acce..400b8a78e 100644 --- a/src/block/BaseBanner.php +++ b/src/block/BaseBanner.php @@ -112,7 +112,14 @@ abstract class BaseBanner extends Transparent{ return SupportType::NONE(); } + private function canBeSupportedBy(Block $block) : bool{ + return $block->isSolid(); + } + public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ + if(!$this->canBeSupportedBy($blockReplace->getSide($this->getSupportingFace()))){ + return false; + } if($item instanceof ItemBanner){ $this->color = $item->getColor(); $this->setPatterns($item->getPatterns()); @@ -124,7 +131,7 @@ abstract class BaseBanner extends Transparent{ abstract protected function getSupportingFace() : int; public function onNearbyBlockChange() : void{ - if($this->getSide($this->getSupportingFace())->getId() === BlockLegacyIds::AIR){ + if(!$this->canBeSupportedBy($this->getSide($this->getSupportingFace()))){ $this->position->getWorld()->useBreakOn($this->position); } } From cbeae906e1ae0f593c495c43e8d0584cfea7439c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Jan 2023 21:34:44 +0000 Subject: [PATCH 7/8] Torch: remove unused variable --- src/block/Torch.php | 1 - src/block/WaterLily.php | 16 +++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/block/Torch.php b/src/block/Torch.php index 77046867a..768c21223 100644 --- a/src/block/Torch.php +++ b/src/block/Torch.php @@ -65,7 +65,6 @@ class Torch extends Flowable{ } public function onNearbyBlockChange() : void{ - $below = $this->getSide(Facing::DOWN); $face = Facing::opposite($this->facing); if(!$this->canBeSupportedBy($this->getSide($face), $this->facing)){ diff --git a/src/block/WaterLily.php b/src/block/WaterLily.php index 25af0a8b0..8263330f6 100644 --- a/src/block/WaterLily.php +++ b/src/block/WaterLily.php @@ -23,9 +23,12 @@ declare(strict_types=1); namespace pocketmine\block; +use pocketmine\item\Item; use pocketmine\math\AxisAlignedBB; use pocketmine\math\Facing; use pocketmine\math\Vector3; +use pocketmine\player\Player; +use pocketmine\world\BlockTransaction; class WaterLily extends Flowable{ @@ -40,8 +43,19 @@ class WaterLily extends Flowable{ return !$blockReplace instanceof Water && parent::canBePlacedAt($blockReplace, $clickVector, $face, $isClickedBlock); } + private function canBeSupportedBy(Block $block) : bool{ + return $block instanceof Water; + } + + public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ + if(!$this->canBeSupportedBy($blockReplace->getSide(Facing::DOWN))){ + return false; + } + return parent::place($tx, $item, $blockReplace, $blockClicked, $face, $clickVector, $player); + } + public function onNearbyBlockChange() : void{ - if(!($this->getSide(Facing::DOWN) instanceof Water)){ + if(!$this->canBeSupportedBy($this->getSide(Facing::DOWN))){ $this->position->getWorld()->useBreakOn($this->position); } } From f4a1d690752466f22f8f4187a45054476471ebd6 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Jan 2023 21:45:25 +0000 Subject: [PATCH 8/8] Bell: fixed support requirements this somehow got overlooked in the support types refactor. --- src/block/Bell.php | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/block/Bell.php b/src/block/Bell.php index 4af738d59..00a53a9be 100644 --- a/src/block/Bell.php +++ b/src/block/Bell.php @@ -114,14 +114,13 @@ final class Bell extends Transparent{ return $this; } - private function canBeSupportedBy(Block $block) : bool{ - //TODO: this isn't the actual logic, but it's the closest approximation we can support for now - return $block->isSolid(); + private function canBeSupportedBy(Block $block, int $face) : bool{ + return !$block->getSupportType($face)->equals(SupportType::NONE()); } public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ if($face === Facing::UP){ - if(!$this->canBeSupportedBy($tx->fetchBlock($this->position->down()))){ + if(!$this->canBeSupportedBy($tx->fetchBlock($this->position->down()), Facing::UP)){ return false; } if($player !== null){ @@ -129,18 +128,18 @@ final class Bell extends Transparent{ } $this->setAttachmentType(BellAttachmentType::FLOOR()); }elseif($face === Facing::DOWN){ - if(!$this->canBeSupportedBy($tx->fetchBlock($this->position->up()))){ + if(!$this->canBeSupportedBy($tx->fetchBlock($this->position->up()), Facing::DOWN)){ return false; } $this->setAttachmentType(BellAttachmentType::CEILING()); }else{ $this->setFacing($face); - if($this->canBeSupportedBy($tx->fetchBlock($this->position->getSide(Facing::opposite($face))))){ + if($this->canBeSupportedBy($tx->fetchBlock($this->position->getSide(Facing::opposite($face))), $face)){ $this->setAttachmentType(BellAttachmentType::ONE_WALL()); }else{ return false; } - if($this->canBeSupportedBy($tx->fetchBlock($this->position->getSide($face)))){ + if($this->canBeSupportedBy($tx->fetchBlock($this->position->getSide($face)), Facing::opposite($face))){ $this->setAttachmentType(BellAttachmentType::TWO_WALLS()); } } @@ -149,10 +148,10 @@ final class Bell extends Transparent{ public function onNearbyBlockChange() : void{ if( - ($this->attachmentType->equals(BellAttachmentType::CEILING()) && !$this->canBeSupportedBy($this->getSide(Facing::UP))) || - ($this->attachmentType->equals(BellAttachmentType::FLOOR()) && !$this->canBeSupportedBy($this->getSide(Facing::DOWN))) || - ($this->attachmentType->equals(BellAttachmentType::ONE_WALL()) && !$this->canBeSupportedBy($this->getSide(Facing::opposite($this->facing)))) || - ($this->attachmentType->equals(BellAttachmentType::TWO_WALLS()) && (!$this->canBeSupportedBy($this->getSide($this->facing)) || !$this->canBeSupportedBy($this->getSide(Facing::opposite($this->facing))))) + ($this->attachmentType->equals(BellAttachmentType::CEILING()) && !$this->canBeSupportedBy($this->getSide(Facing::UP), Facing::DOWN)) || + ($this->attachmentType->equals(BellAttachmentType::FLOOR()) && !$this->canBeSupportedBy($this->getSide(Facing::DOWN), Facing::UP)) || + ($this->attachmentType->equals(BellAttachmentType::ONE_WALL()) && !$this->canBeSupportedBy($this->getSide(Facing::opposite($this->facing)), $this->facing)) || + ($this->attachmentType->equals(BellAttachmentType::TWO_WALLS()) && (!$this->canBeSupportedBy($this->getSide($this->facing), Facing::opposite($this->facing)) || !$this->canBeSupportedBy($this->getSide(Facing::opposite($this->facing)), $this->facing))) ){ $this->position->getWorld()->useBreakOn($this->position); }