Fixed pointers! Debug printing

I have finally, finally fixed the dangling pointer issue. It turned out that using TSharedPtr and TWeakPtr instead of TWeakObjectPtr was a terrible mistake. Changing everything over to TWeakObjectPtr has fixed the issue. This should also fix the segfault that I've been encountering since UnrealMovementComponent was a thing, which is awesome.

I added the ability to toggle Debug logging for UnrealMovementComponent, which offers more information regarding actor spawning

GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.cpp

@@ -28,6 +28,7 @@ UnrealMovementComponent::UnrealMovementComponent()
     : gv::PooledComponentManager<UnrealMovementComponentData>(100)
 {
 	DebugName = "UnrealMovementComponent";
+	DebugPrint = false;
 }
 
 UnrealMovementComponent::~UnrealMovementComponent()
@@ -64,69 +65,50 @@ void UnrealMovementComponent::Update(float deltaSeconds)
 		bool shouldActorExist = gv::EntityLOD::ShouldRenderForPlayer(worldPosition);
 		bool moveActor = false;
 
-		if (currentComponent->data.SpawnParams.CharacterToSpawn)
-		{
-			struct bullshit
-			{
-				void OnActorDestroyed(gv::Entity ent)
-				{
-				}
-			};
-			std::cout << "Spawning " << &currentComponent->data.SpawnParams.CharacterToSpawn
-			          << " in world " << World << "\n";
-			gv::Position position;
-			bullshit someBullshit;
-			/*TSharedPtr<ACharacter> newLockedCharacter(
-			    AGalavantUnrealMain::CreateActorForEntity<ACharacter>(
-			        World, currentComponent->data.SpawnParams.CharacterToSpawn, 0, position,
-			        std::bind(&bullshit::OnActorDestroyed, &someBullshit,
-			   std::placeholders::_1)));*/
-			ACharacter* newLockedCharacter = AGalavantUnrealMain::CreateActorForEntity<ACharacter>(
-			    World, currentComponent->data.SpawnParams.CharacterToSpawn, 0, position,
-			    std::bind(&bullshit::OnActorDestroyed, &someBullshit, std::placeholders::_1));
-			LOGD << "Spawned character. What the fuck";
-		}
-
-		TSharedPtr<AActor> lockedActor(currentComponent->data.Actor.Pin());
-		TSharedPtr<ACharacter> lockedCharacter(currentComponent->data.Character.Pin());
-
 		// Debug Actor/Entity lifetime
 		if (GEngine)
 		{
 			bool spawnDiscrepancy =
-			    (shouldActorExist && (!lockedActor.IsValid() && !lockedCharacter.IsValid())) ||
-			    (!shouldActorExist && (lockedActor.IsValid() || lockedCharacter.IsValid()));
+			    (shouldActorExist && (!currentComponent->data.Actor.IsValid() &&
+			                          !currentComponent->data.Character.IsValid())) ||
+			    (!shouldActorExist && (currentComponent->data.Actor.IsValid() ||
+			                           currentComponent->data.Character.IsValid()));
 			// Use entity as string key so it'll overwrite
 			GEngine->AddOnScreenDebugMessage(
 			    /*key=*/(uint64)currentComponent->entity, /*timeToDisplay=*/1.5f,
 			    spawnDiscrepancy ? FColor::Red : FColor::Green,
 			    FString::Printf(TEXT("Entity %d Actor: %d Character: %d Should Render: %d"),
-			                    currentComponent->entity, lockedActor.IsValid(),
-			                    lockedCharacter.IsValid(), shouldActorExist));
+			                    currentComponent->entity, currentComponent->data.Actor.IsValid(),
+			                    currentComponent->data.Character.IsValid(), shouldActorExist));
 		}
 
 		SpawnActorIfNecessary(currentComponent);
 
