First pass of pointer conversion; still asserting

I learned from [this page](https://wiki.unrealengine.com/How_To_Prevent_Crashes_Due_To_Dangling_Actor_Pointers) that I was segfaulting in UnrealMovementComponent::Update() because storing raw pointers to UObjects is treacherous. I converted everything in UnrealMovementComponentManager to use Shared and Weak pointers for Actors and Characters. 

This seemed all fine and dandy, but now I'm running into an infuriating ensure assert where ACharacter can't be constructed because it's already registered. What is perplexing about this is that I can construct the AgentCharacters just fine in GalavantUnrealMain::Tick(), but as soon as I do it in UnrealMovementComponent::Update() with the same exact parameters everything goes to shit.

I'm at a loss as to what is broken as it seems impossible that my changes could've broken things the way they've been broken.

GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.cpp

@@ -67,7 +67,7 @@ void UnrealActorEntityOnDestroy(AActor* actor, int entity)
 
 	findIt->second.IsBeingDestroyed = true;
 
-	findIt->second.OnDestroyCallback(actor);
+	findIt->second.OnDestroyCallback((gv::Entity)entity);
 
 	// The actor has been removed, so there's no reason to track its lifetime
 	// Note that whatever happens in callbackOnDestroy() could spawn and track an Actor which has

GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.h

@@ -8,6 +8,8 @@
 
 #include <functional>
 
+#include <iostream>
+
 // This module tracks Actor-Entity pairs and ensures that if an Actor was destroyed the components
 // for the associated Entity will know about it. This is to fix the problem where Unreal can destroy
 // an Actor without our Entity system knowing about it
@@ -16,7 +18,7 @@ namespace ActorEntityManager
 void Clear();
 
 // Advanced setup: if actor is destroyed, call callback
-typedef std::function<void(const AActor*)> TrackActorLifetimeCallback;
+typedef std::function<void(gv::Entity)> TrackActorLifetimeCallback;
 void TrackActorLifetime(AActor* actor, TrackActorLifetimeCallback callback);
 
 bool IsActorActive(AActor* actor);
@@ -30,6 +32,10 @@ T* CreateActorForEntity(UWorld* world, TSubclassOf<T> actorType, gv::Entity enti
 	if (!world)
 		return nullptr;
 
+	std::cout << "UWorld* world " << world << " TSubclassOf<T> actorType " << actorType.Get()
+	          << " gv::Entity entity " << entity << " const gv::Position& position " << position.X
+	          << ", " << position.Y << ", " << position.Z << "\n";
+
 	FActorSpawnParameters spawnParams;
 	FVector positionVector(ToFVector(position));
 	FRotator defaultRotation(0.f, 0.f, 0.f);

GalavantUnreal/Source/GalavantUnreal/Characters/AgentCharacter.h

@@ -8,7 +8,7 @@
 
 #include "AgentCharacter.generated.h"
 
-UCLASS()
+UCLASS(config = Game)
 class GALAVANTUNREAL_API AAgentCharacter : public ACharacter
 {
 	GENERATED_BODY()

GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.cpp

@@ -18,6 +18,10 @@
 
 #include <functional>
 
+#include <iostream>
+
+#include "GalavantUnrealMain.h"
+
 UnrealMovementComponent g_UnrealMovementComponentManager;
 
 UnrealMovementComponent::UnrealMovementComponent()
@@ -60,65 +64,64 @@ 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 &&
-			     (!currentComponent->data.Actor && !currentComponent->data.Character)) ||
-			    (!shouldActorExist &&
-			     (currentComponent->data.Actor || currentComponent->data.Character));
+			    (shouldActorExist && (!lockedActor.IsValid() && !lockedCharacter.IsValid())) ||
+			    (!shouldActorExist && (lockedActor.IsValid() || lockedCharacter.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, currentComponent->data.Actor,
-			                    currentComponent->data.Character, shouldActorExist));
+			                    currentComponent->entity, lockedActor.IsValid(),
+			                    lockedCharacter.IsValid(), shouldActorExist));
 		}
 
 		SpawnActorIfNecessary(currentComponent);
 
-		USceneComponent* sceneComponent = nullptr;
-		if (currentComponent->data.Actor)
+		if (lockedActor.IsValid())
 		{
-			if (!ActorEntityManager::IsActorActive(currentComponent->data.Actor))
-			{
-				currentComponent->data.Actor = nullptr;
-				LOGW << "Entity " << currentComponent->entity << " had pointer to inactive actor!";
-			}
-			else
-				sceneComponent = currentComponent->data.Actor->GetRootComponent();
+			trueWorldPosition = lockedActor->GetActorLocation();
+			worldPosition = ToPosition(trueWorldPosition);
 		}
