Player: fixed broken behaviour of entity spawning on chunk send, closes #3355

src/entity/Entity.php

@@ -1528,7 +1528,10 @@ abstract class Entity{
 
 	public function spawnTo(Player $player) : void{
 		$id = spl_object_id($player);
-		if(!isset($this->hasSpawned[$id]) and $player->isUsingChunk($this->location->getFloorX() >> 4, $this->location->getFloorZ() >> 4)){
+		//TODO: this will cause some visible lag during chunk resends; if the player uses a spawn egg in a chunk, the
+		//created entity won't be visible until after the resend arrives. However, this is better than possibly crashing
+		//the player by sending them entities too early.
+		if(!isset($this->hasSpawned[$id]) and $player->hasReceivedChunk($this->location->getFloorX() >> 4, $this->location->getFloorZ() >> 4)){
 			$this->hasSpawned[$id] = $player;
 
 			$this->sendSpawnPacket($player);

src/player/Player.php

@@ -189,7 +189,10 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 	/** @var GameMode */
 	protected $gamemode;
 
-	/** @var bool[] chunkHash => bool (true = sent, false = needs sending) */
+	/**
+	 * @var UsedChunkStatus[] chunkHash => status
+	 * @phpstan-var array<int, UsedChunkStatus>
+	 */
 	protected $usedChunks = [];
 	/** @var bool[] chunkHash => dummy */
 	protected $loadQueue = [];
@@ -286,7 +289,7 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		//load the spawn chunk so we can see the terrain
 		$world->registerChunkLoader($this, $spawn->getFloorX() >> 4, $spawn->getFloorZ() >> 4, true);
 		$world->registerChunkListener($this, $spawn->getFloorX() >> 4, $spawn->getFloorZ() >> 4);
-		$this->usedChunks[World::chunkHash($spawn->getFloorX() >> 4, $spawn->getFloorZ() >> 4)] = false;
+		$this->usedChunks[World::chunkHash($spawn->getFloorX() >> 4, $spawn->getFloorZ() >> 4)] = UsedChunkStatus::NEEDED();
 
 		if($namedtag === null){
 			$namedtag = EntityFactory::createBaseNBT($spawn);
@@ -717,7 +720,7 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		$oldWorld = $this->location->getWorld();
 		if(parent::switchWorld($targetWorld)){
 			if($oldWorld !== null){
-				foreach($this->usedChunks as $index => $d){
+				foreach($this->usedChunks as $index => $status){
 					World::getXZ($index, $X, $Z);
 					$this->unloadChunk($X, $Z, $oldWorld);
 				}
@@ -778,7 +781,7 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 
 			++$count;
 
-			$this->usedChunks[$index] = false;
+			$this->usedChunks[$index] = UsedChunkStatus::NEEDED();
 			$this->getWorld()->registerChunkLoader($this, $X, $Z, true);
 			$this->getWorld()->registerChunkListener($this, $X, $Z);
 
@@ -787,21 +790,21 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 			}
 
 			unset($this->loadQueue[$index]);
-			$this->usedChunks[$index] = true;
+			$this->usedChunks[$index] = UsedChunkStatus::REQUESTED();
 
-			$this->networkSession->startUsingChunk($X, $Z, function(int $chunkX, int $chunkZ) : void{
+			$this->networkSession->startUsingChunk($X, $Z, function(int $chunkX, int $chunkZ) use ($index) : void{
+				$this->usedChunks[$index] = UsedChunkStatus::SENT();
 				if($this->spawned){
 					$this->spawnEntitiesOnChunk($chunkX, $chunkZ);
 				}elseif($this->spawnChunkLoadCount++ === $this->spawnThreshold){
 					$this->spawned = true;
 
-					foreach($this->usedChunks as $chunkHash => $hasSent){
+					foreach($this->usedChunks as $chunkHash => $status){
-						if(!$hasSent){
+						if($status->equals(UsedChunkStatus::SENT())){
-							continue;
-						}
 							World::getXZ($chunkHash, $_x, $_z);
 							$this->spawnEntitiesOnChunk($_x, $_z);
 						}
+					}
 
 					$this->networkSession->onTerrainReady();
 				}
@@ -894,13 +897,13 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		$unloadChunks = $this->usedChunks;
 
 		foreach($this->selectChunks() as $hash){
-			if(!isset($this->usedChunks[$hash]) or $this->usedChunks[$hash] === false){
+			if(!isset($this->usedChunks[$hash]) or $this->usedChunks[$hash]->equals(UsedChunkStatus::NEEDED())){
 				$newOrder[$hash] = true;
 			}
 			unset($unloadChunks[$hash]);
 		}
 
-		foreach($unloadChunks as $index => $bool){
+		foreach($unloadChunks as $index => $status){
 			World::getXZ($index, $X, $Z);
 			$this->unloadChunk($X, $Z);
 		}
@@ -917,6 +920,11 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		return isset($this->usedChunks[World::chunkHash($chunkX, $chunkZ)]);
 	}
 
+	public function hasReceivedChunk(int $chunkX, int $chunkZ) : bool{
+		$status = $this->usedChunks[World::chunkHash($chunkX, $chunkZ)] ?? null;
+		return $status !== null and $status->equals(UsedChunkStatus::SENT());
+	}
+
 	public function doChunkRequests() : void{
 		if($this->nextChunkOrderRun !== PHP_INT_MAX and $this->nextChunkOrderRun-- <= 0){
 			$this->nextChunkOrderRun = PHP_INT_MAX;
@@ -1982,7 +1990,7 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		$this->hiddenPlayers = [];
 
 		if($this->location->isValid()){
-			foreach($this->usedChunks as $index => $d){
+			foreach($this->usedChunks as $index => $status){
 				World::getXZ($index, $chunkX, $chunkZ);
 				$this->unloadChunk($chunkX, $chunkZ);
 			}
@@ -2342,7 +2350,7 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 
 	public function onChunkChanged(Chunk $chunk) : void{
 		if(isset($this->usedChunks[$hash = World::chunkHash($chunk->getX(), $chunk->getZ())])){
-			$this->usedChunks[$hash] = false;
+			$this->usedChunks[$hash] = UsedChunkStatus::NEEDED();
 			$this->nextChunkOrderRun = 0;
 		}
 	}

src/player/UsedChunkStatus.php

@@ -0,0 +1,47 @@
+<?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\player;
+
+use pocketmine\utils\EnumTrait;
+
+/**
+ * This doc-block is generated automatically, do not modify it manually.
+ * This must be regenerated whenever registry members are added, removed or changed.
+ * @see RegistryTrait::_generateMethodAnnotations()
+ *
+ * @method static self NEEDED()
+ * @method static self REQUESTED()
+ * @method static self SENT()
+ */
+final class UsedChunkStatus{
+	use EnumTrait;
+
+	protected static function setup() : void{
+		self::registerAll(
+			new self("NEEDED"),
+			new self("REQUESTED"),
+			new self("SENT")
+		);
+	}
+}