6 files changed, 96 insertions, 129 deletions

diff --git a/src/ChunkGeneratorThread.cpp b/src/ChunkGeneratorThread.cpp
index c87c1db21..cb68c6831 100644
--- a/src/ChunkGeneratorThread.cpp
+++ b/src/ChunkGeneratorThread.cpp
@@ -121,10 +121,10 @@ void cChunkGeneratorThread::WaitForQueueEmpty(void)
 
 
 
-int cChunkGeneratorThread::GetQueueLength(void) const
+size_t cChunkGeneratorThread::GetQueueLength(void) const
 {
 	cCSLock Lock(m_CS);
-	return static_cast<int>(m_Queue.size());
+	return m_Queue.size();
 }
 
 
diff --git a/src/ChunkGeneratorThread.h b/src/ChunkGeneratorThread.h
index f7a4b3da3..7d47549d3 100644
--- a/src/ChunkGeneratorThread.h
+++ b/src/ChunkGeneratorThread.h
@@ -93,7 +93,7 @@ public:
 
 	void WaitForQueueEmpty();
 
-	int GetQueueLength() const;
+	size_t GetQueueLength() const;
 
 	int GetSeed() const;
 
diff --git a/src/Root.cpp b/src/Root.cpp
index b3e7f61ee..b8d4f9db8 100644
--- a/src/Root.cpp
+++ b/src/Root.cpp
@@ -308,9 +308,6 @@ void cRoot::Start(std::unique_ptr<cSettingsRepositoryInterface> a_OverridesRepo)
 	delete m_FurnaceRecipe;   m_FurnaceRecipe = nullptr;
 	delete m_CraftingRecipes; m_CraftingRecipes = nullptr;
 
-	LOG("Unloading worlds...");
-	UnloadWorlds();
-
 	LOGD("Stopping plugin manager...");
 	delete m_PluginManager; m_PluginManager = nullptr;
 
@@ -456,11 +453,10 @@ void cRoot::LoadWorlds(cDeadlockDetect & a_dd, cSettingsRepositoryInterface & a_
 		a_Settings.AddValue("WorldPaths", "world_nether", "world_nether");
 		a_Settings.AddValue("WorldPaths", "world_the_end", "world_the_end");
 
-		AStringVector WorldNames{ "world", "world_nether", "world_the_end" };
-		m_pDefaultWorld = new cWorld("world", "world", a_dd, WorldNames);
-		m_WorldsByName["world"] = m_pDefaultWorld;
-		m_WorldsByName["world_nether"] = new cWorld("world_nether", "world_nether", a_dd, WorldNames, dimNether, "world");
-		m_WorldsByName["world_the_end"] = new cWorld("world_the_end", "world_the_end", a_dd, WorldNames, dimEnd, "world");
+		const AStringVector WorldNames{ "world", "world_nether", "world_the_end" };
+		m_pDefaultWorld = &m_WorldsByName.try_emplace("world", "world", "world", a_dd, WorldNames).first->second;
+		m_WorldsByName.try_emplace("world_nether", "world_nether", "world_nether", a_dd, WorldNames, dimNether, "world");
+		m_WorldsByName.try_emplace("world_the_end", "world_the_end", "world_the_end", a_dd, WorldNames, dimEnd, "world");
 		return;
 	}
 
