From e98f93a079c2cfea7b5478e2cb2934874e0b88e0 Mon Sep 17 00:00:00 2001
From: Tiger Wang <ziwei.tiger@outlook.com>
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 ++---
 2 files changed, 43 insertions(+), 21 deletions(-)

(limited to 'src/Entities')

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<sWorldChangeInfo>();
-	*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<sWorldChangeInfo> 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;
-- 
cgit v1.2.3