-		if (lockedActor.IsValid())
+		if (currentComponent->data.Actor.IsValid())
 		{
-			trueWorldPosition = lockedActor->GetActorLocation();
+			trueWorldPosition = currentComponent->data.Actor->GetActorLocation();
 			worldPosition = ToPosition(trueWorldPosition);
 		}
-		else if (lockedCharacter.IsValid())
+		else if (currentComponent->data.Character.IsValid())
 		{
-			trueWorldPosition = lockedCharacter->GetActorLocation();
+			trueWorldPosition = currentComponent->data.Character->GetActorLocation();
 			worldPosition = ToPosition(trueWorldPosition);
 		}
 		else
-			currentComponent->data.Actor = currentComponent->data.Character = nullptr;
+		{
+			currentComponent->data.Actor.Reset();
+			currentComponent->data.Character.Reset();
+		}
 
 		// Destroy the actor if we are far away
-		if (!shouldActorExist && (lockedActor.IsValid() || lockedCharacter.IsValid()))
+		if (!shouldActorExist &&
+		    (currentComponent->data.Actor.IsValid() || currentComponent->data.Character.IsValid()))
 		{
 			DestroyActor(currentComponent);
 
-			LOGD << "Entity " << currentComponent->entity
-			     << " destroyed its actor because it shouldn't be rendered to the player";
+			LOGD_IF(DebugPrint)
+			    << "Entity " << currentComponent->entity
+			    << " destroyed its actor because it shouldn't be rendered to the player";
 		}
 		else
 			moveActor = true;
@@ -185,11 +167,11 @@ void UnrealMovementComponent::Update(float deltaSeconds)
 
 			if (moveActor)
 			{
-				if (lockedActor.IsValid())
-					lockedActor->AddActorLocalOffset(deltaVelocity, false, nullptr,
-					                                 ETeleportType::None);
-				else if (lockedCharacter.IsValid())
-					lockedCharacter->AddMovementInput(deltaVelocity, 1.f);
+				if (currentComponent->data.Actor.IsValid())
+					currentComponent->data.Actor->AddActorLocalOffset(deltaVelocity, false, nullptr,
+					                                                  ETeleportType::None);
+				else if (currentComponent->data.Character.IsValid())
+					currentComponent->data.Character->AddMovementInput(deltaVelocity, 1.f);
 			}
 
 			worldPosition += ToPosition(deltaVelocity);
