Fix Block Entity Placement in Generation (#5060)

* block area in chunk desc now handles block entities
some minor changes
block entities validate and correct their position when put into the world

* fixed checkstyle

* Fixed Build

* Removed Empty File

---------

Co-authored-by: 12xx12 <12xx12100@gmail.com>
Co-authored-by: Alexander Harkness <me@bearbin.net>

src/BlockArea.h

@@ -163,7 +163,7 @@ public:
 	/** Expands the internal contents by the specified amount of blocks from each border */
 	void Expand(int a_SubMinX, int a_AddMaxX, int a_SubMinY, int a_AddMaxY, int a_SubMinZ, int a_AddMaxZ);
 
-	/** Merges another block area into this one, using the specified block combinating strategy
+	/** Merges another block area into this one, using the specified block combining strategy
 	This function combines another BlockArea into the current object.
 	The a_RelX, a_RelY and a_RelZ parameters specify the coords of this BA where a_Src should be copied.
 	If both areas contain baBlockEntities, the BEs are merged (with preference of keeping this' ones) (MergeBlockEntities()).
@@ -236,7 +236,7 @@ public:
 	*/
 	void Merge(const cBlockArea & a_Src, int a_RelX, int a_RelY, int a_RelZ, eMergeStrategy a_Strategy);
 
-	/** Merges another block area into this one, using the specified block combinating strategy.
+	/** Merges another block area into this one, using the specified block combining strategy.
 	See Merge() above for details. */
 	void Merge(const cBlockArea & a_Src, const Vector3i & a_RelMinCoords, eMergeStrategy a_Strategy);
 
@@ -362,7 +362,7 @@ public:
 	bool HasBlockMetas    (void) const { return (m_BlockMetas    != nullptr); }
 	bool HasBlockLights   (void) const { return (m_BlockLight    != nullptr); }
 	bool HasBlockSkyLights(void) const { return (m_BlockSkyLight != nullptr); }
-	bool HasBlockEntities (void) const { return (m_BlockEntities != nullptr); }
+	bool HasBlockEntities (void) const { return m_BlockEntities.operator bool(); }
 
 	/** Returns the count of blocks that are not air.
 	Returns 0 if blocktypes not available. Block metas are ignored (if present, air with any meta is still considered air). */
@@ -423,7 +423,8 @@ public:
 	bool ForEachBlockEntity(cBlockEntityCallback a_Callback);
 
 	/** Direct read-only access to block entities. */
-	const cBlockEntities & GetBlockEntities(void) const { ASSERT(HasBlockEntities()); return *m_BlockEntities; }
+	const cBlockEntities & GetBlockEntities(void) const { ASSERT(HasBlockEntities()); return *m_BlockEntities.get(); }
+	cBlockEntities &       GetBlockEntities(void)       { ASSERT(HasBlockEntities()); return *m_BlockEntities.get(); }
 
 
 

src/Chunk.cpp

@@ -370,15 +370,25 @@ void cChunk::SetAllData(SetChunkData && a_SetChunkData)
 	m_BlockEntities = std::move(a_SetChunkData.BlockEntities);
 
 	// Check that all block entities have a valid blocktype at their respective coords (DEBUG-mode only):
-#ifndef NDEBUG
+
 	for (auto & KeyPair : m_BlockEntities)
 	{
+#ifndef NDEBUG
 		cBlockEntity * Block = KeyPair.second.get();
 		BLOCKTYPE EntityBlockType = Block->GetBlockType();
 		BLOCKTYPE WorldBlockType = GetBlock(Block->GetRelX(), Block->GetPosY(), Block->GetRelZ());
 		ASSERT(WorldBlockType == EntityBlockType);
-	}
 #endif
+		// Reset Pointer
+		KeyPair.second->SetWorld(nullptr);
+
+		auto Pos = cChunkDef::RelativeToAbsolute({KeyPair.second->GetRelX(), 0, KeyPair.second->GetRelZ()}, {m_PosX, m_PosZ});
+		if ((Pos.x != KeyPair.second->GetPosX()) || (Pos.z != KeyPair.second->GetPosZ()))
+		{
+			KeyPair.second->SetPos(Pos.addedY(KeyPair.second->GetPosY()));
+		}
+		KeyPair.second->SetWorld(m_World);
+	}
 
 	// Set the chunk data as valid.
 	// This may be needed for some simulators that perform actions upon block adding (Vaporize),

src/ChunkDataCallback.h

@@ -18,7 +18,7 @@
 /** Interface class used for getting data out of a chunk using the GetAllData() function.
 Implementation must use the pointers immediately and NOT store any of them for later use
 The virtual methods are called in the same order as they're declared here. */