@@ -475,8 +471,7 @@ void cRoot::LoadWorlds(cDeadlockDetect & a_dd, cSettingsRepositoryInterface & a_
 	// Get the default world
 	AString DefaultWorldName = a_Settings.GetValueSet("Worlds", "DefaultWorld", "world");
 	AString DefaultWorldPath = a_Settings.GetValueSet("WorldPaths", DefaultWorldName, DefaultWorldName);
-	m_pDefaultWorld = new cWorld(DefaultWorldName, DefaultWorldPath, a_dd, WorldNames);
-	m_WorldsByName[ DefaultWorldName ] = m_pDefaultWorld;
+	m_pDefaultWorld = &m_WorldsByName.try_emplace(DefaultWorldName, DefaultWorldName, DefaultWorldPath, a_dd, WorldNames).first->second;
 
 	// Then load the other worlds
 	if (Worlds.size() <= 0)
@@ -520,7 +515,6 @@ void cRoot::LoadWorlds(cDeadlockDetect & a_dd, cSettingsRepositoryInterface & a_
 			continue;
 		}
 		FoundAdditionalWorlds = true;
-		cWorld * NewWorld;
 		AString LowercaseName = StrToLower(WorldName);
 		AString WorldPath = a_Settings.GetValueSet("WorldPaths", WorldName, WorldName);
 		AString NetherAppend = "_nether";
@@ -573,8 +567,7 @@ void cRoot::LoadWorlds(cDeadlockDetect & a_dd, cSettingsRepositoryInterface & a_
 			}
 			Dimension = dimEnd;
 		}
-		NewWorld = new cWorld(WorldName, WorldPath, a_dd, WorldNames, Dimension, LinkTo);
-		m_WorldsByName[WorldName] = NewWorld;
+		m_WorldsByName.try_emplace(WorldName, WorldName, WorldPath, a_dd, WorldNames, Dimension, LinkTo);
 	}  // for i - Worlds
 
 	if (!FoundAdditionalWorlds)
