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

@@ -25,7 +25,7 @@ namespace pocketmine\block;
 
 use pocketmine\block\utils\RailPoweredByRedstoneTrait;
 
-class ActivatorRail extends BaseRail{
+class ActivatorRail extends StraightOnlyRail{
 	use RailPoweredByRedstoneTrait;
 
 	//TODO

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<int, list<int>> $lookup
 	 */
-	protected static function searchState(array $connections, array $lookup) : int{
+	protected static function searchState(array $connections, array $lookup) : ?int{
-		$meta = array_search($connections, $lookup, true);
+		$shape = array_search($connections, $lookup, true);
-		if($meta === false){
+		if($shape === false){
-			$meta = array_search(array_reverse($connections), $lookup, true);
+			$shape = array_search(array_reverse($connections), $lookup, true);
 		}
-		if($meta === false){
+		return $shape === false ? null : $shape;
-			throw new \InvalidArgumentException("No meta value matches connections " . implode(", ", array_map('\dechex', $connections)));
-		}
-
-		return $meta;
 	}
 
 	/**
-	 * 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{
+	abstract protected function setShapeFromConnections(array $connections) : void;
-		return self::searchState($connections, self::CONNECTIONS);
-	}
 
 	/**
 	 * 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);
+			$other = $this->getSide($connection & ~RailConnectionInfo::FLAG_ASCEND);
-			$otherConnection = Facing::opposite($connection & ~self::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<int, true>
 	 */
 	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;
 				}

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{
+	public function readStateFromData(int $id, int $stateMeta) : void{
-		$stateMeta &= ~BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED;
+		parent::readStateFromData($id, $stateMeta & ~BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED);
-		return isset(self::CONNECTIONS[$stateMeta]) ? $stateMeta : null;
+		$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{
+	public function getStateBitmask() : int{
-		parent::readStateFromData($id, $stateMeta);
+		return 0b1111;
-		$this->activated = ($stateMeta & BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED) !== 0;
-	}
-
-	protected function getCurrentShapeConnections() : array{
-		return self::CONNECTIONS[$this->railShape];
 	}
 
 	//TODO

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;
 }

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 = [
+	public function readStateFromData(int $id, int $stateMeta) : void{
-		BlockLegacyMetadata::RAIL_CURVE_SOUTHEAST => [
+		if(!isset(RailConnectionInfo::CONNECTIONS[$stateMeta]) && !isset(RailConnectionInfo::CURVE_CONNECTIONS[$stateMeta])){
-			Facing::SOUTH,
+			throw new InvalidBlockStateException("No rail shape matches metadata $stateMeta");
-			Facing::EAST
+		}
-		],
+		$this->railShape = $stateMeta;
-		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;
 	}
 
-	protected function getShapeForConnections(array $connections) : int{
+	protected function writeStateToMeta() : int{
-		try{
+		//TODO: railShape won't be plain metadata in future
-			return self::searchState($connections, self::CURVE_CONNECTIONS);
+		return $this->railShape;
-		}catch(\InvalidArgumentException $e){
+	}
-			return parent::getShapeForConnections($connections);
+
+	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,

src/block/StraightOnlyRail.php

@@ -0,0 +1,64 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\block;
+
+use pocketmine\block\utils\InvalidBlockStateException;
+use pocketmine\block\utils\RailConnectionInfo;
+
+/**
+ * Simple non-curvable rail.
+ */
+class StraightOnlyRail extends BaseRail{
+
+	private int $railShape = BlockLegacyMetadata::RAIL_STRAIGHT_NORTH_SOUTH;
+
+	public function readStateFromData(int $id, int $stateMeta) : void{
+		$railShape = $stateMeta & ~BlockLegacyMetadata::REDSTONE_RAIL_FLAG_POWERED;
+		if(!isset(RailConnectionInfo::CONNECTIONS[$railShape])){
+			throw new InvalidBlockStateException("No rail shape matches meta $stateMeta");
+		}
+		$this->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];
+	}
+}

src/block/utils/RailConnectionInfo.php

@@ -0,0 +1,82 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\block\utils;
+
+use pocketmine\block\BlockLegacyMetadata;
+use pocketmine\math\Facing;
+
+final class RailConnectionInfo{
+
+	public const FLAG_ASCEND = 1 << 24; //used to indicate direction-up
+
+	public 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
+		]
+	];
+
+	/* 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
+		]
+	];
+}

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{
+	protected function writeStateToMeta() : int{
-		return self::CONNECTIONS[$this->railShape];
+		//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;
 	}
 }