1 files changed, 77 insertions, 50 deletions

diff --git a/src/Chunk.cpp b/src/Chunk.cpp
index 646aea985..f2b936a3f 100644
--- a/src/Chunk.cpp
+++ b/src/Chunk.cpp
@@ -136,13 +136,12 @@ cChunk::~cChunk()
 	// 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 (auto Entity : Entities)
+	for (auto & Entity : Entities)
 	{
 		if (!Entity->IsPlayer())
 		{
 			// Scheduling a normal destruction is neither possible (Since this chunk will be gone till the schedule occurs) nor necessary.
 			Entity->DestroyNoScheduling(false);  // No point in broadcasting in an unloading chunk. Chunks unload when no one is nearby.
-			delete Entity;
 		}
 	}
 
@@ -300,9 +299,9 @@ void cChunk::GetAllData(cChunkDataCallback & a_Callback)
 
 	a_Callback.ChunkData(m_ChunkData);
 
-	for (auto Entity : m_Entities)
+	for (const auto & Entity : m_Entities)
 	{
-		a_Callback.Entity(Entity);
+		a_Callback.Entity(Entity.get());
 	}
 
 	for (auto & KeyPair : m_BlockEntities)
@@ -534,7 +533,7 @@ void cChunk::CollectMobCensus(cMobCensus & toFill)
 	}
 
 	Vector3d currentPosition;
-	for (auto entity : m_Entities)
+	for (auto & entity : m_Entities)
 	{
 		// LOGD("Counting entity #%i (%s)", (*itr)->GetUniqueID(), (*itr)->GetClass());
 		if (entity->IsMob())
@@ -637,7 +636,7 @@ void cChunk::SpawnMobs(cMobSpawner & a_MobSpawner)
 			continue;
 		}
 
