From e98f93a079c2cfea7b5478e2cb2934874e0b88e0 Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sun, 5 Apr 2020 13:41:14 +0100 Subject: Only store IDs across ticks --- src/Entities/Entity.cpp | 58 +++++++++++++++++++++++++++++++-------------- src/Entities/Entity.h | 6 ++--- src/NetherPortalScanner.cpp | 19 +++++++++++---- src/NetherPortalScanner.h | 9 ++++--- 4 files changed, 64 insertions(+), 28 deletions(-) diff --git a/src/Entities/Entity.cpp b/src/Entities/Entity.cpp index 23a29a3a4..e0ea32e78 100644 --- a/src/Entities/Entity.cpp +++ b/src/Entities/Entity.cpp @@ -73,6 +73,7 @@ cEntity::cEntity(eEntityType a_EntityType, Vector3d a_Pos, double a_Width, doubl m_Height(a_Height), m_InvulnerableTicks(0) { + m_WorldChangeInfo.m_NewWorld = nullptr; } @@ -1406,7 +1407,7 @@ bool cEntity::DetectPortal() cWorld * TargetWorld = cRoot::Get()->GetWorld(GetWorld()->GetLinkedOverworldName()); ASSERT(TargetWorld != nullptr); // The linkage checker should have prevented this at startup. See cWorld::start() LOGD("Jumping %s -> %s", DimensionToString(dimNether).c_str(), DimensionToString(DestionationDim).c_str()); - new cNetherPortalScanner(this, TargetWorld, TargetPos, cChunkDef::Height); + new cNetherPortalScanner(*this, TargetWorld, TargetPos, cChunkDef::Height); return true; } // Nether portal in the overworld @@ -1438,7 +1439,7 @@ bool cEntity::DetectPortal() cWorld * TargetWorld = cRoot::Get()->GetWorld(GetWorld()->GetLinkedNetherWorldName()); ASSERT(TargetWorld != nullptr); // The linkage checker should have prevented this at startup. See cWorld::start() LOGD("Jumping %s -> %s", DimensionToString(dimOverworld).c_str(), DimensionToString(DestionationDim).c_str()); - new cNetherPortalScanner(this, TargetWorld, TargetPos, (cChunkDef::Height / 2)); + new cNetherPortalScanner(*this, TargetWorld, TargetPos, (cChunkDef::Height / 2)); return true; } } @@ -1587,25 +1588,46 @@ bool cEntity::MoveToWorld(cWorld * a_World, Vector3d a_NewPosition, bool a_SetPo return false; } + if (m_WorldChangeInfo.m_NewWorld != nullptr) + { + // Avoid scheduling multiple warp tasks + return true; + } + // Create new world change info - auto NewWCI = cpp14::make_unique(); - *NewWCI = { a_World, a_NewPosition, a_SetPortalCooldown, a_ShouldSendRespawn }; + m_WorldChangeInfo = { a_World, a_NewPosition, a_SetPortalCooldown, a_ShouldSendRespawn }; - // Publish atomically - auto OldWCI = m_WorldChangeInfo.exchange(std::move(NewWCI)); + // TODO: move to capture when C++14 + const auto EntityID = GetUniqueID(); - if (OldWCI == nullptr) - { - // Schedule a new world change. - GetWorld()->QueueTask( - [this](cWorld & a_CurWorld) - { - auto WCI = m_WorldChangeInfo.exchange(nullptr); - cWorld::cLock Lock(a_CurWorld); - DoMoveToWorld(*WCI); - } - ); - } + /* Requirements: + Only one world change in-flight at any time + No ticking during world changes + The last invocation takes effect + + As of writing, cWorld ticks entities, clients, and then processes tasks + We may call MoveToWorld (any number of times - consider multiple /portal commands within a tick) in the first and second stages + Queue a task onto the third stage to invoke DoMoveToWorld ONCE with the last given destination world + Store entity IDs in case client tick found the player disconnected and immediately destroys the object + + After the move begins, no further calls to MoveToWorld is possible since neither the client nor entity is ticked + This remains until the warp is complete and the destination world resumes ticking. + */ + GetWorld()->QueueTask( + [EntityID](cWorld & a_CurWorld) + { + a_CurWorld.DoWithEntityByID( + EntityID, + [](cEntity & a_Entity) + { + auto & WCI = a_Entity.m_WorldChangeInfo; + a_Entity.DoMoveToWorld(WCI); + WCI.m_NewWorld = nullptr; + return true; + } + ); + } + ); return true; } diff --git a/src/Entities/Entity.h b/src/Entities/Entity.h index 25784081f..573bc34cf 100644 --- a/src/Entities/Entity.h +++ b/src/Entities/Entity.h @@ -489,7 +489,7 @@ public: /** Returns true if a world change is scheduled to happen. */ bool IsWorldChangeScheduled() const { - return (m_WorldChangeInfo.load() != nullptr); + return (m_WorldChangeInfo.m_NewWorld != nullptr); } /** Updates clients of changes in the entity. */ @@ -662,8 +662,8 @@ protected: cWorld * m_World; - /** If not nullptr, a world change is scheduled and a task is queued in the current world. */ - cAtomicUniquePtr m_WorldChangeInfo; + /** If field m_NewWorld not nullptr, a world change is scheduled and a task is queued in the current world. */ + sWorldChangeInfo m_WorldChangeInfo; /** Whether the entity is capable of taking fire or lava damage. */ bool m_IsFireproof; diff --git a/src/NetherPortalScanner.cpp b/src/NetherPortalScanner.cpp index 8de0a6dcc..386da61fb 100644 --- a/src/NetherPortalScanner.cpp +++ b/src/NetherPortalScanner.cpp @@ -17,8 +17,9 @@ const double cNetherPortalScanner::AcrossOffset = 0.5; -cNetherPortalScanner::cNetherPortalScanner(cEntity * a_MovingEntity, cWorld * a_DestinationWorld, Vector3d a_DestPosition, int a_MaxY) : - m_Entity(a_MovingEntity), +cNetherPortalScanner::cNetherPortalScanner(cEntity & a_MovingEntity, cWorld * a_DestinationWorld, Vector3d a_DestPosition, int a_MaxY) : + m_EntityID(a_MovingEntity.GetUniqueID()), + m_SourceWorld(*a_MovingEntity.GetWorld()), m_World(a_DestinationWorld), m_FoundPortal(false), m_BuildPlatform(true), @@ -296,8 +297,18 @@ void cNetherPortalScanner::OnDisabled(void) Position.z += OutOffset; } - FLOGD("Placing player at {0}", Position); - m_Entity->MoveToWorld(m_World, Position, true, false); + // Lookup our warping entity by ID + // Necessary as they may have been destroyed in the meantime (#4582) + m_SourceWorld.DoWithEntityByID( + m_EntityID, + [this, &Position](cEntity & a_Entity) + { + FLOGD("Placing player at {0}", Position); + a_Entity.MoveToWorld(m_World, Position, true, false); + return true; + } + ); + delete this; } diff --git a/src/NetherPortalScanner.h b/src/NetherPortalScanner.h index f3df57c0d..3d285053a 100644 --- a/src/NetherPortalScanner.h +++ b/src/NetherPortalScanner.h @@ -15,7 +15,7 @@ class cWorld; class cNetherPortalScanner : public cChunkStay { public: - cNetherPortalScanner(cEntity * a_MovingEntity, cWorld * a_DestinationWorld, Vector3d a_DestPosition, int a_MaxY); + cNetherPortalScanner(cEntity & a_MovingEntity, cWorld * a_DestinationWorld, Vector3d a_DestPosition, int a_MaxY); virtual void OnChunkAvailable(int a_ChunkX, int a_ChunkY) override; virtual bool OnAllChunksAvailable(void) override; virtual void OnDisabled(void) override; @@ -48,8 +48,11 @@ private: /** Whether the given location is a valid location to build a portal. */ bool IsValidBuildLocation(Vector3i a_BlockPosition); - /** The entity that's being moved. */ - cEntity * m_Entity; + /** The ID of the entity that's being moved. */ + UInt32 m_EntityID; + + /** The world we're moving the entity from, used to query the entity ID. */ + cWorld & m_SourceWorld; /** The world we're moving the entity to. */ cWorld * m_World; -- cgit v1.2.3