Spectation: add dedicated pathway for spectator mode (#5303)

* Spectation: add dedicated pathway for spectator mode

+ Sync player rotation with spectated entity.
+ Add dedicated infrastructure to cPlayer for handling spectation, instead of misusing entity riding.
* Avoid infinite recursion when exiting spectation, fixes #5296

* AttachTo: Change parameter to reference

src/ClientHandle.cpp

@@ -1757,7 +1757,7 @@ void cClientHandle::HandleUseEntity(UInt32 a_TargetEntityID, bool a_IsLeftClick)
 	{
 		m_Player->GetWorld()->DoWithEntityByID(a_TargetEntityID, [=](cEntity & a_Entity)
 		{
-			m_Player->AttachTo(&a_Entity);
+			m_Player->SpectateEntity(&a_Entity);
 			return true;
 		});
 		return;

src/Entities/Boat.cpp

@@ -17,17 +17,18 @@
 class cBoatCollisionCallback
 {
 public:
-	cBoatCollisionCallback(cBoat * a_Boat, cEntity * a_Attachee) :
+
+	cBoatCollisionCallback(cBoat & a_Boat, cEntity * a_Attachee) :
 		m_Boat(a_Boat), m_Attachee(a_Attachee)
 	{
 	}
 
 	bool operator()(cEntity & a_Entity)
 	{
-		// Checks if boat is empty and if given entity is a mob
+		// Checks if boat is empty and if given entity is a mob:
-		if ((m_Attachee == nullptr) && (a_Entity.IsMob()))
+		if ((m_Attachee == nullptr) && a_Entity.IsMob())
 		{
-			// if so attach and return true
+			// If so attach and stop iterating:
 			a_Entity.AttachTo(m_Boat);
 			return true;
 		}
@@ -36,7 +37,8 @@ public:
 	}
 
 protected:
-	cBoat * m_Boat;
+
+	cBoat & m_Boat;
 	cEntity * m_Attachee;
 };
 
@@ -159,7 +161,7 @@ void cBoat::OnRightClicked(cPlayer & a_Player)
 	}
 
 	// Attach the player to this boat
-	a_Player.AttachTo(this);
+	a_Player.AttachTo(*this);
 }
 
 
@@ -349,7 +351,7 @@ void cBoat::HandlePhysics(std::chrono::milliseconds a_Dt, cChunk & a_Chunk)
 	normal physics calcualtions */
 
 	// Calculate boat's bounding box, run collision callback on all entities in said box
-	cBoatCollisionCallback BoatCollisionCallback(this, m_Attachee);
+	cBoatCollisionCallback BoatCollisionCallback(*this, m_Attachee);
 	Vector3d BoatPosition = GetPosition();
 	cBoundingBox bbBoat(
 		Vector3d(BoatPosition.x, floor(BoatPosition.y), BoatPosition.z), GetWidth() / 2, GetHeight());

src/Entities/Entity.cpp

@@ -1994,26 +1994,25 @@ cEntity * cEntity::GetAttached()
 
 
 
-void cEntity::AttachTo(cEntity * a_AttachTo)
+void cEntity::AttachTo(cEntity & a_AttachTo)
 {
-	if (m_AttachedTo == a_AttachTo)
+	if (m_AttachedTo == &a_AttachTo)
 	{
-		// Already attached to that entity, nothing to do here
+		// Already attached to that entity, nothing to do here:
 		return;
 	}
+
 	if (m_AttachedTo != nullptr)
 	{
 		// Detach from any previous entity:
 		Detach();
 	}
 
-	// Update state information
+	// Update state information:
-	m_AttachedTo = a_AttachTo;
+	m_AttachedTo = &a_AttachTo;
-	a_AttachTo->m_Attachee = this;
+	a_AttachTo.m_Attachee = this;
-	if (a_AttachTo != nullptr)
+
-	{
+	m_World->BroadcastAttachEntity(*this, a_AttachTo);
-		m_World->BroadcastAttachEntity(*this, *a_AttachTo);
-	}
 }
 
 
@@ -2024,13 +2023,16 @@ void cEntity::Detach(void)
 {
 	if (m_AttachedTo == nullptr)
 	{
-		// Already not attached to any entity, our work is done
+		// Already not attached to any entity, our work is done:
 		return;
 	}
+
 	m_World->BroadcastDetachEntity(*this, *m_AttachedTo);
 
 	m_AttachedTo->m_Attachee = nullptr;
 	m_AttachedTo = nullptr;
+
+	OnDetach();
 }
 
 
