Store cChunk::m_BlockEntities in a map (#3717)

* Store block entities in a map from block index
* Cleanup ForEachBlockEntity
* Cleanup DoWithBlockEntityAt

src/BlockID.h

@@ -1212,3 +1212,31 @@ extern cItem GetIniItemSet(cIniFile & a_IniFile, const char * a_Section, const c
 
 
 
+/** Base case for IsOneOf to handle empty template aguments. */
+template <class = void>
+bool IsOneOf(BLOCKTYPE a_BlockType)
+{
+	return false;
+}
+
+
+/** Returns true if a_BlockType is equal to any of the variadic template arguments.
+Some example usage:
+\code
+	IsOneOf<>(E_BLOCK_AIR)                           == false
+	IsOneOf<E_BLOCK_AIR>(E_BLOCK_DIRT)               == false
+	IsOneOf<E_BLOCK_AIR, E_BLOCK_DIRT>(E_BLOCK_DIRT) == true
+\endcode
+The implementation is ugly but it is equivalent to this C++17 fold expression:
+\code
+	((a_BlockType == Types) || ...)
+\endcode
+Just written to be valid without fold expressions or SFINAE. */
+template <BLOCKTYPE Head, BLOCKTYPE ... Tail>
+bool IsOneOf(BLOCKTYPE a_BlockType)
+{
+	return ((a_BlockType == Head) || (IsOneOf<Tail...>(a_BlockType)));
+}
+
+
+

src/Chunk.cpp

@@ -15,6 +15,7 @@
 #include "BlockEntities/BeaconEntity.h"
 #include "BlockEntities/BrewingstandEntity.h"
 #include "BlockEntities/ChestEntity.h"
+#include "BlockEntities/CommandBlockEntity.h"
 #include "BlockEntities/DispenserEntity.h"
 #include "BlockEntities/DropperEntity.h"
 #include "BlockEntities/FlowerPotEntity.h"
@@ -125,22 +126,22 @@ cChunk::~cChunk()
 
 	// LOGINFO("### delete cChunk() (%i, %i) from %p, thread 0x%x ###", m_PosX, m_PosZ, this, GetCurrentThreadId());
 
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	for (auto & KeyPair : m_BlockEntities)
 	{
-		delete *itr;
+		delete KeyPair.second;
 	}
 	m_BlockEntities.clear();
 
 	// Remove and destroy all entities that are not players:
 	cEntityList Entities;
 	std::swap(Entities, m_Entities);  // Need another list because cEntity destructors check if they've been removed from chunk
-	for (cEntityList::const_iterator itr = Entities.begin(); itr != Entities.end(); ++itr)
+	for (auto Entity : Entities)
 	{
-		if (!(*itr)->IsPlayer())
+		if (!Entity->IsPlayer())
 		{
 			// Scheduling a normal destruction is neither possible (Since this chunk will be gone till the schedule occurs) nor necessary.
-			(*itr)->DestroyNoScheduling(false);  // No point in broadcasting in an unloading chunk. Chunks unload when no one is nearby.
+			Entity->DestroyNoScheduling(false);  // No point in broadcasting in an unloading chunk. Chunks unload when no one is nearby.
-			delete *itr;
+			delete Entity;
 		}
 	}
 
@@ -298,14 +299,14 @@ void cChunk::GetAllData(cChunkDataCallback & a_Callback)
 
 	a_Callback.ChunkData(m_ChunkData);
 
-	for (cEntityList::iterator itr = m_Entities.begin(); itr != m_Entities.end(); ++itr)
+	for (auto Entity : m_Entities)
 	{
-		a_Callback.Entity(*itr);
+		a_Callback.Entity(Entity);
 	}
 
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	for (auto & KeyPair : m_BlockEntities)
 	{
-		a_Callback.BlockEntity(*itr);
+		a_Callback.BlockEntity(KeyPair.second);
 	}
 }
 
@@ -335,27 +336,28 @@ void cChunk::SetAllData(cSetChunkData & a_SetChunkData)
 	}
 
 	// Clear the block entities present - either the loader / saver has better, or we'll create empty ones:
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	for (auto & KeyPair : m_BlockEntities)
 	{
-		delete *itr;
+		delete KeyPair.second;
 	}
 	m_BlockEntities.clear();
 	std::swap(a_SetChunkData.GetBlockEntities(), m_BlockEntities);
 
 	// Check that all block entities have a valid blocktype at their respective coords (DEBUG-mode only):
 	#ifdef _DEBUG
-		for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+		for (auto & KeyPair : m_BlockEntities)
 		{
-			BLOCKTYPE EntityBlockType = (*itr)->GetBlockType();
+			cBlockEntity * Block = KeyPair.second;
-			BLOCKTYPE WorldBlockType = GetBlock((*itr)->GetRelX(), (*itr)->GetPosY(), (*itr)->GetRelZ());
+			BLOCKTYPE EntityBlockType = Block->GetBlockType();
+			BLOCKTYPE WorldBlockType = GetBlock(Block->GetRelX(), Block->GetPosY(), Block->GetRelZ());
 			ASSERT(WorldBlockType == EntityBlockType);
-		}  // for itr - m_BlockEntities
+		}  // for KeyPair - m_BlockEntities
 	#endif  // _DEBUG
 
 	// Set all block entities' World variable:
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	for (auto & KeyPair : m_BlockEntities)
 	{
-		(*itr)->SetWorld(m_World);
+		KeyPair.second->SetWorld(m_World);
 	}
 
 	// Create block entities that the loader didn't load; fill them with defaults
