From 07b7fd4ad3fef714399009efd5d4a903a561a6f2 Mon Sep 17 00:00:00 2001 From: LogicParrot Date: Tue, 2 Feb 2016 17:37:21 +0200 Subject: Fixed cChunk::m_Entities corruption upon world travel --- src/Chunk.cpp | 29 ++++++++++++++++++++++++++++- src/Chunk.h | 5 ++++- src/Entities/Entity.cpp | 13 +++++++++++-- src/Entities/Player.cpp | 18 ++++++++++++++---- 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/Chunk.cpp b/src/Chunk.cpp index da13e3b10..1e3e749fd 100644 --- a/src/Chunk.cpp +++ b/src/Chunk.cpp @@ -603,6 +603,7 @@ void cChunk::SpawnMobs(cMobSpawner & a_MobSpawner) void cChunk::Tick(std::chrono::milliseconds a_Dt) { + m_IsInTick = true; BroadcastPendingBlockChanges(); CheckBlocks(); @@ -637,7 +638,7 @@ void cChunk::Tick(std::chrono::milliseconds a_Dt) else if ((*itr)->IsWorldTravellingFrom(m_World)) { // Remove all entities that are travelling to another world - LOGD("Removing entity from [%d, %d] that's travelling between worlds.", m_PosX, m_PosZ); + LOGD("Removing entity from [%d, %d] that's travelling between worlds. (Scheduled)", m_PosX, m_PosZ); MarkDirty(); (*itr)->SetWorldTravellingFrom(nullptr); itr = m_Entities.erase(itr); @@ -659,6 +660,7 @@ void cChunk::Tick(std::chrono::milliseconds a_Dt) } // for itr - m_Entitites[] ApplyWeatherToTop(); + m_IsInTick = false; } @@ -1910,6 +1912,31 @@ void cChunk::RemoveEntity(cEntity * a_Entity) +void cChunk::SafeRemoveEntity(cEntity * a_Entity) +{ + if (!m_IsInTick) + { + LOGD("Removing entity from [%d, %d] that's travelling between worlds. (immediate)", m_PosX, m_PosZ); + // If we're not in a tick, just remove it. + m_Entities.remove(a_Entity); + } + else + { + // If we are in a tick, we don't want to invalidate the iterator, so we schedule the removal. Removal will be done in cChunk::tick() + a_Entity->SetWorldTravellingFrom(GetWorld()); + } + + // Mark as dirty if it was a server-generated entity: + if (!a_Entity->IsPlayer()) + { + MarkDirty(); + } +} + + + + + bool cChunk::HasEntity(UInt32 a_EntityID) { for (cEntityList::const_iterator itr = m_Entities.begin(), end = m_Entities.end(); itr != end; ++itr) diff --git a/src/Chunk.h b/src/Chunk.h index d944af10a..41bc79746 100644 --- a/src/Chunk.h +++ b/src/Chunk.h @@ -260,6 +260,9 @@ public: void AddEntity(cEntity * a_Entity); void RemoveEntity(cEntity * a_Entity); + /** RemoveEntity is dangerous if the chunk is inside the tick() method because it invalidates the iterator. + This will safely remove an entity. */ + void SafeRemoveEntity(cEntity * a_Entity); bool HasEntity(UInt32 a_EntityID); /** Calls the callback for each entity; returns true if all entities processed, false if the callback aborted by returning true */ @@ -502,7 +505,7 @@ private: /** If the chunk fails to load, should it be queued in the generator or reset back to invalid? */ bool m_ShouldGenerateIfLoadFailed; - + bool m_IsInTick; // True if the chunk is executing the tick() method. bool m_IsLightValid; // True if the blocklight and skylight are calculated bool m_IsDirty; // True if the chunk has changed since it was last saved bool m_IsSaving; // True if the chunk is being saved diff --git a/src/Entities/Entity.cpp b/src/Entities/Entity.cpp index d4097f734..ff03bae3c 100644 --- a/src/Entities/Entity.cpp +++ b/src/Entities/Entity.cpp @@ -1499,8 +1499,17 @@ bool cEntity::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d return false; } - // Remove all links to the old world - SetWorldTravellingFrom(GetWorld()); // cChunk::Tick() handles entity removal + // Remove entity from chunk + if (!GetWorld()->DoWithChunk(GetChunkX(), GetChunkZ(), [this](cChunk & a_Chunk) -> bool + { + a_Chunk.SafeRemoveEntity(this); + return true; + })) + { + LOGD("Entity Teleportation failed! Didn't find the source chunk!\n"); + return false; + } + GetWorld()->BroadcastDestroyEntity(*this); SetPosition(a_NewPosition); diff --git a/src/Entities/Player.cpp b/src/Entities/Player.cpp index 767ee2061..7ba6b2bf6 100644 --- a/src/Entities/Player.cpp +++ b/src/Entities/Player.cpp @@ -1695,6 +1695,20 @@ bool cPlayer::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d return false; } + // Remove player from chunk + if (!GetWorld()->DoWithChunk(GetChunkX(), GetChunkZ(), [this](cChunk & a_Chunk) -> bool + { + a_Chunk.SafeRemoveEntity(this); + return true; + })) + { + LOGD("Entity Teleportation failed! Didn't find the source chunk!\n"); + return false; + } + + // Remove player from world + GetWorld()->RemovePlayer(this, false); + // Send the respawn packet: if (a_ShouldSendRespawn && (m_ClientHandle != nullptr)) { @@ -1704,10 +1718,6 @@ bool cPlayer::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d // Broadcast for other people that the player is gone. GetWorld()->BroadcastDestroyEntity(*this); - // Remove player from the old world - SetWorldTravellingFrom(GetWorld()); // cChunk handles entity removal - GetWorld()->RemovePlayer(this, false); - SetPosition(a_NewPosition); // Queue adding player to the new world, including all the necessary adjustments to the object -- cgit v1.2.3