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/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 + 7 files changed, 177 insertions(+), 87 deletions(-) (limited to 'src/Entities') 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; -- cgit v1.2.3