@@ -457,18 +459,7 @@ void cChunk::WriteBlockArea(cBlockArea & a_Area, int a_MinBlockX, int a_MinBlock
 
 bool cChunk::HasBlockEntityAt(int a_BlockX, int a_BlockY, int a_BlockZ)
 {
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	return (GetBlockEntity(a_BlockX, a_BlockY, a_BlockZ) != nullptr);
-	{
-		if (
-			((*itr)->GetPosX() == a_BlockX) &&
-			((*itr)->GetPosY() == a_BlockY) &&
-			((*itr)->GetPosZ() == a_BlockZ)
-		)
-		{
-			return true;
-		}
-	}  // for itr - m_BlockEntities[]
-	return false;
 }
 
 
@@ -638,9 +629,9 @@ void cChunk::Tick(std::chrono::milliseconds a_Dt)
 	TickBlocks();
 
 	// Tick all block entities in this chunk:
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	for (auto & KeyPair : m_BlockEntities)
 	{
-		m_IsDirty = (*itr)->Tick(a_Dt, *this) | m_IsDirty;
+		m_IsDirty = KeyPair.second->Tick(a_Dt, *this) | m_IsDirty;
 	}
 
 	for (cEntityList::iterator itr = m_Entities.begin(); itr != m_Entities.end();)
@@ -1382,7 +1373,7 @@ void cChunk::CreateBlockEntities(void)
 					{
 						if (!HasBlockEntityAt(x + m_PosX * Width, y, z + m_PosZ * Width))
 						{
-							m_BlockEntities.push_back(cBlockEntity::CreateByBlockType(
+							AddBlockEntityClean(cBlockEntity::CreateByBlockType(
 								BlockType, GetMeta(x, y, z),
 								x + m_PosX * Width, y, z + m_PosZ * Width, m_World
 							));
@@ -1513,7 +1504,8 @@ void cChunk::SetBlock(int a_RelX, int a_RelY, int a_RelZ, BLOCKTYPE a_BlockType,
 		case E_BLOCK_MOB_SPAWNER:
 		case E_BLOCK_BREWING_STAND:
 		{
-			AddBlockEntity(cBlockEntity::CreateByBlockType(a_BlockType, a_BlockMeta, WorldPos.x, WorldPos.y, WorldPos.z, m_World));
+			// Fast set block has already marked dirty
+			AddBlockEntityClean(cBlockEntity::CreateByBlockType(a_BlockType, a_BlockMeta, WorldPos.x, WorldPos.y, WorldPos.z, m_World));
 			break;
 		}
 	}  // switch (a_BlockType)
@@ -1663,13 +1655,11 @@ void cChunk::SendBlockTo(int a_RelX, int a_RelY, int a_RelZ, cClientHandle * a_C
 	a_Client->SendBlockChange(wp.x, wp.y, wp.z, GetBlock(a_RelX, a_RelY, a_RelZ), GetMeta(a_RelX, a_RelY, a_RelZ));
 
 	// FS #268 - if a BlockEntity digging is cancelled by a plugin, the entire block entity must be re-sent to the client:
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), end = m_BlockEntities.end(); itr != end; ++itr)
+	cBlockEntity * Block = GetBlockEntity(wp.x, wp.y, wp.z);
+	if (Block != nullptr)
 	{
-		if (((*itr)->GetPosX() == wp.x) && ((*itr)->GetPosY() == wp.y) && ((*itr)->GetPosZ() == wp.z))
+		Block->SendTo(*a_Client);
-		{
+	}
-			(*itr)->SendTo(*a_Client);
-		}
-	}  // for itr - m_BlockEntities
 }
 
 
@@ -1679,7 +1669,19 @@ void cChunk::SendBlockTo(int a_RelX, int a_RelY, int a_RelZ, cClientHandle * a_C
 void cChunk::AddBlockEntity(cBlockEntity * a_BlockEntity)
 {
 	MarkDirty();
-	m_BlockEntities.push_back(a_BlockEntity);
+	AddBlockEntityClean(a_BlockEntity);
+}
+
+
+
+
+
+void cChunk::AddBlockEntityClean(cBlockEntity * a_BlockEntity)
+{
+	int Idx = MakeIndex(a_BlockEntity->GetRelX(), a_BlockEntity->GetPosY(), a_BlockEntity->GetRelZ());
+	auto Result = m_BlockEntities.insert({ Idx, a_BlockEntity });
+	UNUSED(Result);
+	ASSERT(Result.second);  // No block entity already at this position
 }
 
 
@@ -1694,19 +1696,11 @@ cBlockEntity * cChunk::GetBlockEntity(int a_BlockX, int a_BlockY, int a_BlockZ)
 	ASSERT(a_BlockZ >= m_PosZ * cChunkDef::Width);
 	ASSERT(a_BlockZ < m_PosZ * cChunkDef::Width + cChunkDef::Width);
 
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	int RelX = a_BlockX - m_PosX * cChunkDef::Width;
-	{
+	int RelZ = a_BlockZ - m_PosZ * cChunkDef::Width;
-		if (
-			((*itr)->GetPosX() == a_BlockX) &&
-			((*itr)->GetPosY() == a_BlockY) &&
-			((*itr)->GetPosZ() == a_BlockZ)
-		)
-		{
-			return *itr;
-		}
-	}  // for itr - m_BlockEntities[]
 
-	return nullptr;
+	auto itr = m_BlockEntities.find(MakeIndex(RelX, a_BlockY, RelZ));
+	return (itr == m_BlockEntities.end()) ? nullptr : itr->second;
 }
 
 
@@ -1837,25 +1831,24 @@ void cChunk::CollectPickupsByPlayer(cPlayer & a_Player)
 bool cChunk::SetSignLines(int a_PosX, int a_PosY, int a_PosZ, const AString & a_Line1, const AString & a_Line2, const AString & a_Line3, const AString & a_Line4)
 {
 	// Also sends update packets to all clients in the chunk
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	auto Entity = GetBlockEntity(a_PosX, a_PosY, a_PosZ);
+	if (Entity == nullptr)
 	{
-		if (
+		return false;  // Not a block entity
-			((*itr)->GetPosX() == a_PosX) &&
+	}
-			((*itr)->GetPosY() == a_PosY) &&
+	if (
-			((*itr)->GetPosZ() == a_PosZ) &&
+		(Entity->GetBlockType() != E_BLOCK_WALLSIGN) &&
-			(
+		(Entity->GetBlockType() != E_BLOCK_SIGN_POST)
-				((*itr)->GetBlockType() == E_BLOCK_WALLSIGN) ||
+	)
-				((*itr)->GetBlockType() == E_BLOCK_SIGN_POST)
+	{
-			)
+		return false;  // Not a sign
-		)
+	}
-		{
+
-			MarkDirty();
+	MarkDirty();
-			reinterpret_cast<cSignEntity *>(*itr)->SetLines(a_Line1, a_Line2, a_Line3, a_Line4);
+	auto Sign = static_cast<cSignEntity *>(Entity);
-			m_World->BroadcastBlockEntity(a_PosX, a_PosY, a_PosZ);
+	Sign->SetLines(a_Line1, a_Line2, a_Line3, a_Line4);
-			return true;
+	m_World->BroadcastBlockEntity(a_PosX, a_PosY, a_PosZ);
-		}
+	return true;
-	}  // for itr - m_BlockEntities[]
-	return false;
 }
 
 
@@ -1865,7 +1858,9 @@ bool cChunk::SetSignLines(int a_PosX, int a_PosY, int a_PosZ, const AString & a_
 void cChunk::RemoveBlockEntity(cBlockEntity * a_BlockEntity)
 {
 	MarkDirty();
-	m_BlockEntities.remove(a_BlockEntity);
+	ASSERT(a_BlockEntity != nullptr);
+	int Idx = MakeIndex(a_BlockEntity->GetRelX(), a_BlockEntity->GetPosY(), a_BlockEntity->GetRelZ());
+	m_BlockEntities.erase(Idx);
 }
 
 
@@ -2076,17 +2071,24 @@ bool cChunk::DoWithEntityByID(UInt32 a_EntityID, cLambdaEntityCallback a_Callbac
 
 
 
-bool cChunk::ForEachBlockEntity(cBlockEntityCallback & a_Callback)
+template <class tyEntity, BLOCKTYPE... tBlocktype>
+bool cChunk::GenericForEachBlockEntity(cItemCallback<tyEntity>& a_Callback)
 {
 	// The blockentity list is locked by the parent chunkmap's CS
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+	for (auto & KeyPair : m_BlockEntities)
 	{
-		++itr2;
+		cBlockEntity * Block = KeyPair.second;
-		if (a_Callback.Item(*itr))
+		if (
+			(sizeof...(tBlocktype) == 0) ||  // Let empty list mean all block entities
+			(IsOneOf<tBlocktype...>(Block->GetBlockType()))
+		)
 		{
-			return false;
+			if (a_Callback.Item(static_cast<tyEntity *>(Block)))
+			{
+				return false;
+			}
 		}
-	}  // for itr - m_BlockEntitites[]
+	}  // for KeyPair - m_BlockEntitites[]
 	return true;
 }
 
@@ -2094,18 +2096,20 @@ bool cChunk::ForEachBlockEntity(cBlockEntityCallback & a_Callback)
 
 
 
+bool cChunk::ForEachBlockEntity(cBlockEntityCallback & a_Callback)
+{
+	return GenericForEachBlockEntity<cBlockEntity>(a_Callback);
+}
+
+
+
+
+
 bool cChunk::ForEachBrewingstand(cBrewingstandCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericForEachBlockEntity<cBrewingstandEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_BREWING_STAND
-	{
+	>(a_Callback);
-		++itr2;
-		if (a_Callback.Item(reinterpret_cast<cBrewingstandEntity *>(*itr)))
-		{
-			return false;
-		}
-	}  // for itr - m_BlockEntitites[]
-	return true;
 }
 
 
@@ -2114,20 +2118,9 @@ bool cChunk::ForEachBrewingstand(cBrewingstandCallback & a_Callback)
 
 bool cChunk::ForEachChest(cChestCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericForEachBlockEntity<cChestEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_CHEST
-	{
+	>(a_Callback);
-		++itr2;
-		if ((*itr)->GetBlockType() != E_BLOCK_CHEST)
-		{
-			continue;
-		}
-		if (a_Callback.Item(reinterpret_cast<cChestEntity *>(*itr)))
-		{
-			return false;
-		}
-	}  // for itr - m_BlockEntitites[]
-	return true;
 }
 
 
@@ -2136,20 +2129,9 @@ bool cChunk::ForEachChest(cChestCallback & a_Callback)
 
 bool cChunk::ForEachDispenser(cDispenserCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericForEachBlockEntity<cDispenserEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_DISPENSER
-	{
+	>(a_Callback);
-		++itr2;
-		if ((*itr)->GetBlockType() != E_BLOCK_DISPENSER)
-		{
-			continue;
-		}
-		if (a_Callback.Item(reinterpret_cast<cDispenserEntity *>(*itr)))
-		{
-			return false;
-		}
-	}  // for itr - m_BlockEntitites[]
-	return true;
 }
 
 
@@ -2158,20 +2140,9 @@ bool cChunk::ForEachDispenser(cDispenserCallback & a_Callback)
 
 bool cChunk::ForEachDropper(cDropperCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericForEachBlockEntity<cDropperEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_DROPPER
-	{
+	>(a_Callback);
-		++itr2;
-		if ((*itr)->GetBlockType() != E_BLOCK_DROPPER)
-		{
-			continue;
-		}
-		if (a_Callback.Item(reinterpret_cast<cDropperEntity *>(*itr)))
-		{
-			return false;
-		}
-	}  // for itr - m_BlockEntitites[]
-	return true;
 }
 
 
@@ -2180,20 +2151,10 @@ bool cChunk::ForEachDropper(cDropperCallback & a_Callback)
 
 bool cChunk::ForEachDropSpenser(cDropSpenserCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericForEachBlockEntity<cDropSpenserEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_DISPENSER,
-	{
+		E_BLOCK_DROPPER
-		++itr2;
+	>(a_Callback);
-		if (((*itr)->GetBlockType() != E_BLOCK_DISPENSER) && ((*itr)->GetBlockType() != E_BLOCK_DROPPER))
-		{
-			continue;
-		}
-		if (a_Callback.Item(reinterpret_cast<cDropSpenserEntity *>(*itr)))
-		{
-			return false;
-		}
-	}  // for itr - m_BlockEntitites[]
-	return true;
 }
 
 
@@ -2201,29 +2162,34 @@ bool cChunk::ForEachDropSpenser(cDropSpenserCallback & a_Callback)
 
 
 bool cChunk::ForEachFurnace(cFurnaceCallback & a_Callback)
+{
+	return GenericForEachBlockEntity<cFurnaceEntity,
+		E_BLOCK_FURNACE,
+		E_BLOCK_LIT_FURNACE
+	>(a_Callback);
+}
+
+
+
+
+
+template <class tyEntity, BLOCKTYPE... tBlocktype>
+bool cChunk::GenericDoWithBlockEntityAt(int a_BlockX, int a_BlockY, int a_BlockZ, cItemCallback<tyEntity>& a_Callback)
 {
 	// The blockentity list is locked by the parent chunkmap's CS
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+	cBlockEntity * Block = GetBlockEntity(a_BlockX, a_BlockY, a_BlockZ);
+	if (Block == nullptr)
 	{
-		++itr2;
+		return false;  // No block entity here
-		switch ((*itr)->GetBlockType())
+	}
-		{
+	if (
-			case E_BLOCK_FURNACE:
+		(sizeof...(tBlocktype) != 0) &&  // Let empty list mean all block entities
-			case E_BLOCK_LIT_FURNACE:
+		(!IsOneOf<tBlocktype...>(Block->GetBlockType()))
-			{
+	)
-				break;
+	{
-			}
+		return false;  // Not any of the given tBlocktypes
-			default:
+	}
-			{
+	return !a_Callback.Item(static_cast<tyEntity *>(Block));
-				continue;
-			}
-		}
-		if (a_Callback.Item(reinterpret_cast<cFurnaceEntity *>(*itr)))
-		{
-			return false;
-		}
-	}  // for itr - m_BlockEntitites[]
-	return true;
 }
 
 
@@ -2232,24 +2198,7 @@ bool cChunk::ForEachFurnace(cFurnaceCallback & a_Callback)
 
 bool cChunk::DoWithBlockEntityAt(int a_BlockX, int a_BlockY, int a_BlockZ, cBlockEntityCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cBlockEntity>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
-	{
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-
-		if (a_Callback.Item(*itr))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2257,30 +2206,9 @@ bool cChunk::DoWithBlockEntityAt(int a_BlockX, int a_BlockY, int a_BlockZ, cBloc
 
 bool cChunk::DoWithBeaconAt(int a_BlockX, int a_BlockY, int a_BlockZ, cBeaconCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cBeaconEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_BEACON
-	{
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if ((*itr)->GetBlockType() != E_BLOCK_BEACON)
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here
-		if (a_Callback.Item(reinterpret_cast<cBeaconEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2289,30 +2217,9 @@ bool cChunk::DoWithBeaconAt(int a_BlockX, int a_BlockY, int a_BlockZ, cBeaconCal
 
 bool cChunk::DoWithBrewingstandAt(int a_BlockX, int a_BlockY, int a_BlockZ, cBrewingstandCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cBrewingstandEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_BREWING_STAND
-	{
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if ((*itr)->GetBlockType() != E_BLOCK_BREWING_STAND)
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here
-		if (a_Callback.Item(reinterpret_cast<cBrewingstandEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2321,30 +2228,10 @@ bool cChunk::DoWithBrewingstandAt(int a_BlockX, int a_BlockY, int a_BlockZ, cBre
 
 bool cChunk::DoWithChestAt(int a_BlockX, int a_BlockY, int a_BlockZ, cChestCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cChestEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_CHEST,
-	{
+		E_BLOCK_TRAPPED_CHEST
-		++itr2;
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if (((*itr)->GetBlockType() != E_BLOCK_CHEST) && ((*itr)->GetBlockType() != E_BLOCK_TRAPPED_CHEST))  // Trapped chests use normal chests' handlers
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here
-		if (a_Callback.Item(reinterpret_cast<cChestEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2353,30 +2240,9 @@ bool cChunk::DoWithChestAt(int a_BlockX, int a_BlockY, int a_BlockZ, cChestCallb
 
 bool cChunk::DoWithDispenserAt(int a_BlockX, int a_BlockY, int a_BlockZ, cDispenserCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cDispenserEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_DISPENSER
-	{
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if ((*itr)->GetBlockType() != E_BLOCK_DISPENSER)
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here
-		if (a_Callback.Item(reinterpret_cast<cDispenserEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2385,30 +2251,9 @@ bool cChunk::DoWithDispenserAt(int a_BlockX, int a_BlockY, int a_BlockZ, cDispen
 
 bool cChunk::DoWithDropperAt(int a_BlockX, int a_BlockY, int a_BlockZ, cDropperCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cDropperEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_DROPPER
-	{
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if ((*itr)->GetBlockType() != E_BLOCK_DROPPER)
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here
-		if (a_Callback.Item(reinterpret_cast<cDropperEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2417,30 +2262,10 @@ bool cChunk::DoWithDropperAt(int a_BlockX, int a_BlockY, int a_BlockZ, cDropperC
 
 bool cChunk::DoWithDropSpenserAt(int a_BlockX, int a_BlockY, int a_BlockZ, cDropSpenserCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cDropSpenserEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_DISPENSER,
-	{
+		E_BLOCK_DROPPER
-		++itr2;
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if (((*itr)->GetBlockType() != E_BLOCK_DISPENSER) && ((*itr)->GetBlockType() != E_BLOCK_DROPPER))
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here
-		if (a_Callback.Item(reinterpret_cast<cDropSpenserEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2449,38 +2274,10 @@ bool cChunk::DoWithDropSpenserAt(int a_BlockX, int a_BlockY, int a_BlockZ, cDrop
 
 bool cChunk::DoWithFurnaceAt(int a_BlockX, int a_BlockY, int a_BlockZ, cFurnaceCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cFurnaceEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_FURNACE,
-	{
+		E_BLOCK_LIT_FURNACE
-		++itr2;
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		switch ((*itr)->GetBlockType())
-		{
-			case E_BLOCK_FURNACE:
-			case E_BLOCK_LIT_FURNACE:
-			{
-				break;
-			}
-			default:
-			{
-				// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-				return false;
-			}
-		}  // switch (BlockType)
-
-		// The correct block entity is here,
-		if (a_Callback.Item(reinterpret_cast<cFurnaceEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2489,30 +2286,9 @@ bool cChunk::DoWithFurnaceAt(int a_BlockX, int a_BlockY, int a_BlockZ, cFurnaceC
 
 bool cChunk::DoWithNoteBlockAt(int a_BlockX, int a_BlockY, int a_BlockZ, cNoteBlockCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cNoteEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_NOTE_BLOCK
-	{
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if ((*itr)->GetBlockType() != E_BLOCK_NOTE_BLOCK)
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here
-		if (a_Callback.Item(reinterpret_cast<cNoteEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2521,30 +2297,9 @@ bool cChunk::DoWithNoteBlockAt(int a_BlockX, int a_BlockY, int a_BlockZ, cNoteBl
 
 bool cChunk::DoWithCommandBlockAt(int a_BlockX, int a_BlockY, int a_BlockZ, cCommandBlockCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cCommandBlockEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_COMMAND_BLOCK
-	{
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if ((*itr)->GetBlockType() != E_BLOCK_COMMAND_BLOCK)
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here,
-		if (a_Callback.Item(reinterpret_cast<cCommandBlockEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2553,30 +2308,9 @@ bool cChunk::DoWithCommandBlockAt(int a_BlockX, int a_BlockY, int a_BlockZ, cCom
 
 bool cChunk::DoWithMobHeadAt(int a_BlockX, int a_BlockY, int a_BlockZ, cMobHeadCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cMobHeadEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_HEAD
-	{
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if ((*itr)->GetBlockType() != E_BLOCK_HEAD)
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here,
-		if (a_Callback.Item(reinterpret_cast<cMobHeadEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2585,30 +2319,9 @@ bool cChunk::DoWithMobHeadAt(int a_BlockX, int a_BlockY, int a_BlockZ, cMobHeadC
 
 bool cChunk::DoWithFlowerPotAt(int a_BlockX, int a_BlockY, int a_BlockZ, cFlowerPotCallback & a_Callback)
 {
-	// The blockentity list is locked by the parent chunkmap's CS
+	return GenericDoWithBlockEntityAt<cFlowerPotEntity,
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), itr2 = itr; itr != m_BlockEntities.end(); itr = itr2)
+		E_BLOCK_FLOWER_POT
-	{
+	>(a_BlockX, a_BlockY, a_BlockZ, a_Callback);
-		++itr2;
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
-		{
-			continue;
-		}
-		if ((*itr)->GetBlockType() != E_BLOCK_FLOWER_POT)
-		{
-			// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
-			return false;
-		}
-
-		// The correct block entity is here
-		if (a_Callback.Item(reinterpret_cast<cFlowerPotEntity *>(*itr)))
-		{
-			return false;
-		}
-		return true;
-	}  // for itr - m_BlockEntitites[]
-
-	// Not found:
-	return false;
 }
 
 
@@ -2618,31 +2331,25 @@ bool cChunk::DoWithFlowerPotAt(int a_BlockX, int a_BlockY, int a_BlockZ, cFlower
 bool cChunk::GetSignLines(int a_BlockX, int a_BlockY, int a_BlockZ, AString & a_Line1, AString & a_Line2, AString & a_Line3, AString & a_Line4)
 {
 	// The blockentity list is locked by the parent chunkmap's CS
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end(); ++itr)
+	auto Entity = GetBlockEntity(a_BlockX, a_BlockY, a_BlockZ);
+	if (Entity == nullptr)
 	{
-		if (((*itr)->GetPosX() != a_BlockX) || ((*itr)->GetPosY() != a_BlockY) || ((*itr)->GetPosZ() != a_BlockZ))
+		return false;  // Not a block entity
-		{
+	}
-			continue;
+	if (
-		}
+		(Entity->GetBlockType() != E_BLOCK_WALLSIGN) &&
-		switch ((*itr)->GetBlockType())
+		(Entity->GetBlockType() != E_BLOCK_SIGN_POST)
-		{
+	)
-			case E_BLOCK_WALLSIGN:
+	{
-			case E_BLOCK_SIGN_POST:
+		return false;  // Not a sign
-			{
+	}
-				a_Line1 = reinterpret_cast<cSignEntity *>(*itr)->GetLine(0);
-				a_Line2 = reinterpret_cast<cSignEntity *>(*itr)->GetLine(1);
-				a_Line3 = reinterpret_cast<cSignEntity *>(*itr)->GetLine(2);
-				a_Line4 = reinterpret_cast<cSignEntity *>(*itr)->GetLine(3);
-				return true;
-			}
-		}  // switch (BlockType)
 
-		// There is a block entity here, but of different type. No other block entity can be here, so we can safely bail out
+	auto Sign = static_cast<cSignEntity *>(Entity);
-		return false;
+	a_Line1 = Sign->GetLine(0);
-	}  // for itr - m_BlockEntitites[]
+	a_Line2 = Sign->GetLine(1);
-
+	a_Line3 = Sign->GetLine(2);
-	// Not found:
+	a_Line4 = Sign->GetLine(3);
-	return false;
+	return true;
 }
 
 

src/Chunk.h

@@ -276,6 +276,12 @@ public:
 	bool DoWithEntityByID(UInt32 a_EntityID, cEntityCallback & a_Callback, bool & a_CallbackResult);  // Lua-accessible
 	bool DoWithEntityByID(UInt32 a_EntityID, cLambdaEntityCallback a_Callback, bool & a_CallbackResult);  // Lambda version
 
+	/** Calls the callback for each tyEntity; returns true if all block entities processed, false if the callback aborted by returning true
+	tBlocktypes are all blocktypes convertible to tyEntity which are to be called. If no block type is given the callback is called for every block entity
+	Accessible only from within Chunk.cpp */
+	template <class tyEntity, BLOCKTYPE... tBlocktype>
+	bool GenericForEachBlockEntity(cItemCallback<tyEntity>& a_Callback);
+
 	/** Calls the callback for each block entity; returns true if all block entities processed, false if the callback aborted by returning true */
 	bool ForEachBlockEntity(cBlockEntityCallback & a_Callback);  // Lua-accessible
 
@@ -297,6 +303,12 @@ public:
 	/** Calls the callback for each furnace; returns true if all furnaces processed, false if the callback aborted by returning true */
 	bool ForEachFurnace(cFurnaceCallback & a_Callback);  // Lua-accessible
 
+	/** Calls the callback for the tyEntity at the specified coords; returns false if there's no such block entity at those coords, true if found
+	tBlocktype is a list of the blocktypes to be called. If no BLOCKTYPE template arguments are given the callback is called for any block entity
+	Accessible only from within Chunk.cpp */
+	template <class tyEntity, BLOCKTYPE... tBlocktype>
+	bool GenericDoWithBlockEntityAt(int a_BlockX, int a_BlockY, int a_BlockZ, cItemCallback<tyEntity>& a_Callback);
+
 	/** Calls the callback for the block entity at the specified coords; returns false if there's no block entity at those coords, true if found */
 	bool DoWithBlockEntityAt(int a_BlockX, int a_BlockY, int a_BlockZ, cBlockEntityCallback & a_Callback);  // Lua-acessible
 
@@ -524,7 +536,7 @@ private:
 	// A critical section is not needed, because all chunk access is protected by its parent ChunkMap's csLayers
 	std::vector<cClientHandle *> m_LoadedByClient;
 	cEntityList                  m_Entities;
-	cBlockEntityList             m_BlockEntities;
+	cBlockEntities               m_BlockEntities;
 
 	/** Number of times the chunk has been requested to stay (by various cChunkStay objects); if zero, the chunk can be unloaded */
 	int m_StayCount;
@@ -566,7 +578,10 @@ private:
 	void RemoveBlockEntity(cBlockEntity * a_BlockEntity);
 	void AddBlockEntity   (cBlockEntity * a_BlockEntity);
 
-	/** Creates a block entity for each block that needs a block entity and doesn't have one in the list */
+	/** Add a block entity to the chunk without marking the chunk dirty */
+	void AddBlockEntityClean(cBlockEntity * a_BlockEntity);
+
+	/** Creates a block entity for each block that needs a block entity and doesn't have one already */
 	void CreateBlockEntities(void);
 
 	/** Wakes up each simulator for its specific blocks; through all the blocks in the chunk */

src/ChunkDef.h

@@ -31,8 +31,8 @@ class cEntity;
 class cClientHandle;
 class cBlockEntity;
 
-typedef std::list<cEntity *>        cEntityList;
+typedef std::list<cEntity *>          cEntityList;
-typedef std::list<cBlockEntity *>   cBlockEntityList;
+typedef std::map<int, cBlockEntity *> cBlockEntities;
 
 
 

src/Generating/ChunkDesc.cpp

@@ -573,23 +573,27 @@ void cChunkDesc::RandomFillRelCuboid(
 
 cBlockEntity * cChunkDesc::GetBlockEntity(int a_RelX, int a_RelY, int a_RelZ)
 {
+	auto Idx = cChunkDef::MakeIndex(a_RelX, a_RelY, a_RelZ);
+	auto itr = m_BlockEntities.find(Idx);
+
+	if (itr != m_BlockEntities.end())
+	{
+		// Already in the list:
+		cBlockEntity * BlockEntity = itr->second;
+		if (BlockEntity->GetBlockType() == GetBlockType(a_RelX, a_RelY, a_RelZ))
+		{
+			// Correct type, already present. Return it:
+			return BlockEntity;
+		}
+		else
+		{
+			// Wrong type, the block type has been overwritten. Erase and create new:
+			m_BlockEntities.erase(itr);
+		}
+	}
+
 	int AbsX = a_RelX + m_ChunkX * cChunkDef::Width;
 	int AbsZ = a_RelZ + m_ChunkZ * cChunkDef::Width;
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(), end = m_BlockEntities.end(); itr != end; ++itr)
-	{
-		if (((*itr)->GetPosX() == AbsX) && ((*itr)->GetPosY() == a_RelY) && ((*itr)->GetPosZ() == AbsZ))
-		{
-			// Already in the list:
-			if ((*itr)->GetBlockType() != GetBlockType(a_RelX, a_RelY, a_RelZ))
-			{
-				// Wrong type, the block type has been overwritten. Erase and create new:
-				m_BlockEntities.erase(itr);
-				break;
-			}
-			// Correct type, already present. Return it:
-			return *itr;
-		}
-	}  // for itr - m_BlockEntities[]
 
 	// The block entity is not created yet, try to create it and add to list:
 	cBlockEntity * be = cBlockEntity::CreateByBlockType(GetBlockType(a_RelX, a_RelY, a_RelZ), GetBlockMeta(a_RelX, a_RelY, a_RelZ), AbsX, a_RelY, AbsZ);
@@ -598,7 +602,7 @@ cBlockEntity * cChunkDesc::GetBlockEntity(int a_RelX, int a_RelY, int a_RelZ)
 		// No block entity for this block type
 		return nullptr;
 	}
-	m_BlockEntities.push_back(be);
+	m_BlockEntities.insert({ Idx, be });
 	return be;
 }
 

src/Generating/ChunkDesc.h

@@ -215,7 +215,7 @@ public:
 	inline BlockNibbleBytes &        GetBlockMetasUncompressed(void) { return *(reinterpret_cast<BlockNibbleBytes *>(m_BlockArea.GetBlockMetas())); }
 	inline cChunkDef::HeightMap &    GetHeightMap             (void) { return m_HeightMap; }
 	inline cEntityList &             GetEntities              (void) { return m_Entities; }
-	inline cBlockEntityList &        GetBlockEntities         (void) { return m_BlockEntities; }
+	inline cBlockEntities &          GetBlockEntities         (void) { return m_BlockEntities; }
 
 	/** Compresses the metas from the BlockArea format (1 meta per byte) into regular format (2 metas per byte) */
 	void CompressBlockMetas(cChunkDef::BlockNibbles & a_DestMetas);
@@ -233,7 +233,7 @@ private:
 	cBlockArea              m_BlockArea;
 	cChunkDef::HeightMap    m_HeightMap;
 	cEntityList             m_Entities;       // Individual entities are NOT owned by this object!
-	cBlockEntityList        m_BlockEntities;  // Individual block entities are NOT owned by this object!
+	cBlockEntities          m_BlockEntities;  // Individual block entities are NOT owned by this object!
 
 	bool m_bUseDefaultBiomes;
 	bool m_bUseDefaultHeight;

src/SetChunkData.cpp

@@ -34,7 +34,7 @@ cSetChunkData::cSetChunkData(
 	const cChunkDef::HeightMap * a_HeightMap,
 	const cChunkDef::BiomeMap * a_Biomes,
 	cEntityList && a_Entities,
-	cBlockEntityList && a_BlockEntities,
+	cBlockEntities && a_BlockEntities,
 	bool a_ShouldMarkDirty
 ) :
 	m_ChunkX(a_ChunkX),
@@ -119,23 +119,21 @@ void cSetChunkData::CalculateHeightMap(void)
 
 void cSetChunkData::RemoveInvalidBlockEntities(void)
 {
-	for (cBlockEntityList::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end();)
+	for (cBlockEntities::iterator itr = m_BlockEntities.begin(); itr != m_BlockEntities.end();)
 	{
-		BLOCKTYPE EntityBlockType = (*itr)->GetBlockType();
+		cBlockEntity * BlockEntity = itr->second;
-		BLOCKTYPE WorldBlockType = cChunkDef::GetBlock(m_BlockTypes, (*itr)->GetRelX(), (*itr)->GetPosY(), (*itr)->GetRelZ());
+		BLOCKTYPE EntityBlockType = BlockEntity->GetBlockType();
+		BLOCKTYPE WorldBlockType = cChunkDef::GetBlock(m_BlockTypes, BlockEntity->GetRelX(), BlockEntity->GetPosY(), BlockEntity->GetRelZ());
 		if (EntityBlockType != WorldBlockType)
 		{
 			// Bad blocktype, remove the block entity:
 			LOGD("Block entity blocktype mismatch at {%d, %d, %d}: entity for blocktype %s(%d) in block %s(%d). Deleting the block entity.",
-				(*itr)->GetPosX(), (*itr)->GetPosY(), (*itr)->GetPosZ(),
+				BlockEntity->GetPosX(), BlockEntity->GetPosY(), BlockEntity->GetPosZ(),
 				ItemTypeToString(EntityBlockType).c_str(), EntityBlockType,
 				ItemTypeToString(WorldBlockType).c_str(),  WorldBlockType
 			);
-			cBlockEntityList::iterator itr2 = itr;
+			delete BlockEntity;
-			++itr2;
+			itr = m_BlockEntities.erase(itr);
-			delete *itr;
-			m_BlockEntities.erase(itr);
-			itr = itr2;
 		}
 		else
 		{

src/SetChunkData.h

@@ -24,7 +24,7 @@ public:
 
 	/** Constructs a new instance based on data existing elsewhere, will copy all the memory. Prefer to use the
 	other constructor as much as possible.
-	Will move the entity and blockentity lists into the internal storage, and invalidate a_Entities and
+	Will move the entity list and blockentities into the internal storage, and invalidate a_Entities and
 	a_BlockEntities.
 	When passing an lvalue, a_Entities and a_BlockEntities must be explicitly converted to an rvalue beforehand
 	with std::move().
@@ -44,7 +44,7 @@ public:
 		const cChunkDef::HeightMap * a_HeightMap,
 		const cChunkDef::BiomeMap * a_Biomes,
 		cEntityList && a_Entities,
-		cBlockEntityList && a_BlockEntities,
+		cBlockEntities && a_BlockEntities,
 		bool a_ShouldMarkDirty
 	);
 
@@ -73,7 +73,7 @@ public:
 	cEntityList & GetEntities(void) { return m_Entities; }
 
 	/** Returns the internal storage for block entities, read-write. */
-	cBlockEntityList & GetBlockEntities(void) { return m_BlockEntities; }
+	cBlockEntities & GetBlockEntities(void) { return m_BlockEntities; }
 
 	/** Returns whether both light arrays stored in this object are valid. */
 	bool IsLightValid(void) const { return m_IsLightValid; }
@@ -108,7 +108,7 @@ protected:
 	cChunkDef::HeightMap m_HeightMap;
 	cChunkDef::BiomeMap m_Biomes;
 	cEntityList m_Entities;
-	cBlockEntityList m_BlockEntities;
+	cBlockEntities m_BlockEntities;
 
 	bool m_IsLightValid;
 	bool m_IsHeightMapValid;

src/WorldStorage/WSSAnvil.cpp

@@ -417,7 +417,7 @@ bool cWSSAnvil::LoadChunkFromNBT(const cChunkCoords & a_Chunk, const cParsedNBT
 
 	// Load the entities from NBT:
 	cEntityList      Entities;
-	cBlockEntityList BlockEntities;
+	cBlockEntities   BlockEntities;
 	LoadEntitiesFromNBT     (Entities,      a_NBT, a_NBT.FindChildByName(Level, "Entities"));
 	LoadBlockEntitiesFromNBT(BlockEntities, a_NBT, a_NBT.FindChildByName(Level, "TileEntities"), BlockTypes, MetaData);
 
@@ -639,7 +639,7 @@ void cWSSAnvil::LoadEntitiesFromNBT(cEntityList & a_Entities, const cParsedNBT &
 
 
 
-void cWSSAnvil::LoadBlockEntitiesFromNBT(cBlockEntityList & a_BlockEntities, const cParsedNBT & a_NBT, int a_TagIdx, BLOCKTYPE * a_BlockTypes, NIBBLETYPE * a_BlockMetas)
+void cWSSAnvil::LoadBlockEntitiesFromNBT(cBlockEntities & a_BlockEntities, const cParsedNBT & a_NBT, int a_TagIdx, BLOCKTYPE * a_BlockTypes, NIBBLETYPE * a_BlockMetas)
 {
 	if ((a_TagIdx < 0) || (a_NBT.GetType(a_TagIdx) != TAG_List))
 	{
@@ -673,7 +673,10 @@ void cWSSAnvil::LoadBlockEntitiesFromNBT(cBlockEntityList & a_BlockEntities, con
 		}
 
 		// Add the BlockEntity to the loaded data:
-		a_BlockEntities.push_back(be.release());
+		auto Idx = cChunkDef::MakeIndex(be->GetRelX(), be->GetPosY(), be->GetRelZ());
+		a_BlockEntities.insert({ Idx, be.get() });
+		// Release after inserting in case it throws.
+		be.release();
 	}  // for Child - tag children
 }
 

src/WorldStorage/WSSAnvil.h

@@ -131,7 +131,7 @@ protected:
 	void LoadEntitiesFromNBT(cEntityList & a_Entitites, const cParsedNBT & a_NBT, int a_Tag);
 
 	/** Loads the chunk's BlockEntities from NBT data (a_Tag is the Level\\TileEntities list tag; may be -1) */
-	void LoadBlockEntitiesFromNBT(cBlockEntityList & a_BlockEntitites, const cParsedNBT & a_NBT, int a_Tag, BLOCKTYPE * a_BlockTypes, NIBBLETYPE * a_BlockMetas);
+	void LoadBlockEntitiesFromNBT(cBlockEntities & a_BlockEntitites, const cParsedNBT & a_NBT, int a_Tag, BLOCKTYPE * a_BlockTypes, NIBBLETYPE * a_BlockMetas);
 
 	/** Loads the data for a block entity from the specified NBT tag.
 	Returns the loaded block entity, or nullptr upon failure. */