Minor fixes which took way too much time #5060
Conversation
some minor changes block entities validate and correct their position when put into the world
|
The underlying issue is that somewhere in the process of placing a prefab structure the block entities lose their position |
|
Awesome! Note that the description of the PR should contain "Fixes #5035" (you placed the # at the wrong position) for Github to mark the issue as related and to close it automatically once this is merged. |
|
Thank you for the reminder - yeah I was a bit frustrated when opening this PR |
|
Thanks! |
| @@ -362,7 +362,7 @@ class cBlockArea | |||
| 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(); } | |||
There was a problem hiding this comment.
You mean for the weird access to the bool operator?
Normally you can use it simply. I don't know why the compiler denied it here
| for (auto & KeyPair : m_BlockEntities) | ||
| { | ||
| // Reset Pointer | ||
| KeyPair.second->SetWorld(nullptr); |
There was a problem hiding this comment.
For whatever reason in debug mode the nether is mega weird on startup and some block entities have already a set world. This is just for safety
| @@ -220,7 +220,8 @@ class cChunkDesc | |||
| 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(); } | |||
There was a problem hiding this comment.
The comment said there should be read only access
| m_BlockEntities.erase(itr); | ||
| m_BlockArea.GetBlockEntities().erase(Iterator); | ||
| } | ||
| } | ||
|
|
||
| int AbsX = a_RelX + m_Coords.m_ChunkX * cChunkDef::Width; | ||
| 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}); | ||
| if (be == nullptr) | ||
| auto BlockEntity = cBlockEntity::CreateByBlockType(GetBlockType(a_RelX, a_RelY, a_RelZ), GetBlockMeta(a_RelX, a_RelY, a_RelZ), {AbsX, a_RelY, AbsZ}); | ||
| 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)); |
There was a problem hiding this comment.
Does BlockArea not keep track of its internal blockentities list correctly?
There was a problem hiding this comment.
No. When using the prefab the positions are not set correctly.
The relative position is correct but the absolute is wrong. This only happens when using a prefab. This code is only a bad fix - I didn't find the underlying issue.
The problem is that a block entity is not informed by the world where it's actual position is. The broadcasting, loading, saving is done with the position stored in the block entity.
|
As you say there seems to be an underlying issue, I think this warrants further investigation (e.g. with ChunkDesc seemingly doing the work of BlockArea). |
|
Yeah, I think that's a bug inside |
cBlockAreaincChunkDescnow handles block entitiessome minor changes, typos...
Block entities validate and correct their position when put into the world.
Fixes #5035
testes with jungle temples