-		else if (currentComponent->data.Character)
+		else if (lockedCharacter.IsValid())
 		{
-			if (!ActorEntityManager::IsActorActive(currentComponent->data.Character))
-			{
-				currentComponent->data.Character = nullptr;
-				LOGW << "Entity " << currentComponent->entity << " had pointer to inactive actor!";
-			}
-			else
-				sceneComponent = currentComponent->data.Character->GetRootComponent();
+			trueWorldPosition = lockedCharacter->GetActorLocation();
+			worldPosition = ToPosition(trueWorldPosition);
 		}
+		else
+			currentComponent->data.Actor = currentComponent->data.Character = nullptr;
 
-		if (!sceneComponent && shouldActorExist)
-		{
-			LOGD << "Entity " << currentComponent->entity << " detected with no SceneComponent; "
-			                                                 "was the actor destroyed? It should "
-			                                                 "be respawned later...";
-
-			// Nothing in this block below makes sense anymore
-			// If the actor/character has been destroyed for some reason, make sure we reset these
-			// so it'll be spawned. All actors/characters should have scene components
-			/*currentComponent->data.Actor = nullptr;
-			currentComponent->data.Character = nullptr;*/
-			// Don't destroy an entity just because it has lost its actor. Why was this code here?
-			// entitiesToUnsubscribe.push_back(currentComponent->entity);
-			// continue;
-		}
 		// Destroy the actor if we are far away
