From 63261bfb052e2041c1bdede42d2f7dbd3bfb651c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 02:16:19 +0000 Subject: [PATCH 1/5] Don't hand out a remote actor the game failed to place Actor::Create ignored the result of ModManager::Spawn. When the game doesn't place the reference, the new Character keeps form ID 0 and gets no handle, so the GamePtr returned by Create holds its only reference. Callers store it in a raw Actor* and the actor is freed at the end of that statement, then moved with MoveTo and used as a remote actor. Engine work queued for it can later run on the freed actor, as in a crash at SkyrimSE 13641+0x47 under TESObjectREFR::MoveRefToNewSpace (19799) from BSTaskPool, with a form ID 0 "Deer" and no parent cell. Return null from Create when the spawn yields no handle or form ID. Callers already handle a null actor; the debug spawner now checks it too, and the spawn request drops the entity it created for the actor. Not verified in Ghidra and not built or tested in game. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018fvQxNnHTLXfMV8j1PBLiQ --- Code/client/Games/Skyrim/Actor.cpp | 11 ++++++++++- Code/client/Services/Debug/DebugService.cpp | 2 ++ Code/client/Services/Generic/CharacterService.cpp | 5 +++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Code/client/Games/Skyrim/Actor.cpp b/Code/client/Games/Skyrim/Actor.cpp index 9e1933e862..188fe21f1f 100644 --- a/Code/client/Games/Skyrim/Actor.cpp +++ b/Code/client/Games/Skyrim/Actor.cpp @@ -234,7 +234,16 @@ GamePtr Actor::Create(TESNPC* apBaseForm) noexcept if (pCell && !(pCell->cellFlags & 1)) pCell = nullptr; - ModManager::Get()->Spawn(position, rotation, pCell, pWorldSpace, pActor); + // If the game doesn't place the reference, the actor has no form ID or handle and the returned GamePtr + // holds its only reference, so callers that keep a raw Actor* use it after it is freed. Moving it queues + // engine work that can then run on the freed actor (crash at SkyrimSE 13641 under MoveRefToNewSpace, + // with a form ID 0 actor). Don't hand out an actor that was never placed. + const uint32_t cRefHandle = ModManager::Get()->Spawn(position, rotation, pCell, pWorldSpace, pActor); + if (cRefHandle == 0 || pActor->formID == 0) + { + spdlog::error("Actor::Create: failed to place actor with base form {:X}", apBaseForm ? apBaseForm->formID : 0); + return {}; + } pActor->ForcePosition(position); diff --git a/Code/client/Services/Debug/DebugService.cpp b/Code/client/Services/Debug/DebugService.cpp index c36d6693e0..8502d92fff 100644 --- a/Code/client/Services/Debug/DebugService.cpp +++ b/Code/client/Services/Debug/DebugService.cpp @@ -87,6 +87,8 @@ void __declspec(noinline) DebugService::PlaceActorInWorld() noexcept const auto pPlayerBaseForm = static_cast(PlayerCharacter::Get()->baseForm); auto pActor = Actor::Create(pPlayerBaseForm); + if (!pActor) + return; const Inventory inventory = PlayerCharacter::Get()->GetActorInventory(); pActor->SetActorInventory(inventory); diff --git a/Code/client/Services/Generic/CharacterService.cpp b/Code/client/Services/Generic/CharacterService.cpp index 7d3ac21af5..bee390eecd 100644 --- a/Code/client/Services/Generic/CharacterService.cpp +++ b/Code/client/Services/Generic/CharacterService.cpp @@ -588,6 +588,11 @@ void CharacterService::OnCharacterSpawn(const CharacterSpawnRequest& acMessage) if (!pActor) { spdlog::error("Actor object {:X} could not be created.", acMessage.ServerId); + + // The custom form branch created this entity for the new actor + if (acMessage.FormId == GameId{} && entity) + m_world.destroy(*entity); + return; } From be8eec129c342c1b0ef5f886a4c831909164f125 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 03:45:42 +0000 Subject: [PATCH 2/5] Log Characters that end up with form ID 0 Diagnostic for the SkyrimSE+1AF607 crash: the crashing Deer had form ID 0, and the client log shows Deer/Elk actors with form ID 0 being loaded just before it. Log the caller chain (module+offset) when a Character is constructed without a form ID and when one gets its 3D loaded, so the code creating these actors can be found. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018fvQxNnHTLXfMV8j1PBLiQ --- Code/client/Games/Skyrim/Actor.cpp | 45 ++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/Code/client/Games/Skyrim/Actor.cpp b/Code/client/Games/Skyrim/Actor.cpp index 188fe21f1f..5b48b683b6 100644 --- a/Code/client/Games/Skyrim/Actor.cpp +++ b/Code/client/Games/Skyrim/Actor.cpp @@ -448,12 +448,51 @@ bool TP_MAKE_THISCALL(HookRemoveSpell, Actor, MagicItem* apSpell) return result; } + +namespace +{ +// Diagnostic for SkyrimSE+1AF607: Characters that end up with form ID 0 later crash on a stale parent cell. +// Logs the caller chain as module+offset so the code that creates or loads them can be found. +void LogFormIdZeroActor(const char* acpWhere, const Actor* apActor) noexcept +{ + void* frames[24]{}; + const USHORT cCount = RtlCaptureStackBackTrace(1, static_cast(std::size(frames)), frames, nullptr); + + std::string stack; + for (USHORT i = 0; i < cCount; ++i) + { + HMODULE hModule = nullptr; + char moduleName[MAX_PATH] = "?"; + if (GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, static_cast(frames[i]), &hModule) && hModule) + { + char path[MAX_PATH]{}; + if (GetModuleFileNameA(hModule, path, MAX_PATH)) + { + const char* cpName = std::strrchr(path, '\\'); + strncpy_s(moduleName, cpName ? cpName + 1 : path, _TRUNCATE); + } + } + + const auto cOffset = reinterpret_cast(frames[i]) - reinterpret_cast(hModule); + stack += fmt::format(" {}+{:X}", moduleName, cOffset); + } + + const auto* pNpc = Cast(apActor->baseForm); + spdlog::warn("[FormIdZeroActor] {}: actor {}, base {:X} ({}), parent cell {}, flags {:X}, thread {}, stack:{}", acpWhere, fmt::ptr(apActor), + apActor->baseForm ? apActor->baseForm->formID : 0, pNpc && pNpc->fullName.value.AsAscii() ? pNpc->fullName.value.AsAscii() : "", fmt::ptr(apActor->parentCell), + apActor->flags, GetCurrentThreadId(), stack); +} +} // namespace + Actor* TP_MAKE_THISCALL(HookCharacterConstructor, Actor) { TP_EMPTY_HOOK_PLACEHOLDER; TiltedPhoques::ThisCall(RealCharacterConstructor, apThis); + if (apThis->formID == 0) + LogFormIdZeroActor("Character constructor", apThis); + return apThis; } @@ -463,6 +502,9 @@ Actor* TP_MAKE_THISCALL(HookCharacterConstructor2, Actor, uint8_t aUnk) TiltedPhoques::ThisCall(RealCharacterConstructor2, apThis, aUnk); + if (apThis->formID == 0) + LogFormIdZeroActor("Character constructor 2", apThis); + return apThis; } @@ -963,6 +1005,9 @@ bool TP_MAKE_THISCALL(HookSpawnActorInWorld, Actor) spdlog::info("Spawn Actor: {:X}, and NPC {}", apThis->formID, pNpc->fullName.value); } + if (apThis->formID == 0) + LogFormIdZeroActor("UpdateReference3D", apThis); + return TiltedPhoques::ThisCall(RealSpawnActorInWorld, apThis); } From a299ac5dda174c20a7384ce1748b0076b8714361 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 03:52:35 +0000 Subject: [PATCH 3/5] Don't move remote actors past the edge of the loaded area The owner streams positions from its own loaded area, which can lie past the edge of ours. Interpolation moved remote actors there anyway, so the game moved them into cells we don't have loaded and then into cells as they unloaded. Ghidra on 1.6.1170 shows the per-frame actor update (37352) queues task 0x54 with a raw copy of parentCell->worldspace when an actor leaves its exterior cell, and 19799/13641 later use it unchecked. A crash dump had that worldspace come from a parent cell that was no longer a cell. Hold remote actors at their last position inside the loaded grid until they come back or are removed. Interiors and other worldspaces are unchanged. Not tested in game. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018fvQxNnHTLXfMV8j1PBLiQ --- Code/client/Systems/InterpolationSystem.cpp | 24 ++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/Code/client/Systems/InterpolationSystem.cpp b/Code/client/Systems/InterpolationSystem.cpp index e5fdb7e30a..6fd44a8554 100644 --- a/Code/client/Systems/InterpolationSystem.cpp +++ b/Code/client/Systems/InterpolationSystem.cpp @@ -7,6 +7,8 @@ #include #include +#include +#include #include namespace @@ -20,6 +22,24 @@ constexpr float cMaxPositionCorrection = 256.f; // Time constant of the correction fading out, ~95% of it is gone after three times this constexpr double cPositionCorrectionTime = 50.0; constexpr size_t cMaxTimePoints = 64; + +// The owner streams positions from its own loaded area, which can lie past the edge of ours. Moving an actor +// there makes the game move it into a cell we don't have loaded, and then into cells as they unload, which +// leaves it pointing at a freed cell. Its next cell change then crashes (SkyrimSE 13641 under task 0x54). +bool IsInLoadedArea(const Actor* apActor, const glm::vec3& acPosition) noexcept +{ + // Interiors are loaded as a whole, and the loaded grid only describes the player's own worldspace + const auto* pWorldSpace = apActor->GetWorldSpace(); + if (!pWorldSpace || pWorldSpace != PlayerCharacter::Get()->GetWorldSpace()) + return true; + + const TES* pTES = TES::Get(); + if (!pTES) + return true; + + const auto cCoords = GridCellCoords::CalculateGridCellCoords(acPosition.x, acPosition.y); + return GridCellCoords::IsCellInGridCell(cCoords, GridCellCoords(pTES->centerGridX, pTES->centerGridY), false); +} } // namespace void InterpolationSystem::Update(Actor* apActor, InterpolationComponent& aInterpolationComponent, const double aNow) noexcept @@ -96,7 +116,9 @@ void InterpolationSystem::Update(Actor* apActor, InterpolationComponent& aInterp if (!apActor) return; - apActor->ForcePosition(aInterpolationComponent.Position); + // Past the edge of the loaded area the actor waits at its last loaded position until it comes back or is removed + if (IsInLoadedArea(apActor, aInterpolationComponent.Position)) + apActor->ForcePosition(aInterpolationComponent.Position); apActor->LoadAnimationVariables(second.Variables); if (apActor->currentProcess && apActor->currentProcess->middleProcess) From 704422bb93fae923d3e8b7bfe68b56731248fcbd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 04:00:10 +0000 Subject: [PATCH 4/5] Log actor and player placement around assignment and removal Diagnostic for the follower spawn/remove loop (Esbern removed 6,183 times in one log): log the actor's and the player's cell, worldspace and position when an actor is added, before and after a remote actor is moved to its owner, and when it is removed, to show what brings it back. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018fvQxNnHTLXfMV8j1PBLiQ --- .../Services/Generic/CharacterService.cpp | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/Code/client/Services/Generic/CharacterService.cpp b/Code/client/Services/Generic/CharacterService.cpp index bee390eecd..f24923eafd 100644 --- a/Code/client/Services/Generic/CharacterService.cpp +++ b/Code/client/Services/Generic/CharacterService.cpp @@ -227,6 +227,26 @@ void CharacterService::DeleteTempActor(const uint32_t aFormId) noexcept } } +namespace +{ +// Diagnostic for actors that loop between spawn and removal (e.g. a follower both saves share): where the actor +// is, and where the player is, at each step. +void LogActorPlacement(const char* acpWhat, uint32_t aFormId) noexcept +{ + auto* pActor = Cast(TESForm::GetById(aFormId)); + const auto* pPlayer = PlayerCharacter::Get(); + if (!pActor || !pPlayer) + return; + + const auto* pCell = pActor->GetParentCellEx(); + const auto* pPlayerCell = pPlayer->GetParentCellEx(); + spdlog::info("[ActorPlacement] {} {:X}: cell {:X}, worldspace {:X}, position ({:.0f}, {:.0f}, {:.0f}), remote {}; player cell {:X}, worldspace {:X}, position ({:.0f}, {:.0f}, {:.0f})", + acpWhat, aFormId, pCell ? pCell->formID : 0, pActor->GetWorldSpace() ? pActor->GetWorldSpace()->formID : 0, pActor->position.x, pActor->position.y, + pActor->position.z, pActor->GetExtension()->IsRemote(), pPlayerCell ? pPlayerCell->formID : 0, pPlayer->GetWorldSpace() ? pPlayer->GetWorldSpace()->formID : 0, + pPlayer->position.x, pPlayer->position.y, pPlayer->position.z); +} +} // namespace + void CharacterService::OnActorAdded(const ActorAddedEvent& acEvent) noexcept { Actor* pActor = Cast(TESForm::GetById(acEvent.FormId)); @@ -260,11 +280,15 @@ void CharacterService::OnActorAdded(const ActorAddedEvent& acEvent) noexcept m_world.emplace_or_replace(entity, acEvent.FormId); m_world.emplace_or_replace(entity); + LogActorPlacement("Added", acEvent.FormId); + ProcessNewEntity(entity); } void CharacterService::OnActorRemoved(const ActorRemovedEvent& acEvent) noexcept { + LogActorPlacement("Removing", acEvent.FormId); + if (auto* pActor = Cast(TESForm::GetById(acEvent.FormId))) pActor->GetExtension()->Reconciliation = ActorExtension::ReconciliationStage::None; @@ -485,8 +509,15 @@ void CharacterService::OnAssignCharacter(const AssignCharacterResponse& acMessag ReconcileActorData(cEntity, pActor, acMessage.OwnershipEpoch, actorData, true, false); + spdlog::info("[ActorPlacement] Moving remote {:X} to owner's worldspace {:X}:{:X}, cell {:X}:{:X}, position ({:.0f}, {:.0f}, {:.0f})", pActor->formID, + acMessage.WorldSpaceId.ModId, acMessage.WorldSpaceId.BaseId, acMessage.CellId.ModId, acMessage.CellId.BaseId, acMessage.Position.x, acMessage.Position.y, + acMessage.Position.z); + LogActorPlacement("Before move", pActor->formID); + MoveActor(pActor, acMessage.WorldSpaceId, acMessage.CellId, acMessage.Position); + LogActorPlacement("After move", pActor->formID); + // The owner's leveled pick rides the assignment response for actors we discovered ourselves ApplyLeveledNpcPick(pActor, acMessage.LeveledNpcPickId); } From d2c857e48942ae53c231f0d1e32e9529828d6f1d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 04:01:53 +0000 Subject: [PATCH 5/5] Revert "Don't move remote actors past the edge of the loaded area" Holding a remote actor at the edge of the loaded area keeps it loaded and stuck there, so it never unloads the way Skyrim expects when an NPC walks off on its schedule. This reverts commit a299ac5. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018fvQxNnHTLXfMV8j1PBLiQ --- Code/client/Systems/InterpolationSystem.cpp | 24 +-------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/Code/client/Systems/InterpolationSystem.cpp b/Code/client/Systems/InterpolationSystem.cpp index 6fd44a8554..e5fdb7e30a 100644 --- a/Code/client/Systems/InterpolationSystem.cpp +++ b/Code/client/Systems/InterpolationSystem.cpp @@ -7,8 +7,6 @@ #include #include -#include -#include #include namespace @@ -22,24 +20,6 @@ constexpr float cMaxPositionCorrection = 256.f; // Time constant of the correction fading out, ~95% of it is gone after three times this constexpr double cPositionCorrectionTime = 50.0; constexpr size_t cMaxTimePoints = 64; - -// The owner streams positions from its own loaded area, which can lie past the edge of ours. Moving an actor -// there makes the game move it into a cell we don't have loaded, and then into cells as they unload, which -// leaves it pointing at a freed cell. Its next cell change then crashes (SkyrimSE 13641 under task 0x54). -bool IsInLoadedArea(const Actor* apActor, const glm::vec3& acPosition) noexcept -{ - // Interiors are loaded as a whole, and the loaded grid only describes the player's own worldspace - const auto* pWorldSpace = apActor->GetWorldSpace(); - if (!pWorldSpace || pWorldSpace != PlayerCharacter::Get()->GetWorldSpace()) - return true; - - const TES* pTES = TES::Get(); - if (!pTES) - return true; - - const auto cCoords = GridCellCoords::CalculateGridCellCoords(acPosition.x, acPosition.y); - return GridCellCoords::IsCellInGridCell(cCoords, GridCellCoords(pTES->centerGridX, pTES->centerGridY), false); -} } // namespace void InterpolationSystem::Update(Actor* apActor, InterpolationComponent& aInterpolationComponent, const double aNow) noexcept @@ -116,9 +96,7 @@ void InterpolationSystem::Update(Actor* apActor, InterpolationComponent& aInterp if (!apActor) return; - // Past the edge of the loaded area the actor waits at its last loaded position until it comes back or is removed - if (IsInLoadedArea(apActor, aInterpolationComponent.Position)) - apActor->ForcePosition(aInterpolationComponent.Position); + apActor->ForcePosition(aInterpolationComponent.Position); apActor->LoadAnimationVariables(second.Variables); if (apActor->currentProcess && apActor->currentProcess->middleProcess)