@@ -2306,6 +2308,14 @@ void cEntity::BroadcastLeashedMobs()
 
 
 
+void cEntity::OnDetach()
+{
+}
+
+
+
+
+
 void cEntity::BroadcastDeathMessage(TakeDamageInfo & a_TDI)
 {
 	cPluginManager * PluginManager = cRoot::Get()->GetPluginManager();

src/Entities/Entity.h

@@ -452,11 +452,11 @@ public:
 	/** Gets entity (vehicle) attached to this entity */
 	cEntity * GetAttached();
 
-	/** Attaches to the specified entity; detaches from any previous one first */
+	/** Attaches to the specified entity; detaches from any previous one first. */
-	virtual void AttachTo(cEntity * a_AttachTo);
+	void AttachTo(cEntity & a_AttachTo);
 
-	/** Detaches from the currently attached entity, if any */
+	/** Detaches from the currently attached entity, if any. */
-	virtual void Detach(void);
+	void Detach(void);
 
 	/** Returns true if this entity is attached to the specified entity */
 	bool IsAttachedTo(const cEntity * a_Entity) const;
@@ -578,10 +578,10 @@ protected:
 	float m_Health;
 	float m_MaxHealth;
 
-	/** The entity to which this entity is attached (vehicle), nullptr if none */
+	/** The entity to which this entity is attached (vehicle), nullptr if none. */
 	cEntity * m_AttachedTo;
 
-	/** The entity which is attached to this entity (rider), nullptr if none */
+	/** The entity which is attached to this entity (rider), nullptr if none. */
 	cEntity * m_Attachee;
 
 	/** Stores whether head yaw has been set manually */
@@ -683,6 +683,9 @@ protected:
 	/** If has any mobs are leashed, broadcasts every leashed entity to this. */
 	void BroadcastLeashedMobs();
 
+	/** Called when this entity dismounts from m_AttachedTo. */
+	virtual void OnDetach();
+
 private:
 
 	/** Whether the entity is ticking or not. If not, it is scheduled for removal or world-teleportation. */

src/Entities/Floater.cpp

@@ -180,7 +180,7 @@ void cFloater::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk)
 		a_Chunk.ForEachEntity(Callback);
 		if (Callback.HasHit())
 		{
-			AttachTo(Callback.GetHitEntity());
+			AttachTo(*Callback.GetHitEntity());
 			Callback.GetHitEntity()->TakeDamage(*this);  // TODO: the player attacked the mob not the floater.
 			m_AttachedMobID = Callback.GetHitEntity()->GetUniqueID();
 		}

src/Entities/Minecart.cpp

@@ -24,17 +24,17 @@
 class cMinecartAttachCallback
 {
 public:
-	cMinecartAttachCallback(cMinecart * a_Minecart, cEntity * a_Attachee) :
+	cMinecartAttachCallback(cMinecart & a_Minecart, cEntity * a_Attachee) :
 		m_Minecart(a_Minecart), m_Attachee(a_Attachee)
 	{
 	}
 
-	bool operator () (cEntity & a_Entity)
+	bool operator()(cEntity & a_Entity)
 	{
-		// Check if minecart is empty and if given entity is a mob
+		// Check if minecart is empty and if given entity is a mob:
-		if ((m_Attachee == nullptr) && (a_Entity.IsMob()))
+		if ((m_Attachee == nullptr) && a_Entity.IsMob())
 		{
-			// if so, attach to minecart and return true
+			// If so, attach to minecart and stop iterating:
 			a_Entity.AttachTo(m_Minecart);
 			return true;
 		}
@@ -42,7 +42,8 @@ public:
 	}
 
 protected:
-	cMinecart * m_Minecart;
+
+	cMinecart & m_Minecart;
 	cEntity * m_Attachee;
 };
 
@@ -1084,7 +1085,7 @@ bool cMinecart::TestEntityCollision(NIBBLETYPE a_RailMeta)
 	}
 
 	// Collision was true, create bounding box for minecart, call attach callback for all entities within that box
-	cMinecartAttachCallback MinecartAttachCallback(this, m_Attachee);
+	cMinecartAttachCallback MinecartAttachCallback(*this, m_Attachee);
 	Vector3d MinecartPosition = GetPosition();
 	cBoundingBox bbMinecart(Vector3d(MinecartPosition.x, floor(MinecartPosition.y), MinecartPosition.z), GetWidth() / 2, GetHeight());
 	m_World->ForEachEntityInBox(bbMinecart, MinecartAttachCallback);
