Clean up entity/tile data loading from world providers

src/world/World.php

@@ -75,6 +75,7 @@ use pocketmine\utils\ReversePriorityQueue;
 use pocketmine\world\biome\Biome;
 use pocketmine\world\biome\BiomeRegistry;
 use pocketmine\world\format\Chunk;
+use pocketmine\world\format\io\ChunkData;
 use pocketmine\world\format\io\exception\CorruptedChunkException;
 use pocketmine\world\format\io\WritableWorldProvider;
 use pocketmine\world\format\LightArray;
@@ -1127,7 +1128,11 @@ class World implements ChunkManager{
 			foreach($this->chunks as $chunkHash => $chunk){
 				if($chunk->isDirty()){
 					self::getXZ($chunkHash, $chunkX, $chunkZ);
-					$this->provider->saveChunk($chunkX, $chunkZ, $chunk);
+					$this->provider->saveChunk($chunkX, $chunkZ, new ChunkData(
+						$chunk,
+						array_map(fn(Entity $e) => $e->saveNBT(), $chunk->getSavableEntities()),
+						array_map(fn(Tile $t) => $t->saveNBT(), $chunk->getTiles()),
+					));
 					$chunk->clearDirtyFlags();
 				}
 			}
@@ -2443,31 +2448,31 @@ class World implements ChunkManager{
 			return null;
 		}
 
-		$this->chunks[$chunkHash] = $chunk;
+		$this->chunks[$chunkHash] = $chunk->getChunk();
 		unset($this->blockCache[$chunkHash]);
 
 		$this->initChunk($x, $z, $chunk);
 
-		(new ChunkLoadEvent($this, $x, $z, $chunk, false))->call();
+		(new ChunkLoadEvent($this, $x, $z, $this->chunks[$chunkHash], false))->call();
 
 		if(!$this->isChunkInUse($x, $z)){
 			$this->logger->debug("Newly loaded chunk $x $z has no loaders registered, will be unloaded at next available opportunity");
 			$this->unloadChunkRequest($x, $z);
 		}
 		foreach($this->getChunkListeners($x, $z) as $listener){
-			$listener->onChunkLoaded($x, $z, $chunk);
+			$listener->onChunkLoaded($x, $z, $this->chunks[$chunkHash]);
 		}
 
 		$this->timings->syncChunkLoad->stopTiming();
 
-		return $chunk;
+		return $this->chunks[$chunkHash];
 	}
 
