1 files changed, 88 insertions, 271 deletions

diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp
index 4ec01744b..ffb968a0c 100644
--- a/src/ClientHandle.cpp
+++ b/src/ClientHandle.cpp
@@ -68,13 +68,12 @@ float cClientHandle::FASTBREAK_PERCENTAGE;
 // cClientHandle:
 
 cClientHandle::cClientHandle(const AString & a_IPString, int a_ViewDistance) :
-	m_LastSentDimension(dimNotSet),
 	m_ForgeHandshake(this),
 	m_CurrentViewDistance(a_ViewDistance),
 	m_RequestedViewDistance(a_ViewDistance),
 	m_IPString(a_IPString),
 	m_Player(nullptr),
-	m_CachedSentChunk(0, 0),
+	m_CachedSentChunk(0x7fffffff, 0x7fffffff),
 	m_HasSentDC(false),
 	m_LastStreamedChunkX(0x7fffffff),  // bogus chunk coords to force streaming upon login
 	m_LastStreamedChunkZ(0x7fffffff),
@@ -116,26 +115,6 @@ cClientHandle::~cClientHandle()
 
 	LOGD("Deleting client \"%s\" at %p", GetUsername().c_str(), static_cast<void *>(this));
 
-	{
-		cCSLock Lock(m_CSChunkLists);
-		m_LoadedChunks.clear();
-		m_ChunksToSend.clear();
-	}
-
-	if (m_Player != nullptr)
-	{
-		cWorld * World = m_Player->GetWorld();
-		if (World != nullptr)
-		{
-			m_Player->GetWorld()->RemoveClientFromChunkSender(this);
-		}
-		// Send the Offline PlayerList packet:
-		cRoot::Get()->BroadcastPlayerListsRemovePlayer(*m_Player);
-
-		m_PlayerPtr.reset();
-		m_Player = nullptr;
-	}
-
 	LOGD("ClientHandle at %p deleted", static_cast<void *>(this));
 }
 
@@ -145,12 +124,7 @@ cClientHandle::~cClientHandle()
 
 void cClientHandle::Destroy(void)
 {
-	{
-		cCSLock Lock(m_CSOutgoingData);
-		m_Link.reset();
-	}
-
-	if (!SetState(csDestroying))
+	if (!SetState(csDestroyed))
 	{
 		// Already called
 		LOGD("%s: client %p, \"%s\" already destroyed, bailing out", __FUNCTION__, static_cast<void *>(this), m_Username.c_str());
@@ -158,41 +132,12 @@ void cClientHandle::Destroy(void)
 	}
 
 	LOGD("%s: destroying client %p, \"%s\" @ %s", __FUNCTION__, static_cast<void *>(this), m_Username.c_str(), m_IPString.c_str());
-	auto player = m_Player;
-	auto Self = std::move(m_Self);  // Keep ourself alive for at least as long as this function
-	SetState(csDestroyed);
-
-	if (player == nullptr)
-	{
-		return;
-	}
-
-	// Atomically decrement player count (in world or server thread)
-	cRoot::Get()->GetServer()->PlayerDestroyed();
 
-	auto world = player->GetWorld();
-	if (world != nullptr)
 	{
-		player->StopEveryoneFromTargetingMe();
-		player->SetIsTicking(false);
-
-		if (!m_PlayerPtr)
-		{
-			// If our own smart pointer is unset, player has been transferred to world
-			ASSERT(world->IsPlayerReferencedInWorldOrChunk(*player));
-
-			m_PlayerPtr = world->RemovePlayer(*player);
-
-			// And RemovePlayer should have returned a valid smart pointer
-			ASSERT(m_PlayerPtr);
-		}
-		else
-		{
-			// If ownership was not transferred, our own smart pointer should be valid and RemovePlayer's should not
-			ASSERT(!world->IsPlayerReferencedInWorldOrChunk(*player));
-		}
+		cCSLock Lock(m_CSOutgoingData);
+		m_Link->Shutdown();  // Cleanly close the connection
+		m_Link.reset();  // Release the strong reference cTCPLink holds to ourself
 	}
-	player->RemoveClientHandle();
 }
 
 