-		cEntity * newMob = a_MobSpawner.TryToSpawnHere(this, TryX, TryY, TryZ, Biome, MaxNbOfSuccess);
+		auto newMob = a_MobSpawner.TryToSpawnHere(this, TryX, TryY, TryZ, Biome, MaxNbOfSuccess);
 		if (newMob == nullptr)
 		{
 			continue;
@@ -661,7 +660,7 @@ void cChunk::Tick(std::chrono::milliseconds a_Dt)
 	// If we are not valid, tick players and bailout
 	if (!IsValid())
 	{
-		for (auto Entity : m_Entities)
+		for (const auto & Entity : m_Entities)
 		{
 			if (Entity->IsPlayer())
 			{
@@ -686,7 +685,7 @@ void cChunk::Tick(std::chrono::milliseconds a_Dt)
 		m_IsDirty = KeyPair.second->Tick(a_Dt, *this) | m_IsDirty;
 	}
 
-	for (cEntityList::iterator itr = m_Entities.begin(); itr != m_Entities.end();)
+	for (auto itr = m_Entities.begin(); itr != m_Entities.end();)
 	{
 		// Do not tick mobs that are detached from the world. They're either scheduled for teleportation or for removal.
 		if (!(*itr)->IsTicking())
@@ -711,20 +710,22 @@ void cChunk::Tick(std::chrono::milliseconds a_Dt)
 			continue;
 		}
 
-		if ((((*itr)->GetChunkX() != m_PosX) ||
-			((*itr)->GetChunkZ() != m_PosZ))
+		if (
+			((*itr)->GetChunkX() != m_PosX) ||
+			((*itr)->GetChunkZ() != m_PosZ)
 		)
 		{
-			// This block is very similar to RemoveEntity, except it uses an iterator to avoid scanning the whole m_Entities
-			// The entity moved out of the chunk, move it to the neighbor
-
-			(*itr)->SetParentChunk(nullptr);
-			MoveEntityToNewChunk(*itr);
 			// Mark as dirty if it was a server-generated entity:
 			if (!(*itr)->IsPlayer())
 			{
 				MarkDirty();
 			}
+
+			// This block is very similar to RemoveEntity, except it uses an iterator to avoid scanning the whole m_Entities
+			// The entity moved out of the chunk, move it to the neighbor
+			(*itr)->SetParentChunk(nullptr);
+			MoveEntityToNewChunk(std::move(*itr));
+
 			itr = m_Entities.erase(itr);
 		}
 		else
@@ -753,7 +754,7 @@ void cChunk::TickBlock(int a_RelX, int a_RelY, int a_RelZ)
 
 
 
-void cChunk::MoveEntityToNewChunk(cEntity * a_Entity)
+void cChunk::MoveEntityToNewChunk(OwnedEntity a_Entity)
 {
 	cChunk * Neighbor = GetNeighborChunk(a_Entity->GetChunkX() * cChunkDef::Width, a_Entity->GetChunkZ() * cChunkDef::Width);
 	if (Neighbor == nullptr)
@@ -767,28 +768,29 @@ void cChunk::MoveEntityToNewChunk(cEntity * a_Entity)
 	}
 
 	ASSERT(Neighbor != this);  // Moving into the same chunk? wtf?
-	Neighbor->AddEntity(a_Entity);
+	auto & Entity = *a_Entity;
+	Neighbor->AddEntity(std::move(a_Entity));
 
 	class cMover :
 		public cClientDiffCallback
 	{
 		virtual void Removed(cClientHandle * a_Client) override
 		{
-			a_Client->SendDestroyEntity(*m_Entity);
+			a_Client->SendDestroyEntity(m_Entity);
 		}
 
 		virtual void Added(cClientHandle * a_Client) override
 		{
-			m_Entity->SpawnOn(*a_Client);
+			m_Entity.SpawnOn(*a_Client);
 		}
 
-		cEntity * m_Entity;
+		cEntity & m_Entity;
 
 	public:
-		cMover(cEntity * a_CallbackEntity) :
+		cMover(cEntity & a_CallbackEntity) :
 			m_Entity(a_CallbackEntity)
 		{}
-	} Mover(a_Entity);
+	} Mover(Entity);
 
 	m_ChunkMap->CompareChunkClients(this, Neighbor, Mover);
 }
@@ -1869,15 +1871,15 @@ void cChunk::CollectPickupsByPlayer(cPlayer & a_Player)
 	double PosY = a_Player.GetPosY();
 	double PosZ = a_Player.GetPosZ();
 
-	for (cEntityList::iterator itr = m_Entities.begin(); itr != m_Entities.end(); ++itr)
+	for (auto & Entity : m_Entities)
 	{
-		if ((!(*itr)->IsPickup()) && (!(*itr)->IsProjectile()))
+		if ((!Entity->IsPickup()) && (!Entity->IsProjectile()))
 		{
 			continue;  // Only pickups and projectiles can be picked up
 		}
-		float DiffX = static_cast<float>((*itr)->GetPosX() - PosX);
-		float DiffY = static_cast<float>((*itr)->GetPosY() - PosY);
-		float DiffZ = static_cast<float>((*itr)->GetPosZ() - PosZ);
+		float DiffX = static_cast<float>(Entity->GetPosX() - PosX);
+		float DiffY = static_cast<float>(Entity->GetPosY() - PosY);
+		float DiffZ = static_cast<float>(Entity->GetPosZ() - PosZ);
 		float SqrDist = DiffX * DiffX + DiffY * DiffY + DiffZ * DiffZ;
 		if (SqrDist < 1.5f * 1.5f)  // 1.5 block
 		{
@@ -1887,13 +1889,13 @@ void cChunk::CollectPickupsByPlayer(cPlayer & a_Player)
 			);
 			*/
 			MarkDirty();
-			if ((*itr)->IsPickup())
+			if (Entity->IsPickup())
 			{
-				(reinterpret_cast<cPickup *>(*itr))->CollectedBy(a_Player);
+				reinterpret_cast<cPickup *>(Entity.get())->CollectedBy(a_Player);
 			}
 			else
 			{
-				(reinterpret_cast<cProjectileEntity *>(*itr))->CollectedBy(a_Player);
+				reinterpret_cast<cProjectileEntity *>(Entity.get())->CollectedBy(a_Player);
 			}
 		}
 		else if (SqrDist < 5 * 5)
@@ -1989,7 +1991,7 @@ void cChunk::RemoveClient(cClientHandle * a_Client)
 
 	if (!a_Client->IsDestroyed())
 	{
-		for (auto Entity : m_Entities)
+		for (auto & Entity : m_Entities)
 		{
 			/*
 			// DEBUG:
@@ -2027,34 +2029,59 @@ bool cChunk::HasAnyClients(void) const
 
 
 
-void cChunk::AddEntity(cEntity * a_Entity)
+void cChunk::AddEntity(OwnedEntity a_Entity)
 {
 	if (!a_Entity->IsPlayer())
 	{
 		MarkDirty();
 	}
 
+	auto EntityPtr = a_Entity.get();
+
 	ASSERT(std::find(m_Entities.begin(), m_Entities.end(), a_Entity) == m_Entities.end());  // Not there already
+	m_Entities.emplace_back(std::move(a_Entity));
 
-	m_Entities.push_back(a_Entity);
-	ASSERT(a_Entity->GetParentChunk() == nullptr);
-	a_Entity->SetParentChunk(this);
+	ASSERT(EntityPtr->GetParentChunk() == nullptr);
+	EntityPtr->SetParentChunk(this);
 }
 
 
 
 
 
-void cChunk::RemoveEntity(cEntity * a_Entity)
+OwnedEntity cChunk::RemoveEntity(cEntity & a_Entity)
 {
-	ASSERT(a_Entity->GetParentChunk() == this);
-	a_Entity->SetParentChunk(nullptr);
-	m_Entities.remove(a_Entity);
+	ASSERT(a_Entity.GetParentChunk() == this);
+	ASSERT(!a_Entity.IsTicking());
+	a_Entity.SetParentChunk(nullptr);
+
 	// Mark as dirty if it was a server-generated entity:
-	if (!a_Entity->IsPlayer())
+	if (!a_Entity.IsPlayer())
 	{
 		MarkDirty();
 	}
+
+	OwnedEntity Removed;
+	m_Entities.erase(
+		std::remove_if(
+			m_Entities.begin(),
+			m_Entities.end(),
+			[&a_Entity, &Removed](decltype(m_Entities)::value_type & a_Value)
+			{
+				if (a_Value.get() == &a_Entity)
+				{
+					ASSERT(!Removed);
+					Removed = std::move(a_Value);
+					return true;
+				}
+
+				return false;
+			}
+		),
+		m_Entities.end()
+	);
+
+	return Removed;
 }
 
 
@@ -2063,13 +2090,13 @@ void cChunk::RemoveEntity(cEntity * a_Entity)
 
 bool cChunk::HasEntity(UInt32 a_EntityID)
 {
-	for (cEntityList::const_iterator itr = m_Entities.begin(), end = m_Entities.end(); itr != end; ++itr)
+	for (const auto & Entity : m_Entities)
 	{
-		if ((*itr)->GetUniqueID() == a_EntityID)
+		if (Entity->GetUniqueID() == a_EntityID)
 		{
 			return true;
 		}
-	}  // for itr - m_Entities[]
+	}
 	return false;
 }
 
@@ -2080,14 +2107,14 @@ bool cChunk::HasEntity(UInt32 a_EntityID)
 bool cChunk::ForEachEntity(cEntityCallback & a_Callback)
 {
 	// The entity list is locked by the parent chunkmap's CS
-	for (cEntityList::iterator itr = m_Entities.begin(), itr2 = itr; itr != m_Entities.end(); itr = itr2)
+	for (auto itr = m_Entities.begin(), itr2 = itr; itr != m_Entities.end(); itr = itr2)
 	{
 		++itr2;
 		if (!(*itr)->IsTicking())
 		{
 			continue;
 		}
-		if (a_Callback.Item(*itr))
+		if (a_Callback.Item(itr->get()))
 		{
 			return false;
 		}
@@ -2102,7 +2129,7 @@ bool cChunk::ForEachEntity(cEntityCallback & a_Callback)
 bool cChunk::ForEachEntityInBox(const cBoundingBox & a_Box, cEntityCallback & a_Callback)
 {
 	// The entity list is locked by the parent chunkmap's CS
-	for (cEntityList::iterator itr = m_Entities.begin(), itr2 = itr; itr != m_Entities.end(); itr = itr2)
+	for (auto itr = m_Entities.begin(), itr2 = itr; itr != m_Entities.end(); itr = itr2)
 	{
 		++itr2;
 		if (!(*itr)->IsTicking())
@@ -2115,7 +2142,7 @@ bool cChunk::ForEachEntityInBox(const cBoundingBox & a_Box, cEntityCallback & a_
 			// The entity is not in the specified box
 			continue;
 		}
-		if (a_Callback.Item(*itr))
+		if (a_Callback.Item(itr->get()))
 		{
 			return false;
 		}
@@ -2139,11 +2166,11 @@ bool cChunk::DoWithEntityByID(UInt32 a_EntityID, cEntityCallback & a_Callback, b
 bool cChunk::DoWithEntityByID(UInt32 a_EntityID, cLambdaEntityCallback a_Callback, bool & a_CallbackResult)
 {
 	// The entity list is locked by the parent chunkmap's CS
-	for (cEntityList::iterator itr = m_Entities.begin(), end = m_Entities.end(); itr != end; ++itr)
+	for (const auto & Entity : m_Entities)
 	{
-		if (((*itr)->GetUniqueID() == a_EntityID) && ((*itr)->IsTicking()))
+		if ((Entity->GetUniqueID() == a_EntityID) && (Entity->IsTicking()))
 		{
-			a_CallbackResult = a_Callback(*itr);
+			a_CallbackResult = a_Callback(Entity.get());
 			return true;
 		}
 	}  // for itr - m_Entitites[]