@@ -1350,8 +1351,8 @@ void cRideableMinecart::OnRightClicked(cPlayer & a_Player)
 		m_Attachee->Detach();
 	}
 
-	// Attach the player to this minecart
+	// Attach the player to this minecart:
-	a_Player.AttachTo(this);
+	a_Player.AttachTo(*this);
 }
 
 

src/Entities/Player.cpp

@@ -140,6 +140,7 @@ cPlayer::cPlayer(const std::shared_ptr<cClientHandle> & a_Client) :
 	m_BowCharge(0),
 	m_FloaterID(cEntity::INVALID_ID),
 	m_Team(nullptr),
+	m_Spectating(nullptr),
 	m_TicksUntilNextSave(PLAYER_INVENTORY_SAVE_INTERVAL),
 	m_SkinParts(0)
 {
@@ -658,6 +659,13 @@ void cPlayer::SetCrouch(const bool a_ShouldCrouch)
 	if (a_ShouldCrouch && IsStanding())
 	{
 		m_BodyStance = BodyStanceCrouching(*this);
+
+		// Handle spectator mode detach:
+		if (IsGameModeSpectator())
+		{
+			SpectateEntity(nullptr);
+		}
+
 		cRoot::Get()->GetPluginManager()->CallHookPlayerCrouched(*this);
 	}
 	else if (!a_ShouldCrouch && IsCrouched())
@@ -1414,17 +1422,30 @@ void cPlayer::SendRotation(double a_YawDegrees, double a_PitchDegrees)
 
 
 
-void cPlayer::SpectateEntity(cEntity * a_Target)
+void cPlayer::SpectateEntity(const cEntity * a_Target)
 {
-	if ((a_Target == nullptr) || (static_cast<cEntity *>(this) == a_Target))
+	if (a_Target == this)
 	{
-		GetClientHandle()->SendCameraSetTo(*this);
+		// Canonicalise self-pointers:
-		m_AttachedTo = nullptr;
+		a_Target = nullptr;
+	}
+
+	if (m_Spectating == a_Target)
+	{
+		// Already spectating requested target:
 		return;
 	}
 
-	m_AttachedTo = a_Target;
+	if (a_Target == nullptr)
-	GetClientHandle()->SendCameraSetTo(*m_AttachedTo);
+	{
+		m_ClientHandle->SendCameraSetTo(*this);
+		m_ClientHandle->SendPlayerMoveLook();
+		m_Spectating = nullptr;
+		return;
+	}
+
+	m_Spectating = a_Target;
+	m_ClientHandle->SendCameraSetTo(*a_Target);
 }
 
 
@@ -2522,78 +2543,6 @@ void cPlayer::SetSkinParts(int a_Parts)
 
 
 
-void cPlayer::AttachTo(cEntity * a_AttachTo)
-{
-	// Different attach, if this is a spectator
-	if (IsGameModeSpectator())
-	{
-		SpectateEntity(a_AttachTo);
-		return;
-	}
-
-	Super::AttachTo(a_AttachTo);
-}
-
-
-
-
-
-void cPlayer::Detach()
-{
-	if (m_AttachedTo == nullptr)
-	{
-		// The player is not attached to anything. Bail out.
-		return;
-	}
-
-	// Different detach, if this is a spectator:
-	if (IsGameModeSpectator())
-	{
-		GetClientHandle()->SendCameraSetTo(*this);
-		TeleportToEntity(*m_AttachedTo);
-		m_AttachedTo = nullptr;
-		return;
-	}
-
-	Super::Detach();
-
-	// If they are teleporting, no need to figure out position:
-	if (m_IsTeleporting)
-	{
-		return;
-	}
-
-	int PosX = POSX_TOINT;
-	int PosY = POSY_TOINT;
-	int PosZ = POSZ_TOINT;
-
-	// Search for a position within an area to teleport player after detachment
-	// Position must be solid land with two air blocks above.
-	// If nothing found, player remains where they are
-	for (int x = PosX - 1; x <= (PosX + 1); ++x)
-	{
-		for (int y = PosY; y <= (PosY + 3); ++y)
-		{
-			for (int z = PosZ - 1; z <= (PosZ + 1); ++z)
-			{
-				if (
-					(m_World->GetBlock({ x, y, z }) == E_BLOCK_AIR) &&
-					(m_World->GetBlock({ x, y + 1, z }) == E_BLOCK_AIR) &&
-					cBlockInfo::IsSolid(m_World->GetBlock({ x, y - 1, z }))
-				)
-				{
-					TeleportToCoords(x + 0.5, y, z + 0.5);
-					return;
-				}
-			}
-		}
-	}
-}
-
-
-
-
-
 AString cPlayer::GetUUIDFileName(const cUUID & a_UUID)
 {
 	AString UUID = a_UUID.ToLongString();
@@ -2980,18 +2929,18 @@ float cPlayer::GetEnchantmentBlastKnockbackReduction()
 
 
 
-bool cPlayer::IsInvisible() const
+bool cPlayer::IsCrouched(void) const
 {
-	return !m_IsVisible || Super::IsInvisible();
+	return std::holds_alternative<BodyStanceCrouching>(m_BodyStance);
 }
 
 
 
 
 
-bool cPlayer::IsCrouched(void) const
+bool cPlayer::IsSprinting(void) const
 {
-	return std::holds_alternative<BodyStanceCrouching>(m_BodyStance);
+	return std::holds_alternative<BodyStanceSprinting>(m_BodyStance);
 }
 
 
@@ -3007,9 +2956,9 @@ bool cPlayer::IsElytraFlying(void) const
 
 
 
-bool cPlayer::IsSprinting(void) const
+bool cPlayer::IsInvisible() const
 {
-	return std::holds_alternative<BodyStanceSprinting>(m_BodyStance);
+	return !m_IsVisible || Super::IsInvisible();
 }
 
 
@@ -3067,6 +3016,45 @@ void cPlayer::OnAddToWorld(cWorld & a_World)
 
 
 
+void cPlayer::OnDetach()
+{
+	if (m_IsTeleporting)
+	{
+		// If they are teleporting, no need to figure out position:
+		return;
+	}
+
+	int PosX = POSX_TOINT;
+	int PosY = POSY_TOINT;
+	int PosZ = POSZ_TOINT;
+
+	// Search for a position within an area to teleport player after detachment
+	// Position must be solid land with two air blocks above.
+	// If nothing found, player remains where they are.
+	for (int x = PosX - 1; x <= (PosX + 1); ++x)
+	{
+		for (int y = PosY; y <= (PosY + 3); ++y)
+		{
+			for (int z = PosZ - 1; z <= (PosZ + 1); ++z)
+			{
+				if (
+					(m_World->GetBlock({ x, y, z }) == E_BLOCK_AIR) &&
+					(m_World->GetBlock({ x, y + 1, z }) == E_BLOCK_AIR) &&
+					cBlockInfo::IsSolid(m_World->GetBlock({ x, y - 1, z }))
+				)
+				{
+					TeleportToCoords(x + 0.5, y, z + 0.5);
+					return;
+				}
+			}
+		}
+	}
+}
+
+
+
+
+
 void cPlayer::OnRemoveFromWorld(cWorld & a_World)
 {
 	Super::OnRemoveFromWorld(a_World);
@@ -3174,20 +3162,6 @@ void cPlayer::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk)
 		}
 	}
 