@@ -301,9 +246,6 @@ void cClientHandle::Kick(const AString & a_Reason)
 
 void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties)
 {
-	// Atomically increment player count (in server thread)
-	cRoot::Get()->GetServer()->PlayerCreated();
-
 	{
 		cCSLock lock(m_CSState);
 		/*
@@ -351,95 +293,67 @@ void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, c
 
 void cClientHandle::FinishAuthenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties)
 {
-	cWorld * World;
-	{
-		// Spawn player (only serversided, so data is loaded)
-		m_PlayerPtr = std::make_unique<cPlayer>(m_Self, GetUsername());
-		m_Player = m_PlayerPtr.get();
-		/*
-		LOGD("Created a new cPlayer object at %p for client %s @ %s (%p)",
-			static_cast<void *>(m_Player),
-			m_Username.c_str(), m_IPString.c_str(), static_cast<void *>(this)
-		);
-		//*/
-		InvalidateCachedSentChunk();
-		m_Self.reset();
+	// Serverside spawned player (so data are loaded).
+	auto Player = std::make_unique<cPlayer>(shared_from_this());
+	m_Player = Player.get();
 
+	/*
+	LOGD("Created a new cPlayer object at %p for client %s @ %s (%p)",
+		static_cast<void *>(m_Player),
+		m_Username.c_str(), m_IPString.c_str(), static_cast<void *>(this)
+	);
+	//*/
 
-		// New player use default world
-		// Player who can load from disk, use loaded world
-		if (m_Player->GetWorld() == nullptr)
-		{
-			World = cRoot::Get()->GetWorld(m_Player->GetLoadedWorldName());
-			if (World == nullptr)
-			{
-				World = cRoot::Get()->GetDefaultWorld();
-				m_Player->SetPosition(World->GetSpawnX(), World->GetSpawnY(), World->GetSpawnZ());
-			}
-			m_Player->SetWorld(World);
-		}
-		else
-		{
-			World = m_Player->GetWorld();
-		}
-
-		m_Player->SetIP (m_IPString);
+	cWorld * World = cRoot::Get()->GetWorld(m_Player->GetLoadedWorldName());
+	if (World == nullptr)
+	{
+		World = cRoot::Get()->GetDefaultWorld();
+	}
 
-		if (!cRoot::Get()->GetPluginManager()->CallHookPlayerJoined(*m_Player))
-		{
-			cRoot::Get()->BroadcastChatJoin(Printf("%s has joined the game", GetUsername().c_str()));
-			LOGINFO("Player %s has joined the game", m_Username.c_str());
-		}
+	// Atomically increment player count (in server thread):
+	cRoot::Get()->GetServer()->PlayerCreated();
 
-		m_ConfirmPosition = m_Player->GetPosition();
+	if (!cRoot::Get()->GetPluginManager()->CallHookPlayerJoined(*m_Player))
+	{
+		cRoot::Get()->BroadcastChatJoin(Printf("%s has joined the game", a_Name.c_str()));
+		LOGINFO("Player %s has joined the game", a_Name.c_str());
+	}
 
-		// Return a server login packet
-		m_Protocol->SendLogin(*m_Player, *World);
-		m_LastSentDimension = World->GetDimension();
+	// TODO: this accesses the world spawn from the authenticator thread
+	// World spawn should be sent in OnAddedToWorld.
+	// Return a server login packet:
+	m_Protocol->SendLogin(*m_Player, *World);
 
-		// Send Weather if raining:
-		if ((World->GetWeather() == 1) || (World->GetWeather() == 2))
+	if (m_Player->GetKnownRecipes().empty())
+	{
+		SendInitRecipes(0);
+	}
+	else
+	{
+		for (const auto KnownRecipe : m_Player->GetKnownRecipes())
 		{
-			m_Protocol->SendWeather(World->GetWeather());
+			SendInitRecipes(KnownRecipe);
 		}
-
-		// Send time:
-		m_Protocol->SendTimeUpdate(World->GetWorldAge(), World->GetTimeOfDay(), World->IsDaylightCycleEnabled());
-
-		// Send contents of the inventory window
-		m_Protocol->SendWholeInventory(*m_Player->GetWindow());
-
-		// Send health
-		m_Player->SendHealth();
-
-		// Send experience
-		m_Player->SendExperience();
-
-		// Send hotbar active slot
-		m_Player->SendHotbarActiveSlot();
-
-		// Send player list items
-		SendPlayerListAddPlayer(*m_Player);
-		cRoot::Get()->BroadcastPlayerListsAddPlayer(*m_Player);
-		cRoot::Get()->SendPlayerLists(m_Player);
-
-		SetState(csAuthenticated);
 	}
 
-	// Query player team
-	m_Player->UpdateTeam();
-
-	// Send scoreboard data
-	World->GetScoreBoard().SendTo(*this);
+	// Send player list items:
+	SendPlayerListAddPlayer(*m_Player);  // Add ourself
+	cRoot::Get()->BroadcastPlayerListsAddPlayer(*m_Player);  // Add ourself to everyone else
+	cRoot::Get()->SendPlayerLists(m_Player);  // Add everyone else to ourself
 
-	// Send statistics
+	// Send statistics:
 	SendStatistics(m_Player->GetStatManager());
 
 	// Delay the first ping until the client "settles down"
 	// This should fix #889, "BadCast exception, cannot convert bit to fm" error in client
 	m_PingStartTime = std::chrono::steady_clock::now() + std::chrono::seconds(3);  // Send the first KeepAlive packet in 3 seconds
 
-	cRoot::Get()->GetPluginManager()->CallHookPlayerSpawned(*m_Player);
+	// Remove the client handle from the server, it will be ticked from its cPlayer object from now on:
+	cRoot::Get()->GetServer()->ClientMovedToWorld(this);
+
+	SetState(csDownloadingWorld);
+	m_Player->Initialize(std::move(Player), *World);
+
 	// LOGD("Client %s @ %s (%p) has been fully authenticated", m_Username.c_str(), m_IPString.c_str(), static_cast<void *>(this));
 }
 
@@ -449,10 +363,6 @@ void cClientHandle::FinishAuthenticate(const AString & a_Name, const cUUID & a_U
 
 bool cClientHandle::StreamNextChunk(void)
 {
-	if ((m_State < csAuthenticated) || (m_State >= csDestroying))
-	{
-		return true;
-	}
 	ASSERT(m_Player != nullptr);
 
 	int ChunkPosX = m_Player->GetChunkX();
@@ -609,12 +519,6 @@ void cClientHandle::UnloadOutOfRangeChunks(void)
 
 void cClientHandle::StreamChunk(int a_ChunkX, int a_ChunkZ, cChunkSender::Priority a_Priority)
 {
-	if (m_State >= csDestroying)
-	{
-		// Don't stream chunks to clients that are being destroyed
-		return;
-	}
-
 	cWorld * World = m_Player->GetWorld();
 	ASSERT(World != nullptr);
 
@@ -820,12 +724,6 @@ void cClientHandle::HandlePlayerAbilities(bool a_IsFlying, float FlyingSpeed, fl
 
 void cClientHandle::HandlePlayerPos(double a_PosX, double a_PosY, double a_PosZ, double a_Stance, bool a_IsOnGround)
 {
-	if ((m_Player == nullptr) || (m_State != csPlaying))
-	{
-		// The client hasn't been spawned yet and sends nonsense, we know better
-		return;
-	}
-
 	if (m_Player->IsFrozen())
 	{
 		// Ignore client-side updates if the player is frozen
@@ -1580,11 +1478,6 @@ void cClientHandle::HandleChat(const AString & a_Message)
 
 void cClientHandle::HandlePlayerLook(float a_Rotation, float a_Pitch, bool a_IsOnGround)
 {
-	if ((m_Player == nullptr) || (m_State != csPlaying))
-	{
-		return;
-	}
-
 	m_Player->SetYaw        (a_Rotation);
 	m_Player->SetHeadYaw    (a_Rotation);
 	m_Player->SetPitch      (a_Pitch);
@@ -1838,11 +1731,12 @@ void cClientHandle::HandleUseItem(eHand a_Hand)
 
 void cClientHandle::HandleRespawn(void)
 {
-	if (m_Player == nullptr)
+	if (m_Player->GetHealth() > 0)
 	{
-		Destroy();
+		Kick("What is not dead may not live again. Hacked client?");
 		return;
 	}
+
 	m_Player->Respawn();
 	cRoot::Get()->GetPluginManager()->CallHookPlayerSpawned(*m_Player);
 }
@@ -1961,10 +1855,6 @@ void cClientHandle::HandleEntitySprinting(UInt32 a_EntityID, bool a_IsSprinting)
 
 void cClientHandle::HandleUnmount(void)
 {
-	if (m_Player == nullptr)
-	{
-		return;
-	}
 	m_Player->Detach();
 }
 
@@ -2009,8 +1899,14 @@ void cClientHandle::SendData(const ContiguousByteBufferView a_Data)
 		return;
 	}
 
-	cCSLock Lock(m_CSOutgoingData);
-	m_OutgoingData += a_Data;
+	// Due to cTCPLink's design of holding a strong pointer to ourself, we need to explicitly reset m_Link.
+	// This means we need to check it's not nullptr before trying to send, but also capture the link,
+	// to prevent it being reset between the null check and the Send:
+	if (auto Link = m_Link; Link != nullptr)
+	{
+		cCSLock Lock(m_CSOutgoingData);
+		Link->Send(a_Data.data(), a_Data.size());
+	}
 }
 
 
@@ -2087,28 +1983,15 @@ void cClientHandle::Tick(float a_Dt)
 
 	try
 	{
-		ProcessProtocolInOut();
+		ProcessProtocolIn();
 	}
 	catch (const std::exception & Oops)
 	{
 		Kick(Oops.what());
-		return;  // Return early to give a chance to send the kick packet before link shutdown
 	}
 
-	// If player has been kicked, terminate the connection:
-	if (m_State == csKicked)
+	if (IsDestroyed())
 	{
-		m_Link->Shutdown();
-	}
-
-	// If destruction is queued, destroy now:
-	if (m_State == csQueuedForDestruction)
-	{
-		LOGD("Client %s @ %s (%p) has been queued for destruction, destroying now.",
-			m_Username.c_str(), m_IPString.c_str(), static_cast<void *>(this)
-		);
-		GetPlayer()->GetStatManager().AddValue(Statistic::LeaveGame);
-		Destroy();
 		return;
 	}
 
@@ -2119,12 +2002,6 @@ void cClientHandle::Tick(float a_Dt)
 		return;
 	}
 
-	// Only process further if the player object is valid:
-	if (m_Player == nullptr)
-	{
-		return;
-	}
-
 	// Freeze the player if they are standing in a chunk not yet sent to the client
 	m_HasSentPlayerChunk = false;
 	if (m_Player->GetParentChunk() != nullptr)
@@ -2160,6 +2037,12 @@ void cClientHandle::Tick(float a_Dt)
 		{
 			m_Protocol->SendPlayerMoveLook();
 			m_State = csPlaying;
+
+			// Send resource pack (after a MoveLook, because sending it before the initial MoveLook cancels the download screen):
+			if (const auto & ResourcePackUrl = cRoot::Get()->GetServer()->GetResourcePackUrl(); !ResourcePackUrl.empty())
+			{
+				SendResourcePack(ResourcePackUrl);
+			}
 		}
 	}  // lock(m_CSState)
 
@@ -2174,24 +2057,21 @@ void cClientHandle::Tick(float a_Dt)
 		}
 	}
 
-	if ((m_State >= csAuthenticated) && (m_State < csQueuedForDestruction))
+	// Stream 4 chunks per tick
+	for (int i = 0; i < 4; i++)
 	{
-		// Stream 4 chunks per tick
-		for (int i = 0; i < 4; i++)
+		// Stream the next chunk
+		if (StreamNextChunk())
 		{
-			// Stream the next chunk
-			if (StreamNextChunk())
-			{
-				// Streaming finished. All chunks are loaded.
-				break;
-			}
+			// Streaming finished. All chunks are loaded.
+			break;
 		}
+	}
 
-		// Unload all chunks that are out of the view distance (every 5 seconds)
-		if ((m_Player->GetWorld()->GetWorldAge() % 100) == 0)
-		{
-			UnloadOutOfRangeChunks();
-		}
+	// Unload all chunks that are out of the view distance (every 5 seconds)
+	if ((m_Player->GetWorld()->GetWorldAge() % 100) == 0)
+	{
+		UnloadOutOfRangeChunks();
 	}
 
 	// Handle block break animation:
@@ -2220,33 +2100,7 @@ void cClientHandle::Tick(float a_Dt)
 
 void cClientHandle::ServerTick(float a_Dt)
 {
-	ProcessProtocolInOut();
-
-	// If destruction is queued, destroy now:
-	if (m_State == csQueuedForDestruction)
-	{
-		LOGD("Client %s @ %s (%p) has been queued for destruction, destroying now.",
-			m_Username.c_str(), m_IPString.c_str(), static_cast<void *>(this)
-		);
-		Destroy();
-		return;
-	}
-
-	{
-		cCSLock lock(m_CSState);
-		if (m_State == csAuthenticated)
-		{
-			StreamNextChunk();
-
-			// Remove the client handle from the server, it will be ticked from its cPlayer object from now on
-			cRoot::Get()->GetServer()->ClientMovedToWorld(this);
-
-			// Add the player to the world (start ticking from there):
-			m_State = csDownloadingWorld;
-			m_Player->Initialize(std::move(m_PlayerPtr), *(m_Player->GetWorld()));
-			return;
-		}
-	}  // lock(m_CSState)
+	ProcessProtocolIn();
 
 	m_TicksSinceLastPacket += 1;
 	if (m_TicksSinceLastPacket > 600)  // 30 seconds
@@ -2538,10 +2392,7 @@ void cClientHandle::SendDisconnect(const AString & a_Reason)
 		LOGD("Sending a DC: \"%s\"", StripColorCodes(a_Reason).c_str());
 		m_Protocol.SendDisconnect(*this, a_Reason);
 		m_HasSentDC = true;
-		// csKicked means m_Link will be shut down on the next tick. The
-		// disconnect packet data is sent in the tick thread so the connection
-		// is closed there after the data is sent.
-		SetState(csKicked);
+		Destroy();
 	}
 }
 
@@ -2906,19 +2757,18 @@ void cClientHandle::SendResetTitle()
 
 void cClientHandle::SendRespawn(eDimension a_Dimension, bool a_ShouldIgnoreDimensionChecks)
 {
-	if ((!a_ShouldIgnoreDimensionChecks) && (a_Dimension == m_LastSentDimension))
+	if (!a_ShouldIgnoreDimensionChecks && (a_Dimension == m_Player->GetWorld()->GetDimension()))
 	{
 		// The client goes crazy if we send a respawn packet with the dimension of the current world
 		// So we send a temporary one first.
 		// This is not needed when the player dies, hence the a_ShouldIgnoreDimensionChecks flag.
-		// a_ShouldIgnoreDimensionChecks is true only at cPlayer::respawn, which is called after
+		// a_ShouldIgnoreDimensionChecks is true only at cPlayer::Respawn, which is called after
 		// the player dies.
 		eDimension TemporaryDimension = (a_Dimension == dimOverworld) ? dimNether : dimOverworld;
 		m_Protocol->SendRespawn(TemporaryDimension);
 	}
+
 	m_Protocol->SendRespawn(a_Dimension);
-	m_Protocol->SendExperience();
-	m_LastSentDimension = a_Dimension;
 }
 
 
@@ -3285,7 +3135,7 @@ bool cClientHandle::HasPluginChannel(const AString & a_PluginChannel)
 
 bool cClientHandle::WantsSendChunk(int a_ChunkX, int a_ChunkZ)
 {
-	if (m_State >= csQueuedForDestruction)
+	if (m_State >= csDestroyed)
 	{
 		return false;
 	}
@@ -3300,7 +3150,7 @@ bool cClientHandle::WantsSendChunk(int a_ChunkX, int a_ChunkZ)
 
 void cClientHandle::AddWantedChunk(int a_ChunkX, int a_ChunkZ)
 {
-	if (m_State >= csQueuedForDestruction)
+	if (m_State >= csDestroyed)
 	{
 		return;
 	}
@@ -3368,17 +3218,7 @@ void cClientHandle::SocketClosed(void)
 	}
 
 	// Queue self for destruction:
-	SetState(csQueuedForDestruction);
-}
-
-
-
-
-
-void cClientHandle::SetSelf(cClientHandlePtr a_Self)
-{
-	ASSERT(m_Self == nullptr);
-	m_Self = std::move(a_Self);
+	Destroy();
 }
 
 
@@ -3400,7 +3240,7 @@ bool cClientHandle::SetState(eState a_NewState)
 
 
 
-void cClientHandle::ProcessProtocolInOut(void)
+void cClientHandle::ProcessProtocolIn(void)
 {
 	// Process received network data:
 	AString IncomingData;
@@ -3413,21 +3253,6 @@ void cClientHandle::ProcessProtocolInOut(void)
 	{
 		m_Protocol.HandleIncomingData(*this, IncomingData);
 	}
-
-	// Send any queued outgoing data:
-	ContiguousByteBuffer OutgoingData;
-	{
-		cCSLock Lock(m_CSOutgoingData);
-		std::swap(OutgoingData, m_OutgoingData);
-	}
-
-	// Capture the link to prevent it being reset between the null check and the Send:
-	auto Link = m_Link;
-
-	if ((Link != nullptr) && !OutgoingData.empty())
-	{
-		Link->Send(OutgoingData.data(), OutgoingData.size());
-	}
 }
 
 
@@ -3464,10 +3289,6 @@ void cClientHandle::OnRemoteClosed(void)
 		m_Username.c_str(), m_IPString.c_str()
 	);
 	//*/
-	{
-		cCSLock Lock(m_CSOutgoingData);
-		m_Link.reset();
-	}
 	SocketClosed();
 }
 
@@ -3480,9 +3301,5 @@ void cClientHandle::OnError(int a_ErrorCode, const AString & a_ErrorMsg)
 	LOGD("An error has occurred on client link for %s @ %s: %d (%s). Client disconnected.",
 		m_Username.c_str(), m_IPString.c_str(), a_ErrorCode, a_ErrorMsg.c_str()
 	);
-	{
-		cCSLock Lock(m_CSOutgoingData);
-		m_Link.reset();
-	}
 	SocketClosed();
 }