-class cChunkDataCallback abstract
+class cChunkDataCallback
 {
 public:
 
@@ -44,7 +44,7 @@ public:
 	/** Called for each entity in the chunk */
 	virtual void Entity(cEntity * a_Entity) { UNUSED(a_Entity); }
 
-	/** Called for each blockentity in the chunk */
+	/** Called for each block entity in the chunk */
 	virtual void BlockEntity(cBlockEntity * a_Entity) { UNUSED(a_Entity); }
 } ;
 

src/Generating/ChunkDesc.cpp

@@ -573,12 +573,12 @@ void cChunkDesc::RandomFillRelCuboid(
 cBlockEntity * cChunkDesc::GetBlockEntity(int a_RelX, int a_RelY, int a_RelZ)
 {
 	const auto Idx = cChunkDef::MakeIndex(a_RelX, a_RelY, a_RelZ);
-	const auto itr = m_BlockEntities.find(Idx);
+	const auto Iterator = m_BlockArea.GetBlockEntities().find(Idx);
 
-	if (itr != m_BlockEntities.end())
+	if (Iterator != m_BlockArea.GetBlockEntities().end())
 	{
 		// Already in the list:
-		cBlockEntity * BlockEntity = itr->second.get();
+		cBlockEntity * BlockEntity = Iterator->second.get();
 		if (BlockEntity->GetBlockType() == GetBlockType(a_RelX, a_RelY, a_RelZ))
 		{
 			// Correct type, already present. Return it:
@@ -587,7 +587,7 @@ cBlockEntity * cChunkDesc::GetBlockEntity(int a_RelX, int a_RelY, int a_RelZ)
 		else
 		{
 			// Wrong type, the block type has been overwritten. Erase and create new:
-			m_BlockEntities.erase(itr);
+			m_BlockArea.GetBlockEntities().erase(Iterator);
 		}
 	}
 
@@ -595,13 +595,13 @@ cBlockEntity * cChunkDesc::GetBlockEntity(int a_RelX, int a_RelY, int a_RelZ)
 	int AbsZ = a_RelZ + m_Coords.m_ChunkZ * cChunkDef::Width;
 
 	// The block entity is not created yet, try to create it and add to list:
-	auto be = cBlockEntity::CreateByBlockType(GetBlockType(a_RelX, a_RelY, a_RelZ), GetBlockMeta(a_RelX, a_RelY, a_RelZ), {AbsX, a_RelY, AbsZ});
+	auto BlockEntity = cBlockEntity::CreateByBlockType(GetBlockType(a_RelX, a_RelY, a_RelZ), GetBlockMeta(a_RelX, a_RelY, a_RelZ), {AbsX, a_RelY, AbsZ});
-	if (be == nullptr)
+	if (BlockEntity == nullptr)
 	{
 		// No block entity for this block type
 		return nullptr;
 	}
-	auto res = m_BlockEntities.emplace(Idx, std::move(be));
+	auto res = m_BlockArea.GetBlockEntities().emplace(Idx, std::move(BlockEntity));
 	return res.first->second.get();
 }
 

src/Generating/ChunkDesc.h

@@ -238,7 +238,8 @@ 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 cBlockEntities &          GetBlockEntities         (void) { return m_BlockEntities; }
+	inline const cBlockEntities &    GetBlockEntities         (void) const { return m_BlockArea.GetBlockEntities(); }
+	inline cBlockEntities &          GetBlockEntities         (void) { return m_BlockArea.GetBlockEntities(); }
 
 	inline const cChunkDef::BiomeMap &     GetBiomeMap()   const { return m_BiomeMap; }
 	inline const cChunkDef::BlockTypes &   GetBlockTypes() const { return *(reinterpret_cast<cChunkDef::BlockTypes *>(m_BlockArea.GetBlockTypes())); }
@@ -259,7 +260,6 @@ private:
 	cBlockArea              m_BlockArea;
 	cChunkDef::HeightMap    m_HeightMap;
 	cEntityList             m_Entities;
-	cBlockEntities          m_BlockEntities;  // Individual block entities are NOT owned by this object!
 
 	bool m_bUseDefaultBiomes;
 	bool m_bUseDefaultHeight;

src/Generating/MineShafts.cpp

@@ -26,7 +26,7 @@ in a depth-first processing. Each of the descendants will branch randomly, if no
 
 
 
-class cMineShaft abstract
+class cMineShaft
 {
 public:
 	enum eKind

src/Generating/MineShafts.h

@@ -35,7 +35,7 @@ protected:
 	class cMineShaftSystem;  // fwd: MineShafts.cpp
 
 	int    m_GridSize;            ///< Average spacing of the systems
-	int    m_MaxSystemSize;       ///< Maximum blcok size of a mineshaft system
+	int    m_MaxSystemSize;       ///< Maximum block size of a mineshaft system
 	int    m_ProbLevelCorridor;   ///< Probability level of a branch object being the corridor
 	int    m_ProbLevelCrossing;   ///< Probability level of a branch object being the crossing, minus Corridor
 	int    m_ProbLevelStaircase;  ///< Probability level of a branch object being the staircase, minus Crossing