From 5096741b2948abe33a0e9c897d7601ae3bab32e4 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 20 Sep 2020 13:29:09 +0100 Subject: [PATCH] World::getChunk() behaviour now matches that of a regular ChunkManager Various bugs existed for a while with stuff using chunk managers instead of worlds when interacting with terrain due to a behavioural inconsistency between World::getChunk() (return from cache or load from disk), and SimpleChunkManager::getChunk() (return from cache only). This change brings the two in line. World::getOrLoadChunk() has been added as a replacement, which has the same behaviour as the old getChunk() and also makes it more obvious that there is an issue with code using it during refactoring. --- src/entity/Entity.php | 2 +- src/network/mcpe/ChunkCache.php | 2 +- src/player/Player.php | 4 +-- src/world/World.php | 46 +++++++++++++++---------- src/world/light/LightPopulationTask.php | 2 +- 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/entity/Entity.php b/src/entity/Entity.php index c90930eed..550852f40 100644 --- a/src/entity/Entity.php +++ b/src/entity/Entity.php @@ -1351,7 +1351,7 @@ abstract class Entity{ if($this->chunk !== null){ $this->chunk->removeEntity($this); } - $this->chunk = $this->getWorld()->getChunk($chunkX, $chunkZ, true); + $this->chunk = $this->getWorld()->getOrLoadChunk($chunkX, $chunkZ, true); $this->chunkX = $chunkX; $this->chunkZ = $chunkZ; diff --git a/src/network/mcpe/ChunkCache.php b/src/network/mcpe/ChunkCache.php index 5985b9e3c..22817b3ec 100644 --- a/src/network/mcpe/ChunkCache.php +++ b/src/network/mcpe/ChunkCache.php @@ -110,7 +110,7 @@ class ChunkCache implements ChunkListener{ new ChunkRequestTask( $chunkX, $chunkZ, - $this->world->getChunk($chunkX, $chunkZ), + $this->world->getOrLoadChunk($chunkX, $chunkZ), $this->caches[$chunkHash], $this->compressor, function() use ($chunkX, $chunkZ) : void{ diff --git a/src/player/Player.php b/src/player/Player.php index 1a00dc2c2..c8cb135f3 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -733,7 +733,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ $world = $world ?? $this->getWorld(); $index = World::chunkHash($x, $z); if(isset($this->usedChunks[$index])){ - foreach($world->getChunk($x, $z)->getEntities() as $entity){ + foreach($world->getOrLoadChunk($x, $z)->getEntities() as $entity){ if($entity !== $this){ $entity->despawnFrom($this); } @@ -747,7 +747,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ } protected function spawnEntitiesOnChunk(int $chunkX, int $chunkZ) : void{ - foreach($this->getWorld()->getChunk($chunkX, $chunkZ)->getEntities() as $entity){ + foreach($this->getWorld()->getOrLoadChunk($chunkX, $chunkZ)->getEntities() as $entity){ if($entity !== $this and !$entity->isFlaggedForDespawn()){ $entity->spawnTo($this); } diff --git a/src/world/World.php b/src/world/World.php index 6f736233e..c2d8306f2 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -754,7 +754,7 @@ class World implements ChunkManager{ } World::getXZ($index, $chunkX, $chunkZ); if(count($blocks) > 512){ - $chunk = $this->getChunk($chunkX, $chunkZ); + $chunk = $this->getOrLoadChunk($chunkX, $chunkZ); foreach($this->getChunkPlayers($chunkX, $chunkZ) as $p){ $p->onChunkChanged($chunk); } @@ -1241,7 +1241,7 @@ class World implements ChunkManager{ * @return int bitmap, (id << 4) | data */ public function getFullBlock(int $x, int $y, int $z) : int{ - return $this->getChunk($x >> 4, $z >> 4, false)->getFullBlock($x & 0x0f, $y, $z & 0x0f); + return $this->getOrLoadChunk($x >> 4, $z >> 4, false)->getFullBlock($x & 0x0f, $y, $z & 0x0f); } public function isInWorld(int $x, int $y, int $z) : bool{ @@ -1653,7 +1653,7 @@ class World implements ChunkManager{ if(!$this->isChunkLoaded($x, $z)){ continue; } - foreach($this->getChunk($x, $z)->getEntities() as $ent){ + foreach($this->getOrLoadChunk($x, $z)->getEntities() as $ent){ /** @var Entity|null $entity */ if($ent->canBeCollidedWith() and ($entity === null or ($ent !== $entity and $entity->canCollideWith($ent))) and $ent->boundingBox->intersectsWith($bb)){ $nearby[] = $ent; @@ -1684,7 +1684,7 @@ class World implements ChunkManager{ if(!$this->isChunkLoaded($x, $z)){ continue; } - foreach($this->getChunk($x, $z)->getEntities() as $ent){ + foreach($this->getOrLoadChunk($x, $z)->getEntities() as $ent){ if($ent !== $entity and $ent->boundingBox->intersectsWith($bb)){ $nearby[] = $ent; } @@ -1727,7 +1727,7 @@ class World implements ChunkManager{ if(!$this->isChunkLoaded($x, $z)){ continue; } - foreach($this->getChunk($x, $z)->getEntities() as $entity){ + foreach($this->getOrLoadChunk($x, $z)->getEntities() as $entity){ if(!($entity instanceof $entityType) or $entity->isFlaggedForDespawn() or (!$includeDead and !$entity->isAlive())){ continue; } @@ -1773,7 +1773,7 @@ class World implements ChunkManager{ * Returns the tile at the specified x,y,z coordinates, or null if it does not exist. */ public function getTileAt(int $x, int $y, int $z) : ?Tile{ - return ($chunk = $this->getChunk($x >> 4, $z >> 4)) !== null ? $chunk->getTile($x & 0x0f, $y, $z & 0x0f) : null; + return ($chunk = $this->getOrLoadChunk($x >> 4, $z >> 4)) !== null ? $chunk->getTile($x & 0x0f, $y, $z & 0x0f) : null; } /** @@ -1782,7 +1782,7 @@ class World implements ChunkManager{ * @return int 0-15 */ public function getBlockSkyLightAt(int $x, int $y, int $z) : int{ - return $this->getChunk($x >> 4, $z >> 4, true)->getBlockSkyLight($x & 0x0f, $y, $z & 0x0f); + return $this->getOrLoadChunk($x >> 4, $z >> 4, true)->getBlockSkyLight($x & 0x0f, $y, $z & 0x0f); } /** @@ -1791,11 +1791,11 @@ class World implements ChunkManager{ * @return int 0-15 */ public function getBlockLightAt(int $x, int $y, int $z) : int{ - return $this->getChunk($x >> 4, $z >> 4, true)->getBlockLight($x & 0x0f, $y, $z & 0x0f); + return $this->getOrLoadChunk($x >> 4, $z >> 4, true)->getBlockLight($x & 0x0f, $y, $z & 0x0f); } public function getBiomeId(int $x, int $z) : int{ - return $this->getChunk($x >> 4, $z >> 4, true)->getBiomeId($x & 0x0f, $z & 0x0f); + return $this->getOrLoadChunk($x >> 4, $z >> 4, true)->getBiomeId($x & 0x0f, $z & 0x0f); } public function getBiome(int $x, int $z) : Biome{ @@ -1803,7 +1803,7 @@ class World implements ChunkManager{ } public function setBiomeId(int $x, int $z, int $biomeId) : void{ - $this->getChunk($x >> 4, $z >> 4, true)->setBiomeId($x & 0x0f, $z & 0x0f, $biomeId); + $this->getOrLoadChunk($x >> 4, $z >> 4, true)->setBiomeId($x & 0x0f, $z & 0x0f, $biomeId); } /** @@ -1819,7 +1819,7 @@ class World implements ChunkManager{ * * @param bool $create Whether to create an empty chunk as a placeholder if the chunk does not exist */ - public function getChunk(int $chunkX, int $chunkZ, bool $create = false) : ?Chunk{ + public function getOrLoadChunk(int $chunkX, int $chunkZ, bool $create = false) : ?Chunk{ if(isset($this->chunks[$index = World::chunkHash($chunkX, $chunkZ)])){ return $this->chunks[$index]; }elseif($this->loadChunk($chunkX, $chunkZ, $create)){ @@ -1829,11 +1829,19 @@ class World implements ChunkManager{ return null; } + public function getChunk(int $chunkX, int $chunkZ, bool $create = false) : ?Chunk{ + $hash = World::chunkHash($chunkX, $chunkZ); + if(isset($this->chunks[$hash])){ + return $this->chunks[$hash]; + } + return $create ? ($this->chunks[$hash] = new Chunk($chunkX, $chunkZ)) : null; + } + /** * Returns the chunk containing the given Vector3 position. */ public function getChunkAtPosition(Vector3 $pos, bool $create = false) : ?Chunk{ - return $this->getChunk($pos->getFloorX() >> 4, $pos->getFloorZ() >> 4, $create); + return $this->getOrLoadChunk($pos->getFloorX() >> 4, $pos->getFloorZ() >> 4, $create); } /** @@ -1849,7 +1857,7 @@ class World implements ChunkManager{ if($i === 4){ continue; //center chunk } - $result[$i] = $this->getChunk($x + $xx - 1, $z + $zz - 1, false); + $result[$i] = $this->getOrLoadChunk($x + $xx - 1, $z + $zz - 1, false); } } @@ -1883,7 +1891,7 @@ class World implements ChunkManager{ unset($this->chunkPopulationQueue[$index]); if($chunk !== null){ - $oldChunk = $this->getChunk($x, $z, false); + $oldChunk = $this->getOrLoadChunk($x, $z, false); $this->setChunk($x, $z, $chunk, false); if(($oldChunk === null or !$oldChunk->isPopulated()) and $chunk->isPopulated()){ (new ChunkPopulateEvent($this, $chunk))->call(); @@ -1916,7 +1924,7 @@ class World implements ChunkManager{ $chunk->setZ($chunkZ); $chunkHash = World::chunkHash($chunkX, $chunkZ); - $oldChunk = $this->getChunk($chunkX, $chunkZ, false); + $oldChunk = $this->getOrLoadChunk($chunkX, $chunkZ, false); if($oldChunk !== null and $oldChunk !== $chunk){ if($deleteEntitiesAndTiles){ foreach($oldChunk->getEntities() as $player){ @@ -1964,7 +1972,7 @@ class World implements ChunkManager{ * @return int 0-255 */ public function getHighestBlockAt(int $x, int $z) : int{ - return $this->getChunk($x >> 4, $z >> 4, true)->getHighestBlockAt($x & 0x0f, $z & 0x0f); + return $this->getOrLoadChunk($x >> 4, $z >> 4, true)->getHighestBlockAt($x & 0x0f, $z & 0x0f); } /** @@ -1979,12 +1987,12 @@ class World implements ChunkManager{ } public function isChunkGenerated(int $x, int $z) : bool{ - $chunk = $this->getChunk($x, $z); + $chunk = $this->getOrLoadChunk($x, $z); return $chunk !== null ? $chunk->isGenerated() : false; } public function isChunkPopulated(int $x, int $z) : bool{ - $chunk = $this->getChunk($x, $z); + $chunk = $this->getOrLoadChunk($x, $z); return $chunk !== null ? $chunk->isPopulated() : false; } @@ -2360,7 +2368,7 @@ class World implements ChunkManager{ } } - $chunk = $this->getChunk($x, $z, true); + $chunk = $this->getOrLoadChunk($x, $z, true); if(!$chunk->isPopulated()){ Timings::$populationTimer->startTiming(); diff --git a/src/world/light/LightPopulationTask.php b/src/world/light/LightPopulationTask.php index bd734202f..af3499ea7 100644 --- a/src/world/light/LightPopulationTask.php +++ b/src/world/light/LightPopulationTask.php @@ -81,7 +81,7 @@ class LightPopulationTask extends AsyncTask{ $world = $this->fetchLocal(self::TLS_KEY_WORLD); if(!$world->isClosed() and $world->isChunkLoaded($this->chunkX, $this->chunkZ)){ /** @var Chunk $chunk */ - $chunk = $world->getChunk($this->chunkX, $this->chunkZ); + $chunk = $world->getOrLoadChunk($this->chunkX, $this->chunkZ); //TODO: calculated light information might not be valid if the terrain changed during light calculation /** @var int[] $heightMapArray */