Merge pull request #3165 from LogicParrot/chunkLayer2

ChunkLayers now stored in std::map [revised]

src/ChunkMap.cpp

@@ -51,24 +51,10 @@ cChunkMap::cChunkMap(cWorld * a_World) :
 
 
 
-cChunkMap::~cChunkMap()
-{
-	cCSLock Lock(m_CSLayers);
-	while (!m_Layers.empty())
-	{
-		delete m_Layers.back();
-		m_Layers.pop_back();  // Must pop, because further chunk deletions query the chunkmap for entities and that would touch deleted data
-	}
-}
-
-
-
-
-
 void cChunkMap::RemoveLayer(cChunkLayer * a_Layer)
 {
 	cCSLock Lock(m_CSLayers);
-	m_Layers.remove(a_Layer);
+	m_Layers.erase(std::pair<int, int>(a_Layer->GetX(), a_Layer->GetZ()));
 }
 
 
@@ -78,23 +64,26 @@ void cChunkMap::RemoveLayer(cChunkLayer * a_Layer)
 cChunkMap::cChunkLayer * cChunkMap::GetLayer(int a_LayerX, int a_LayerZ)
 {
 	cCSLock Lock(m_CSLayers);
-	for (cChunkLayerList::const_iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	auto it = m_Layers.find(std::pair<int, int>(a_LayerX, a_LayerZ));
+	if (it !=m_Layers.end())
 	{
-		if (((*itr)->GetX() == a_LayerX) && ((*itr)->GetZ() == a_LayerZ))
+		cChunkLayer & Layer = it->second;
-		{
+		return &Layer;
-			return *itr;
-		}
 	}
 
 	// Not found, create new:
-	cChunkLayer * Layer = new cChunkLayer(a_LayerX, a_LayerZ, this, *m_Pool);
+	auto EmplaceResult = m_Layers.emplace(std::piecewise_construct,
-	if (Layer == nullptr)
+		std::forward_as_tuple(a_LayerX, a_LayerZ),
-	{
+		std::forward_as_tuple(a_LayerX, a_LayerZ, this, *m_Pool));
-		LOGERROR("cChunkMap: Cannot create new layer, server out of memory?");
+
-		return nullptr;
+	// EmplaceResult.second = a bool, if false, then the item is already there.
-	}
+	// EmplaceResult.first = an iterator pointing to the inserted item
-	m_Layers.push_back(Layer);
+	// EmplaceResult.first->second = the inserted chunk layer
-	return Layer;
+	// EmplaceResult.first->first = the item's key (an std::pair if chunk X and chunk Z) - we don't need this
+
+	ASSERT (EmplaceResult.second == true);  // Make sure the element did not exist prior to the insertion
+	cChunkLayer & Layer = (EmplaceResult.first->second);
+	return &Layer;
 }
 
 
@@ -116,13 +105,12 @@ cChunkMap::cChunkLayer * cChunkMap::FindLayer(int a_LayerX, int a_LayerZ)
 {
 	ASSERT(m_CSLayers.IsLockedByCurrentThread());
 
-	for (cChunkLayerList::const_iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	auto it = m_Layers.find(std::pair<int, int>(a_LayerX, a_LayerZ));
+	if (it != m_Layers.end())
 	{
-		if (((*itr)->GetX() == a_LayerX) && ((*itr)->GetZ() == a_LayerZ))
+		cChunkLayer & Layer = it->second;
-		{
+		return &Layer;
-			return *itr;
+	}
-		}
-	}  // for itr - m_Layers[]
 
 	// Not found
 	return nullptr;
@@ -1591,9 +1579,10 @@ void cChunkMap::RemoveClientFromChunks(cClientHandle * a_Client)
 {
 	cCSLock Lock(m_CSLayers);
 
-	for (cChunkLayerList::const_iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr : m_Layers)
 	{
-		(*itr)->RemoveClient(a_Client);
+		cChunkLayer & Layer = itr.second;
+		Layer.RemoveClient(a_Client);
 	}  // for itr - m_Layers[]
 }
 
@@ -1643,9 +1632,10 @@ void cChunkMap::AddEntityIfNotPresent(cEntity * a_Entity)
 bool cChunkMap::HasEntity(UInt32 a_UniqueID)
 {
 	cCSLock Lock(m_CSLayers);
-	for (cChunkLayerList::const_iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr : m_Layers)
 	{
-		if ((*itr)->HasEntity(a_UniqueID))
+		cChunkLayer & Layer = itr.second;
+		if (Layer.HasEntity(a_UniqueID))
 		{
 			return true;
 		}
@@ -1677,9 +1667,10 @@ void cChunkMap::RemoveEntity(cEntity * a_Entity)
 bool cChunkMap::ForEachEntity(cEntityCallback & a_Callback)
 {
 	cCSLock Lock(m_CSLayers);
-	for (cChunkLayerList::const_iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr : m_Layers)
 	{
-		if (!(*itr)->ForEachEntity(a_Callback))
+		cChunkLayer & Layer = itr.second;
+		if (!Layer.ForEachEntity(a_Callback))
 		{
 			return false;
 		}
@@ -1938,9 +1929,10 @@ bool cChunkMap::DoWithEntityByID(UInt32 a_UniqueID, cEntityCallback & a_Callback
 {
 	cCSLock Lock(m_CSLayers);
 	bool res = false;
-	for (cChunkLayerList::const_iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr : m_Layers)
 	{
-		if ((*itr)->DoWithEntityByID(a_UniqueID, a_Callback, res))
+		cChunkLayer & Layer = itr.second;
+		if (Layer.DoWithEntityByID(a_UniqueID, a_Callback, res))
 		{
 			return res;
 		}
@@ -2494,14 +2486,14 @@ bool cChunkMap::ForEachChunkInRect(int a_MinChunkX, int a_MaxChunkX, int a_MinCh
 bool cChunkMap::ForEachLoadedChunk(std::function<bool(int, int)> a_Callback)
 {
 	cCSLock Lock(m_CSLayers);
-	for (cChunkLayerList::const_iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)  // iterate over ALL loaded layers
+	for (auto & itr: m_Layers)
 	{
-		cChunkLayer * layer = *itr;
+		cChunkLayer & Layer = itr.second;
 		for (int x = 0; x < LAYER_SIZE; x++)
 		{
 			for (int z = 0; z < LAYER_SIZE; z++)
 			{
-				cChunkPtr p = layer->FindChunk(layer->GetX() * LAYER_SIZE + x, layer->GetZ() * LAYER_SIZE + z);
+				cChunkPtr p = Layer.FindChunk(Layer.GetX() * LAYER_SIZE + x, Layer.GetZ() * LAYER_SIZE + z);
 				if ((p != nullptr) && p->IsValid())  // if chunk is loaded
 				{
 					if (a_Callback(p->GetPosX(), p->GetPosZ()))
@@ -2562,10 +2554,11 @@ void cChunkMap::GetChunkStats(int & a_NumChunksValid, int & a_NumChunksDirty)
 	a_NumChunksValid = 0;
 	a_NumChunksDirty = 0;
 	cCSLock Lock(m_CSLayers);
-	for (cChunkLayerList::const_iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr : m_Layers)
 	{
+		cChunkLayer & Layer = itr.second;
 		int NumValid = 0, NumDirty = 0;
-		(*itr)->GetChunkStats(NumValid, NumDirty);
+		Layer.GetChunkStats(NumValid, NumDirty);
 		a_NumChunksValid += NumValid;
 		a_NumChunksDirty += NumDirty;
 	}  // for itr - m_Layers[]
@@ -2645,9 +2638,10 @@ void cChunkMap::SetNextBlockTick(int a_BlockX, int a_BlockY, int a_BlockZ)
 void cChunkMap::CollectMobCensus(cMobCensus & a_ToFill)
 {
 	cCSLock Lock(m_CSLayers);
-	for (auto && layer: m_Layers)
+	for (auto & itr: m_Layers)
 	{
-		layer->CollectMobCensus(a_ToFill);
+		cChunkLayer & Layer = itr.second;
+		Layer.CollectMobCensus(a_ToFill);
 	}  // for itr - m_Layers
 }
 
@@ -2659,9 +2653,10 @@ void cChunkMap::CollectMobCensus(cMobCensus & a_ToFill)
 void cChunkMap::SpawnMobs(cMobSpawner & a_MobSpawner)
 {
 	cCSLock Lock(m_CSLayers);
-	for (auto && layer: m_Layers)
+	for (auto & itr: m_Layers)
 	{
-		layer->SpawnMobs(a_MobSpawner);
+		cChunkLayer & Layer = itr.second;
+		Layer.SpawnMobs(a_MobSpawner);
 	}  // for itr - m_Layers
 }
 
@@ -2672,9 +2667,10 @@ void cChunkMap::SpawnMobs(cMobSpawner & a_MobSpawner)
 void cChunkMap::Tick(std::chrono::milliseconds a_Dt)
 {
 	cCSLock Lock(m_CSLayers);
-	for (cChunkLayerList::iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr: m_Layers)
 	{
-		(*itr)->Tick(a_Dt);
+		cChunkLayer & Layer = itr.second;
+		Layer.Tick(a_Dt);
 	}  // for itr - m_Layers
 }
 
@@ -2702,9 +2698,10 @@ void cChunkMap::TickBlock(int a_BlockX, int a_BlockY, int a_BlockZ)
 void cChunkMap::UnloadUnusedChunks(void)
 {
 	cCSLock Lock(m_CSLayers);
-	for (cChunkLayerList::iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr: m_Layers)
 	{
-		(*itr)->UnloadUnusedChunks();
+		cChunkLayer & Layer = itr.second;
+		Layer.UnloadUnusedChunks();
 	}  // for itr - m_Layers
 }
 
@@ -2715,9 +2712,10 @@ void cChunkMap::UnloadUnusedChunks(void)
 void cChunkMap::SaveAllChunks(void)
 {
 	cCSLock Lock(m_CSLayers);
-	for (cChunkLayerList::iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr: m_Layers)
 	{
-		(*itr)->Save();
+		cChunkLayer & Layer = itr.second;
+		Layer.Save();
 	}  // for itr - m_Layers[]
 }
 
@@ -2729,9 +2727,10 @@ int cChunkMap::GetNumChunks(void)
 {
 	cCSLock Lock(m_CSLayers);
 	int NumChunks = 0;
-	for (cChunkLayerList::iterator itr = m_Layers.begin(); itr != m_Layers.end(); ++itr)
+	for (auto & itr: m_Layers)
 	{
-		NumChunks += (*itr)->GetNumChunksLoaded();
+		cChunkLayer & Layer = itr.second;
+		NumChunks += Layer.GetNumChunksLoaded();
 	}
 	return NumChunks;
 }

src/ChunkMap.h

@@ -12,7 +12,6 @@
 
 
 
-
 class cWorld;
 class cWorldInterface;
 class cItem;
@@ -68,7 +67,6 @@ public:
 	static const int LAYER_SIZE = 32;
 
 	cChunkMap(cWorld * a_World);
-	~cChunkMap();
 
 	// Broadcast respective packets to all clients of the chunk where the event is taking place
 	// (Please keep these alpha-sorted)
@@ -424,6 +422,8 @@ private:
 		);
 		~cChunkLayer();
 
+		cChunkLayer(const cChunkLayer & a_That) = delete;
+
 		/** Always returns an assigned chunkptr, but the chunk needn't be valid (loaded / generated) - callers must check */
 		cChunkPtr GetChunk( int a_ChunkX, int a_ChunkZ);
 
@@ -506,7 +506,7 @@ private:
 	void RemoveLayer(cChunkLayer * a_Layer);
 
 	cCriticalSection m_CSLayers;
-	cChunkLayerList  m_Layers;
+	std::map<std::pair<int, int>, cChunkLayer>  m_Layers;
 	cEvent           m_evtChunkValid;  // Set whenever any chunk becomes valid, via ChunkValidated()
 
 	cWorld * m_World;