-		else if (!shouldActorExist &&
-		         (currentComponent->data.Actor || currentComponent->data.Character))
+		if (!shouldActorExist && (lockedActor.IsValid() || lockedCharacter.IsValid()))
 		{
 			DestroyActor(currentComponent);
 
@@ -128,18 +131,7 @@ void UnrealMovementComponent::Update(float deltaSeconds)
 		else
 			moveActor = true;
 
-		// World position is always superceded by the actual Actor position
-		if (moveActor && sceneComponent)
-		{
-			// TODO: This motherfucker is still crashing
-			/*LOGD << "Entity " << currentComponent->entity << " actor "
-			     << &currentComponent->data.Actor << " Character "
-			     << &currentComponent->data.Character;*/
-			trueWorldPosition = sceneComponent->GetComponentLocation();
-			worldPosition = ToPosition(trueWorldPosition);
-			// LOGD << "Done retrieving position";
-		}
-		else
+		if (!moveActor)
 			trueWorldPosition = ToFVector(worldPosition);
 
 		// Decide where we're going to go
@@ -193,11 +185,11 @@ void UnrealMovementComponent::Update(float deltaSeconds)
 
 			if (moveActor)
 			{
-				if (currentComponent->data.Actor)
-					sceneComponent->AddLocalOffset(deltaVelocity, false, nullptr,
-					                               ETeleportType::None);
-				else if (currentComponent->data.Character)
-					currentComponent->data.Character->AddMovementInput(deltaVelocity, 1.f);
+				if (lockedActor.IsValid())
+					lockedActor->AddActorLocalOffset(deltaVelocity, false, nullptr,
+					                                 ETeleportType::None);
+				else if (lockedCharacter.IsValid())
+					lockedCharacter->AddMovementInput(deltaVelocity, 1.f);
 			}
 
 			worldPosition += ToPosition(deltaVelocity);
@@ -306,14 +298,14 @@ void UnrealMovementComponent::SpawnActorIfNecessary(
 	if (!World || !component)
 		return;
 
-	if (component->data.Actor || component->data.Character)
+	TSharedPtr<AActor> lockedActor(component->data.Actor.Pin());
+	TSharedPtr<ACharacter> lockedCharacter(component->data.Character.Pin());
+
+	if (lockedActor.IsValid() || lockedCharacter.IsValid())
 		return;
 
 	if (gv::EntityLOD::ShouldRenderForPlayer(component->data.WorldPosition))
 	{
-		AActor* newActor = nullptr;
-		ACharacter* newCharacter = nullptr;
-
 		// TODO: Store rotation
 		// FRotator defaultRotation(0.f, 0.f, 0.f);
 		FVector position(ToFVector(component->data.WorldPosition));
@@ -348,19 +340,25 @@ void UnrealMovementComponent::SpawnActorIfNecessary(
 		LOGD << "Entity " << component->entity
 		     << " spawning actor/character because it should still be rendered";
 
+		TWeakPtr<AActor> newActor;
+		TWeakPtr<ACharacter> newCharacter;
+
 		if (component->data.SpawnParams.ActorToSpawn)
 		{
-			newActor = ActorEntityManager::CreateActorForEntity<AActor>(
+			TSharedPtr<AActor> newLockedActor(ActorEntityManager::CreateActorForEntity<AActor>(
 			    World, component->data.SpawnParams.ActorToSpawn, component->entity,
-			    ToPosition(position),
-			    std::bind(&UnrealMovementComponent::OnActorDestroyed, this, std::placeholders::_1));
+			    ToPosition(position), std::bind(&UnrealMovementComponent::OnActorDestroyed, this,
+			                                    std::placeholders::_1)));
+			newActor = newLockedActor;
 		}
 		else if (component->data.SpawnParams.CharacterToSpawn)
 		{
-			newCharacter = ActorEntityManager::CreateActorForEntity<ACharacter>(
-			    World, component->data.SpawnParams.CharacterToSpawn, component->entity,
-			    ToPosition(position),
-			    std::bind(&UnrealMovementComponent::OnActorDestroyed, this, std::placeholders::_1));
+			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;
 		}
 		else
 		{
@@ -368,15 +366,12 @@ void UnrealMovementComponent::SpawnActorIfNecessary(
 			return;
 		}
 
-		if (!newCharacter && !newActor)
+		if (!newCharacter.IsValid() && !newActor.IsValid())
 			LOGE << "Unable to spawn entity " << component->entity << "!";
 		else
 		{
 			component->data.Actor = newActor;
 			component->data.Character = newCharacter;
-
-			if (newActor)
-				newActor->Entity = component->entity;
 		}
 	}
 }
@@ -384,16 +379,18 @@ void UnrealMovementComponent::SpawnActorIfNecessary(
 void UnrealMovementComponent::DestroyActor(
     gv::PooledComponent<UnrealMovementComponentData>* component)
 {
-	if (component->data.Actor)
-		component->data.Actor->Destroy();
-	if (component->data.Character)
-		component->data.Character->Destroy();
+	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;
 }
 
 // @Callback: TrackActorLifetimeCallback
-void UnrealMovementComponent::OnActorDestroyed(const AActor* actor)
+void UnrealMovementComponent::OnActorDestroyed(gv::Entity entity)
 {
 	gv::PooledComponentManager<UnrealMovementComponentData>::FragmentedPoolIterator it =
 	    gv::PooledComponentManager<UnrealMovementComponentData>::NULL_POOL_ITERATOR;
@@ -402,12 +399,13 @@ void UnrealMovementComponent::OnActorDestroyed(const AActor* actor)
 	     it != gv::PooledComponentManager<UnrealMovementComponentData>::NULL_POOL_ITERATOR;
 	     currentComponent = GetNextActivePooledComponent(it))
 	{
-		if (currentComponent->data.Actor == actor || currentComponent->data.Character == actor)
+		if (currentComponent->entity == entity)
 		{
 			if (gv::EntityLOD::ShouldRenderForPlayer(currentComponent->data.WorldPosition))
 			{
-				LOGD << "Entity " << currentComponent->entity << " had its actor " << actor
-				     << " destroyed (possibly against its will); it is in player view";
+				LOGD << "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;
 
-	ACharacter* Character = nullptr;
-	AActor* Actor = nullptr;
+	TWeakPtr<ACharacter> Character = nullptr;
+	TWeakPtr<AActor> Actor = nullptr;
 
 	gv::Position WorldPosition;
 
@@ -79,7 +79,7 @@ public:
 	// TODO: This should return whether it was actually successful (i.e. the entity exists)
 	virtual void PathEntitiesTo(const gv::EntityList& entities, const gv::PositionList& positions);
 
-	void OnActorDestroyed(const AActor* actor);
+	void OnActorDestroyed(gv::Entity entity);
 };
 
 extern UnrealMovementComponent g_UnrealMovementComponentManager;

GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.cpp

@@ -23,6 +23,8 @@
 #include "game/EntityLevelOfDetail.hpp"
 #include "util/StringHashing.hpp"
 
+#include <iostream>
+
 static gv::Logging::Logger s_UnrealLogger(gv::Logging::Severity::debug, &UnrealLogOutput);
 
 // Sets default values
@@ -350,7 +352,7 @@ void AGalavantUnrealMain::InitializeGalavant()
 
 		{
 			gv::g_PlanComponentManager.Initialize(&WorldStateManager, &TaskEventCallbacks);
-			//gv::g_PlanComponentManager.DebugPrint = true;
+			// gv::g_PlanComponentManager.DebugPrint = true;
 		}
 
 		{
@@ -429,6 +431,36 @@ void AGalavantUnrealMain::Tick(float DeltaTime)
 
 	UWorld* world = GetWorld();
 
+	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,6 +10,8 @@
 #include "game/agent/htnTasks/InteractTasks.hpp"
 #include "util/CallbackContainer.hpp"
 
+#include "ActorEntityManagement.h"
+
 #include "GalaEntityComponents/UnrealMovementComponent.hpp"
 #include "CombatFx.hpp"
 
@@ -84,6 +86,14 @@ 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();