@@ -593,11 +586,12 @@ void cRoot::LoadWorlds(cDeadlockDetect & a_dd, cSettingsRepositoryInterface & a_
 
 void cRoot::StartWorlds(cDeadlockDetect & a_DeadlockDetect)
 {
-	for (WorldMap::iterator itr = m_WorldsByName.begin(); itr != m_WorldsByName.end(); ++itr)
+	for (auto & Entry : m_WorldsByName)
 	{
-		itr->second->Start();
-		itr->second->InitializeSpawn();
-		m_PluginManager->CallHookWorldStarted(*itr->second);
+		auto & World = Entry.second;
+		World.Start();
+		World.InitializeSpawn();
+		m_PluginManager->CallHookWorldStarted(World);
 	}
 }
 
@@ -607,24 +601,10 @@ void cRoot::StartWorlds(cDeadlockDetect & a_DeadlockDetect)
 
 void cRoot::StopWorlds(cDeadlockDetect & a_DeadlockDetect)
 {
-	for (WorldMap::iterator itr = m_WorldsByName.begin(); itr != m_WorldsByName.end(); ++itr)
-	{
-		itr->second->Stop(a_DeadlockDetect);
-	}
-}
-
-
-
-
-
-void cRoot::UnloadWorlds(void)
-{
-	m_pDefaultWorld = nullptr;
-	for (WorldMap::iterator itr = m_WorldsByName.begin(); itr != m_WorldsByName.end(); ++itr)
+	for (auto & Entry : m_WorldsByName)
 	{
-		delete itr->second;
+		Entry.second.Stop(a_DeadlockDetect);
 	}
-	m_WorldsByName.clear();
 }
 
 
@@ -633,6 +613,7 @@ void cRoot::UnloadWorlds(void)
 
 cWorld * cRoot::GetDefaultWorld()
 {
+	ASSERT(m_pDefaultWorld != nullptr);
 	return m_pDefaultWorld;
 }
 
@@ -642,10 +623,10 @@ cWorld * cRoot::GetDefaultWorld()
 
 cWorld * cRoot::GetWorld(const AString & a_WorldName)
 {
-	WorldMap::iterator itr = m_WorldsByName.find(a_WorldName);
-	if (itr != m_WorldsByName.end())
+	const auto FindResult = m_WorldsByName.find(a_WorldName);
+	if (FindResult != m_WorldsByName.cend())
 	{
-		return itr->second;
+		return &FindResult->second;
 	}
 
 	return nullptr;
@@ -659,12 +640,9 @@ bool cRoot::ForEachWorld(cWorldListCallback a_Callback)
 {
 	for (auto & World : m_WorldsByName)
 	{
-		if (World.second != nullptr)
+		if (a_Callback(World.second))
 		{
-			if (a_Callback(*World.second))
-			{
-				return false;
-			}
+			return false;
 		}
 	}
 	return true;
@@ -755,14 +733,14 @@ void cRoot::AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID
 
 
 
-int cRoot::GetTotalChunkCount(void)
+size_t cRoot::GetTotalChunkCount(void)
 {
-	int res = 0;
-	for (WorldMap::iterator itr = m_WorldsByName.begin(); itr != m_WorldsByName.end(); ++itr)
+	size_t Count = 0;
+	for (const auto & Entry : m_WorldsByName)
 	{
-		res += static_cast<int>(itr->second->GetNumChunks());
+		Count += Entry.second.GetNumChunks();
 	}
-	return res;
+	return Count;
 }
 
 
@@ -771,9 +749,9 @@ int cRoot::GetTotalChunkCount(void)
 
 void cRoot::SaveAllChunks(void)
 {
-	for (WorldMap::iterator itr = m_WorldsByName.begin(); itr != m_WorldsByName.end(); ++itr)
+	for (auto & Entry : m_WorldsByName)
 	{
-		itr->second->QueueSaveAllChunks();
+		Entry.second.QueueSaveAllChunks();
 	}
 }
 
@@ -783,9 +761,9 @@ void cRoot::SaveAllChunks(void)
 
 void cRoot::SetSavingEnabled(bool a_SavingEnabled)
 {
-	for (WorldMap::iterator itr = m_WorldsByName.begin(); itr != m_WorldsByName.end(); ++itr)
+	for (auto & Entry : m_WorldsByName)
 	{
-		itr->second->SetSavingEnabled(a_SavingEnabled);
+		Entry.second.SetSavingEnabled(a_SavingEnabled);
 	}
 }
 
@@ -795,10 +773,10 @@ void cRoot::SetSavingEnabled(bool a_SavingEnabled)
 
 void cRoot::SendPlayerLists(cPlayer * a_DestPlayer)
 {
-	for (const auto & itr : m_WorldsByName)
+	for (auto & Entry : m_WorldsByName)
 	{
-		itr.second->SendPlayerList(a_DestPlayer);
-	}  // for itr - m_WorldsByName[]
+		Entry.second.SendPlayerList(a_DestPlayer);
+	}
 }
 
 
@@ -807,10 +785,10 @@ void cRoot::SendPlayerLists(cPlayer * a_DestPlayer)
 
 void cRoot::BroadcastPlayerListsAddPlayer(const cPlayer & a_Player, const cClientHandle * a_Exclude)
 {
-	for (const auto & itr : m_WorldsByName)
+	for (auto & Entry : m_WorldsByName)
 	{
-		itr.second->BroadcastPlayerListAddPlayer(a_Player);
-	}  // for itr - m_WorldsByName[]
+		Entry.second.BroadcastPlayerListAddPlayer(a_Player);
+	}
 }
 
 
@@ -819,10 +797,10 @@ void cRoot::BroadcastPlayerListsAddPlayer(const cPlayer & a_Player, const cClien
 
 void cRoot::BroadcastPlayerListsRemovePlayer(const cPlayer & a_Player, const cClientHandle * a_Exclude)
 {
-	for (const auto & itr : m_WorldsByName)
+	for (auto & Entry : m_WorldsByName)
 	{
-		itr.second->BroadcastPlayerListRemovePlayer(a_Player);
-	}  // for itr - m_WorldsByName[]
+		Entry.second.BroadcastPlayerListRemovePlayer(a_Player);
+	}
 }
 
 
@@ -831,10 +809,10 @@ void cRoot::BroadcastPlayerListsRemovePlayer(const cPlayer & a_Player, const cCl
 
 void cRoot::BroadcastChat(const AString & a_Message, eMessageType a_ChatPrefix)
 {
-	for (WorldMap::iterator itr = m_WorldsByName.begin(), end = m_WorldsByName.end(); itr != end; ++itr)
+	for (auto & Entry : m_WorldsByName)
 	{
-		itr->second->BroadcastChat(a_Message, nullptr, a_ChatPrefix);
-	}  // for itr - m_WorldsByName[]
+		Entry.second.BroadcastChat(a_Message, nullptr, a_ChatPrefix);
+	}
 }
 
 
@@ -843,10 +821,10 @@ void cRoot::BroadcastChat(const AString & a_Message, eMessageType a_ChatPrefix)
 
 void cRoot::BroadcastChat(const cCompositeChat & a_Message)
 {
-	for (WorldMap::iterator itr = m_WorldsByName.begin(), end = m_WorldsByName.end(); itr != end; ++itr)
+	for (auto & Entry : m_WorldsByName)
 	{
-		itr->second->BroadcastChat(a_Message);
-	}  // for itr - m_WorldsByName[]
+		Entry.second.BroadcastChat(a_Message);
+	}
 }
 
 
