Mostly phase out EmptySubChunk

copy-on-write and zero-layer SubChunk objects are much easier to manage and have less cognitive overhead.

obviously, the goal is to get rid of EmptySubChunk completely, but right now it does still serve a purpose (filling in dummy values for reading out-of-bounds on chunks), and that's a problem that takes a little more work to fix.

src/world/World.php

@@ -64,7 +64,6 @@ use pocketmine\utils\Limits;
 use pocketmine\utils\ReversePriorityQueue;
 use pocketmine\world\biome\Biome;
 use pocketmine\world\format\Chunk;
-use pocketmine\world\format\EmptySubChunk;
 use pocketmine\world\format\io\exception\CorruptedChunkException;
 use pocketmine\world\format\io\WritableWorldProvider;
 use pocketmine\world\generator\Generator;
@@ -983,7 +982,7 @@ class World implements ChunkManager{
 
 
 			foreach($chunk->getSubChunks() as $Y => $subChunk){
-				if(!($subChunk instanceof EmptySubChunk)){
+				if(!$subChunk->isEmptyFast()){
 					$k = mt_rand(0, 0xfffffffff); //36 bits
 					for($i = 0; $i < 3; ++$i){
 						$x = $k & 0x0f;

src/world/format/Chunk.php

@@ -68,7 +68,7 @@ class Chunk{
 	/** @var bool */
 	protected $terrainPopulated = false;
 
-	/** @var \SplFixedArray|SubChunkInterface[] */
+	/** @var \SplFixedArray|SubChunk[] */
 	protected $subChunks;
 
 	/** @var Tile[] */
@@ -90,13 +90,13 @@ class Chunk{
 	protected $NBTentities = [];
 
 	/**
-	 * @param int                 $chunkX
-	 * @param int                 $chunkZ
-	 * @param SubChunkInterface[] $subChunks
-	 * @param CompoundTag[]       $entities
-	 * @param CompoundTag[]       $tiles
-	 * @param string              $biomeIds
-	 * @param int[]               $heightMap
+	 * @param int           $chunkX
+	 * @param int           $chunkZ
+	 * @param SubChunk[]    $subChunks
+	 * @param CompoundTag[] $entities
+	 * @param CompoundTag[] $tiles
+	 * @param string        $biomeIds
+	 * @param int[]         $heightMap
 	 */
 	public function __construct(int $chunkX, int $chunkZ, array $subChunks = [], ?array $entities = null, ?array $tiles = null, string $biomeIds = "", array $heightMap = []){
 		$this->x = $chunkX;
@@ -105,7 +105,7 @@ class Chunk{
 		$this->subChunks = new \SplFixedArray(Chunk::MAX_SUBCHUNKS);
 
 		foreach($this->subChunks as $y => $null){
-			$this->subChunks[$y] = $subChunks[$y] ?? EmptySubChunk::getInstance();
+			$this->subChunks[$y] = $subChunks[$y] ?? new SubChunk(BlockLegacyIds::AIR << 4, []);
 		}
 
 		if(count($heightMap) === 256){
@@ -183,7 +183,7 @@ class Chunk{
 	 * @param int $block
 	 */
 	public function setFullBlock(int $x, int $y, int $z, int $block) : void{
-		$this->getSubChunk($y >> 4, true)->setFullBlock($x, $y & 0xf, $z, $block);
+		$this->getSubChunk($y >> 4)->setFullBlock($x, $y & 0xf, $z, $block);
 		$this->dirtyFlags |= self::DIRTY_FLAG_TERRAIN;
 	}
 
@@ -209,7 +209,7 @@ class Chunk{
 	 * @param int $level 0-15
 	 */
 	public function setBlockSkyLight(int $x, int $y, int $z, int $level) : void{
-		$this->getSubChunk($y >> 4, true)->getBlockSkyLightArray()->set($x & 0xf, $y & 0x0f, $z & 0xf, $level);
+		$this->getSubChunk($y >> 4)->getBlockSkyLightArray()->set($x & 0xf, $y & 0x0f, $z & 0xf, $level);
 	}
 
 	/**
@@ -217,7 +217,7 @@ class Chunk{
 	 */
 	public function setAllBlockSkyLight(int $level) : void{
 		for($y = $this->getHighestSubChunkIndex(); $y >= 0; --$y){
-			$this->getSubChunk($y, true)->setBlockSkyLightArray(LightArray::fill($level));
+			$this->getSubChunk($y)->setBlockSkyLightArray(LightArray::fill($level));
 		}
 	}
 
@@ -243,7 +243,7 @@ class Chunk{
 	 * @param int $level 0-15
 	 */
 	public function setBlockLight(int $x, int $y, int $z, int $level) : void{
-		$this->getSubChunk($y >> 4, true)->getBlockLightArray()->set($x & 0xf, $y & 0x0f, $z & 0xf, $level);
+		$this->getSubChunk($y >> 4)->getBlockLightArray()->set($x & 0xf, $y & 0x0f, $z & 0xf, $level);
 	}
 
 	/**
@@ -251,7 +251,7 @@ class Chunk{
 	 */
 	public function setAllBlockLight(int $level) : void{
 		for($y = $this->getHighestSubChunkIndex(); $y >= 0; --$y){
-			$this->getSubChunk($y, true)->setBlockLightArray(LightArray::fill($level));
+			$this->getSubChunk($y)->setBlockLightArray(LightArray::fill($level));
 		}
 	}
 
@@ -659,16 +659,13 @@ class Chunk{
 	/**
 	 * Returns the subchunk at the specified subchunk Y coordinate, or an empty, unmodifiable stub if it does not exist or the coordinate is out of range.
 	 *
-	 * @param int  $y
-	 * @param bool $generateNew Whether to create a new, modifiable subchunk if there is not one in place
+	 * @param int $y
 	 *
 	 * @return SubChunkInterface
 	 */
-	public function getSubChunk(int $y, bool $generateNew = false) : SubChunkInterface{
+	public function getSubChunk(int $y) : SubChunkInterface{
 		if($y < 0 or $y >= $this->subChunks->getSize()){
-			return EmptySubChunk::getInstance();
-		}elseif($generateNew and $this->subChunks[$y] instanceof EmptySubChunk){
-			$this->subChunks[$y] = new SubChunk(BlockLegacyIds::AIR << 4, []);
+			return EmptySubChunk::getInstance(); //TODO: drop this and throw an exception here
 		}
 
 		return $this->subChunks[$y];
@@ -677,24 +674,20 @@ class Chunk{
 	/**
 	 * Sets a subchunk in the chunk index
 	 *
-	 * @param int                    $y
-	 * @param SubChunkInterface|null $subChunk
-	 * @param bool                   $allowEmpty Whether to check if the chunk is empty, and if so replace it with an empty stub
+	 * @param int           $y
+	 * @param SubChunk|null $subChunk
 	 */
-	public function setSubChunk(int $y, ?SubChunkInterface $subChunk, bool $allowEmpty = false) : void{
+	public function setSubChunk(int $y, ?SubChunk $subChunk) : void{
 		if($y < 0 or $y >= $this->subChunks->getSize()){
 			throw new \InvalidArgumentException("Invalid subchunk Y coordinate $y");
 		}
-		if($subChunk === null or ($subChunk->isEmpty() and !$allowEmpty)){
-			$this->subChunks[$y] = EmptySubChunk::getInstance();
-		}else{
-			$this->subChunks[$y] = $subChunk;
-		}
+
+		$this->subChunks[$y] = $subChunk ?? new SubChunk(BlockLegacyIds::AIR << 4, []);
 		$this->setDirtyFlag(self::DIRTY_FLAG_TERRAIN, true);
 	}
 
 	/**
-	 * @return \SplFixedArray|SubChunkInterface[]
+	 * @return \SplFixedArray|SubChunk[]
 	 */
 	public function getSubChunks() : \SplFixedArray{
 		return $this->subChunks;
@@ -707,8 +700,7 @@ class Chunk{
 	 */
 	public function getHighestSubChunkIndex() : int{
 		for($y = $this->subChunks->count() - 1; $y >= 0; --$y){
-			if($this->subChunks[$y] instanceof EmptySubChunk){
-				//No need to thoroughly prune empties at runtime, this will just reduce performance.
+			if($this->subChunks[$y]->isEmptyFast()){
 				continue;
 			}
 			break;
@@ -733,9 +725,6 @@ class Chunk{
 		foreach($this->subChunks as $y => $subChunk){
 			if($subChunk instanceof SubChunk){
 				$subChunk->collectGarbage();
-				if($subChunk->isEmpty()){
-					$this->subChunks[$y] = EmptySubChunk::getInstance();
-				}
 			}
 		}
 	}

src/world/format/EmptySubChunk.php

@@ -35,7 +35,11 @@ class EmptySubChunk implements SubChunkInterface{
 		return self::$instance;
 	}
 
-	public function isEmpty(bool $checkLight = true) : bool{
+	public function isEmptyAuthoritative() : bool{
+		return true;
+	}
+
+	public function isEmptyFast() : bool{
 		return true;
 	}
 

src/world/format/SubChunk.php

@@ -52,21 +52,13 @@ class SubChunk implements SubChunkInterface{
 		$this->blockLight = $blockLight ?? new LightArray(LightArray::ZERO);
 	}
 
-	public function isEmpty(bool $checkLight = true) : bool{
-		foreach($this->blockLayers as $layer){
-			$palette = $layer->getPalette();
-			foreach($palette as $p){
-				if($p !== $this->defaultBlock){
-					return false;
-				}
-			}
-		}
-		return
-			(!$checkLight or (
-				$this->skyLight->getData() === LightArray::FIFTEEN and
-				$this->blockLight->getData() === LightArray::ZERO
-			)
-		);
+	public function isEmptyAuthoritative() : bool{
+		$this->collectGarbage();
+		return $this->isEmptyFast();
+	}
+
+	public function isEmptyFast() : bool{
+		return empty($this->blockLayers);
 	}
 
 	public function getFullBlock(int $x, int $y, int $z) : int{

src/world/format/SubChunkInterface.php

@@ -26,11 +26,21 @@ namespace pocketmine\world\format;
 interface SubChunkInterface{
 
 	/**
-	 * @param bool $checkLight
+	 * Returns whether this subchunk contains any non-air blocks.
+	 * This function will do a slow check, usually by garbage collecting first.
+	 * This is typically useful for disk saving.
 	 *
 	 * @return bool
 	 */
-	public function isEmpty(bool $checkLight = true) : bool;
+	public function isEmptyAuthoritative() : bool;
+
+	/**
+	 * Returns a non-authoritative bool to indicate whether the chunk contains any blocks.
+	 * This is a fast check, but may be inaccurate if the chunk has been modified and not garbage-collected.
+	 *
+	 * @return bool
+	 */
+	public function isEmptyFast() : bool;
 
 	/**
 	 * @param int $x

src/world/format/io/FastChunkSerializer.php

@@ -26,7 +26,6 @@ namespace pocketmine\world\format\io;
 use pocketmine\block\BlockLegacyIds;
 use pocketmine\utils\BinaryStream;
 use pocketmine\world\format\Chunk;
-use pocketmine\world\format\EmptySubChunk;
 use pocketmine\world\format\LightArray;
 use pocketmine\world\format\PalettedBlockArray;
 use pocketmine\world\format\SubChunk;
@@ -64,9 +63,6 @@ final class FastChunkSerializer{
 			$count = 0;
 			$subStream = new BinaryStream();
 			foreach($chunk->getSubChunks() as $y => $subChunk){
-				if($subChunk instanceof EmptySubChunk){
-					continue;
-				}
 				++$count;
 
 				$subStream->putByte($y);

src/world/format/io/leveldb/LevelDB.php

@@ -438,7 +438,7 @@ class LevelDB extends BaseWorldProvider implements WritableWorldProvider{
 			$subChunks = $chunk->getSubChunks();
 			foreach($subChunks as $y => $subChunk){
 				$key = $index . self::TAG_SUBCHUNK_PREFIX . chr($y);
-				if($subChunk->isEmpty(false)){ //MCPE doesn't save light anymore as of 1.1
+				if($subChunk->isEmptyAuthoritative()){
 					$write->delete($key);
 				}else{
 					$subStream = new BinaryStream();

src/world/generator/Flat.php

@@ -156,7 +156,7 @@ class Flat extends Generator{
 
 		$count = count($this->structure);
 		for($sy = 0; $sy < $count; $sy += 16){
-			$subchunk = $this->chunk->getSubChunk($sy >> 4, true);
+			$subchunk = $this->chunk->getSubChunk($sy >> 4);
 			for($y = 0; $y < 16 and isset($this->structure[$y | $sy]); ++$y){
 				$id = $this->structure[$y | $sy];
 

src/world/light/LightPopulationTask.php

@@ -95,10 +95,10 @@ class LightPopulationTask extends AsyncTask{
 			$blockLightArrays = igbinary_unserialize($this->resultBlockLightArrays);
 
 			foreach($skyLightArrays as $y => $array){
-				$chunk->getSubChunk($y, true)->setBlockSkyLightArray($array);
+				$chunk->getSubChunk($y)->setBlockSkyLightArray($array);
 			}
 			foreach($blockLightArrays as $y => $array){
-				$chunk->getSubChunk($y, true)->setBlockLightArray($array);
+				$chunk->getSubChunk($y)->setBlockLightArray($array);
 			}
 			$chunk->setLightPopulated();
 		}

src/world/utils/SubChunkIteratorManager.php

@@ -26,8 +26,7 @@ namespace pocketmine\world\utils;
 use pocketmine\utils\Utils;
 use pocketmine\world\ChunkManager;
 use pocketmine\world\format\Chunk;
-use pocketmine\world\format\EmptySubChunk;
-use pocketmine\world\format\SubChunkInterface;
+use pocketmine\world\format\SubChunk;
 
 class SubChunkIteratorManager{
 	/** @var ChunkManager */
@@ -35,7 +34,7 @@ class SubChunkIteratorManager{
 
 	/** @var Chunk|null */
 	public $currentChunk;
-	/** @var SubChunkInterface|null */
+	/** @var SubChunk|null */
 	public $currentSubChunk;
 
 	/** @var int */
@@ -67,11 +66,12 @@ class SubChunkIteratorManager{
 		if($this->currentSubChunk === null or $this->currentY !== ($y >> 4)){
 			$this->currentY = $y >> 4;
 
-			$this->currentSubChunk = $this->currentChunk->getSubChunk($y >> 4, $create);
-			if($this->currentSubChunk instanceof EmptySubChunk){
+			if($this->currentY < 0 or $this->currentY >= $this->currentChunk->getHeight()){
 				$this->currentSubChunk = null;
 				return false;
 			}
+
+			$this->currentSubChunk = $this->currentChunk->getSubChunk($y >> 4);
 			if($this->onSubChunkChangeFunc !== null){
 				($this->onSubChunkChangeFunc)();
 			}