serverTick

11 files changed, 57 insertions, 218 deletions

diff --git a/src/ChunkSender.cpp b/src/ChunkSender.cpp
index 1d55cf743..169861e87 100644
--- a/src/ChunkSender.cpp
+++ b/src/ChunkSender.cpp
@@ -233,12 +233,6 @@ void cChunkSender::SendChunk(int a_ChunkX, int a_ChunkZ, std::unordered_set<cCli
 		}
 	}
 
-	// If the chunk has no clients, no need to packetize it:
-	if (!m_World.HasChunkAnyClients(a_ChunkX, a_ChunkZ))
-	{
-		return;
-	}
-
 	// If the chunk is not valid, do nothing - whoever needs it has queued it for loading / generating
 	if (!m_World.IsChunkValid(a_ChunkX, a_ChunkZ))
 	{
diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp
index 8897063a0..aca81cc0a 100644
--- a/src/ClientHandle.cpp
+++ b/src/ClientHandle.cpp
@@ -175,6 +175,7 @@ void cClientHandle::Destroy(void)
 	{
 		m_Player->RemoveClientHandle();
 	}
+	m_Player = nullptr;
 	m_State = csDestroyed;
 }
 
@@ -1679,27 +1680,12 @@ bool cClientHandle::CheckMultiLogin(const AString & a_Username)
 	}
 	
 	// Check if the player is waiting to be transferred to the World.
-	if (cRoot::Get()->GetServer()->IsPlayerInQueue(a_Username))
+	if (cRoot::Get()->GetServer()->IsClientOnServer(a_Username))
 	{
 		Kick("A player of the username is already logged in");
 		return false;
 	}
 
-	class cCallback :
-		public cPlayerListCallback
-	{
-		virtual bool Item(cPlayer * a_Player) override
-		{
-			return true;
-		}
-	} Callback;
-
-	// Check if the player is in any World.
-	if (cRoot::Get()->DoWithPlayer(a_Username, Callback))
-	{
-		Kick("A player of the username is already logged in");
-		return false;
-	}
 	return true;
 }
 
@@ -1887,6 +1873,17 @@ void cClientHandle::Tick(float a_Dt)
 			Link->Send(OutgoingData.data(), OutgoingData.size());
 		}
 	}
+
+
+	if (m_State == csAuthenticated)
+	{
+		StreamNextChunk();
+		
+		// Add the player to the world (start ticking from there):
+		m_State = csDownloadingWorld;
+		m_Player->GetWorld()->AddPlayer(m_Player);
+		return;
+	}
 	
 	m_TicksSinceLastPacket += 1;
 	if (m_TicksSinceLastPacket > 600)  // 30 seconds time-out
@@ -1962,54 +1959,6 @@ void cClientHandle::Tick(float a_Dt)
 
 
 
-void cClientHandle::ServerTick(float a_Dt)
-{
-	// Process received network data:
-	AString IncomingData;
-	{
-		cCSLock Lock(m_CSIncomingData);
-		std::swap(IncomingData, m_IncomingData);
-	}
-	if (!IncomingData.empty())
-	{
-		m_Protocol->DataReceived(IncomingData.data(), IncomingData.size());
-	}
-	
-	// Send any queued outgoing data:
-	AString OutgoingData;
-	{
-		cCSLock Lock(m_CSOutgoingData);
-		std::swap(OutgoingData, m_OutgoingData);
-	}
-	if ((m_Link != nullptr) && !OutgoingData.empty())
-	{
-		m_Link->Send(OutgoingData.data(), OutgoingData.size());
-	}
-	
-	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->GetWorld()->AddPlayer(m_Player);
-		return;
-	}
-	
-	m_TicksSinceLastPacket += 1;
-	if (m_TicksSinceLastPacket > 600)  // 30 seconds
-	{
-		SendDisconnect("Nooooo!! You timed out! D: Come back!");
-	}
-}
-
-
-
-
-
 void cClientHandle::SendAttachEntity(const cEntity & a_Entity, const cEntity * a_Vehicle)
 {
 	m_Protocol->SendAttachEntity(a_Entity, a_Vehicle);
@@ -2176,7 +2125,7 @@ void cClientHandle::SendChatSystem(const cCompositeChat & a_Message)
 
 void cClientHandle::SendChunkData(int a_ChunkX, int a_ChunkZ, cChunkDataSerializer & a_Serializer)
 {
-	ASSERT(m_Player != nullptr);
+	if (m_Player != nullptr) return;
 	
 	// Check chunks being sent, erase them from m_ChunksToSend:
 	bool Found = false;
@@ -3007,15 +2956,10 @@ void cClientHandle::SocketClosed(void)
 		LOGD("Client %s @ %s disconnected", m_Username.c_str(), m_IPString.c_str());
 		cRoot::Get()->GetPluginManager()->CallHookDisconnect(*this, "Player disconnected");
 	}
-	if ((m_State < csDestroying) && (m_Player != nullptr))
-	{
-		cWorld * World = m_Player->GetWorld();
-		if (World != nullptr)
-		{
-			World->RemovePlayer(m_Player, true);  // Must be called before cPlayer::Destroy() as otherwise cChunk tries to delete the player, and then we do it again
-		}
-	}
-	Destroy();
+
+	// This must be the last thing this method does, as it may destroy the this pointer.
+	cRoot::Get()->GetServer()->QueueRemoveAndDestroyClient(this);
+	
 }
 
 