-	// Handle the player detach, when the player is in spectator mode
-	if (
-		(IsGameModeSpectator()) &&
-		(m_AttachedTo != nullptr) &&
-		(
-			(m_AttachedTo->IsDestroyed()) ||  // Watching entity destruction
-			(m_AttachedTo->GetHealth() <= 0) ||  // Watching entity dead
-			(IsCrouched())  // Or the player wants to be detached
-		)
-	)
-	{
-		Detach();
-	}
-
 	if (!a_Chunk.IsValid())
 	{
 		// Players are ticked even if the parent chunk is invalid.
@@ -3217,6 +3191,15 @@ void cPlayer::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk)
 		m_BowCharge += 1;
 	}
 
+	// Handle syncing our position with the entity being spectated:
+	if (IsGameModeSpectator() && (m_Spectating != nullptr))
+	{
+		SetYaw(m_Spectating->GetYaw());
+		SetPitch(m_Spectating->GetPitch());
+		SetRoll(m_Spectating->GetRoll());
+		SetPosition(m_Spectating->GetPosition());
+	}
+
 	if (IsElytraFlying())
 	{
 		// Damage elytra, once per second:

src/Entities/Player.h

@@ -184,7 +184,7 @@ public:
 	void SendRotation(double a_YawDegrees, double a_PitchDegrees);
 
 	/** Spectates the target entity. If a_Target is nullptr or a pointer to self, end spectation. */
-	void SpectateEntity(cEntity * a_Target);
+	void SpectateEntity(const cEntity * a_Target);
 
 	/** Returns the position where projectiles thrown by this player should start, player eye position + adjustment */
 	Vector3d GetThrowStartPos(void) const;
@@ -591,8 +591,6 @@ public:
 	void AddKnownItem(const cItem & a_Item);
 
 	// cEntity overrides:
-	virtual void AttachTo(cEntity * a_AttachTo) override;
-	virtual void Detach(void) override;
 	virtual cItem GetEquippedWeapon(void) const override { return m_Inventory.GetEquippedItem(); }
 	virtual cItem GetEquippedHelmet(void) const override { return m_Inventory.GetEquippedHelmet(); }
 	virtual cItem GetEquippedChestplate(void) const override { return m_Inventory.GetEquippedChestplate(); }
@@ -600,7 +598,6 @@ public:
 	virtual cItem GetEquippedBoots(void) const override { return m_Inventory.GetEquippedBoots(); }
 	virtual cItem GetOffHandEquipedItem(void) const override { return m_Inventory.GetShieldSlot(); }
 	virtual bool IsCrouched(void) const override;
-	virtual bool IsElytraFlying(void) const override;
 	virtual bool IsOnGround(void) const override { return m_bTouchGround; }
 	virtual bool IsSprinting(void) const override;
 
@@ -730,6 +727,9 @@ private:
 
 	cTeam * m_Team;
 
+	/** The entity that this player is spectating, nullptr if none. */
+	const cEntity * m_Spectating;
+
 	StatisticsManager m_Stats;
 
 	/** How long till the player's inventory will be saved
@@ -788,9 +788,11 @@ private:
 	virtual bool DoTakeDamage(TakeDamageInfo & TDI) override;
 	virtual float GetEnchantmentBlastKnockbackReduction() override;
 	virtual void HandlePhysics(std::chrono::milliseconds a_Dt, cChunk &) override { UNUSED(a_Dt); }
+	virtual bool IsElytraFlying(void) const override;
 	virtual bool IsInvisible() const override;
 	virtual bool IsRclking(void) const override { return IsEating() || IsChargingBow(); }
 	virtual void OnAddToWorld(cWorld & a_World) override;
+	virtual void OnDetach() override;
 	virtual void OnRemoveFromWorld(cWorld & a_World) override;
 	virtual void SpawnOn(cClientHandle & a_Client) override;
 	virtual void Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk) override;

src/Mobs/Horse.cpp

@@ -152,7 +152,7 @@ void cHorse::OnRightClicked(cPlayer & a_Player)
 		}
 		else
 		{
-			a_Player.AttachTo(this);
+			a_Player.AttachTo(*this);
 		}
 	}
 	else if (a_Player.GetEquippedItem().IsEmpty())
@@ -177,7 +177,7 @@ void cHorse::OnRightClicked(cPlayer & a_Player)
 			}
 
 			m_TameAttemptTimes++;
-			a_Player.AttachTo(this);
+			a_Player.AttachTo(*this);
 		}
 	}
 	else

src/Mobs/Pig.cpp

@@ -67,8 +67,8 @@ void cPig::OnRightClicked(cPlayer & a_Player)
 			m_Attachee->Detach();
 		}
 
-		// Attach the player to this pig
+		// Attach the player to this pig:
-		a_Player.AttachTo(this);
+		a_Player.AttachTo(*this);
 	}
 	else if (a_Player.GetEquippedItem().m_ItemType == E_ITEM_SADDLE)
 	{

tests/Generating/Stubs.cpp

@@ -696,7 +696,7 @@ void cEntity::BroadcastMovementUpdate(class cClientHandle const * a_ClientHandle
 
 
 
-void cEntity::AttachTo(class cEntity * a_Entity)
+void cEntity::AttachTo(class cEntity & a_Entity)
 {
 }
 
@@ -766,6 +766,15 @@ void cEntity::ResetPosition(class Vector3<double> a_Pos)
 
 
 
+void cEntity::OnDetach()
+{
+
+}
+
+
+
+
+
 cPawn::cPawn(enum cEntity::eEntityType, float a_Width, float a_Height) :
 	cEntity(etMonster, Vector3d(), a_Height, a_Width)
 {