From aac592f98598aae327d07b2a1bb155e33c6d51b1 Mon Sep 17 00:00:00 2001
From: peterbell10 <peterbell10@live.co.uk>
Date: Fri, 3 Apr 2020 22:23:38 +0100
Subject: Manage block entity lifetime with unique_ptr (#4080)

---
 src/Chunk.cpp | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

(limited to 'src/Chunk.cpp')

diff --git a/src/Chunk.cpp b/src/Chunk.cpp
index d9601e451..17e130cdd 100644
--- a/src/Chunk.cpp
+++ b/src/Chunk.cpp
@@ -111,10 +111,6 @@ cChunk::~cChunk()
 
 	// LOGINFO("### delete cChunk() (%i, %i) from %p, thread 0x%x ###", m_PosX, m_PosZ, this, GetCurrentThreadId());
 
-	for (auto & KeyPair : m_BlockEntities)
-	{
-		delete KeyPair.second;
-	}
 	m_BlockEntities.clear();
 
 	// Remove and destroy all entities that are not players:
@@ -306,7 +302,7 @@ void cChunk::GetAllData(cChunkDataCallback & a_Callback)
 
 	for (auto & KeyPair : m_BlockEntities)
 	{
-		a_Callback.BlockEntity(KeyPair.second);
+		a_Callback.BlockEntity(KeyPair.second.get());
 	}
 }
 
@@ -326,17 +322,13 @@ void cChunk::SetAllData(cSetChunkData & a_SetChunkData)
 	m_IsLightValid = a_SetChunkData.IsLightValid();
 
 	// Clear the block entities present - either the loader / saver has better, or we'll create empty ones:
-	for (auto & KeyPair : m_BlockEntities)
-	{
-		delete KeyPair.second;
-	}
 	m_BlockEntities = std::move(a_SetChunkData.GetBlockEntities());
 
 	// Check that all block entities have a valid blocktype at their respective coords (DEBUG-mode only):
 	#ifdef _DEBUG
 		for (auto & KeyPair : m_BlockEntities)
 		{
-			cBlockEntity * Block = KeyPair.second;
+			cBlockEntity * Block = KeyPair.second.get();
 			BLOCKTYPE EntityBlockType = Block->GetBlockType();
 			BLOCKTYPE WorldBlockType = GetBlock(Block->GetRelX(), Block->GetPosY(), Block->GetRelZ());
 			ASSERT(WorldBlockType == EntityBlockType);
@@ -449,7 +441,6 @@ void cChunk::WriteBlockArea(cBlockArea & a_Area, int a_MinBlockX, int a_MinBlock
 	{
 		if (affectedArea.IsInside(itr->second->GetPos()))
 		{
-			delete itr->second;
 			itr = m_BlockEntities.erase(itr);
 		}
 		else
@@ -483,7 +474,7 @@ void cChunk::WriteBlockArea(cBlockArea & a_Area, int a_MinBlockX, int a_MinBlock
 			}
 			auto clone = be->Clone({posX, posY, posZ});
 			clone->SetWorld(m_World);
-			AddBlockEntityClean(clone);
+			AddBlockEntityClean(std::move(clone));
 			m_World->BroadcastBlockEntity({posX, posY, posZ});
 		}
 	}
@@ -1347,8 +1338,6 @@ void cChunk::SetBlock(Vector3i a_RelPos, BLOCKTYPE a_BlockType, NIBBLETYPE a_Blo
 	{
 		BlockEntity->Destroy();
 		RemoveBlockEntity(BlockEntity);
-		delete BlockEntity;
-		BlockEntity = nullptr;
 	}
 
 	// If the new block is a block entity, create the entity object:
@@ -1528,20 +1517,20 @@ void cChunk::SendBlockTo(int a_RelX, int a_RelY, int a_RelZ, cClientHandle * a_C
 
 
 
-void cChunk::AddBlockEntity(cBlockEntity * a_BlockEntity)
+void cChunk::AddBlockEntity(OwnedBlockEntity a_BlockEntity)
 {
 	MarkDirty();
-	AddBlockEntityClean(a_BlockEntity);
+	AddBlockEntityClean(std::move(a_BlockEntity));
 }
 
 
 
 
 
-void cChunk::AddBlockEntityClean(cBlockEntity * a_BlockEntity)
+void cChunk::AddBlockEntityClean(OwnedBlockEntity a_BlockEntity)
 {
 	int Idx = MakeIndex(a_BlockEntity->GetRelX(), a_BlockEntity->GetPosY(), a_BlockEntity->GetRelZ());
-	auto Result = m_BlockEntities.insert({ Idx, a_BlockEntity });
+	auto Result = m_BlockEntities.emplace(Idx, std::move(a_BlockEntity));
 	UNUSED(Result);
 	ASSERT(Result.second);  // No block entity already at this position
 }
@@ -1561,7 +1550,7 @@ cBlockEntity * cChunk::GetBlockEntity(Vector3i a_AbsPos)
 	}
 
 	auto itr = m_BlockEntities.find(static_cast<size_t>(MakeIndexNoCheck(relPos)));
-	return (itr == m_BlockEntities.end()) ? nullptr : itr->second;
+	return (itr == m_BlockEntities.end()) ? nullptr : itr->second.get();
 }
 
 
@@ -1572,7 +1561,7 @@ cBlockEntity * cChunk::GetBlockEntityRel(Vector3i a_RelPos)
 {
 	ASSERT(IsValidRelPos(a_RelPos));
 	auto itr = m_BlockEntities.find(static_cast<size_t>(MakeIndexNoCheck(a_RelPos)));
-	return (itr == m_BlockEntities.end()) ? nullptr : itr->second;
+	return (itr == m_BlockEntities.end()) ? nullptr : itr->second.get();
 }
 
 
@@ -1959,7 +1948,7 @@ bool cChunk::GenericForEachBlockEntity(cFunctionRef<bool(tyEntity &)> a_Callback
 	// The blockentity list is locked by the parent chunkmap's CS
 	for (auto & KeyPair : m_BlockEntities)
 	{
-		cBlockEntity * Block = KeyPair.second;
+		cBlockEntity * Block = KeyPair.second.get();
 		if (
 			(sizeof...(tBlocktype) == 0) ||  // Let empty list mean all block entities
 			(IsOneOf<tBlocktype...>(Block->GetBlockType()))
-- 
cgit v1.2.3