From 19f2335035c9bb9c97807065bd8c8080ca21cb31 Mon Sep 17 00:00:00 2001 From: Macoy Madson Date: Sat, 7 Oct 2017 21:17:08 -0700 Subject: [PATCH] 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/ActorEntityManagement.cpp | 2 +- .../GalavantUnreal/ActorEntityManagement.h | 8 +- .../Characters/AgentCharacter.h | 2 +- .../UnrealMovementComponent.cpp | 158 +++++++++--------- .../UnrealMovementComponent.hpp | 6 +- .../GalavantUnreal/GalavantUnrealMain.cpp | 34 +++- .../GalavantUnreal/GalavantUnrealMain.h | 10 ++ 7 files changed, 133 insertions(+), 87 deletions(-) diff --git a/GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.cpp b/GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.cpp index 11778ce..2aeb32e 100644 --- a/GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.cpp +++ b/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 diff --git a/GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.h b/GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.h index 9572b68..9ea90b5 100644 --- a/GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.h +++ b/GalavantUnreal/Source/GalavantUnreal/ActorEntityManagement.h @@ -8,6 +8,8 @@ #include +#include + // 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 TrackActorLifetimeCallback; +typedef std::function TrackActorLifetimeCallback; void TrackActorLifetime(AActor* actor, TrackActorLifetimeCallback callback); bool IsActorActive(AActor* actor); @@ -30,6 +32,10 @@ T* CreateActorForEntity(UWorld* world, TSubclassOf actorType, gv::Entity enti if (!world) return nullptr; + std::cout << "UWorld* world " << world << " TSubclassOf 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); diff --git a/GalavantUnreal/Source/GalavantUnreal/Characters/AgentCharacter.h b/GalavantUnreal/Source/GalavantUnreal/Characters/AgentCharacter.h index e829344..2bd4e4b 100644 --- a/GalavantUnreal/Source/GalavantUnreal/Characters/AgentCharacter.h +++ b/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() diff --git a/GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.cpp b/GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.cpp index 69910bc..3a80baf 100644 --- a/GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.cpp +++ b/GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.cpp @@ -18,6 +18,10 @@ #include +#include + +#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 " << ¤tComponent->data.SpawnParams.CharacterToSpawn + << " in world " << World << "\n"; + gv::Position position; + bullshit someBullshit; + /*TSharedPtr newLockedCharacter( + AGalavantUnrealMain::CreateActorForEntity( + World, currentComponent->data.SpawnParams.CharacterToSpawn, 0, position, + std::bind(&bullshit::OnActorDestroyed, &someBullshit, + std::placeholders::_1)));*/ + ACharacter* newLockedCharacter = AGalavantUnrealMain::CreateActorForEntity( + World, currentComponent->data.SpawnParams.CharacterToSpawn, 0, position, + std::bind(&bullshit::OnActorDestroyed, &someBullshit, std::placeholders::_1)); + LOGD << "Spawned character. What the fuck"; + } + + TSharedPtr lockedActor(currentComponent->data.Actor.Pin()); + TSharedPtr 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 " - << ¤tComponent->data.Actor << " Character " - << ¤tComponent->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 lockedActor(component->data.Actor.Pin()); + TSharedPtr 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 newActor; + TWeakPtr newCharacter; + if (component->data.SpawnParams.ActorToSpawn) { - newActor = ActorEntityManager::CreateActorForEntity( + TSharedPtr newLockedActor(ActorEntityManager::CreateActorForEntity( 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( - World, component->data.SpawnParams.CharacterToSpawn, component->entity, - ToPosition(position), - std::bind(&UnrealMovementComponent::OnActorDestroyed, this, std::placeholders::_1)); + TSharedPtr newLockedCharacter( + ActorEntityManager::CreateActorForEntity( + 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* component) { - if (component->data.Actor) - component->data.Actor->Destroy(); - if (component->data.Character) - component->data.Character->Destroy(); + TSharedPtr lockedActor(component->data.Actor.Pin()); + TSharedPtr 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::FragmentedPoolIterator it = gv::PooledComponentManager::NULL_POOL_ITERATOR; @@ -402,12 +399,13 @@ void UnrealMovementComponent::OnActorDestroyed(const AActor* actor) it != gv::PooledComponentManager::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; diff --git a/GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.hpp b/GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.hpp index 9b8a94b..6079816 100644 --- a/GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.hpp +++ b/GalavantUnreal/Source/GalavantUnreal/GalaEntityComponents/UnrealMovementComponent.hpp @@ -26,8 +26,8 @@ struct UnrealMovementComponentData { MovementComponentActorSpawnParams SpawnParams; - ACharacter* Character = nullptr; - AActor* Actor = nullptr; + TWeakPtr Character = nullptr; + TWeakPtr 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; \ No newline at end of file diff --git a/GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.cpp b/GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.cpp index 818c03d..7201a9d 100644 --- a/GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.cpp +++ b/GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.cpp @@ -23,6 +23,8 @@ #include "game/EntityLevelOfDetail.hpp" #include "util/StringHashing.hpp" +#include + 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 newLockedCharacter( + ActorEntityManager::CreateActorForEntity( + world, DefaultAgentCharacter, 0, position, + std::bind(&bullshit::OnActorDestroyed, &someBullshit, + std::placeholders::_1))); + + LOGD << "Spawned character. What the fuck"; + + TSharedPtr newLockedCharacter2( + ActorEntityManager::CreateActorForEntity( + 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(); diff --git a/GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.h b/GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.h index 0fa41d7..39bdbc1 100644 --- a/GalavantUnreal/Source/GalavantUnreal/GalavantUnrealMain.h +++ b/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 + static T* CreateActorForEntity(UWorld* world, TSubclassOf actorType, gv::Entity entity, + const gv::Position& position, + ActorEntityManager::TrackActorLifetimeCallback callback) + { + return ActorEntityManager::CreateActorForEntity(world, actorType, 0, position, callback); + } + private: void InitializeEntityTests(); void InitializeProceduralWorld();