From 23c33f1b4b9c87c13488a844fc74b6f32dccdf57 Mon Sep 17 00:00:00 2001
From: David Gilbert <gilbert@vr.rwth-aachen.de>
Date: Tue, 6 Aug 2024 14:27:59 +0200
Subject: [PATCH] feature(cave, replication): Makes cluster join logic more
 robust

---
 .../Private/Core/RWTHVRGameModeBase.cpp       | 38 +++++++---------
 .../Pawn/ClusterRepresentationActor.cpp       | 45 +++++++++++++------
 .../Public/Core/RWTHVRPlayerState.h           | 18 ++++++--
 .../Public/Pawn/ClusterRepresentationActor.h  | 10 +++--
 4 files changed, 68 insertions(+), 43 deletions(-)

diff --git a/Source/RWTHVRToolkit/Private/Core/RWTHVRGameModeBase.cpp b/Source/RWTHVRToolkit/Private/Core/RWTHVRGameModeBase.cpp
index c064de87..172f00f8 100644
--- a/Source/RWTHVRToolkit/Private/Core/RWTHVRGameModeBase.cpp
+++ b/Source/RWTHVRToolkit/Private/Core/RWTHVRGameModeBase.cpp
@@ -28,9 +28,7 @@ FString ARWTHVRGameModeBase::InitNewPlayer(APlayerController* NewPlayerControlle
 	// Check if we're using our custom PlayerState so that we can save the player type there.
 	// If not, just ingore all related args.
 	ARWTHVRPlayerState* State = Cast<ARWTHVRPlayerState>(NewPlayerController->PlayerState);
-
-	bool bIsClusterNode = false;
-
+	
 	if (State != nullptr)
 	{
 		int32 ClusterId = -1;
@@ -51,20 +49,25 @@ FString ARWTHVRGameModeBase::InitNewPlayer(APlayerController* NewPlayerControlle
 			const EPlayerType Type =
 				NodeName == PrimaryNodeId ? EPlayerType::nDisplayPrimary : EPlayerType::nDisplaySecondary;
 			State->RequestSetPlayerType(Type);
-			bIsClusterNode = true;
 		}
 		else if (GetNetMode() == NM_Standalone && URWTHVRUtilities::IsRoomMountedMode())
 		{
 			ClusterId = 0;
-			bIsClusterNode = true;
 		}
+		State->SetCorrespondingClusterId(ClusterId);
+	}
 
+	return Super::InitNewPlayer(NewPlayerController, UniqueId, Options, Portal);
+}
+
+void ARWTHVRGameModeBase::PostLogin(APlayerController* NewPlayer)
+{
+	if (ARWTHVRPlayerState* State = Cast<ARWTHVRPlayerState>(NewPlayer->PlayerState); State != nullptr)
+	{		
 		// If we're in none-standalone netmode, this is only executed on the server, as the GM only exists there.
 		// On standalone, this is executed on every node.
-
-		State->SetCorrespondingClusterId(ClusterId);
-
-		if (bIsClusterNode)
+		int32 ClusterId = State->GetCorrespondingClusterId();
+		if (ClusterId >= 0) // we're either standalone (0) or in an acutal cluster
 		{
 			AClusterRepresentationActor** ClusterRepresentationPtr = ConnectedClusters.Find(ClusterId);
 			AClusterRepresentationActor* ClusterRepresentation;
@@ -75,7 +78,6 @@ FString ARWTHVRGameModeBase::InitNewPlayer(APlayerController* NewPlayerControlle
 				SpawnParameters.Name = FName(*FString::Printf(TEXT("ClusterRepresentation_%d"), ClusterId));
 				SpawnParameters.NameMode = FActorSpawnParameters::ESpawnActorNameMode::Requested;
 				ClusterRepresentation = GetWorld()->SpawnActor<AClusterRepresentationActor>(SpawnParameters);
-				ClusterRepresentation->ClusterId = ClusterId;
 				UE_LOGFMT(Toolkit, Display,
 						  "ARWTHVRGameModeBase: Spawned ClusterRepresentationActor {Name} for Cluster {Id}",
 						  ClusterRepresentation->GetName(), ClusterId);
@@ -90,25 +92,17 @@ FString ARWTHVRGameModeBase::InitNewPlayer(APlayerController* NewPlayerControlle
 					  *ClusterRepresentation->GetName(), ClusterId);
 
 			// Double check for sanity
-			check(ClusterId == ClusterRepresentation->ClusterId);
-
+			check(ClusterRepresentation != nullptr);
+			
 			State->SetCorrespondingClusterActor(ClusterRepresentation);
 
 			if (State->GetPlayerType() == EPlayerType::nDisplayPrimary)
 			{
 				// We're the owner of the actor!
-				ClusterRepresentation->SetOwner(NewPlayerController);
+				ClusterRepresentation->SetOwner(NewPlayer);
 			}
 		}
-	}
-
-	return Super::InitNewPlayer(NewPlayerController, UniqueId, Options, Portal);
-}
-
-void ARWTHVRGameModeBase::PostLogin(APlayerController* NewPlayer)
-{
-	if (const ARWTHVRPlayerState* State = Cast<ARWTHVRPlayerState>(NewPlayer->PlayerState); State != nullptr)
-	{
+		
 		// Do we already have an auto-possessing pawn possessed?
 		if (NewPlayer->GetPawn() && NewPlayer->GetPawn()->IsValidLowLevelFast())
 		{
diff --git a/Source/RWTHVRToolkit/Private/Pawn/ClusterRepresentationActor.cpp b/Source/RWTHVRToolkit/Private/Pawn/ClusterRepresentationActor.cpp
index f3ec515b..57c16ee8 100644
--- a/Source/RWTHVRToolkit/Private/Pawn/ClusterRepresentationActor.cpp
+++ b/Source/RWTHVRToolkit/Private/Pawn/ClusterRepresentationActor.cpp
@@ -26,7 +26,13 @@ AClusterRepresentationActor::AClusterRepresentationActor()
 void AClusterRepresentationActor::BeginPlay()
 {
 	Super::BeginPlay();
+	// will fail if we're in replicated mode and PlayerState has not yet replicated fully
+	// Therefore we also execute this
+	AttachDCRAIfRequired();
+}
 
+void AClusterRepresentationActor::AttachDCRAIfRequired(const ARWTHVRPlayerState* OptionalPlayerState)
+{
 	// We need to identify the correct ClusterRepresentationActor to do the attachment.
 	// 1. Either we are the local net owner -> Primary Node Pawn
 	// This is hard to do as things might not be synchronized yet.
@@ -38,30 +44,43 @@ void AClusterRepresentationActor::BeginPlay()
 	if (!URWTHVRUtilities::IsRoomMountedMode())
 		return;
 
+	if (bIsAttached)
+	{
+		UE_LOGFMT(Toolkit, Display,
+				  "AClusterRepresentationActor::AttachDCRAIfRequired: Already attached, skipping repeated attachment.");
+		return;
+	}
+	
+	UE_LOGFMT(Toolkit, Display, "AClusterRepresentationActor::AttachDCRAIfRequired: Starting DCRA Attachment process.");
+
 	// This should give us the first local player controller
 	const auto* PlayerController = UGameplayStatics::GetPlayerController(GetWorld(), 0);
-	
+
 	// Only run this for the local controller - redundant, but double check
 	if (!PlayerController || !PlayerController->IsLocalController())
+	{
+		UE_LOGFMT(
+			Toolkit, Warning,
+			"AClusterRepresentationActor::AttachDCRAIfRequired: PlayerController not valid or not locally controlled.");
 		return;
-
-	const auto* PlayerState = PlayerController->GetPlayerState<ARWTHVRPlayerState>();
+	}
+	const auto* PlayerState =
+		OptionalPlayerState != nullptr ? OptionalPlayerState : PlayerController->GetPlayerState<ARWTHVRPlayerState>();
 	if (!PlayerState)
 	{
-		UE_LOGFMT(
-			Toolkit, Error,
-			"AClusterRepresentationActor::BeginPlay: PlayerState is not valid or not of type ARWTHVRPlayerState.");
+		UE_LOGFMT(Toolkit, Error,
+				  "AClusterRepresentationActor::AttachDCRAIfRequired: PlayerState is not valid or not of type "
+				  "ARWTHVRPlayerState.");
 		return;
 	}
 
 	// The local player this is executed on corresponds to this actor
 	if (PlayerState->GetCorrespondingClusterActor() == this)
 	{
-		check(PlayerState->GetCorrespondingClusterId() == ClusterId);
-		UE_LOGFMT(Toolkit, Display, "AClusterRepresentationActor::BeginPlay: Attaching DCRA to {Name} with Id: {Id}.",
-				  GetName(), ClusterId);
+		UE_LOGFMT(Toolkit, Display, "AClusterRepresentationActor::AttachDCRAIfRequired: Attaching DCRA to {Name}.",
+				  GetName());
 
-		AttachDCRA();
+		bIsAttached = AttachDCRA();
 	}
 }
 
@@ -76,7 +95,7 @@ bool AClusterRepresentationActor::AttachDCRA()
 
 	if (URWTHVRUtilities::IsRoomMountedMode())
 	{
-		UE_LOGFMT(Toolkit, Display, "{Name}: Trying to attach DCRA for ClusterId {Id}", GetName(), ClusterId);
+		UE_LOGFMT(Toolkit, Display, "{Name}: Trying to attach DCRA", GetName());
 		auto DCRA = IDisplayCluster::Get().GetGameMgr()->GetRootActor();
 
 		if (!IsValid(DCRA))
@@ -94,8 +113,8 @@ bool AClusterRepresentationActor::AttachDCRA()
 		{
 			if (!DCRA->IsPrimaryRootActor())
 			{
-				UE_LOGFMT(Toolkit, Error, "Found DCRA {Name} is not the primary DCRA of Cluster with Id {Id}!",
-						  DCRA->GetName(), ClusterId);
+				UE_LOGFMT(Toolkit, Error, "Found DCRA {Name} is not the primary DCRA of Cluster with Id!",
+						  DCRA->GetName());
 				return false;
 			}
 		}
diff --git a/Source/RWTHVRToolkit/Public/Core/RWTHVRPlayerState.h b/Source/RWTHVRToolkit/Public/Core/RWTHVRPlayerState.h
index c5e8f46f..f5fcb06d 100644
--- a/Source/RWTHVRToolkit/Public/Core/RWTHVRPlayerState.h
+++ b/Source/RWTHVRToolkit/Public/Core/RWTHVRPlayerState.h
@@ -5,6 +5,9 @@
 #include "CoreMinimal.h"
 #include "PlayerType.h"
 #include "GameFramework/PlayerState.h"
+#include "Logging/StructuredLog.h"
+#include "Pawn/ClusterRepresentationActor.h"
+#include "Utility/RWTHVRUtilities.h"
 #include "RWTHVRPlayerState.generated.h"
 
 
@@ -28,10 +31,17 @@ private:
 	UPROPERTY(Replicated, Category = PlayerState, BlueprintGetter = GetCorrespondingClusterId, meta = (AllowPrivateAccess))
 	int32 CorrespondingClusterId = -1;
 
-	/** Replicated cluster actor for this player. Is nullptr in case player is not a cluster*/
-	UPROPERTY(Replicated, Category = PlayerState, BlueprintGetter = GetCorrespondingClusterActor,
-			  meta = (AllowPrivateAccess))
-	AClusterRepresentationActor* CorrespondingClusterActor = nullptr;
+	/** Replicated cluster actor for this player. Is nullptr in case player is not a cluster.
+	 * As this is not guaranteed to be valid on BeginPlay, we need to do a callback to the CorrespondingClusterActor here...
+	 */
+	UPROPERTY(ReplicatedUsing = OnRep_CorrespondingClusterActor)
+	TObjectPtr<AClusterRepresentationActor> CorrespondingClusterActor;
+
+	UFUNCTION()
+	virtual void OnRep_CorrespondingClusterActor()
+	{
+		CorrespondingClusterActor->AttachDCRAIfRequired(this);
+	}
 	
 	UFUNCTION(Reliable, Server)
 	void ServerSetPlayerTypeRpc(EPlayerType NewPlayerType);
diff --git a/Source/RWTHVRToolkit/Public/Pawn/ClusterRepresentationActor.h b/Source/RWTHVRToolkit/Public/Pawn/ClusterRepresentationActor.h
index 1db7a420..c3c041f8 100644
--- a/Source/RWTHVRToolkit/Public/Pawn/ClusterRepresentationActor.h
+++ b/Source/RWTHVRToolkit/Public/Pawn/ClusterRepresentationActor.h
@@ -7,7 +7,9 @@
 #include "ClusterRepresentationActor.generated.h"
 
 
+class ARWTHVRPlayerState;
 class ADisplayClusterRootActor;
+
 UCLASS()
 class RWTHVRTOOLKIT_API AClusterRepresentationActor : public AActor
 {
@@ -17,13 +19,13 @@ public:
 	// Sets default values for this actor's properties
 	AClusterRepresentationActor();
 
-	UPROPERTY(VisibleAnywhere, BlueprintReadOnly)
-	int32 ClusterId = -1;
-
 	virtual void BeginPlay() override;
-	
+
+	void AttachDCRAIfRequired(const ARWTHVRPlayerState* OptionalPlayerState = nullptr);
 
 private:
 	bool AttachDCRA();
 	ADisplayClusterRootActor* SpawnDCRA();
+
+	bool bIsAttached = false;
 };
-- 
GitLab