-	private function initChunk(int $chunkX, int $chunkZ, Chunk $chunk) : void{
+	private function initChunk(int $chunkX, int $chunkZ, ChunkData $chunkData) : void{
-		if($chunk->NBTentities !== null){
+		if(count($chunkData->getEntityNBT()) !== 0){
 			$this->timings->syncChunkLoadEntities->startTiming();
 			$entityFactory = EntityFactory::getInstance();
-			foreach($chunk->NBTentities as $k => $nbt){
+			foreach($chunkData->getEntityNBT() as $k => $nbt){
 				try{
 					$entity = $entityFactory->createFromData($this, $nbt);
 				}catch(NbtDataException $e){
@@ -2489,14 +2494,14 @@ class World implements ChunkManager{
 				//here, because entities currently add themselves to the world
 			}
 
-			$chunk->setDirtyFlag(Chunk::DIRTY_FLAG_ENTITIES, true);
+			$this->getChunk($chunkX, $chunkZ)?->setDirtyFlag(Chunk::DIRTY_FLAG_ENTITIES, true);
-			$chunk->NBTentities = null;
 			$this->timings->syncChunkLoadEntities->stopTiming();
 		}
-		if($chunk->NBTtiles !== null){
+
+		if(count($chunkData->getTileNBT()) !== 0){
 			$this->timings->syncChunkLoadTileEntities->startTiming();
 			$tileFactory = TileFactory::getInstance();
-			foreach($chunk->NBTtiles as $k => $nbt){
+			foreach($chunkData->getTileNBT() as $k => $nbt){
 				try{
 					$tile = $tileFactory->createFromData($this, $nbt);
 				}catch(NbtDataException $e){
@@ -2513,8 +2518,7 @@ class World implements ChunkManager{
 				}
 			}
 
-			$chunk->setDirtyFlag(Chunk::DIRTY_FLAG_TILES, true);
+			$this->getChunk($chunkX, $chunkZ)?->setDirtyFlag(Chunk::DIRTY_FLAG_TILES, true);
-			$chunk->NBTtiles = null;
 			$this->timings->syncChunkLoadTileEntities->stopTiming();
 		}
 	}
@@ -2564,7 +2568,11 @@ class World implements ChunkManager{
 			if($trySave and $this->getAutoSave() and $chunk->isDirty()){
 				$this->timings->syncChunkSave->startTiming();
 				try{
-					$this->provider->saveChunk($x, $z, $chunk);
+					$this->provider->saveChunk($x, $z, new ChunkData(
+						$chunk,
+						array_map(fn(Entity $e) => $e->saveNBT(), $chunk->getSavableEntities()),
+						array_map(fn(Tile $t) => $t->saveNBT(), $chunk->getTiles()),
+					));
 				}finally{
 					$this->timings->syncChunkSave->stopTiming();
 				}

src/world/format/Chunk.php

@@ -31,7 +31,6 @@ use pocketmine\block\BlockLegacyIds;
 use pocketmine\block\tile\Tile;
 use pocketmine\data\bedrock\BiomeIds;
 use pocketmine\entity\Entity;
-use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\player\Player;
 use function array_fill;
 use function array_filter;
@@ -72,18 +71,10 @@ class Chunk{
 	/** @var BiomeArray */
 	protected $biomeIds;
 
-	/** @var CompoundTag[]|null */
-	public $NBTtiles;
-
-	/** @var CompoundTag[]|null */
-	public $NBTentities;
-
 	/**
-	 * @param SubChunk[]    $subChunks
+	 * @param SubChunk[] $subChunks
-	 * @param CompoundTag[] $entities
-	 * @param CompoundTag[] $tiles
 	 */
-	public function __construct(array $subChunks = [], ?array $entities = null, ?array $tiles = null, ?BiomeArray $biomeIds = null, ?HeightArray $heightMap = null){
+	public function __construct(array $subChunks = [], ?BiomeArray $biomeIds = null, ?HeightArray $heightMap = null){
 		$this->subChunks = new \SplFixedArray(Chunk::MAX_SUBCHUNKS);
 
 		foreach($this->subChunks as $y => $null){
@@ -93,9 +84,6 @@ class Chunk{
 		$val = ($this->subChunks->getSize() * 16);
 		$this->heightMap = $heightMap ?? new HeightArray(array_fill(0, 256, $val));
 		$this->biomeIds = $biomeIds ?? BiomeArray::fill(BiomeIds::OCEAN);
-
-		$this->NBTtiles = $tiles;
-		$this->NBTentities = $entities;
 	}
 
 	/**
@@ -292,20 +280,6 @@ class Chunk{
 		}
 	}
 
-	/**
-	 * @return CompoundTag[]
-	 */
-	public function getNBTtiles() : array{
-		return $this->NBTtiles ?? array_map(function(Tile $tile) : CompoundTag{ return $tile->saveNBT(); }, $this->tiles);
-	}
-
-	/**
-	 * @return CompoundTag[]
-	 */
-	public function getNBTentities() : array{
-		return $this->NBTentities ?? array_map(function(Entity $entity) : CompoundTag{ return $entity->saveNBT(); }, $this->getSavableEntities());
-	}
-
 	public function getBiomeIdArray() : string{
 		return $this->biomeIds->getData();
 	}

src/world/format/io/ChunkData.php

@@ -0,0 +1,48 @@
+<?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\world\format\io;
+
+use pocketmine\nbt\tag\CompoundTag;
+use pocketmine\world\format\Chunk;
+
+final class ChunkData{
+
+	/**
+	 * @param CompoundTag[] $entityNBT
+	 * @param CompoundTag[] $tileNBT
+	 */
+	public function __construct(
+		private Chunk $chunk,
+		private array $entityNBT,
+		private array $tileNBT
+	){}
+
+	public function getChunk() : Chunk{ return $this->chunk; }
+
+	/** @return CompoundTag[] */
+	public function getEntityNBT() : array{ return $this->entityNBT; }
+
+	/** @return CompoundTag[] */
+	public function getTileNBT() : array{ return $this->tileNBT; }
+}

src/world/format/io/FastChunkSerializer.php

@@ -143,7 +143,7 @@ final class FastChunkSerializer{
 			$heightMap = new HeightArray(array_values($unpackedHeightMap));
 		}
 
-		$chunk = new Chunk($subChunks, null, null, $biomeIds, $heightMap);
+		$chunk = new Chunk($subChunks, $biomeIds, $heightMap);
 		$chunk->setPopulated($terrainPopulated);
 		$chunk->setLightPopulated($lightPopulated);
 		$chunk->clearDirtyFlags();

src/world/format/io/FormatConverter.php

@@ -149,7 +149,7 @@ class FormatConverter{
 		$thisRound = $start;
 		foreach($this->oldProvider->getAllChunks(true, $this->logger) as $coords => $chunk){
 			[$chunkX, $chunkZ] = $coords;
-			$chunk->setDirty();
+			$chunk->getChunk()->setDirty();
 			$new->saveChunk($chunkX, $chunkZ, $chunk);
 			$counter++;
 			if(($counter % $this->chunksPerProgressUpdate) === 0){

src/world/format/io/WorldProvider.php

@@ -23,7 +23,6 @@ declare(strict_types=1);
 
 namespace pocketmine\world\format\io;
 
-use pocketmine\world\format\Chunk;
 use pocketmine\world\format\io\exception\CorruptedChunkException;
 
 interface WorldProvider{
@@ -44,7 +43,7 @@ interface WorldProvider{
 	 *
 	 * @throws CorruptedChunkException
 	 */
-	public function loadChunk(int $chunkX, int $chunkZ) : ?Chunk;
+	public function loadChunk(int $chunkX, int $chunkZ) : ?ChunkData;
 
 	/**
 	 * Performs garbage collection in the world provider, such as cleaning up regions in Region-based worlds.
@@ -64,8 +63,8 @@ interface WorldProvider{
 	/**
 	 * Returns a generator which yields all the chunks in this world.
 	 *
-	 * @return \Generator|Chunk[]
+	 * @return \Generator|ChunkData[]
-	 * @phpstan-return \Generator<array{int, int}, Chunk, void, void>
+	 * @phpstan-return \Generator<array{int, int}, ChunkData, void, void>
 	 * @throws CorruptedChunkException
 	 */
 	public function getAllChunks(bool $skipCorrupted = false, ?\Logger $logger = null) : \Generator;

src/world/format/io/WritableWorldProvider.php

@@ -23,11 +23,9 @@ declare(strict_types=1);
 
 namespace pocketmine\world\format\io;
 
-use pocketmine\world\format\Chunk;
-
 interface WritableWorldProvider extends WorldProvider{
 	/**
 	 * Saves a chunk (usually to disk).
 	 */
-	public function saveChunk(int $chunkX, int $chunkZ, Chunk $chunk) : void;
+	public function saveChunk(int $chunkX, int $chunkZ, ChunkData $chunkData) : void;
 }

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

@@ -36,6 +36,7 @@ use pocketmine\utils\BinaryStream;
 use pocketmine\world\format\BiomeArray;
 use pocketmine\world\format\Chunk;
 use pocketmine\world\format\io\BaseWorldProvider;
+use pocketmine\world\format\io\ChunkData;
 use pocketmine\world\format\io\ChunkUtils;
 use pocketmine\world\format\io\data\BedrockWorldData;
 use pocketmine\world\format\io\exception\CorruptedChunkException;
@@ -232,7 +233,7 @@ class LevelDB extends BaseWorldProvider implements WritableWorldProvider{
 	/**
 	 * @throws CorruptedChunkException
 	 */
-	public function loadChunk(int $chunkX, int $chunkZ) : ?Chunk{
+	public function loadChunk(int $chunkX, int $chunkZ) : ?ChunkData{
 		$index = LevelDB::chunkIndex($chunkX, $chunkZ);
 
 		$chunkVersionRaw = $this->db->get($index . self::TAG_VERSION);
@@ -405,8 +406,6 @@ class LevelDB extends BaseWorldProvider implements WritableWorldProvider{
 
 		$chunk = new Chunk(
 			$subChunks,
-			$entities,
-			$tiles,
 			$biomeArray
 		);
 
@@ -423,16 +422,17 @@ class LevelDB extends BaseWorldProvider implements WritableWorldProvider{
 			$chunk->setDirty(); //trigger rewriting chunk to disk if it was converted from an older format
 		}
 
-		return $chunk;
+		return new ChunkData($chunk, $entities, $tiles);
 	}
 
-	public function saveChunk(int $chunkX, int $chunkZ, Chunk $chunk) : void{
+	public function saveChunk(int $chunkX, int $chunkZ, ChunkData $chunkData) : void{
 		$idMap = LegacyBlockIdToStringIdMap::getInstance();
 		$index = LevelDB::chunkIndex($chunkX, $chunkZ);
 
 		$write = new \LevelDBWriteBatch();
 		$write->put($index . self::TAG_VERSION, chr(self::CURRENT_LEVEL_CHUNK_VERSION));
 
+		$chunk = $chunkData->getChunk();
 		if($chunk->getDirtyFlag(Chunk::DIRTY_FLAG_TERRAIN)){
 			$subChunks = $chunk->getSubChunks();
 			foreach($subChunks as $y => $subChunk){
@@ -474,8 +474,8 @@ class LevelDB extends BaseWorldProvider implements WritableWorldProvider{
 		//TODO: use this properly
 		$write->put($index . self::TAG_STATE_FINALISATION, chr($chunk->isPopulated() ? self::FINALISATION_DONE : self::FINALISATION_NEEDS_POPULATION));
 
-		$this->writeTags($chunk->getNBTtiles(), $index . self::TAG_BLOCK_ENTITY, $write);
+		$this->writeTags($chunkData->getTileNBT(), $index . self::TAG_BLOCK_ENTITY, $write);
-		$this->writeTags($chunk->getNBTentities(), $index . self::TAG_ENTITY, $write);
+		$this->writeTags($chunkData->getEntityNBT(), $index . self::TAG_ENTITY, $write);
 
 		$write->delete($index . self::TAG_DATA_2D_LEGACY);
 		$write->delete($index . self::TAG_LEGACY_TERRAIN);

src/world/format/io/region/LegacyAnvilChunkTrait.php

@@ -31,6 +31,7 @@ use pocketmine\nbt\tag\IntArrayTag;
 use pocketmine\nbt\tag\ListTag;
 use pocketmine\world\format\BiomeArray;
 use pocketmine\world\format\Chunk;
+use pocketmine\world\format\io\ChunkData;
 use pocketmine\world\format\io\ChunkUtils;
 use pocketmine\world\format\io\exception\CorruptedChunkException;
 use pocketmine\world\format\SubChunk;
@@ -49,7 +50,7 @@ trait LegacyAnvilChunkTrait{
 	/**
 	 * @throws CorruptedChunkException
 	 */
-	protected function deserializeChunk(string $data) : Chunk{
+	protected function deserializeChunk(string $data) : ChunkData{
 		$decompressed = @zlib_decode($data);
 		if($decompressed === false){
 			throw new CorruptedChunkException("Failed to decompress chunk NBT");
@@ -89,12 +90,14 @@ trait LegacyAnvilChunkTrait{
 
 		$result = new Chunk(
 			$subChunks,
-			($entitiesTag = $chunk->getTag("Entities")) instanceof ListTag ? self::getCompoundList("Entities", $entitiesTag) : [],
-			($tilesTag = $chunk->getTag("TileEntities")) instanceof ListTag ? self::getCompoundList("TileEntities", $tilesTag) : [],
 			$biomeArray
 		);
 		$result->setPopulated($chunk->getByte("TerrainPopulated", 0) !== 0);
-		return $result;
+		return new ChunkData(
+			$result,
+			($entitiesTag = $chunk->getTag("Entities")) instanceof ListTag ? self::getCompoundList("Entities", $entitiesTag) : [],
+			($tilesTag = $chunk->getTag("TileEntities")) instanceof ListTag ? self::getCompoundList("TileEntities", $tilesTag) : [],
+		);
 	}
 
 	abstract protected function deserializeSubChunk(CompoundTag $subChunk) : SubChunk;

src/world/format/io/region/McRegion.php

@@ -33,6 +33,7 @@ use pocketmine\nbt\tag\IntArrayTag;
 use pocketmine\nbt\tag\ListTag;
 use pocketmine\world\format\BiomeArray;
 use pocketmine\world\format\Chunk;
+use pocketmine\world\format\io\ChunkData;
 use pocketmine\world\format\io\ChunkUtils;
 use pocketmine\world\format\io\exception\CorruptedChunkException;
 use pocketmine\world\format\io\SubChunkConverter;
@@ -43,7 +44,7 @@ class McRegion extends RegionWorldProvider{
 	/**
 	 * @throws CorruptedChunkException
 	 */
-	protected function deserializeChunk(string $data) : Chunk{
+	protected function deserializeChunk(string $data) : ChunkData{
 		$decompressed = @zlib_decode($data);
 		if($decompressed === false){
 			throw new CorruptedChunkException("Failed to decompress chunk NBT");
@@ -81,14 +82,13 @@ class McRegion extends RegionWorldProvider{
 			$biomeIds = $makeBiomeArray($biomesTag->getValue());
 		}
 
-		$result = new Chunk(
+		$result = new Chunk($subChunks, $biomeIds);
-			$subChunks,
+		$result->setPopulated($chunk->getByte("TerrainPopulated", 0) !== 0);
+		return new ChunkData(
+			$result,
 			($entitiesTag = $chunk->getTag("Entities")) instanceof ListTag ? self::getCompoundList("Entities", $entitiesTag) : [],
 			($tilesTag = $chunk->getTag("TileEntities")) instanceof ListTag ? self::getCompoundList("TileEntities", $tilesTag) : [],
-			$biomeIds
 		);
-		$result->setPopulated($chunk->getByte("TerrainPopulated", 0) !== 0);
-		return $result;
 	}
 
 	protected static function getRegionFileExtension() : string{

src/world/format/io/region/RegionWorldProvider.php

@@ -27,8 +27,8 @@ use pocketmine\nbt\NBT;
 use pocketmine\nbt\tag\ByteArrayTag;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\ListTag;
-use pocketmine\world\format\Chunk;
 use pocketmine\world\format\io\BaseWorldProvider;
+use pocketmine\world\format\io\ChunkData;
 use pocketmine\world\format\io\data\JavaWorldData;
 use pocketmine\world\format\io\exception\CorruptedChunkException;
 use pocketmine\world\format\io\WorldData;
@@ -146,7 +146,7 @@ abstract class RegionWorldProvider extends BaseWorldProvider{
 	/**
 	 * @throws CorruptedChunkException
 	 */
-	abstract protected function deserializeChunk(string $data) : Chunk;
+	abstract protected function deserializeChunk(string $data) : ChunkData;
 
 	/**
 	 * @return CompoundTag[]
@@ -185,7 +185,7 @@ abstract class RegionWorldProvider extends BaseWorldProvider{
 	/**
 	 * @throws CorruptedChunkException
 	 */
-	public function loadChunk(int $chunkX, int $chunkZ) : ?Chunk{
+	public function loadChunk(int $chunkX, int $chunkZ) : ?ChunkData{
 		$regionX = $regionZ = null;
 		self::getRegionIndex($chunkX, $chunkZ, $regionX, $regionZ);
 		assert(is_int($regionX) and is_int($regionZ));

src/world/format/io/region/WritableRegionWorldProvider.php

@@ -23,7 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\world\format\io\region;
 
-use pocketmine\world\format\Chunk;
+use pocketmine\world\format\io\ChunkData;
 use pocketmine\world\format\io\data\JavaWorldData;
 use pocketmine\world\format\io\WritableWorldProvider;
 use pocketmine\world\WorldCreationOptions;
@@ -51,10 +51,10 @@ abstract class WritableRegionWorldProvider extends RegionWorldProvider implement
 		JavaWorldData::generate($path, $name, $options, static::getPcWorldFormatVersion());
 	}
 
-	abstract protected function serializeChunk(Chunk $chunk) : string;
+	abstract protected function serializeChunk(ChunkData $chunk) : string;
 
-	public function saveChunk(int $chunkX, int $chunkZ, Chunk $chunk) : void{
+	public function saveChunk(int $chunkX, int $chunkZ, ChunkData $chunkData) : void{
 		self::getRegionIndex($chunkX, $chunkZ, $regionX, $regionZ);
-		$this->loadRegion($regionX, $regionZ)->writeChunk($chunkX & 0x1f, $chunkZ & 0x1f, $this->serializeChunk($chunk));
+		$this->loadRegion($regionX, $regionZ)->writeChunk($chunkX & 0x1f, $chunkZ & 0x1f, $this->serializeChunk($chunkData));
 	}
 }