diff --git a/src/ClientHandle.h b/src/ClientHandle.h
index 32e894d41..60a09f30b 100644
--- a/src/ClientHandle.h
+++ b/src/ClientHandle.h
@@ -131,14 +131,15 @@ public:  // tolua_export
 
 	/** Called while the client is being ticked from the world via its cPlayer object */
 	void Tick(float a_Dt);
-	
-	/** Called while the client is being ticked from the cServer object */
-	void ServerTick(float a_Dt);
 
 	void Destroy(void);
 	
 	bool IsPlaying   (void) const { return (m_State == csPlaying); }
-	bool IsDestroyed (void) const { return (m_State == csDestroyed); }
+	bool IsDestroyed (void) const
+	{
+		ASSERT((m_State != csDestroyed) || (m_Player == nullptr));
+		return (m_State == csDestroyed);
+	}
 	bool IsDestroying(void) const { return (m_State == csDestroying); }
 
 	// The following functions send the various packets:
diff --git a/src/Entities/Player.cpp b/src/Entities/Player.cpp
index 3bea60af7..5c38f19bd 100644
--- a/src/Entities/Player.cpp
+++ b/src/Entities/Player.cpp
@@ -144,6 +144,7 @@ cPlayer::cPlayer(cClientHandlePtr a_Client, const AString & a_PlayerName) :
 
 cPlayer::~cPlayer(void)
 {
+	ASSERT((m_World == nullptr) || m_World->HasEntity(m_UniqueID));
 	if (!cRoot::Get()->GetPluginManager()->CallHookPlayerDestroyed(*this))
 	{
 		cRoot::Get()->BroadcastChatLeave(Printf("%s has left the game", GetName().c_str()));
@@ -163,6 +164,8 @@ cPlayer::~cPlayer(void)
 	m_InventoryWindow = nullptr;
 
 	LOGD("Player %p deleted", static_cast<void *>(this));
+
+	ASSERT((m_World == nullptr) || m_World->HasEntity(m_UniqueID));
 }
 
 
@@ -1278,7 +1281,6 @@ void cPlayer::SetCapabilities()
 	if (IsGameModeSpectator())
 	{
 		SetVisible(false);
-		SetCanFly(true);
 	}
 	else
 	{
diff --git a/src/Generating/ChunkGenerator.cpp b/src/Generating/ChunkGenerator.cpp
index bfa3344b9..977c88b4e 100644
--- a/src/Generating/ChunkGenerator.cpp
+++ b/src/Generating/ChunkGenerator.cpp
@@ -260,7 +260,7 @@ void cChunkGenerator::Execute(void)
 		}
 
 		// Skip the chunk if the generator is overloaded:
-		if (SkipEnabled && !m_ChunkSink->HasChunkAnyClients(item.m_ChunkX, item.m_ChunkZ))
+		if (SkipEnabled && !m_ChunkSink->IsNeeded(item.m_ChunkX, item.m_ChunkZ))
 		{
 			LOGWARNING("Chunk generator overloaded, skipping chunk [%d, %d]", item.m_ChunkX, item.m_ChunkZ);
 			if (item.m_Callback != nullptr)
diff --git a/src/Generating/ChunkGenerator.h b/src/Generating/ChunkGenerator.h
index b7968356f..e33dc5e17 100644
--- a/src/Generating/ChunkGenerator.h
+++ b/src/Generating/ChunkGenerator.h
@@ -100,7 +100,7 @@ public:
 		
 		/** Called when the generator is overloaded to skip chunks that are no longer needed.
 		If this callback returns false, the chunk is not generated. */
-		virtual bool HasChunkAnyClients(int a_ChunkX, int a_ChunkZ) = 0;
+		virtual bool IsNeeded(int a_ChunkX, int a_ChunkZ) = 0;
 
 		/** Called to check whether the specified chunk is in the queued state.
 		Currently used only in Debug-mode asserts. */
diff --git a/src/Protocol/MojangAPI.cpp b/src/Protocol/MojangAPI.cpp
index 73b3bd8c0..6a39595b4 100644
--- a/src/Protocol/MojangAPI.cpp
+++ b/src/Protocol/MojangAPI.cpp
@@ -525,9 +525,12 @@ AString cMojangAPI::MakeUUIDDashed(const AString & a_UUID)
 			res.append(a_UUID, 20, 12);
 			return StrToLower(res);
 		}
+		default:
+		{
+			LOGWARNING("%s: Not an UUID: \"%s\".", __FUNCTION__, a_UUID.c_str());
+			return "";
+		}
 	}
