From 0ecd68e4a777a570cacf35d02f7ba5d2dd678c47 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 27 Oct 2020 18:01:31 +0000 Subject: [PATCH] LightUpdate: remove premature optimisation which breaks when mass-update lighting is used when setBlockSkyLightArray/setBlockLightArray was used, currentLightArray would retain a reference to the old light array, which would cause false readings if SubChunkExplorer didn't move away from that subchunk and back. This causes a small degradation of performance, but I think it can be implemented differently anyway. This also fixes #3816. --- src/world/light/BlockLightUpdate.php | 5 +++-- src/world/light/LightUpdate.php | 23 +++++++++++------------ src/world/light/SkyLightUpdate.php | 5 +++-- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/world/light/BlockLightUpdate.php b/src/world/light/BlockLightUpdate.php index 323e5cd36..6cd86e4d7 100644 --- a/src/world/light/BlockLightUpdate.php +++ b/src/world/light/BlockLightUpdate.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace pocketmine\world\light; +use pocketmine\world\format\LightArray; use pocketmine\world\utils\SubChunkExplorer; use pocketmine\world\utils\SubChunkExplorerStatus; use function max; @@ -46,8 +47,8 @@ class BlockLightUpdate extends LightUpdate{ $this->lightEmitters = $lightEmitters; } - protected function updateLightArrayRef() : void{ - $this->currentLightArray = $this->subChunkExplorer->currentSubChunk->getBlockLightArray(); + protected function getCurrentLightArray() : LightArray{ + return $this->subChunkExplorer->currentSubChunk->getBlockLightArray(); } public function recalculateNode(int $x, int $y, int $z) : void{ diff --git a/src/world/light/LightUpdate.php b/src/world/light/LightUpdate.php index 639e3416a..8fb5569e2 100644 --- a/src/world/light/LightUpdate.php +++ b/src/world/light/LightUpdate.php @@ -47,9 +47,6 @@ abstract class LightUpdate{ /** @var SubChunkExplorer */ protected $subChunkExplorer; - /** @var LightArray|null */ - protected $currentLightArray = null; - /** * @param \SplFixedArray|int[] $lightFilters * @phpstan-param \SplFixedArray $lightFilters @@ -58,16 +55,15 @@ abstract class LightUpdate{ $this->lightFilters = $lightFilters; $this->subChunkExplorer = $subChunkExplorer; - $this->subChunkExplorer->onSubChunkChange(\Closure::fromCallable([$this, 'updateLightArrayRef'])); } - abstract protected function updateLightArrayRef() : void; + abstract protected function getCurrentLightArray() : LightArray; abstract public function recalculateNode(int $x, int $y, int $z) : void; protected function getEffectiveLight(int $x, int $y, int $z) : int{ if($this->subChunkExplorer->moveTo($x, $y, $z, false) !== SubChunkExplorerStatus::INVALID){ - return $this->currentLightArray->get($x & 0xf, $y & 0xf, $z & 0xf); + return $this->getCurrentLightArray()->get($x & 0xf, $y & 0xf, $z & 0xf); } return 0; } @@ -97,10 +93,11 @@ abstract class LightUpdate{ $context = new LightPropagationContext(); foreach($this->updateNodes as $blockHash => [$x, $y, $z, $newLevel]){ if($this->subChunkExplorer->moveTo($x, $y, $z, false) !== SubChunkExplorerStatus::INVALID){ - $oldLevel = $this->currentLightArray->get($x & 0xf, $y & 0xf, $z & 0xf); + $lightArray = $this->getCurrentLightArray(); + $oldLevel = $lightArray->get($x & 0xf, $y & 0xf, $z & 0xf); if($oldLevel !== $newLevel){ - $this->currentLightArray->set($x & 0xf, $y & 0xf, $z & 0xf, $newLevel); + $lightArray->set($x & 0xf, $y & 0xf, $z & 0xf, $newLevel); if($oldLevel < $newLevel){ //light increased $context->spreadVisited[$blockHash] = true; $context->spreadQueue->enqueue([$x, $y, $z]); @@ -172,10 +169,11 @@ abstract class LightUpdate{ } protected function computeRemoveLight(int $x, int $y, int $z, int $oldAdjacentLevel, LightPropagationContext $context) : void{ - $current = $this->currentLightArray->get($x & 0xf, $y & 0xf, $z & 0xf); + $lightArray = $this->getCurrentLightArray(); + $current = $lightArray->get($x & 0xf, $y & 0xf, $z & 0xf); if($current !== 0 and $current < $oldAdjacentLevel){ - $this->currentLightArray->set($x & 0xf, $y & 0xf, $z & 0xf, 0); + $lightArray->set($x & 0xf, $y & 0xf, $z & 0xf, 0); if(!isset($context->removalVisited[$index = World::blockHash($x, $y, $z)])){ $context->removalVisited[$index] = true; @@ -192,11 +190,12 @@ abstract class LightUpdate{ } protected function computeSpreadLight(int $x, int $y, int $z, int $newAdjacentLevel, LightPropagationContext $context) : void{ - $current = $this->currentLightArray->get($x & 0xf, $y & 0xf, $z & 0xf); + $lightArray = $this->getCurrentLightArray(); + $current = $lightArray->get($x & 0xf, $y & 0xf, $z & 0xf); $potentialLight = $newAdjacentLevel - $this->lightFilters[$this->subChunkExplorer->currentSubChunk->getFullBlock($x & 0x0f, $y & 0x0f, $z & 0x0f)]; if($current < $potentialLight){ - $this->currentLightArray->set($x & 0xf, $y & 0xf, $z & 0xf, $potentialLight); + $lightArray->set($x & 0xf, $y & 0xf, $z & 0xf, $potentialLight); if(!isset($context->spreadVisited[$index = World::blockHash($x, $y, $z)]) and $potentialLight > 1){ $context->spreadVisited[$index] = true; diff --git a/src/world/light/SkyLightUpdate.php b/src/world/light/SkyLightUpdate.php index 6927463cb..daaa99239 100644 --- a/src/world/light/SkyLightUpdate.php +++ b/src/world/light/SkyLightUpdate.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace pocketmine\world\light; +use pocketmine\world\format\LightArray; use pocketmine\world\utils\SubChunkExplorer; use pocketmine\world\utils\SubChunkExplorerStatus; use pocketmine\world\World; @@ -47,8 +48,8 @@ class SkyLightUpdate extends LightUpdate{ $this->directSkyLightBlockers = $directSkyLightBlockers; } - protected function updateLightArrayRef() : void{ - $this->currentLightArray = $this->subChunkExplorer->currentSubChunk->getBlockSkyLightArray(); + protected function getCurrentLightArray() : LightArray{ + return $this->subChunkExplorer->currentSubChunk->getBlockSkyLightArray(); } protected function getEffectiveLight(int $x, int $y, int $z) : int{