@@ -298,10 +280,7 @@ void UnrealMovementComponent::SpawnActorIfNecessary(
 	if (!World || !component)
 		return;
 
-	TSharedPtr<AActor> lockedActor(component->data.Actor.Pin());
-	TSharedPtr<ACharacter> lockedCharacter(component->data.Character.Pin());
-
-	if (lockedActor.IsValid() || lockedCharacter.IsValid())
+	if (component->data.Actor.IsValid() || component->data.Character.IsValid())
 		return;
 
 	if (gv::EntityLOD::ShouldRenderForPlayer(component->data.WorldPosition))
@@ -332,33 +311,28 @@ void UnrealMovementComponent::SpawnActorIfNecessary(
 
 		if (!hitWorld)
 		{
-			LOGD << "Entity " << component->entity
-			     << " will NOT spawn actor/character because it doesn't have a place to stand";
+			LOGD_IF(DebugPrint)
+			    << "Entity " << component->entity
+			    << " will NOT spawn actor/character because it doesn't have a place to stand";
 			return;
 		}
 
-		LOGD << "Entity " << component->entity
-		     << " spawning actor/character because it should still be rendered";
-
-		TWeakPtr<AActor> newActor;
-		TWeakPtr<ACharacter> newCharacter;
+		LOGD_IF(DebugPrint) << "Entity " << component->entity
+		                    << " spawning actor/character because it should still be rendered";
 
 		if (component->data.SpawnParams.ActorToSpawn)
 		{
-			TSharedPtr<AActor> newLockedActor(ActorEntityManager::CreateActorForEntity<AActor>(
+			component->data.Actor = ActorEntityManager::CreateActorForEntity<AActor>(
 			    World, component->data.SpawnParams.ActorToSpawn, component->entity,
-			    ToPosition(position), std::bind(&UnrealMovementComponent::OnActorDestroyed, this,
-			                                    std::placeholders::_1)));
-			newActor = newLockedActor;
+			    ToPosition(position),
+			    std::bind(&UnrealMovementComponent::OnActorDestroyed, this, std::placeholders::_1));
 		}
 		else if (component->data.SpawnParams.CharacterToSpawn)
 		{
-			TSharedPtr<ACharacter> newLockedCharacter(
-			    ActorEntityManager::CreateActorForEntity<ACharacter>(
-			        World, component->data.SpawnParams.CharacterToSpawn, component->entity,
-			        ToPosition(position), std::bind(&UnrealMovementComponent::OnActorDestroyed,
-			                                        this, std::placeholders::_1)));
-			newCharacter = newLockedCharacter;
+			component->data.Character = ActorEntityManager::CreateActorForEntity<ACharacter>(
+			    World, component->data.SpawnParams.CharacterToSpawn, component->entity,
+			    ToPosition(position),
+			    std::bind(&UnrealMovementComponent::OnActorDestroyed, this, std::placeholders::_1));
 		}
 		else
 		{
@@ -366,27 +340,21 @@ void UnrealMovementComponent::SpawnActorIfNecessary(
 			return;
 		}
 
-		if (!newCharacter.IsValid() && !newActor.IsValid())
+		if (!component->data.Actor.IsValid() && !component->data.Character.IsValid())
 			LOGE << "Unable to spawn entity " << component->entity << "!";
-		else
-		{
-			component->data.Actor = newActor;
-			component->data.Character = newCharacter;
-		}
 	}
 }
 
 void UnrealMovementComponent::DestroyActor(
     gv::PooledComponent<UnrealMovementComponentData>* component)
 {
-	TSharedPtr<AActor> lockedActor(component->data.Actor.Pin());
-	TSharedPtr<ACharacter> lockedCharacter(component->data.Character.Pin());
-	if (lockedActor.IsValid())
-		lockedActor->Destroy();
-	if (lockedCharacter.IsValid())
-		lockedCharacter->Destroy();
-
-	component->data.Actor = component->data.Character = nullptr;
+	if (component->data.Actor.IsValid())
+		component->data.Actor->Destroy();
+	if (component->data.Character.IsValid())
+		component->data.Character->Destroy();
+
+	component->data.Actor.Reset();
+	component->data.Character.Reset();
 }
 
 // @Callback: TrackActorLifetimeCallback
@@ -403,9 +371,10 @@ void UnrealMovementComponent::OnActorDestroyed(gv::Entity entity)
 		{
 			if (gv::EntityLOD::ShouldRenderForPlayer(currentComponent->data.WorldPosition))
 			{
-				LOGD << "Entity " << currentComponent->entity << " had its actor destroyed "
-				                                                 "(possibly against its will); it "
-				                                                 "is in player view ";
+				LOGD_IF(DebugPrint) << "Entity " << currentComponent->entity
+				                    << " had its actor destroyed "
+				                       "(possibly against its will); it "
+				                       "is in player view ";
 			}
 			currentComponent->data.Actor = nullptr;
 			currentComponent->data.Character = nullptr;

GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.hpp

@@ -26,8 +26,8 @@ struct UnrealMovementComponentData
 {
 	MovementComponentActorSpawnParams SpawnParams;
 
-	TWeakPtr<ACharacter> Character = nullptr;
-	TWeakPtr<AActor> Actor = nullptr;
+	TWeakObjectPtr<ACharacter> Character = nullptr;
+	TWeakObjectPtr<AActor> Actor = nullptr;
 
 	gv::Position WorldPosition;
 
@@ -62,6 +62,8 @@ protected:
 	                                             UnrealMovementComponentRefList& components);
 
 public:
+	bool DebugPrint;
+
 	typedef std::vector<gv::PooledComponent<UnrealMovementComponentData>>
 	    UnrealMovementComponentList;
 

GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.cpp

@@ -349,16 +349,13 @@ void AGalavantUnrealMain::InitializeGalavant()
 		gv::g_EntityComponentManager.AddComponentManager(&g_UnrealMovementComponentManager);
 
 		g_UnrealMovementComponentManager.Initialize(GetWorld(), &TaskEventCallbacks);
+		// g_UnrealMovementComponentManager.DebugPrint = true;
 
-		{
-			gv::g_PlanComponentManager.Initialize(&WorldStateManager, &TaskEventCallbacks);
-			// gv::g_PlanComponentManager.DebugPrint = true;
-		}
+		gv::g_PlanComponentManager.Initialize(&WorldStateManager, &TaskEventCallbacks);
+		// gv::g_PlanComponentManager.DebugPrint = true;
 
-		{
-			gv::g_AgentComponentManager.Initialize(&gv::g_PlanComponentManager);
-			gv::g_AgentComponentManager.DebugPrint = true;
-		}
+		gv::g_AgentComponentManager.Initialize(&gv::g_PlanComponentManager);
+		gv::g_AgentComponentManager.DebugPrint = true;
 
 		gv::g_CombatComponentManager.Initialize(&CombatFxHandler);
 	}
@@ -433,34 +430,6 @@ void AGalavantUnrealMain::Tick(float DeltaTime)
 
 	g_UnrealMovementComponentManager.Initialize(world, &TaskEventCallbacks);
 
-	static bool hasTicked = false;
-	if (!hasTicked)
-	{
-		struct bullshit
-		{
-			void OnActorDestroyed(gv::Entity ent)
-			{
-			}
-		};
-		gv::Position position;
-		bullshit someBullshit;
-		std::cout << "Spawning " << &DefaultAgentCharacter << " in world " << world <<  "\n";
-		TSharedPtr<ACharacter> newLockedCharacter(
-		    ActorEntityManager::CreateActorForEntity<ACharacter>(
-		        world, DefaultAgentCharacter, 0, position,
-		        std::bind(&bullshit::OnActorDestroyed, &someBullshit,
-		                  std::placeholders::_1)));
-
-		LOGD << "Spawned character. What the fuck";
-
-		TSharedPtr<ACharacter> newLockedCharacter2(
-		    ActorEntityManager::CreateActorForEntity<ACharacter>(
-		        world, DefaultAgentCharacter, 0, position,
-		        std::bind(&bullshit::OnActorDestroyed, &someBullshit,
-		                  std::placeholders::_1)));
-		LOGD << "Spawned another character. What the fuck";
-	}
-
 	// Destroy entities now because Unreal might have destroyed actors, so we don't want our code to
 	// break not knowing that
 	gv::g_EntityComponentManager.DestroyEntitiesPendingDestruction();

GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.h

@@ -10,8 +10,6 @@
 #include "game/agent/htnTasks/InteractTasks.hpp"
 #include "util/CallbackContainer.hpp"
 
-#include "ActorEntityManagement.h"
-
 #include "GalaEntityComponents/UnrealMovementComponent.hpp"
 #include "CombatFx.hpp"
 
@@ -86,14 +84,6 @@ public:
 	virtual void InitGame(const FString& MapName, const FString& Options,
 	                      FString& ErrorMessage) override;
 
-	template <class T>
-	static T* CreateActorForEntity(UWorld* world, TSubclassOf<T> actorType, gv::Entity entity,
-	                               const gv::Position& position,
-	                               ActorEntityManager::TrackActorLifetimeCallback callback)
-	{
-		return ActorEntityManager::CreateActorForEntity<T>(world, actorType, 0, position, callback);
-	}
-
 private:
 	void InitializeEntityTests();
 	void InitializeProceduralWorld();