From 83459d0d899786378e8304c92a5b79ddca92c62f Mon Sep 17 00:00:00 2001 From: LogicParrot Date: Sun, 7 Feb 2016 19:07:14 +0200 Subject: Proper entity destruction in non-ticking chunks --- src/BlockEntities/HopperEntity.cpp | 2 +- src/Chunk.cpp | 82 ++++++++++------------- src/Chunk.h | 3 - src/ClientHandle.cpp | 31 ++++----- src/Entities/Entity.cpp | 130 +++++++++++++++++++++++++++---------- src/Entities/Entity.h | 45 ++++++++----- src/Entities/Minecart.cpp | 5 +- src/Entities/Pawn.cpp | 4 +- src/Entities/Pickup.cpp | 6 +- src/Entities/Player.cpp | 72 ++++++++++++-------- src/Entities/Player.h | 2 + src/Inventory.cpp | 2 +- src/Mobs/Monster.cpp | 6 +- src/Mobs/PassiveMonster.cpp | 16 +++-- src/Mobs/PassiveMonster.h | 2 + src/World.cpp | 31 ++++++--- src/World.h | 1 - 17 files changed, 263 insertions(+), 177 deletions(-) (limited to 'src') diff --git a/src/BlockEntities/HopperEntity.cpp b/src/BlockEntities/HopperEntity.cpp index b57f0a638..fbe073ada 100644 --- a/src/BlockEntities/HopperEntity.cpp +++ b/src/BlockEntities/HopperEntity.cpp @@ -195,7 +195,7 @@ bool cHopperEntity::MovePickupsIn(cChunk & a_Chunk, Int64 a_CurrentTick) { ASSERT(a_Entity != nullptr); - if (!a_Entity->IsPickup() || a_Entity->IsDestroyed()) + if (!a_Entity->IsPickup()) { return false; } diff --git a/src/Chunk.cpp b/src/Chunk.cpp index f5d447c45..237bf62c0 100644 --- a/src/Chunk.cpp +++ b/src/Chunk.cpp @@ -139,7 +139,8 @@ cChunk::~cChunk() { if (!(*itr)->IsPlayer()) { - (*itr)->Destroy(false); + // Scheduling a normal destruction is neither possible (Since this chunk will be gone till the schedule occurs) nor necessary. + (*itr)->DestroyNoScheduling(false); // No point in broadcasting in an unloading chunk. Chunks unload when no one is nearby. delete *itr; } } @@ -621,37 +622,44 @@ void cChunk::Tick(std::chrono::milliseconds a_Dt) for (cEntityList::iterator 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()) + { + ++itr; + continue; + } + if (!((*itr)->IsMob())) // Mobs are ticked inside cWorld::TickMobs() (as we don't have to tick them if they are far away from players) { // Tick all entities in this chunk (except mobs): + ASSERT((*itr)->GetParentChunk() == this); (*itr)->Tick(a_Dt, *this); + ASSERT((*itr)->GetParentChunk() == this); } - if ((*itr)->IsDestroyed()) // Remove all entities that were scheduled for removal: + // Do not move mobs that are detached from the world to neighbors. They're either scheduled for teleportation or for removal. + if (!(*itr)->IsTicking()) { - LOGD("Destroying entity #%i (%s)", (*itr)->GetUniqueID(), (*itr)->GetClass()); - MarkDirty(); - cEntity * ToDelete = *itr; - itr = m_Entities.erase(itr); - delete ToDelete; - } - 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. (Scheduled)", m_PosX, m_PosZ); - MarkDirty(); - (*itr)->SetWorldTravellingFrom(nullptr); - itr = m_Entities.erase(itr); + ++itr; + continue; } - else if ( - ((*itr)->GetChunkX() != m_PosX) || - ((*itr)->GetChunkZ() != m_PosZ) + + // Because the schedulded destruction is going to look for them in this chunk. See cEntity::destroy. + 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 - MarkDirty(); + + (*itr)->SetParentChunk(nullptr); MoveEntityToNewChunk(*itr); itr = m_Entities.erase(itr); + // Mark as dirty if it was a server-generated entity: + if (!(*itr)->IsPlayer()) + { + MarkDirty(); + } } else { @@ -1891,6 +1899,8 @@ void cChunk::AddEntity(cEntity * a_Entity) ASSERT(std::find(m_Entities.begin(), m_Entities.end(), a_Entity) == m_Entities.end()); // Not there already m_Entities.push_back(a_Entity); + ASSERT(a_Entity->GetParentChunk() == nullptr); + a_Entity->SetParentChunk(this); } @@ -1899,33 +1909,9 @@ void cChunk::AddEntity(cEntity * a_Entity) void cChunk::RemoveEntity(cEntity * a_Entity) { + ASSERT(a_Entity->GetParentChunk() == this); + a_Entity->SetParentChunk(nullptr); m_Entities.remove(a_Entity); - - // Mark as dirty if it was a server-generated entity: - if (!a_Entity->IsPlayer()) - { - MarkDirty(); - } -} - - - - - -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()) { @@ -1959,7 +1945,7 @@ bool cChunk::ForEachEntity(cEntityCallback & a_Callback) for (cEntityList::iterator itr = m_Entities.begin(), itr2 = itr; itr != m_Entities.end(); itr = itr2) { ++itr2; - if ((*itr)->IsDestroyed()) + if (!(*itr)->IsTicking()) { continue; } @@ -1981,7 +1967,7 @@ bool cChunk::ForEachEntityInBox(const cBoundingBox & a_Box, cEntityCallback & a_ for (cEntityList::iterator itr = m_Entities.begin(), itr2 = itr; itr != m_Entities.end(); itr = itr2) { ++itr2; - if ((*itr)->IsDestroyed()) + if (!(*itr)->IsTicking()) { continue; } @@ -2008,7 +1994,7 @@ bool cChunk::DoWithEntityByID(UInt32 a_EntityID, cEntityCallback & a_Callback, b // 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) { - if (((*itr)->GetUniqueID() == a_EntityID) && (!(*itr)->IsDestroyed())) + if (((*itr)->GetUniqueID() == a_EntityID) && ((*itr)->IsTicking())) { a_CallbackResult = a_Callback.Item(*itr); return true; diff --git a/src/Chunk.h b/src/Chunk.h index 41bc79746..e1f44b197 100644 --- a/src/Chunk.h +++ b/src/Chunk.h @@ -260,9 +260,6 @@ 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 */ diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index fca29c65d..6fb2033f4 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -120,14 +120,15 @@ cClientHandle::~cClientHandle() cWorld * World = m_Player->GetWorld(); if (World != nullptr) { + RemoveFromAllChunks(); + m_Player->GetWorld()->RemoveClientFromChunkSender(this); + m_Player->SetIsTicking(false); if (!m_Username.empty()) { // Send the Offline PlayerList packet: World->BroadcastPlayerListRemovePlayer(*m_Player, this); } - - World->RemovePlayer(m_Player, true); // Must be called before cPlayer::Destroy() as otherwise cChunk tries to delete the player, and then we do it again - m_Player->Destroy(); + m_Player->DestroyNoScheduling(true); } delete m_Player; m_Player = nullptr; @@ -164,18 +165,18 @@ void cClientHandle::Destroy(void) m_State = csDestroying; } - // DEBUG: LOGD("%s: client %p, \"%s\"", __FUNCTION__, static_cast(this), m_Username.c_str()); - if ((m_Player != nullptr) && (m_Player->GetWorld() != nullptr)) - { - RemoveFromAllChunks(); - m_Player->GetWorld()->RemoveClientFromChunkSender(this); - } if (m_Player != nullptr) { + cWorld * World = m_Player->GetWorld(); + if (World != nullptr) + { + World->RemovePlayer(m_Player, true); // TODO this is NOT thread safe. + } m_Player->RemoveClientHandle(); } + m_State = csDestroyed; } @@ -384,7 +385,7 @@ void cClientHandle::Authenticate(const AString & a_Name, const AString & a_UUID, cRoot::Get()->BroadcastPlayerListsAddPlayer(*m_Player); cRoot::Get()->SendPlayerLists(m_Player); - m_Player->Initialize(*World); + m_Player->SetWorld(World); m_State = csAuthenticated; // Query player team @@ -2020,7 +2021,7 @@ void cClientHandle::ServerTick(float a_Dt) // Add the player to the world (start ticking from there): m_State = csDownloadingWorld; - m_Player->GetWorld()->AddPlayer(m_Player); + m_Player->Initialize(*(m_Player->GetWorld())); return; } @@ -3032,14 +3033,6 @@ void cClientHandle::SocketClosed(void) LOGD("Client %s @ %s disconnected", m_Username.c_str(), m_IPString.c_str()); cRoot::Get()->GetPluginManager()->CallHookDisconnect(*this, "Player disconnected"); } - if ((m_State < csDestroying) && (m_Player != nullptr)) - { - cWorld * World = m_Player->GetWorld(); - if (World != nullptr) - { - World->RemovePlayer(m_Player, true); // Must be called before cPlayer::Destroy() as otherwise cChunk tries to delete the player, and then we do it again - } - } Destroy(); } diff --git a/src/Entities/Entity.cpp b/src/Entities/Entity.cpp index d0540b4eb..169bcb4de 100644 --- a/src/Entities/Entity.cpp +++ b/src/Entities/Entity.cpp @@ -39,8 +39,6 @@ cEntity::cEntity(eEntityType a_EntityType, double a_X, double a_Y, double a_Z, d m_Gravity(-9.81f), m_AirDrag(0.02f), m_LastPosition(a_X, a_Y, a_Z), - m_IsInitialized(false), - m_WorldTravellingFrom(nullptr), m_EntityType(a_EntityType), m_World(nullptr), m_IsWorldChangeScheduled(false), @@ -55,6 +53,8 @@ cEntity::cEntity(eEntityType a_EntityType, double a_X, double a_Y, double a_Z, d m_AirLevel(0), m_AirTickTimer(0), m_TicksAlive(0), + m_IsTicking(false), + m_ParentChunk(nullptr), m_HeadYaw(0.0), m_Rot(0.0, 0.0, 0.0), m_Position(a_X, a_Y, a_Z), @@ -77,8 +77,10 @@ cEntity::cEntity(eEntityType a_EntityType, double a_X, double a_Y, double a_Z, d cEntity::~cEntity() { + // Before deleting, the entity needs to have been removed from the world, if ever added ASSERT((m_World == nullptr) || !m_World->HasEntity(m_UniqueID)); + ASSERT(!IsTicking()); /* // DEBUG: @@ -98,12 +100,6 @@ cEntity::~cEntity() { m_Attachee->Detach(); } - - if (m_IsInitialized) - { - LOGWARNING("ERROR: Entity deallocated without being destroyed"); - ASSERT(!"Entity deallocated without being destroyed or unlinked"); - } } @@ -139,7 +135,7 @@ const char * cEntity::GetParentClass(void) const bool cEntity::Initialize(cWorld & a_World) { - if (cPluginManager::Get()->CallHookSpawningEntity(a_World, *this) && !IsPlayer()) + if (cPluginManager::Get()->CallHookSpawningEntity(a_World, *this)) { return false; } @@ -151,9 +147,10 @@ bool cEntity::Initialize(cWorld & a_World) ); */ - m_IsInitialized = true; - m_World = &a_World; - m_World->AddEntity(this); + ASSERT(m_World == nullptr); + ASSERT(GetParentChunk() == nullptr); + a_World.AddEntity(this); + ASSERT(m_World != nullptr); cPluginManager::Get()->CallHookSpawnedEntity(a_World, *this); @@ -175,7 +172,6 @@ void cEntity::WrapHeadYaw(void) - void cEntity::WrapRotation(void) { m_Rot.x = NormalizeAngleDegrees(m_Rot.x); @@ -196,20 +192,60 @@ void cEntity::WrapSpeed(void) +void cEntity::SetParentChunk(cChunk * a_Chunk) +{ + m_ParentChunk = a_Chunk; +} + + + + + +cChunk * cEntity::GetParentChunk() +{ + return m_ParentChunk; +} + + + + + void cEntity::Destroy(bool a_ShouldBroadcast) { - if (!m_IsInitialized) + ASSERT(IsTicking()); + ASSERT(GetParentChunk() != nullptr); + SetIsTicking(false); + + if (a_ShouldBroadcast) { - return; + m_World->BroadcastDestroyEntity(*this); } + cChunk * ParentChunk = GetParentChunk(); + m_World->QueueTask([this, ParentChunk](cWorld & a_World) + { + LOGD("Destroying entity #%i (%s) from chunk (%d, %d)", + this->GetUniqueID(), this->GetClass(), + ParentChunk->GetPosX(), ParentChunk->GetPosZ() + ); + ParentChunk->RemoveEntity(this); + delete this; + }); + Destroyed(); +} + + + + + +void cEntity::DestroyNoScheduling(bool a_ShouldBroadcast) +{ + SetIsTicking(false); if (a_ShouldBroadcast) { m_World->BroadcastDestroyEntity(*this); } - m_IsInitialized = false; - Destroyed(); } @@ -217,10 +253,10 @@ void cEntity::Destroy(bool a_ShouldBroadcast) + void cEntity::TakeDamage(cEntity & a_Attacker) { int RawDamage = a_Attacker.GetRawDamageAgainst(*this); - TakeDamage(dtAttack, &a_Attacker, RawDamage, a_Attacker.GetKnockbackAmountAgainst(*this)); } @@ -833,6 +869,8 @@ void cEntity::SetHealth(int a_Health) void cEntity::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) { + ASSERT(IsTicking()); + ASSERT(GetWorld() != nullptr); m_TicksAlive++; if (m_InvulnerableTicks > 0) @@ -1491,6 +1529,7 @@ bool cEntity::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d { UNUSED(a_ShouldSendRespawn); ASSERT(a_World != nullptr); + ASSERT(IsTicking()); if (GetWorld() == a_World) { @@ -1505,21 +1544,17 @@ bool cEntity::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d return false; } - // 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; - } + // Stop ticking, in preperation for detaching from this world. + SetIsTicking(false); + // Tell others we are gone GetWorld()->BroadcastDestroyEntity(*this); + // Set position to the new position SetPosition(a_NewPosition); + // Stop all mobs from targeting this entity + // Stop this entity from targeting other mobs if (this->IsMob()) { cMonster * Monster = static_cast(this); @@ -1527,14 +1562,21 @@ bool cEntity::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d Monster->StopEveryoneFromTargetingMe(); } - // Queue add to new world - a_World->AddEntity(this); - cWorld * OldWorld = cRoot::Get()->GetWorld(GetWorld()->GetName()); // Required for the hook HOOK_ENTITY_CHANGED_WORLD - SetWorld(a_World); - - // Entity changed the world, call the hook - cRoot::Get()->GetPluginManager()->CallHookEntityChangedWorld(*this, *OldWorld); - + // Queue add to new world and removal from the old one + cWorld * OldWorld = GetWorld(); + cChunk * ParentChunk = GetParentChunk(); + SetWorld(a_World); // Chunks may be streamed before cWorld::AddPlayer() sets the world to the new value + OldWorld->QueueTask([this, ParentChunk, a_World](cWorld & a_OldWorld) + { + LOGD("Warping entity #%i (%s) from world \"%s\" to \"%s\". Source chunk: (%d, %d) ", + this->GetUniqueID(), this->GetClass(), + a_OldWorld.GetName().c_str(), a_World->GetName().c_str(), + ParentChunk->GetPosX(), ParentChunk->GetPosZ() + ); + ParentChunk->RemoveEntity(this); + a_World->AddEntity(this); + cRoot::Get()->GetPluginManager()->CallHookEntityChangedWorld(*this, a_OldWorld); + }); return true; } @@ -1595,6 +1637,16 @@ void cEntity::SetSwimState(cChunk & a_Chunk) +void cEntity::SetIsTicking(bool a_IsTicking) +{ + m_IsTicking = a_IsTicking; + ASSERT(!(m_IsTicking && (m_ParentChunk == nullptr))); // We shouldn't be ticking if we have no parent chunk +} + + + + + void cEntity::DoSetSpeed(double a_SpeedX, double a_SpeedY, double a_SpeedZ) { m_Speed.Set(a_SpeedX, a_SpeedY, a_SpeedZ); @@ -2069,6 +2121,12 @@ void cEntity::SteerVehicle(float a_Forward, float a_Sideways) +bool cEntity::IsTicking(void) const +{ + ASSERT(!(m_IsTicking && (m_ParentChunk == nullptr))); // We shouldn't be ticking if we have no parent chunk + return m_IsTicking; +} + //////////////////////////////////////////////////////////////////////////////// // Get look vector (this is NOT a rotation!) Vector3d cEntity::GetLookVector(void) const diff --git a/src/Entities/Entity.h b/src/Entities/Entity.h index dbfc019e7..75c77e593 100644 --- a/src/Entities/Entity.h +++ b/src/Entities/Entity.h @@ -252,9 +252,16 @@ public: void SteerVehicle(float a_Forward, float a_Sideways); inline UInt32 GetUniqueID(void) const { return m_UniqueID; } - inline bool IsDestroyed(void) const { return !m_IsInitialized; } - /** Schedules the entity for destroying; if a_ShouldBroadcast is set to true, broadcasts the DestroyEntity packet */ + /** Deprecated. Use IsTicking instead. */ + inline bool IsDestroyed() const {return !IsTicking();} + + /** Returns true if the entity is valid and ticking. Returns false if the entity is not ticking and is about to leave + its current world either via teleportation or destruction. + If this returns false, you must stop using the cEntity pointer you have. */ + bool IsTicking(void) const; + + /** Destroys the entity and schedules it for memory freeing; if a_ShouldBroadcast is set to true, broadcasts the DestroyEntity packet */ void Destroy(bool a_ShouldBroadcast = true); /** Makes this pawn take damage from an attack by a_Attacker. Damage values are calculated automatically and DoTakeDamage() called */ @@ -285,6 +292,9 @@ public: // tolua_end + /** Destroy the entity without scheduling memory freeing. This should only be used by cChunk or cClientHandle for internal memory management. */ + void DestroyNoScheduling(bool a_ShouldBroadcast); + /** Makes this entity take damage specified in the a_TDI. The TDI is sent through plugins first, then applied. If it returns false, the entity hasn't receive any damage. */ @@ -408,12 +418,6 @@ public: virtual bool DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d a_NewPosition); - /** Returns if the entity is travelling away from a specified world */ - bool IsWorldTravellingFrom(cWorld * a_World) const { return (m_WorldTravellingFrom == a_World); } - - /** Sets the world the entity will be leaving */ - void SetWorldTravellingFrom(cWorld * a_World) { m_WorldTravellingFrom = a_World; } - /** Updates clients of changes in the entity. */ virtual void BroadcastMovementUpdate(const cClientHandle * a_Exclude = nullptr); @@ -481,6 +485,16 @@ public: /** Sets the internal world pointer to a new cWorld, doesn't update anything else. */ void SetWorld(cWorld * a_World) { m_World = a_World; } + /** Sets the parent chunk, which is the chunk responsible for ticking this entity. + Only cChunk::AddEntity and cChunk::RemoveEntity cChunk::~cChunk should ever call this. */ + void SetParentChunk(cChunk * a_Chunk); + + /** Returns the chunk responsible for ticking this entity. */ + cChunk * GetParentChunk(); + + /** Set the entity's status to either ticking or not ticking. */ + void SetIsTicking(bool a_IsTicking); + protected: /** Structure storing the portal delay timer and cooldown boolean */ struct sPortalCooldownData @@ -538,14 +552,6 @@ protected: Vector3d m_LastPosition; - /** True when entity is initialised (Initialize()) and false when destroyed pending deletion (Destroy()) */ - bool m_IsInitialized; - - /** World entity is travelling from (such as when using portals). - Set to a valid world pointer by MoveToWorld; reset to nullptr when the entity is removed from the old world. - Can't be a simple boolean as context switches between worlds may leave the new chunk processing (and therefore immediately removing) the entity before the old chunk could remove it. */ - cWorld * m_WorldTravellingFrom; - eEntityType m_EntityType; cWorld * m_World; @@ -606,6 +612,13 @@ protected: virtual void SetSwimState(cChunk & a_Chunk); private: + + /** Whether the entity is ticking or not. If not, it is scheduled for removal or world-teleportation. */ + bool m_IsTicking; + + /** The chunk which is responsible for ticking this entity. */ + cChunk * m_ParentChunk; + /** Measured in degrees, [-180, +180) */ double m_HeadYaw; diff --git a/src/Entities/Minecart.cpp b/src/Entities/Minecart.cpp index 3adf5477e..ea51c49c8 100644 --- a/src/Entities/Minecart.cpp +++ b/src/Entities/Minecart.cpp @@ -116,10 +116,7 @@ void cMinecart::SpawnOn(cClientHandle & a_ClientHandle) void cMinecart::HandlePhysics(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) { - if (IsDestroyed()) // Mainly to stop detector rails triggering again after minecart is dead - { - return; - } + ASSERT(IsTicking()); int PosY = POSY_TOINT; if ((PosY <= 0) || (PosY >= cChunkDef::Height)) diff --git a/src/Entities/Pawn.cpp b/src/Entities/Pawn.cpp index c8780c326..3dea74851 100644 --- a/src/Entities/Pawn.cpp +++ b/src/Entities/Pawn.cpp @@ -221,7 +221,7 @@ void cPawn::ClearEntityEffects() void cPawn::NoLongerTargetingMe(cMonster * a_Monster) { - ASSERT(!IsDestroyed()); // Our destroy override is supposed to clear all targets before we're destroyed. + ASSERT(IsTicking()); // Our destroy override is supposed to clear all targets before we're destroyed. for (auto i = m_TargetingMe.begin(); i != m_TargetingMe.end(); ++i) { cMonster * Monster = *i; @@ -241,7 +241,7 @@ void cPawn::NoLongerTargetingMe(cMonster * a_Monster) void cPawn::TargetingMe(cMonster * a_Monster) { - ASSERT(!IsDestroyed()); + ASSERT(IsTicking()); ASSERT(m_TargetingMe.size() < 10000); ASSERT(a_Monster->GetTarget() == this); m_TargetingMe.push_back(a_Monster); diff --git a/src/Entities/Pickup.cpp b/src/Entities/Pickup.cpp index c2bcf3960..bdb9128dc 100644 --- a/src/Entities/Pickup.cpp +++ b/src/Entities/Pickup.cpp @@ -30,11 +30,13 @@ public: virtual bool Item(cEntity * a_Entity) override { - if (!a_Entity->IsPickup() || (a_Entity->GetUniqueID() <= m_Pickup->GetUniqueID()) || a_Entity->IsDestroyed()) + ASSERT(a_Entity->IsTicking()); + if (!a_Entity->IsPickup() || (a_Entity->GetUniqueID() <= m_Pickup->GetUniqueID())) { return false; } + Vector3d EntityPos = a_Entity->GetPosition(); double Distance = (EntityPos - m_Position).Length(); @@ -152,7 +154,7 @@ void cPickup::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) } // Try to combine the pickup with adjacent same-item pickups: - if (!IsDestroyed() && (m_Item.m_ItemCount < m_Item.GetMaxStackSize())) // Don't combine if already full + if ((m_Item.m_ItemCount < m_Item.GetMaxStackSize())) // Don't combine if already full { // By using a_Chunk's ForEachEntity() instead of cWorld's, pickups don't combine across chunk boundaries. // That is a small price to pay for not having to traverse the entire world for each entity. diff --git a/src/Entities/Player.cpp b/src/Entities/Player.cpp index b7f6f4d05..9d18cedc4 100644 --- a/src/Entities/Player.cpp +++ b/src/Entities/Player.cpp @@ -146,6 +146,25 @@ cPlayer::cPlayer(cClientHandlePtr a_Client, const AString & a_PlayerName) : +bool cPlayer::Initialize(cWorld & a_World) +{ + UNUSED(a_World); + ASSERT(GetWorld() != nullptr); + ASSERT(GetParentChunk() == nullptr); + GetWorld()->AddPlayer(this); + + cPluginManager::Get()->CallHookSpawnedEntity(*GetWorld(), *this); + + // Spawn the entity on the clients: + GetWorld()->BroadcastSpawnEntity(*this); + + return true; +} + + + + + cPlayer::~cPlayer(void) { if (!cRoot::Get()->GetPluginManager()->CallHookPlayerDestroyed(*this)) @@ -1682,6 +1701,8 @@ void cPlayer::FreezeInternal(const Vector3d & a_Location, bool a_ManuallyFrozen) bool cPlayer::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d a_NewPosition) { ASSERT(a_World != nullptr); + ASSERT(IsTicking()); + if (GetWorld() == a_World) { // Don't move to same world @@ -1695,21 +1716,19 @@ 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; - } + // Prevent further ticking in this world + SetIsTicking(false); + + // Tell others we are gone + GetWorld()->BroadcastDestroyEntity(*this); // Remove player from world GetWorld()->RemovePlayer(this, false); - // Stop all mobs from targeting this player + // Set position to the new position + SetPosition(a_NewPosition); + + // Stop all mobs from targeting this player StopEveryoneFromTargetingMe(); // Send the respawn packet: @@ -1718,19 +1737,6 @@ bool cPlayer::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d m_ClientHandle->SendRespawn(a_World->GetDimension()); } - // Broadcast for other people that the player is gone. - GetWorld()->BroadcastDestroyEntity(*this); - - SetPosition(a_NewPosition); - - // Stop all mobs from targeting this player - StopEveryoneFromTargetingMe(); - - // Queue adding player to the new world, including all the necessary adjustments to the object - a_World->AddPlayer(this); - cWorld * OldWorld = cRoot::Get()->GetWorld(GetWorld()->GetName()); // Required for the hook HOOK_ENTITY_CHANGED_WORLD - SetWorld(a_World); // Chunks may be streamed before cWorld::AddPlayer() sets the world to the new value - // Update the view distance. m_ClientHandle->SetViewDistance(m_ClientHandle->GetRequestedViewDistance()); @@ -1743,9 +1749,21 @@ bool cPlayer::DoMoveToWorld(cWorld * a_World, bool a_ShouldSendRespawn, Vector3d // Broadcast the player into the new world. a_World->BroadcastSpawnEntity(*this); - // Player changed the world, call the hook - cRoot::Get()->GetPluginManager()->CallHookEntityChangedWorld(*this, *OldWorld); - + // Queue add to new world and removal from the old one + cChunk * ParentChunk = GetParentChunk(); + cWorld * OldWorld = GetWorld(); + SetWorld(a_World); // Chunks may be streamed before cWorld::AddPlayer() sets the world to the new value + OldWorld->QueueTask([this, ParentChunk, a_World](cWorld & a_OldWorld) + { + LOGD("Warping player \"%s\" from world \"%s\" to \"%s\". Source chunk: (%d, %d) ", + this->GetName().c_str(), + a_OldWorld.GetName().c_str(), a_World->GetName().c_str(), + ParentChunk->GetPosX(), ParentChunk->GetPosZ() + ); + ParentChunk->RemoveEntity(this); + a_World->AddPlayer(this); + cRoot::Get()->GetPluginManager()->CallHookEntityChangedWorld(*this, a_OldWorld); + }); return true; } diff --git a/src/Entities/Player.h b/src/Entities/Player.h index efd515b23..6d092eeb3 100644 --- a/src/Entities/Player.h +++ b/src/Entities/Player.h @@ -42,6 +42,8 @@ public: cPlayer(cClientHandlePtr a_Client, const AString & a_PlayerName); + virtual bool Initialize(cWorld & a_World) override; + virtual ~cPlayer(); virtual void SpawnOn(cClientHandle & a_Client) override; diff --git a/src/Inventory.cpp b/src/Inventory.cpp index 35f087d79..5add53a12 100644 --- a/src/Inventory.cpp +++ b/src/Inventory.cpp @@ -713,7 +713,7 @@ void cInventory::OnSlotChanged(cItemGrid * a_ItemGrid, int a_SlotNum) { // Send the neccessary updates to whoever needs them - if (m_Owner.IsDestroyed()) + if (!m_Owner.IsTicking()) { // Owner is not (yet) valid, skip for now return; diff --git a/src/Mobs/Monster.cpp b/src/Mobs/Monster.cpp index 28cb10238..fad54a00d 100644 --- a/src/Mobs/Monster.cpp +++ b/src/Mobs/Monster.cpp @@ -256,7 +256,7 @@ void cMonster::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) } if ((GetTarget() != nullptr)) { - ASSERT(!GetTarget()->IsDestroyed()); + ASSERT(GetTarget()->IsTicking()); if (GetTarget()->IsPlayer()) { @@ -912,7 +912,7 @@ int cMonster::GetSpawnDelay(cMonster::eFamily a_MobFamily) /** Sets the target. */ void cMonster::SetTarget (cPawn * a_NewTarget) { - ASSERT((a_NewTarget == nullptr) || (!IsDestroyed())); + ASSERT((a_NewTarget == nullptr) || (IsTicking())); if (m_Target == a_NewTarget) { return; @@ -928,7 +928,7 @@ void cMonster::SetTarget (cPawn * a_NewTarget) if (a_NewTarget != nullptr) { - ASSERT(!a_NewTarget->IsDestroyed()); + ASSERT(a_NewTarget->IsTicking()); // Notify the new target that we are now targeting it. m_Target->TargetingMe(this); } diff --git a/src/Mobs/PassiveMonster.cpp b/src/Mobs/PassiveMonster.cpp index 53288a54c..000f3d5a4 100644 --- a/src/Mobs/PassiveMonster.cpp +++ b/src/Mobs/PassiveMonster.cpp @@ -65,6 +65,18 @@ void cPassiveMonster::ResetLoveMode() +void cPassiveMonster::Destroyed() +{ + if (m_LovePartner != nullptr) + { + m_LovePartner->ResetLoveMode(); + } +} + + + + + void cPassiveMonster::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) { super::Tick(a_Dt, a_Chunk); @@ -73,10 +85,6 @@ void cPassiveMonster::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) { CheckEventLostPlayer(); } - if ((m_LovePartner != nullptr) && m_LovePartner->IsDestroyed()) - { - m_LovePartner = nullptr; - } // if we have a partner, mate if (m_LovePartner != nullptr) diff --git a/src/Mobs/PassiveMonster.h b/src/Mobs/PassiveMonster.h index 1106ffb91..9a2627417 100644 --- a/src/Mobs/PassiveMonster.h +++ b/src/Mobs/PassiveMonster.h @@ -45,6 +45,8 @@ public: /** Returns whether the monster is tired of breeding and is in the cooldown state. */ bool IsInLoveCooldown() const { return (m_LoveCooldown > 0); } + virtual void Destroyed(void) override; + protected: /** The monster's breeding partner. */ cPassiveMonster * m_LovePartner; diff --git a/src/World.cpp b/src/World.cpp index c704b46bb..d8386119d 100644 --- a/src/World.cpp +++ b/src/World.cpp @@ -1018,6 +1018,8 @@ void cWorld::Tick(std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_La { (*itr)->SetWorld(this); m_ChunkMap->AddEntity(*itr); + ASSERT(!(*itr)->IsTicking()); + (*itr)->SetIsTicking(true); } m_EntitiesToAdd.clear(); } @@ -1026,6 +1028,7 @@ void cWorld::Tick(std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_La AddQueuedPlayers(); m_ChunkMap->Tick(a_Dt); + TickMobs(a_Dt); m_MapManager.TickMaps(); TickClients(static_cast(a_Dt.count())); @@ -1046,7 +1049,6 @@ void cWorld::Tick(std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_La UnloadUnusedChunks(); } - TickMobs(a_Dt); } @@ -1134,7 +1136,16 @@ void cWorld::TickMobs(std::chrono::milliseconds a_Dt) cMobProximityCounter::sIterablePair allCloseEnoughToMoveMobs = MobCensus.GetProximityCounter().getMobWithinThosesDistances(-1, 64 * 16);// MG TODO : deal with this magic number (the 16 is the size of a block) for (cMobProximityCounter::tDistanceToMonster::const_iterator itr = allCloseEnoughToMoveMobs.m_Begin; itr != allCloseEnoughToMoveMobs.m_End; ++itr) { - itr->second.m_Monster.Tick(a_Dt, itr->second.m_Chunk); + cEntity & Entity = itr->second.m_Monster; + cChunk * Chunk = Entity.GetParentChunk(); + if (!Entity.IsTicking()) + { + ++itr; + continue; + } + ASSERT(Chunk == &(itr->second.m_Chunk)); + Entity.Tick(a_Dt, *Chunk); + ASSERT(Chunk == &(itr->second.m_Chunk)); } // remove too far mobs @@ -2919,7 +2930,7 @@ bool cWorld::ForEachPlayer(cPlayerListCallback & a_Callback) for (cPlayerList::iterator itr = m_Players.begin(), itr2 = itr; itr != m_Players.end(); itr = itr2) { ++itr2; - if ((*itr)->IsDestroyed()) + if (!(*itr)->IsTicking()) { continue; } @@ -2941,7 +2952,7 @@ bool cWorld::DoWithPlayer(const AString & a_PlayerName, cPlayerListCallback & a_ cCSLock Lock(m_CSPlayers); for (cPlayerList::iterator itr = m_Players.begin(); itr != m_Players.end(); ++itr) { - if ((*itr)->IsDestroyed()) + if (!(*itr)->IsTicking()) { continue; } @@ -2967,7 +2978,7 @@ bool cWorld::FindAndDoWithPlayer(const AString & a_PlayerNameHint, cPlayerListCa cCSLock Lock(m_CSPlayers); for (cPlayerList::iterator itr = m_Players.begin(); itr != m_Players.end(); ++itr) { - if ((*itr)->IsDestroyed()) + if (!(*itr)->IsTicking()) { continue; } @@ -2999,7 +3010,7 @@ bool cWorld::DoWithPlayerByUUID(const AString & a_PlayerUUID, cPlayerListCallbac cCSLock Lock(m_CSPlayers); for (cPlayerList::iterator itr = m_Players.begin(); itr != m_Players.end(); ++itr) { - if ((*itr)->IsDestroyed()) + if (!(*itr)->IsTicking()) { continue; } @@ -3026,7 +3037,7 @@ cPlayer * cWorld::FindClosestPlayer(const Vector3d & a_Pos, float a_SightLimit, cCSLock Lock(m_CSPlayers); for (cPlayerList::const_iterator itr = m_Players.begin(); itr != m_Players.end(); ++itr) { - if ((*itr)->IsDestroyed()) + if (!(*itr)->IsTicking()) { continue; } @@ -3406,6 +3417,7 @@ void cWorld::ScheduleTask(int a_DelayTicks, std::function a_Tas void cWorld::AddEntity(cEntity * a_Entity) { + a_Entity->SetWorld(this); cCSLock Lock(m_CSEntitiesToAdd); m_EntitiesToAdd.push_back(a_Entity); } @@ -3792,6 +3804,8 @@ void cWorld::AddQueuedPlayers(void) // Add to chunkmap, if not already there (Spawn vs MoveToWorld): m_ChunkMap->AddEntityIfNotPresent(*itr); + ASSERT(!(*itr)->IsTicking()); + (*itr)->SetIsTicking(true); } // for itr - PlayersToAdd[] } // Lock(m_CSPlayers) @@ -3909,6 +3923,3 @@ cBroadcaster cWorld::GetBroadcaster() { return cBroadcaster(this); } - - - diff --git a/src/World.h b/src/World.h index c5df65049..93c566ce5 100644 --- a/src/World.h +++ b/src/World.h @@ -819,7 +819,6 @@ private: virtual void Execute(void) override; } ; - /** Implementation of the callbacks that the ChunkGenerator uses to store new chunks and interface to plugins */ class cChunkGeneratorCallbacks : public cChunkGenerator::cChunkSink, -- cgit v1.2.3