From e97234d42064f9a6323414b33521f1b0922dca6c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 22 Jul 2021 17:35:10 +0100 Subject: [PATCH] Refactor Rail handling to allow LSP-complaint shape handling the reason there hasn't been any API until now is because of how inconvenient it was to expose a LSP-compliant API _and_ use the same base class for handling all the connection logic. This commit fixes that problem by abstracting shape handling away from BaseRail entirely, so that now it deals exclusively with connections. Deciding the shape of rail to use is now the job of the subclasses. --- src/block/ActivatorRail.php | 2 +- src/block/BaseRail.php | 108 ++++-------------- src/block/DetectorRail.php | 17 +-- src/block/PoweredRail.php | 2 +- src/block/Rail.php | 53 ++++----- src/block/StraightOnlyRail.php | 64 +++++++++++ src/block/utils/RailConnectionInfo.php | 82 +++++++++++++ .../utils/RailPoweredByRedstoneTrait.php | 20 ++-- 8 files changed, 210 insertions(+), 138 deletions(-) create mode 100644 src/block/StraightOnlyRail.php create mode 100644 src/block/utils/RailConnectionInfo.php diff --git a/src/block/ActivatorRail.php b/src/block/ActivatorRail.php index 84538db7b..d842be041 100644 --- a/src/block/ActivatorRail.php +++ b/src/block/ActivatorRail.php @@ -25,7 +25,7 @@ namespace pocketmine\block; use pocketmine\block\utils\RailPoweredByRedstoneTrait; -class ActivatorRail extends BaseRail{ +class ActivatorRail extends StraightOnlyRail{ use RailPoweredByRedstoneTrait; //TODO diff --git a/src/block/BaseRail.php b/src/block/BaseRail.php index aeb62b384..52ff91dfa 100644 --- a/src/block/BaseRail.php +++ b/src/block/BaseRail.php @@ -23,74 +23,20 @@ declare(strict_types=1); namespace pocketmine\block; -use pocketmine\block\utils\InvalidBlockStateException; +use pocketmine\block\utils\RailConnectionInfo; use pocketmine\item\Item; use pocketmine\math\Facing; use pocketmine\math\Vector3; use pocketmine\player\Player; use pocketmine\world\BlockTransaction; -use function array_map; use function array_reverse; use function array_search; use function array_shift; use function count; -use function implode; use function in_array; abstract class BaseRail extends Flowable{ - protected const FLAG_ASCEND = 1 << 24; //used to indicate direction-up - - protected const CONNECTIONS = [ - //straights - BlockLegacyMetadata::RAIL_STRAIGHT_NORTH_SOUTH => [ - Facing::NORTH, - Facing::SOUTH - ], - BlockLegacyMetadata::RAIL_STRAIGHT_EAST_WEST => [ - Facing::EAST, - Facing::WEST - ], - - //ascending - BlockLegacyMetadata::RAIL_ASCENDING_EAST => [ - Facing::WEST, - Facing::EAST | self::FLAG_ASCEND - ], - BlockLegacyMetadata::RAIL_ASCENDING_WEST => [ - Facing::EAST, - Facing::WEST | self::FLAG_ASCEND - ], - BlockLegacyMetadata::RAIL_ASCENDING_NORTH => [ - Facing::SOUTH, - Facing::NORTH | self::FLAG_ASCEND - ], - BlockLegacyMetadata::RAIL_ASCENDING_SOUTH => [ - Facing::NORTH, - Facing::SOUTH | self::FLAG_ASCEND - ] - ]; - - protected int $railShape = BlockLegacyMetadata::RAIL_STRAIGHT_NORTH_SOUTH; - - protected function writeStateToMeta() : int{ - return $this->railShape; - } - - public function readStateFromData(int $id, int $stateMeta) : void{ - $railShape = $this->readRailShapeFromMeta($stateMeta); - if($railShape === null){ - throw new InvalidBlockStateException("Invalid rail type meta $stateMeta"); - } - $this->railShape = $railShape; - } - - abstract protected function readRailShapeFromMeta(int $stateMeta) : ?int; - - public function getStateBitmask() : int{ - return 0b1111; - } - public function place(BlockTransaction $tx, Item $item, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector, ?Player $player = null) : bool{ if(!$blockReplace->getSide(Facing::DOWN)->isTransparent()){ return parent::place($tx, $item, $blockReplace, $blockClicked, $face, $clickVector, $player); @@ -108,28 +54,22 @@ abstract class BaseRail extends Flowable{ * @param int[][] $lookup * @phpstan-param array> $lookup */ - protected static function searchState(array $connections, array $lookup) : int{ - $meta = array_search($connections, $lookup, true); - if($meta === false){ - $meta = array_search(array_reverse($connections), $lookup, true); + protected static function searchState(array $connections, array $lookup) : ?int{ + $shape = array_search($connections, $lookup, true); + if($shape === false){ + $shape = array_search(array_reverse($connections), $lookup, true); } - if($meta === false){ - throw new \InvalidArgumentException("No meta value matches connections " . implode(", ", array_map('\dechex', $connections))); - } - - return $meta; + return $shape === false ? null : $shape; } /** - * Returns a meta value for the rail with the given connections. + * Sets the rail shape according to the given connections, if a shape matches. * * @param int[] $connections * - * @throws \InvalidArgumentException if no state matches the given connections + * @throws \InvalidArgumentException if no shape matches the given connections */ - protected function getShapeForConnections(array $connections) : int{ - return self::searchState($connections, self::CONNECTIONS); - } + abstract protected function setShapeFromConnections(array $connections) : void; /** * Returns the connection directions of this rail (depending on the current block state) @@ -149,15 +89,15 @@ abstract class BaseRail extends Flowable{ /** @var int $connection */ foreach($this->getCurrentShapeConnections() as $connection){ - $other = $this->getSide($connection & ~self::FLAG_ASCEND); - $otherConnection = Facing::opposite($connection & ~self::FLAG_ASCEND); + $other = $this->getSide($connection & ~RailConnectionInfo::FLAG_ASCEND); + $otherConnection = Facing::opposite($connection & ~RailConnectionInfo::FLAG_ASCEND); - if(($connection & self::FLAG_ASCEND) !== 0){ + if(($connection & RailConnectionInfo::FLAG_ASCEND) !== 0){ $other = $other->getSide(Facing::UP); }elseif(!($other instanceof BaseRail)){ //check for rail sloping up to meet this one $other = $other->getSide(Facing::DOWN); - $otherConnection |= self::FLAG_ASCEND; + $otherConnection |= RailConnectionInfo::FLAG_ASCEND; } if( @@ -188,7 +128,7 @@ abstract class BaseRail extends Flowable{ Facing::EAST => true ]; foreach($possible as $p => $_){ - $possible[$p | self::FLAG_ASCEND] = true; + $possible[$p | RailConnectionInfo::FLAG_ASCEND] = true; } return $possible; @@ -206,13 +146,13 @@ abstract class BaseRail extends Flowable{ * @phpstan-return array */ protected function getPossibleConnectionDirectionsOneConstraint(int $constraint) : array{ - $opposite = Facing::opposite($constraint & ~self::FLAG_ASCEND); + $opposite = Facing::opposite($constraint & ~RailConnectionInfo::FLAG_ASCEND); $possible = [$opposite => true]; - if(($constraint & self::FLAG_ASCEND) === 0){ + if(($constraint & RailConnectionInfo::FLAG_ASCEND) === 0){ //We can slope the other way if this connection isn't already a slope - $possible[$opposite | self::FLAG_ASCEND] = true; + $possible[$opposite | RailConnectionInfo::FLAG_ASCEND] = true; } return $possible; @@ -227,16 +167,16 @@ abstract class BaseRail extends Flowable{ $continue = false; foreach($possible as $thisSide => $_){ - $otherSide = Facing::opposite($thisSide & ~self::FLAG_ASCEND); + $otherSide = Facing::opposite($thisSide & ~RailConnectionInfo::FLAG_ASCEND); - $other = $this->getSide($thisSide & ~self::FLAG_ASCEND); + $other = $this->getSide($thisSide & ~RailConnectionInfo::FLAG_ASCEND); - if(($thisSide & self::FLAG_ASCEND) !== 0){ + if(($thisSide & RailConnectionInfo::FLAG_ASCEND) !== 0){ $other = $other->getSide(Facing::UP); }elseif(!($other instanceof BaseRail)){ //check if other rails can slope up to meet this one $other = $other->getSide(Facing::DOWN); - $otherSide |= self::FLAG_ASCEND; + $otherSide |= RailConnectionInfo::FLAG_ASCEND; } if(!($other instanceof BaseRail) or count($otherConnections = $other->getConnectedDirections()) >= 2){ @@ -271,12 +211,12 @@ abstract class BaseRail extends Flowable{ */ private function setConnections(array $connections) : void{ if(count($connections) === 1){ - $connections[] = Facing::opposite($connections[0] & ~self::FLAG_ASCEND); + $connections[] = Facing::opposite($connections[0] & ~RailConnectionInfo::FLAG_ASCEND); }elseif(count($connections) !== 2){ throw new \InvalidArgumentException("Expected exactly 2 connections, got " . count($connections)); } - $this->railShape = $this->getShapeForConnections($connections); + $this->setShapeFromConnections($connections); } public function onNearbyBlockChange() : void{ @@ -284,7 +224,7 @@ abstract class BaseRail extends Flowable{ $this->pos->getWorld()->useBreakOn($this->pos); }else{ foreach($this->getCurrentShapeConnections() as $connection){ - if(($connection & self::FLAG_ASCEND) !== 0 and $this->getSide($connection & ~self::FLAG_ASCEND)->isTransparent()){ + if(($connection & RailConnectionInfo::FLAG_ASCEND) !== 0 and $this->getSide($connection & ~RailConnectionInfo::FLAG_ASCEND)->isTransparent()){ $this->pos->getWorld()->useBreakOn($this->pos); break; } diff --git a/src/block/DetectorRail.php b/src/block/DetectorRail.php index 572cb63cd..935fd0a04 100644 --- a/src/block/DetectorRail.php +++ b/src/block/DetectorRail.php @@ -23,7 +23,7 @@ declare(strict_types=1); namespace pocketmine\block; -class DetectorRail extends BaseRail{ +class DetectorRail extends StraightOnlyRail{ protected bool $activated = false; public function isActivated() : bool{ return $this->activated; } @@ -34,22 +34,17 @@ class DetectorRail extends BaseRail{ return $this; } - protected function readRailShapeFromMeta(int $stateMeta) : ?int{ - $stateMeta &= ~BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED; - return isset(self::CONNECTIONS[$stateMeta]) ? $stateMeta : null; + public function readStateFromData(int $id, int $stateMeta) : void{ + parent::readStateFromData($id, $stateMeta & ~BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED); + $this->activated = ($stateMeta & BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED) !== 0; } protected function writeStateToMeta() : int{ return parent::writeStateToMeta() | ($this->activated ? BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED : 0); } - public function readStateFromData(int $id, int $stateMeta) : void{ - parent::readStateFromData($id, $stateMeta); - $this->activated = ($stateMeta & BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED) !== 0; - } - - protected function getCurrentShapeConnections() : array{ - return self::CONNECTIONS[$this->railShape]; + public function getStateBitmask() : int{ + return 0b1111; } //TODO diff --git a/src/block/PoweredRail.php b/src/block/PoweredRail.php index 320227619..0c754d57f 100644 --- a/src/block/PoweredRail.php +++ b/src/block/PoweredRail.php @@ -25,6 +25,6 @@ namespace pocketmine\block; use pocketmine\block\utils\RailPoweredByRedstoneTrait; -class PoweredRail extends BaseRail{ +class PoweredRail extends StraightOnlyRail{ use RailPoweredByRedstoneTrait; } diff --git a/src/block/Rail.php b/src/block/Rail.php index 82968dcdf..0d0415f8b 100644 --- a/src/block/Rail.php +++ b/src/block/Rail.php @@ -23,51 +23,46 @@ declare(strict_types=1); namespace pocketmine\block; +use pocketmine\block\utils\InvalidBlockStateException; +use pocketmine\block\utils\RailConnectionInfo; use pocketmine\math\Facing; class Rail extends BaseRail{ - /* extended meta values for regular rails, to allow curving */ + private int $railShape = BlockLegacyMetadata::RAIL_STRAIGHT_NORTH_SOUTH; - private const CURVE_CONNECTIONS = [ - BlockLegacyMetadata::RAIL_CURVE_SOUTHEAST => [ - Facing::SOUTH, - Facing::EAST - ], - BlockLegacyMetadata::RAIL_CURVE_SOUTHWEST => [ - Facing::SOUTH, - Facing::WEST - ], - BlockLegacyMetadata::RAIL_CURVE_NORTHWEST => [ - Facing::NORTH, - Facing::WEST - ], - BlockLegacyMetadata::RAIL_CURVE_NORTHEAST => [ - Facing::NORTH, - Facing::EAST - ] - ]; - - protected function readRailShapeFromMeta(int $stateMeta) : ?int{ - return isset(self::CURVE_CONNECTIONS[$stateMeta]) || isset(self::CONNECTIONS[$stateMeta]) ? $stateMeta : null; + public function readStateFromData(int $id, int $stateMeta) : void{ + if(!isset(RailConnectionInfo::CONNECTIONS[$stateMeta]) && !isset(RailConnectionInfo::CURVE_CONNECTIONS[$stateMeta])){ + throw new InvalidBlockStateException("No rail shape matches metadata $stateMeta"); + } + $this->railShape = $stateMeta; } - protected function getShapeForConnections(array $connections) : int{ - try{ - return self::searchState($connections, self::CURVE_CONNECTIONS); - }catch(\InvalidArgumentException $e){ - return parent::getShapeForConnections($connections); + protected function writeStateToMeta() : int{ + //TODO: railShape won't be plain metadata in future + return $this->railShape; + } + + public function getStateBitmask() : int{ + return 0b1111; + } + + protected function setShapeFromConnections(array $connections) : void{ + $railShape = self::searchState($connections, RailConnectionInfo::CONNECTIONS) ?? self::searchState($connections, RailConnectionInfo::CURVE_CONNECTIONS); + if($railShape === null){ + throw new \InvalidArgumentException("No rail shape matches these connections"); } + $this->railShape = $railShape; } protected function getCurrentShapeConnections() : array{ - return self::CURVE_CONNECTIONS[$this->railShape] ?? self::CONNECTIONS[$this->railShape]; + return RailConnectionInfo::CURVE_CONNECTIONS[$this->railShape] ?? RailConnectionInfo::CONNECTIONS[$this->railShape]; } protected function getPossibleConnectionDirectionsOneConstraint(int $constraint) : array{ $possible = parent::getPossibleConnectionDirectionsOneConstraint($constraint); - if(($constraint & self::FLAG_ASCEND) === 0){ + if(($constraint & RailConnectionInfo::FLAG_ASCEND) === 0){ foreach([ Facing::NORTH, Facing::SOUTH, diff --git a/src/block/StraightOnlyRail.php b/src/block/StraightOnlyRail.php new file mode 100644 index 000000000..4b393bb92 --- /dev/null +++ b/src/block/StraightOnlyRail.php @@ -0,0 +1,64 @@ +railShape = $railShape; + } + + protected function writeStateToMeta() : int{ + //TODO: railShape won't be plain metadata in the future + return $this->railShape; + } + + public function getStateBitmask() : int{ + return 0b111; + } + + protected function setShapeFromConnections(array $connections) : void{ + $railShape = self::searchState($connections, RailConnectionInfo::CONNECTIONS); + if($railShape === null){ + throw new \InvalidArgumentException("No rail shape matches these connections"); + } + $this->railShape = $railShape; + } + + protected function getCurrentShapeConnections() : array{ + return RailConnectionInfo::CONNECTIONS[$this->railShape]; + } +} diff --git a/src/block/utils/RailConnectionInfo.php b/src/block/utils/RailConnectionInfo.php new file mode 100644 index 000000000..324ee3cfb --- /dev/null +++ b/src/block/utils/RailConnectionInfo.php @@ -0,0 +1,82 @@ + [ + Facing::NORTH, + Facing::SOUTH + ], + BlockLegacyMetadata::RAIL_STRAIGHT_EAST_WEST => [ + Facing::EAST, + Facing::WEST + ], + + //ascending + BlockLegacyMetadata::RAIL_ASCENDING_EAST => [ + Facing::WEST, + Facing::EAST | self::FLAG_ASCEND + ], + BlockLegacyMetadata::RAIL_ASCENDING_WEST => [ + Facing::EAST, + Facing::WEST | self::FLAG_ASCEND + ], + BlockLegacyMetadata::RAIL_ASCENDING_NORTH => [ + Facing::SOUTH, + Facing::NORTH | self::FLAG_ASCEND + ], + BlockLegacyMetadata::RAIL_ASCENDING_SOUTH => [ + Facing::NORTH, + Facing::SOUTH | self::FLAG_ASCEND + ] + ]; + + /* extended meta values for regular rails, to allow curving */ + public const CURVE_CONNECTIONS = [ + BlockLegacyMetadata::RAIL_CURVE_SOUTHEAST => [ + Facing::SOUTH, + Facing::EAST + ], + BlockLegacyMetadata::RAIL_CURVE_SOUTHWEST => [ + Facing::SOUTH, + Facing::WEST + ], + BlockLegacyMetadata::RAIL_CURVE_NORTHWEST => [ + Facing::NORTH, + Facing::WEST + ], + BlockLegacyMetadata::RAIL_CURVE_NORTHEAST => [ + Facing::NORTH, + Facing::EAST + ] + ]; +} diff --git a/src/block/utils/RailPoweredByRedstoneTrait.php b/src/block/utils/RailPoweredByRedstoneTrait.php index 9845eb5b7..f0549429f 100644 --- a/src/block/utils/RailPoweredByRedstoneTrait.php +++ b/src/block/utils/RailPoweredByRedstoneTrait.php @@ -28,21 +28,17 @@ use pocketmine\block\BlockLegacyMetadata; trait RailPoweredByRedstoneTrait{ use PoweredByRedstoneTrait; - protected function readRailShapeFromMeta(int $stateMeta) : ?int{ - $stateMeta &= ~BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED; - return isset(self::CONNECTIONS[$stateMeta]) ? $stateMeta : null; - } - - protected function writeStateToMeta() : int{ - return parent::writeStateToMeta() | ($this->powered ? BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED : 0); - } - public function readStateFromData(int $id, int $stateMeta) : void{ - parent::readStateFromData($id, $stateMeta); + parent::readStateFromData($id, $stateMeta & ~BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED); $this->powered = ($stateMeta & BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED) !== 0; } - protected function getCurrentShapeConnections() : array{ - return self::CONNECTIONS[$this->railShape]; + protected function writeStateToMeta() : int{ + //TODO: railShape won't be plain metadata in the future + return parent::writeStateToMeta() | ($this->powered ? BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED : 0); + } + + public function getStateBitmask() : int{ + return 0b1111; } }