From 4ef47aed62364f9cf1474864e5cf94232b4477af Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Mon, 19 Dec 2016 20:12:23 +0000 Subject: Changed entity ownership model to use smart pointers --- src/Chunk.cpp | 127 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 77 insertions(+), 50 deletions(-) (limited to 'src/Chunk.cpp') diff --git a/src/Chunk.cpp b/src/Chunk.cpp index 3b9739907..5c83f9a47 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) @@ -531,7 +530,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()) @@ -634,7 +633,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; @@ -658,7 +657,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()) { @@ -683,7 +682,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()) @@ -708,20 +707,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 @@ -750,7 +751,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) @@ -764,28 +765,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); } @@ -1866,15 +1868,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((*itr)->GetPosX() - PosX); - float DiffY = static_cast((*itr)->GetPosY() - PosY); - float DiffZ = static_cast((*itr)->GetPosZ() - PosZ); + float DiffX = static_cast(Entity->GetPosX() - PosX); + float DiffY = static_cast(Entity->GetPosY() - PosY); + float DiffZ = static_cast(Entity->GetPosZ() - PosZ); float SqrDist = DiffX * DiffX + DiffY * DiffY + DiffZ * DiffZ; if (SqrDist < 1.5f * 1.5f) // 1.5 block { @@ -1884,13 +1886,13 @@ void cChunk::CollectPickupsByPlayer(cPlayer & a_Player) ); */ MarkDirty(); - if ((*itr)->IsPickup()) + if (Entity->IsPickup()) { - (reinterpret_cast(*itr))->CollectedBy(a_Player); + reinterpret_cast(Entity.get())->CollectedBy(a_Player); } else { - (reinterpret_cast(*itr))->CollectedBy(a_Player); + reinterpret_cast(Entity.get())->CollectedBy(a_Player); } } else if (SqrDist < 5 * 5) @@ -1986,7 +1988,7 @@ void cChunk::RemoveClient(cClientHandle * a_Client) if (!a_Client->IsDestroyed()) { - for (auto Entity : m_Entities) + for (auto & Entity : m_Entities) { /* // DEBUG: @@ -2024,34 +2026,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; } @@ -2060,13 +2087,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; } @@ -2077,14 +2104,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; } @@ -2099,7 +2126,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()) @@ -2112,7 +2139,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; } @@ -2136,11 +2163,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[] -- cgit v1.2.3