Banner: remove Deque usages

originally I introduced this to make it easier to implement the various APIs addPattern removePattern etc, but those were later removed in favour of simple getPatterns() and setPatterns(), allowing plugin developers to use ext-ds APIs to manipulate patterns.
However, ds poses a number of headaches because of mutability combined with by-ref semantics, which make it a pain to use these on the APIs because we can't guarantee that they won't be modified.
As much as arrays suck, they have two significant advantages over ext-ds: 1) they have copy-on-write semantics, and 2) they support PHP 8.0 without any extra work from me.
This commit is contained in:
Dylan K. Taylor 2021-01-15 00:17:56 +00:00
parent 4c0d3d68af
commit a9f8afa077
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
4 changed files with 36 additions and 59 deletions

View File

@ -23,7 +23,6 @@ declare(strict_types=1);
namespace pocketmine\block;
use Ds\Deque;
use pocketmine\block\tile\Banner as TileBanner;
use pocketmine\block\utils\BannerPattern;
use pocketmine\block\utils\DyeColor;
@ -36,28 +35,23 @@ use pocketmine\math\AxisAlignedBB;
use pocketmine\math\Vector3;
use pocketmine\player\Player;
use pocketmine\world\BlockTransaction;
use function array_filter;
use function assert;
use function count;
abstract class BaseBanner extends Transparent{
/** @var DyeColor */
protected $baseColor;
/**
* @var Deque|BannerPattern[]
* @phpstan-var Deque<BannerPattern>
* @var BannerPattern[]
* @phpstan-var list<BannerPattern>
*/
protected $patterns;
protected $patterns = [];
public function __construct(BlockIdentifier $idInfo, string $name, ?BlockBreakInfo $breakInfo = null){
parent::__construct($idInfo, $name, $breakInfo ?? new BlockBreakInfo(1.0, BlockToolType::AXE));
$this->baseColor = DyeColor::BLACK();
$this->patterns = new Deque();
}
public function __clone(){
parent::__clone();
//pattern objects are considered immutable, so they don't need to be copied
$this->patterns = $this->patterns->copy();
}
public function readStateFromWorld() : void{
@ -89,21 +83,21 @@ abstract class BaseBanner extends Transparent{
}
/**
* @return Deque|BannerPattern[]
* @phpstan-return Deque<BannerPattern>
* @return BannerPattern[]
* @phpstan-return list<BannerPattern>
*/
public function getPatterns() : Deque{
public function getPatterns() : array{
return $this->patterns;
}
/**
* @param Deque|BannerPattern[] $patterns
* @phpstan-param Deque<BannerPattern> $patterns
* @param BannerPattern[] $patterns
* @phpstan-param list<BannerPattern> $patterns
* @return $this
*/
public function setPatterns(Deque $patterns) : self{
$checked = $patterns->filter(function($v) : bool{ return $v instanceof BannerPattern; });
if($checked->count() !== $patterns->count()){
public function setPatterns(array $patterns) : self{
$checked = array_filter($patterns, fn($v) => $v instanceof BannerPattern);
if(count($checked) !== count($patterns)){
throw new \TypeError("Deque must only contain " . BannerPattern::class . " objects");
}
$this->patterns = $checked;
@ -140,7 +134,7 @@ abstract class BaseBanner extends Transparent{
public function getDropsForCompatibleTool(Item $item) : array{
$drop = $this->asItem();
if($drop instanceof ItemBanner and !$this->patterns->isEmpty()){
if($drop instanceof ItemBanner and count($this->patterns) > 0){
$drop->setPatterns($this->patterns);
}
@ -149,7 +143,7 @@ abstract class BaseBanner extends Transparent{
public function getPickedItem(bool $addUserData = false) : Item{
$result = $this->asItem();
if($addUserData and $result instanceof ItemBanner and !$this->patterns->isEmpty()){
if($addUserData and $result instanceof ItemBanner and count($this->patterns) > 0){
$result->setPatterns($this->patterns);
}
return $result;

View File

@ -23,7 +23,6 @@ declare(strict_types=1);
namespace pocketmine\block\tile;
use Ds\Deque;
use pocketmine\block\utils\BannerPattern;
use pocketmine\block\utils\DyeColor;
use pocketmine\data\bedrock\DyeColorIdMap;
@ -48,14 +47,13 @@ class Banner extends Spawnable{
private $baseColor;
/**
* @var BannerPattern[]|Deque
* @phpstan-var Deque<BannerPattern>
* @var BannerPattern[]
* @phpstan-var list<BannerPattern>
*/
private $patterns;
private $patterns = [];
public function __construct(World $world, Vector3 $pos){
$this->baseColor = DyeColor::BLACK();
$this->patterns = new Deque();
parent::__construct($world, $pos);
}
@ -115,18 +113,18 @@ class Banner extends Spawnable{
}
/**
* @return BannerPattern[]|Deque
* @phpstan-return Deque<BannerPattern>
* @return BannerPattern[]
* @phpstan-return list<BannerPattern>
*/
public function getPatterns() : Deque{
public function getPatterns() : array{
return $this->patterns;
}
/**
* @param BannerPattern[]|Deque $patterns
* @phpstan-param Deque<BannerPattern> $patterns
* @param BannerPattern[] $patterns
* @phpstan-param list<BannerPattern> $patterns
*/
public function setPatterns(Deque $patterns) : void{
public function setPatterns(array $patterns) : void{
$this->patterns = $patterns;
}

View File

@ -23,7 +23,6 @@ declare(strict_types=1);
namespace pocketmine\item;
use Ds\Deque;
use pocketmine\block\Block;
use pocketmine\block\tile\Banner as TileBanner;
use pocketmine\block\utils\BannerPattern;
@ -41,16 +40,14 @@ class Banner extends ItemBlockWallOrFloor{
private $color;
/**
* @var BannerPattern[]|Deque
* @phpstan-var Deque<BannerPattern>
* @var BannerPattern[]
* @phpstan-var list<BannerPattern>
*/
private $patterns;
private $patterns = [];
public function __construct(ItemIdentifier $identifier, Block $floorVariant, Block $wallVariant){
parent::__construct($identifier, $floorVariant, $wallVariant);
$this->color = DyeColor::BLACK();
$this->patterns = new Deque();
}
public function getColor() : DyeColor{
@ -72,20 +69,20 @@ class Banner extends ItemBlockWallOrFloor{
}
/**
* @return Deque|BannerPattern[]
* @phpstan-return Deque<BannerPattern>
* @return BannerPattern[]
* @phpstan-return list<BannerPattern>
*/
public function getPatterns() : Deque{
public function getPatterns() : array{
return $this->patterns;
}
/**
* @param Deque|BannerPattern[] $patterns
* @phpstan-param Deque<BannerPattern> $patterns
* @param BannerPattern[] $patterns
* @phpstan-param list<BannerPattern> $patterns
*
* @return $this
*/
public function setPatterns(Deque $patterns) : self{
public function setPatterns(array $patterns) : self{
$this->patterns = $patterns;
return $this;
}
@ -97,14 +94,14 @@ class Banner extends ItemBlockWallOrFloor{
protected function deserializeCompoundTag(CompoundTag $tag) : void{
parent::deserializeCompoundTag($tag);
$this->patterns = new Deque();
$this->patterns = [];
$colorIdMap = DyeColorIdMap::getInstance();
$patterns = $tag->getListTag(self::TAG_PATTERNS);
if($patterns !== null){
/** @var CompoundTag $t */
foreach($patterns as $t){
$this->patterns->push(new BannerPattern($t->getString(self::TAG_PATTERN_NAME), $colorIdMap->fromInvertedId($t->getInt(self::TAG_PATTERN_COLOR))));
$this->patterns[] = new BannerPattern($t->getString(self::TAG_PATTERN_NAME), $colorIdMap->fromInvertedId($t->getInt(self::TAG_PATTERN_COLOR)));
}
}
}
@ -112,10 +109,9 @@ class Banner extends ItemBlockWallOrFloor{
protected function serializeCompoundTag(CompoundTag $tag) : void{
parent::serializeCompoundTag($tag);
if(!$this->patterns->isEmpty()){
if(count($this->patterns) > 0){
$patterns = new ListTag();
$colorIdMap = DyeColorIdMap::getInstance();
/** @var BannerPattern $pattern */
foreach($this->patterns as $pattern){
$patterns->push(CompoundTag::create()
->setString(self::TAG_PATTERN_NAME, $pattern->getId())
@ -128,10 +124,4 @@ class Banner extends ItemBlockWallOrFloor{
$tag->removeTag(self::TAG_PATTERNS);
}
}
public function __clone(){
parent::__clone();
//we don't need to duplicate the individual patterns because they are immutable
$this->patterns = $this->patterns->copy();
}
}

View File

@ -1,10 +1,5 @@
parameters:
ignoreErrors:
-
message: "#^Cannot access an offset on Ds\\\\Deque\\<pocketmine\\\\block\\\\utils\\\\BannerPattern\\>\\.$#"
count: 1
path: ../../../src/block/tile/Banner.php
-
message: "#^Cannot access offset int on Ds\\\\Deque\\<pocketmine\\\\item\\\\WritableBookPage\\>\\.$#"
count: 2