@@ -855,10 +833,9 @@ void cRoot::BroadcastChat(const cCompositeChat & a_Message)
 
 bool cRoot::ForEachPlayer(cPlayerListCallback a_Callback)
 {
-	for (WorldMap::iterator itr = m_WorldsByName.begin(), itr2 = itr; itr != m_WorldsByName.end(); itr = itr2)
+	for (auto & Entry : m_WorldsByName)
 	{
-		++itr2;
-		if (!itr->second->ForEachPlayer(a_Callback))
+		if (!Entry.second.ForEachPlayer(a_Callback))
 		{
 			return false;
 		}
@@ -922,9 +899,9 @@ bool cRoot::FindAndDoWithPlayer(const AString & a_PlayerName, cPlayerListCallbac
 
 bool cRoot::DoWithPlayerByUUID(const cUUID & a_PlayerUUID, cPlayerListCallback a_Callback)
 {
-	for (WorldMap::iterator itr = m_WorldsByName.begin(); itr !=  m_WorldsByName.end(); ++itr)
+	for (auto & Entry : m_WorldsByName)
 	{
-		if (itr->second->DoWithPlayerByUUID(a_PlayerUUID, a_Callback))
+		if (Entry.second.DoWithPlayerByUUID(a_PlayerUUID, a_Callback))
 		{
 			return true;
 		}
@@ -938,9 +915,9 @@ bool cRoot::DoWithPlayerByUUID(const cUUID & a_PlayerUUID, cPlayerListCallback a
 
 bool cRoot::DoWithPlayer(const AString & a_PlayerName, cPlayerListCallback a_Callback)
 {
-	for (auto World : m_WorldsByName)
+	for (auto & Entry : m_WorldsByName)
 	{
-		if (World.second->DoWithPlayer(a_PlayerName, a_Callback))
+		if (Entry.second.DoWithPlayer(a_PlayerName, a_Callback))
 		{
 			return true;
 		}
@@ -1072,23 +1049,23 @@ void cRoot::LogChunkStats(cCommandOutputCallback & a_Output)
 	int SumNumInLighting = 0;
 	int SumNumInGenerator = 0;
 	int SumMem = 0;
-	for (WorldMap::iterator itr = m_WorldsByName.begin(), end = m_WorldsByName.end(); itr != end; ++itr)
+	for (auto & Entry : m_WorldsByName)
 	{
-		cWorld * World = itr->second;
-		int NumInGenerator = World->GetGeneratorQueueLength();
-		int NumInSaveQueue = static_cast<int>(World->GetStorageSaveQueueLength());
-		int NumInLoadQueue = static_cast<int>(World->GetStorageLoadQueueLength());
+		auto & World = Entry.second;
+		const auto NumInGenerator = World.GetGeneratorQueueLength();
+		const auto NumInSaveQueue = World.GetStorageSaveQueueLength();
+		const auto NumInLoadQueue = World.GetStorageLoadQueueLength();
 		int NumValid = 0;
 		int NumDirty = 0;
 		int NumInLighting = 0;
-		World->GetChunkStats(NumValid, NumDirty, NumInLighting);
-		a_Output.Out("World %s:", World->GetName().c_str());
+		World.GetChunkStats(NumValid, NumDirty, NumInLighting);
+		a_Output.Out("World %s:", World.GetName().c_str());
 		a_Output.Out("  Num loaded chunks: %d", NumValid);
 		a_Output.Out("  Num dirty chunks: %d", NumDirty);
 		a_Output.Out("  Num chunks in lighting queue: %d", NumInLighting);
-		a_Output.Out("  Num chunks in generator queue: %d", NumInGenerator);
-		a_Output.Out("  Num chunks in storage load queue: %d", NumInLoadQueue);
-		a_Output.Out("  Num chunks in storage save queue: %d", NumInSaveQueue);
+		a_Output.Out("  Num chunks in generator queue: %zu", NumInGenerator);
+		a_Output.Out("  Num chunks in storage load queue: %zu", NumInLoadQueue);
+		a_Output.Out("  Num chunks in storage save queue: %zu", NumInSaveQueue);
 		int Mem = NumValid * static_cast<int>(sizeof(cChunk));
 		a_Output.Out("  Memory used by chunks: %d KiB (%d MiB)", (Mem + 1023) / 1024, (Mem + 1024 * 1024 - 1) / (1024 * 1024));
 		a_Output.Out("  Per-chunk memory size breakdown:");
diff --git a/src/Root.h b/src/Root.h
index 2393871da..2092bb9be 100644
--- a/src/Root.h
+++ b/src/Root.h
@@ -1,13 +1,13 @@
 
 #pragma once
 
-#include "Protocol/Authenticator.h"
-#include "Protocol/MojangAPI.h"
-#include "HTTP/HTTPServer.h"
+#include "BlockTypeRegistry.h"
 #include "Defines.h"
 #include "FunctionRef.h"
+#include "HTTP/HTTPServer.h"
+#include "Protocol/Authenticator.h"
+#include "Protocol/MojangAPI.h"
 #include "RankManager.h"
-#include "BlockTypeRegistry.h"
 
 
 
@@ -146,7 +146,7 @@ public:
 	void TickCommands(void);
 
 	/** Returns the number of chunks loaded */
-	int GetTotalChunkCount(void);  // tolua_export
+	size_t GetTotalChunkCount(void);  // tolua_export
 
 	/** Saves all chunks in all worlds */
 	void SaveAllChunks(void);  // tolua_export
@@ -214,7 +214,7 @@ private:
 		cCommandOutputCallback * m_Output;
 	} ;
 
-	typedef std::map<AString, cWorld *> WorldMap;
+	typedef std::map<AString, cWorld> WorldMap;
 	typedef std::vector<cCommand> cCommandQueue;
 
 	cWorld * m_pDefaultWorld;
@@ -268,12 +268,6 @@ private:
 	/** Stops each world's threads, so that it's safe to unload them */
 	void StopWorlds(cDeadlockDetect & a_DeadlockDetect);
 
-	/** Unloads all worlds from memory */
-	void UnloadWorlds(void);
-
-	/** Does the actual work of executing a command */
-	void DoExecuteConsoleCommand(const AString & a_Cmd);
-
 	static cRoot * s_Root;
 
 	static void InputThread(cRoot & a_Params);
diff --git a/src/World.cpp b/src/World.cpp
index 251c8ce51..6e7c066c9 100644
--- a/src/World.cpp
+++ b/src/World.cpp
@@ -457,21 +457,6 @@ cWorld::~cWorld()
 	delete m_WaterSimulator;     m_WaterSimulator    = nullptr;
 	delete m_LavaSimulator;      m_LavaSimulator     = nullptr;
 	delete m_RedstoneSimulator;  m_RedstoneSimulator = nullptr;
-
-	m_Storage.WaitForFinish();
-
-	if (IsSavingEnabled())
-	{
-		// Unload the scoreboard
-		cScoreboardSerializer Serializer(m_DataPath, &m_Scoreboard);
-		Serializer.Save();
-
-		m_MapManager.SaveMapData();
-	}
-
-	// Explicitly destroy the chunkmap, so that it's guaranteed to be destroyed before the other internals
-	// This fixes crashes on stopping the server, because chunk destructor deletes entities and those access the world.
-	m_ChunkMap.reset();
 }
 
 
@@ -964,11 +949,25 @@ void cWorld::Stop(cDeadlockDetect & a_DeadlockDetect)
 	m_Lighting.Stop();
 	m_Generator.Stop();
 	m_ChunkSender.Stop();
-	m_Storage.Stop();
+	m_Storage.Stop();  // Waits for thread to finish
 
 	a_DeadlockDetect.UntrackCriticalSection(m_CSClients);
 	a_DeadlockDetect.UntrackCriticalSection(m_CSTasks);
 	m_ChunkMap->UntrackInDeadlockDetect(a_DeadlockDetect);
+
+	if (IsSavingEnabled())
+	{
+		// Unload the scoreboard
+		cScoreboardSerializer Serializer(m_DataPath, &m_Scoreboard);
+		Serializer.Save();
+
+		m_MapManager.SaveMapData();
+	}
+
+	// Explicitly destroy the chunkmap, so that it's guaranteed to be destroyed before the other internals
+	// This fixes crashes on stopping the server, because chunk destructor deletes entities and those access the world.
+	// TODO: destructors should only be used for releasing resources, not doing extra work
+	m_ChunkMap.reset();
 }
 
 
diff --git a/src/World.h b/src/World.h
index 860bed5d6..76c0fa3c9 100644
--- a/src/World.h
+++ b/src/World.h
@@ -91,6 +91,18 @@ public:
 		return "cWorld";
 	}
 
+	/** Construct the world and read settings from its ini file.
+	@param a_DeadlockDetect is used for tracking this world's age, detecting a possible deadlock.
+	@param a_WorldNames is a list of all world names, used to validate linked worlds
+	*/
+	cWorld(
+		const AString & a_WorldName, const AString & a_DataPath,
+		cDeadlockDetect & a_DeadlockDetect, const AStringVector & a_WorldNames,
+		eDimension a_Dimension = dimOverworld, const AString & a_LinkedOverworldName = {}
+	);
+
+	virtual ~cWorld() override;
+
 	// tolua_begin
 
 	/** Get whether saving chunks is enabled */
@@ -966,7 +978,7 @@ public:
 	void GetChunkStats(int & a_NumValid, int & a_NumDirty, int & a_NumInLightingQueue);
 
 	// Various queues length queries (cannot be const, they lock their CS):
-	inline int GetGeneratorQueueLength     (void) { return m_Generator.GetQueueLength();   }    // tolua_export
+	inline size_t GetGeneratorQueueLength  (void) { return m_Generator.GetQueueLength();   }    // tolua_export
 	inline size_t GetLightingQueueLength   (void) { return m_Lighting.GetQueueLength();    }    // tolua_export
 	inline size_t GetStorageLoadQueueLength(void) { return m_Storage.GetLoadQueueLength(); }    // tolua_export
 	inline size_t GetStorageSaveQueueLength(void) { return m_Storage.GetSaveQueueLength(); }    // tolua_export
@@ -1104,10 +1116,6 @@ public:
 
 private:
 
-	friend class cRoot;
-
-
-
 	class cTickThread:
 		public cIsThread
 	{
@@ -1327,17 +1335,6 @@ private:
 	/** Queue for the chunk data to be set into m_ChunkMap by the tick thread. Protected by m_CSSetChunkDataQueue */
 	cSetChunkDataPtrs m_SetChunkDataQueue;
 
-	/** Construct the world and read settings from its ini file.
-	@param a_DeadlockDetect is used for tracking this world's age, detecting a possible deadlock.
-	@param a_WorldNames is a list of all world names, used to validate linked worlds
-	*/
-	cWorld(
-		const AString & a_WorldName, const AString & a_DataPath,
-		cDeadlockDetect & a_DeadlockDetect, const AStringVector & a_WorldNames,
-		eDimension a_Dimension = dimOverworld, const AString & a_LinkedOverworldName = {}
-	);
-	virtual ~cWorld() override;
-
 	void Tick(std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_LastTickDurationMSec);
 
 	/** Handles the weather in each tick */