-	LOGWARNING("%s: Not an UUID: \"%s\".", __FUNCTION__, a_UUID.c_str());
-	return "";
 }
 
 
diff --git a/src/Server.cpp b/src/Server.cpp
index 12c45467d..c75fa3c39 100644
--- a/src/Server.cpp
+++ b/src/Server.cpp
@@ -153,16 +153,6 @@ cServer::cServer(void) :
 
 
 
-void cServer::ClientMovedToWorld(const cClientHandle * a_Client)
-{
-	cCSLock Lock(m_CSClients);
-	m_ClientsToRemove.push_back(const_cast<cClientHandle *>(a_Client));
-}
-
-
-
-
-
 void cServer::PlayerCreated(const cPlayer * a_Player)
 {
 	UNUSED(a_Player);
@@ -268,7 +258,7 @@ int cServer::GetNumPlayers(void) const
 
 
 
-bool cServer::IsPlayerInQueue(AString a_Username)
+bool cServer::IsClientOnServer(AString a_Username)
 {
 	cCSLock Lock(m_CSClients);
 	for (auto client : m_Clients)
@@ -284,7 +274,6 @@ bool cServer::IsPlayerInQueue(AString a_Username)
 
 
 
-
 void cServer::PrepareKeys(void)
 {
 	LOGD("Generating protocol encryption keypair...");
@@ -329,7 +318,7 @@ bool cServer::Tick(float a_Dt)
 	// Let the Root process all the queued commands:
 	cRoot::Get()->TickCommands();
 
-	// Tick all clients not yet assigned to a world:
+	// Tick all clients
 	TickClients(a_Dt);
 
 	if (!m_bRestarting)
@@ -354,20 +343,6 @@ void cServer::TickClients(float a_Dt)
 	{
 		cCSLock Lock(m_CSClients);
 
-		// Remove clients that have moved to a world (the world will be ticking them from now on)
-		for (auto itr = m_ClientsToRemove.begin(), end = m_ClientsToRemove.end(); itr != end; ++itr)
-		{
-			for (auto itrC = m_Clients.begin(), endC = m_Clients.end(); itrC != endC; ++itrC)
-			{
-				if (itrC->get() == *itr)
-				{
-					m_Clients.erase(itrC);
-					break;
-				}
-			}
-		}  // for itr - m_ClientsToRemove[]
-		m_ClientsToRemove.clear();
-
 		// Tick the remaining clients, take out those that have been destroyed into RemoveClients
 		for (auto itr = m_Clients.begin(); itr != m_Clients.end();)
 		{
@@ -378,7 +353,7 @@ void cServer::TickClients(float a_Dt)
 				itr = m_Clients.erase(itr);
 				continue;
 			}
-			(*itr)->ServerTick(a_Dt);
+			(*itr)->Tick(a_Dt);
 			++itr;
 		}  // for itr - m_Clients[]
 	}
@@ -687,3 +662,7 @@ void cServer::AuthenticateUser(int a_ClientID, const AString & a_Name, const ASt
 
 
 
+void cServer::QueueRemoveAndDestroyClient(cClientHandle * a_Client)
+{
+	a_Client->Destroy();
+}
diff --git a/src/Server.h b/src/Server.h
index 4d0bc1c18..918a5769e 100644
--- a/src/Server.h
+++ b/src/Server.h
@@ -69,9 +69,9 @@ public:
 	int  GetNumPlayers(void) const;
 	void SetMaxPlayers(int a_MaxPlayers) { m_MaxPlayers = a_MaxPlayers; }
 	
-	/** Check if the player is queued to be transferred to a World.
-	Returns true is Player is found in queue. */
-	bool IsPlayerInQueue(AString a_Username);
+	/** Check if the client is on the server.
+	Returns true is client is found in any world, or in the queue. */
+	bool IsClientOnServer(AString a_Username);
 	
 	/** Can login more than once with same username.
 	Returns false if it is not allowed, true otherwise. */
@@ -107,9 +107,6 @@ public:
 	/** Called by cClientHandle's destructor; stop m_SocketThreads from calling back into a_Client */
 	void ClientDestroying(const cClientHandle * a_Client);
 	
-	/** Don't tick a_Client anymore, it will be ticked from its cPlayer instead */
-	void ClientMovedToWorld(const cClientHandle * a_Client);
-	
 	/** Notifies the server that a player was created; the server uses this to adjust the number of players */
 	void PlayerCreated(const cPlayer * a_Player);
 	
@@ -138,6 +135,9 @@ public:
 	Read from settings, admins should set this to true only when they chain to BungeeCord,
 	it makes the server vulnerable to identity theft through direct connections. */
 	bool ShouldAllowBungeeCord(void) const { return m_ShouldAllowBungeeCord; }
+
+
+	void QueueRemoveAndDestroyClient(cClientHandle * a_Client);
 	
 private:
 
diff --git a/src/World.cpp b/src/World.cpp
index bd06af1b7..6babfd4b2 100644
--- a/src/World.cpp
+++ b/src/World.cpp
@@ -786,15 +786,6 @@ void cWorld::InitialiseAndLoadMobSpawningValues(cIniFile & a_IniFile)
 
 void cWorld::Stop(void)
 {
-	// Delete the clients that have been in this world:
-	{
-		cCSLock Lock(m_CSClients);
-		for (auto itr = m_Clients.begin(); itr != m_Clients.end(); ++itr)
-		{
-			(*itr)->Destroy();
-		}  // for itr - m_Clients[]
-		m_Clients.clear();
-	}
 
 	// Write settings to file; these are all plugin changeable values - keep updated!
 	cIniFile IniFile;
@@ -884,7 +875,6 @@ void cWorld::Tick(std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_La
 	m_ChunkMap->Tick(a_Dt);
 	m_MapManager.TickMaps();
 
-	TickClients(static_cast<float>(a_Dt.count()));
 	TickQueuedBlocks();
 	TickQueuedTasks();
 	
@@ -1048,56 +1038,6 @@ void cWorld::TickQueuedTasks(void)
 
 
 
-void cWorld::TickClients(float a_Dt)
-{
-	cClientHandlePtrs RemoveClients;
-	{
-		cCSLock Lock(m_CSClients);
-		
-		// Remove clients scheduled for removal:
-		for (auto itr = m_ClientsToRemove.begin(), end = m_ClientsToRemove.end(); itr != end; ++itr)
-		{
-			for (auto itrC = m_Clients.begin(), endC = m_Clients.end(); itrC != endC; ++itrC)
-			{
-				if (itrC->get() == *itr)
-				{
-					m_Clients.erase(itrC);
-					break;
-				}
-			}
-		}  // for itr - m_ClientsToRemove[]
-		m_ClientsToRemove.clear();
-		
-		// Add clients scheduled for adding:
-		for (auto itr = m_ClientsToAdd.begin(), end = m_ClientsToAdd.end(); itr != end; ++itr)
-		{
-			ASSERT(std::find(m_Clients.begin(), m_Clients.end(), *itr) == m_Clients.end());
-			m_Clients.push_back(*itr);
-		}  // for itr - m_ClientsToRemove[]
-		m_ClientsToAdd.clear();
-		
-		// Tick the clients, take out those that have been destroyed into RemoveClients
-		for (auto itr = m_Clients.begin(); itr != m_Clients.end();)
-		{
-			if ((*itr)->IsDestroyed())
-			{
-				// Remove the client later, when CS is not held, to avoid deadlock
-				RemoveClients.push_back(*itr);
-				itr = m_Clients.erase(itr);
-				continue;
-			}
-			(*itr)->Tick(a_Dt);
-			++itr;
-		}  // for itr - m_Clients[]
-	}
-
-	// Delete the clients queued for removal:
-	RemoveClients.clear();
-}
-
-
-
-
 
 void cWorld::UpdateSkyDarkness(void)
 {
@@ -2690,15 +2630,16 @@ bool cWorld::IsChunkValid(int a_ChunkX, int a_ChunkZ) const
 
 
 
-bool cWorld::HasChunkAnyClients(int a_ChunkX, int a_ChunkZ) const
+bool cWorld::cChunkGeneratorCallbacks::IsNeeded(int a_ChunkX, int a_ChunkZ)
 {
-	return m_ChunkMap->HasChunkAnyClients(a_ChunkX, a_ChunkZ);
+	return m_World->m_ChunkMap->HasChunkAnyClients(a_ChunkX, a_ChunkZ);
 }
 
 
 
 
 
+
 void cWorld::UnloadUnusedChunks(void)
 {
 	m_LastUnload = std::chrono::duration_cast<cTickTimeLong>(m_WorldAge);
@@ -2754,16 +2695,6 @@ void cWorld::RemovePlayer(cPlayer * a_Player, bool a_RemoveFromChunk)
 		LOGD("Removing player %s from world \"%s\"", a_Player->GetName().c_str(), m_WorldName.c_str());
 		m_Players.remove(a_Player);
 	}
-	
-	// Remove the player's client from the list of clients to be ticked:
-	cClientHandle * Client = a_Player->GetClientHandle();
-	if (Client != nullptr)
-	{
-		Client->RemoveFromWorld();
-		m_ChunkMap->RemoveClientFromChunks(Client);
-		cCSLock Lock(m_CSClients);
-		m_ClientsToRemove.push_back(Client);
-	}
 }
 
 
@@ -2962,12 +2893,12 @@ bool cWorld::DoWithEntityByID(UInt32 a_UniqueID, cEntityCallback & a_Callback)
 
 
 
-
+/*
 void cWorld::CompareChunkClients(int a_ChunkX1, int a_ChunkZ1, int a_ChunkX2, int a_ChunkZ2, cClientDiffCallback & a_Callback)
 {
 	m_ChunkMap->CompareChunkClients(a_ChunkX1, a_ChunkZ1, a_ChunkX2, a_ChunkZ2, a_Callback);
 }
-
+*/
 
 
 
@@ -3506,6 +3437,7 @@ void cWorld::SetChunkAlwaysTicked(int a_ChunkX, int a_ChunkZ, bool a_AlwaysTicke
 
 
 
+
 cRedstoneSimulator * cWorld::InitializeRedstoneSimulator(cIniFile & a_IniFile)
 {
 	AString SimulatorName = a_IniFile.GetValueSet("Physics", "RedstoneSimulator", "Incremental");
@@ -3633,6 +3565,7 @@ void cWorld::AddQueuedPlayers(void)
 		}  // for itr - PlayersToAdd[]
 	}  // Lock(m_CSPlayers)
 
+	/*
 	// Add all the players' clienthandles:
 	{
 		cCSLock Lock(m_CSClients);
@@ -3645,6 +3578,7 @@ void cWorld::AddQueuedPlayers(void)
 			}
 		}  // for itr - PlayersToAdd[]
 	}  // Lock(m_CSClients)
+	*/
 
 	// Stream chunks to all eligible clients:
 	for (cPlayerList::iterator itr = PlayersToAdd.begin(), end = PlayersToAdd.end(); itr != end; ++itr)
@@ -3713,11 +3647,12 @@ bool cWorld::cChunkGeneratorCallbacks::IsChunkQueued(int a_ChunkX, int a_ChunkZ)
 
 
 
-
+/*
 bool cWorld::cChunkGeneratorCallbacks::HasChunkAnyClients(int a_ChunkX, int a_ChunkZ)
 {
 	return m_World->HasChunkAnyClients(a_ChunkX, a_ChunkZ);
 }
+*/
 
 
 
diff --git a/src/World.h b/src/World.h
index 30ac52763..d11b5bb8c 100644
--- a/src/World.h
+++ b/src/World.h
@@ -251,8 +251,6 @@ public:
 
 	/** Returns true iff the chunk is present and valid. */
 	bool IsChunkValid(int a_ChunkX, int a_ChunkZ) const;
-
-	bool HasChunkAnyClients(int a_ChunkX, int a_ChunkZ) const;
 	
 	/** Queues a task to unload unused chunks onto the tick thread. The prefferred way of unloading. */
 	void QueueUnloadUnusedChunks(void);  // tolua_export
@@ -310,9 +308,6 @@ public:
 	/** Calls the callback if the entity with the specified ID is found, with the entity object as the callback param.
 	Returns true if entity found and callback returned false. */
 	bool DoWithEntityByID(UInt32 a_UniqueID, cEntityCallback & a_Callback);  // Exported in ManualBindings.cpp
-
-	/** Compares clients of two chunks, calls the callback accordingly */
-	void CompareChunkClients(int a_ChunkX1, int a_ChunkZ1, int a_ChunkX2, int a_ChunkZ2, cClientDiffCallback & a_Callback);
 	
 	/** Adds client to a chunk, if not already present; returns true if added, false if present */
 	bool AddChunkClient(int a_ChunkX, int a_ChunkZ, cClientHandle * a_Client);
@@ -843,7 +838,7 @@ private:
 		// cChunkSink overrides:
 		virtual void OnChunkGenerated  (cChunkDesc & a_ChunkDesc) override;
 		virtual bool IsChunkValid      (int a_ChunkX, int a_ChunkZ) override;
-		virtual bool HasChunkAnyClients(int a_ChunkX, int a_ChunkZ) override;
+		virtual bool IsNeeded(int a_ChunkX, int a_ChunkZ) override;
 		virtual bool IsChunkQueued     (int a_ChunkX, int a_ChunkZ) override;
 		
 		// cPluginInterface overrides:
@@ -988,17 +983,6 @@ private:
 	/** Tasks that have been queued onto the tick thread, possibly to be executed at target tick in the future; guarded by m_CSTasks */
 	std::vector<std::pair<Int64, std::function<void(cWorld &)>>> m_Tasks;
 	
-	/** Guards m_Clients */
-	cCriticalSection  m_CSClients;
-	
-	/** List of clients in this world, these will be ticked by this world */
-	cClientHandlePtrs m_Clients;
-	
-	/** Clients that are scheduled for removal (ticked in another world), waiting for TickClients() to remove them */
-	cClientHandles m_ClientsToRemove;
-	
-	/** Clients that are scheduled for adding, waiting for TickClients to add them */
-	cClientHandlePtrs m_ClientsToAdd;
 
 	/** Guards m_EntitiesToAdd */
 	cCriticalSection m_CSEntitiesToAdd;
@@ -1032,9 +1016,6 @@ private:
 	
 	/** Executes all tasks queued onto the tick thread */
 	void TickQueuedTasks(void);
-	
-	/** Ticks all clients that are in this world */
-	void TickClients(float a_Dt);
 
 	/** Unloads all chunks immediately. */
 	void UnloadUnusedChunks(void);