From 10e64706490bbb0b425d5f39728c0510ca1b9c50 Mon Sep 17 00:00:00 2001 From: Daniil Date: Sat, 19 Sep 2026 18:32:58 +0300 Subject: [PATCH 1/2] Correctly rebuild leveled NPC base from the original NPC and the owner's pick (#899) * Add `SetLeveledCreature` and `CreateTemplateActorBase` functions * Rebuild leveled NPC base from the original NPC and the owner's pick ...instead of assigning the template directly. This preserves non-inherited data, such as hold guard names and outfits, and prevents native actor resets from disposing of shared static NPC records. Update leveled-creature metadata and reconciliation checks, dispose of replaced temporary bases through the engine, and rename the base predicate to `IsLeveledNpcBase`. * Move the "resolve and apply pick" procedure into `LeveledNpcSystem::ApplyPick` --- Code/client/Games/Skyrim/Actor.cpp | 2 +- .../Skyrim/Components/TESActorBaseData.cpp | 7 +++ .../Skyrim/Components/TESActorBaseData.h | 2 + .../Games/Skyrim/Misc/GarbageCollector.cpp | 17 +++++++ .../Games/Skyrim/Misc/GarbageCollector.h | 11 +++++ Code/client/Games/Skyrim/TESObjectREFR.cpp | 22 +++------ Code/client/Games/Skyrim/TESObjectREFR.h | 4 +- .../Services/Generic/CharacterService.cpp | 33 ++++++------- Code/client/Systems/LeveledNpcSystem.cpp | 47 ++++++++++++++++++- Code/client/Systems/LeveledNpcSystem.h | 15 +++++- 10 files changed, 123 insertions(+), 37 deletions(-) create mode 100644 Code/client/Games/Skyrim/Misc/GarbageCollector.cpp create mode 100644 Code/client/Games/Skyrim/Misc/GarbageCollector.h diff --git a/Code/client/Games/Skyrim/Actor.cpp b/Code/client/Games/Skyrim/Actor.cpp index 6fe8d2c1c..5b5167518 100644 --- a/Code/client/Games/Skyrim/Actor.cpp +++ b/Code/client/Games/Skyrim/Actor.cpp @@ -212,7 +212,7 @@ GamePtr Actor::Create(TESNPC* apBaseForm) noexcept pActor->SetLevelMod(4); pActor->MarkChanged(0x40000000); pActor->SetParentCell(pCell); - pActor->SetBaseForm(apBaseForm); + pActor->SetObjectReference(apBaseForm); auto position = pPlayer->position; auto rotation = pPlayer->rotation; diff --git a/Code/client/Games/Skyrim/Components/TESActorBaseData.cpp b/Code/client/Games/Skyrim/Components/TESActorBaseData.cpp index 5655f35a5..f88f1f57b 100644 --- a/Code/client/Games/Skyrim/Components/TESActorBaseData.cpp +++ b/Code/client/Games/Skyrim/Components/TESActorBaseData.cpp @@ -21,6 +21,13 @@ TESActorBase* HookCreateTemplateActorBase(TESActorBase* apOriginalBase, TESActor return pResult; } +TESActorBase* TESActorBaseData::CreateTemplateActorBase(TESActorBase* apOriginalBase, TESActorBase* apTemplateBase) noexcept +{ + using TCreateTemplateActorBase = decltype(TESActorBaseData::CreateTemplateActorBase); + POINTER_SKYRIMSE(TCreateTemplateActorBase, s_CreateTemplateActorBase, 14375); + return s_CreateTemplateActorBase.Get()(apOriginalBase, apTemplateBase); +} + static TiltedPhoques::Initializer s_actorBaseDataInitHooks( []() { diff --git a/Code/client/Games/Skyrim/Components/TESActorBaseData.h b/Code/client/Games/Skyrim/Components/TESActorBaseData.h index 096f4007c..2dac7e55e 100644 --- a/Code/client/Games/Skyrim/Components/TESActorBaseData.h +++ b/Code/client/Games/Skyrim/Components/TESActorBaseData.h @@ -49,6 +49,8 @@ struct TESActorBaseData : BaseFormComponent actorBaseFlags &= ~BaseFlags::IS_ESSENTIAL; } + static TESActorBase* CreateTemplateActorBase(TESActorBase* apOriginalBase, TESActorBase* apTemplateBase) noexcept; + GameArray factions; }; diff --git a/Code/client/Games/Skyrim/Misc/GarbageCollector.cpp b/Code/client/Games/Skyrim/Misc/GarbageCollector.cpp new file mode 100644 index 000000000..33da7a892 --- /dev/null +++ b/Code/client/Games/Skyrim/Misc/GarbageCollector.cpp @@ -0,0 +1,17 @@ +#include + +#include + +GarbageCollector* GarbageCollector::Get() noexcept +{ + POINTER_SKYRIMSE(GarbageCollector*, s_singleton, 400329); + return *s_singleton.Get(); +} + +void GarbageCollector::Add(TESBoundObject* apObject) noexcept +{ + // The base-object overload used by Actor::RecalcLeveledActor (among other functions); + TP_THIS_FUNCTION(TAdd, void, GarbageCollector, TESBoundObject*); + POINTER_SKYRIMSE(TAdd, s_add, 36460); + TiltedPhoques::ThisCall(s_add, this, apObject); +} diff --git a/Code/client/Games/Skyrim/Misc/GarbageCollector.h b/Code/client/Games/Skyrim/Misc/GarbageCollector.h new file mode 100644 index 000000000..d2e3fd3c9 --- /dev/null +++ b/Code/client/Games/Skyrim/Misc/GarbageCollector.h @@ -0,0 +1,11 @@ +#pragma once + +struct TESBoundObject; + +struct GarbageCollector +{ + static GarbageCollector* Get() noexcept; + + // Uses the engine's immediate/deferred base-object deletion policy. + void Add(TESBoundObject* apObject) noexcept; +}; diff --git a/Code/client/Games/Skyrim/TESObjectREFR.cpp b/Code/client/Games/Skyrim/TESObjectREFR.cpp index fdd5e56b5..0a1ef4b25 100644 --- a/Code/client/Games/Skyrim/TESObjectREFR.cpp +++ b/Code/client/Games/Skyrim/TESObjectREFR.cpp @@ -166,6 +166,13 @@ void TESObjectREFR::SetRotation(float aX, float aY, float aZ) noexcept TiltedPhoques::ThisCall(RealRotateZ, this, aZ); } +void TESObjectREFR::SetLeveledCreature(TESActorBase* apOriginalBase, TESActorBase* apTemplateA) noexcept +{ + TP_THIS_FUNCTION(TSetLeveledCreature, void, TESObjectREFR, TESActorBase*, TESActorBase*); + POINTER_SKYRIMSE(TSetLeveledCreature, s_SetLeveledCreature, 20231); + TiltedPhoques::ThisCall(s_SetLeveledCreature, this, apOriginalBase, apTemplateA); +} + using TiltedPhoques::Serialization; void TESObjectREFR::SaveAnimationVariables(AnimationVariables& aVariables) const noexcept @@ -1128,21 +1135,6 @@ void TP_MAKE_THISCALL(HookLockChange, TESObjectREFR) World::Get().GetRunner().Trigger(LockChangeEvent(apThis->formID, false, 0)); } -// Kept for reference: Actor::GetLeveledPick reads the engine's ExtraLeveledCreature directly. -// Called by Actor::RecalcLeveledActor (37323) and TESActorBaseData::CalcTemplateForRef (14374) in the engine. -void TP_MAKE_THISCALL(HookSetLeveledCreature, TESObjectREFR, TESActorBase* apOriginalBase, TESActorBase* apTemplateBase) -{ - TiltedPhoques::ThisCall(RealSetLeveledCreature, apThis, apOriginalBase, apTemplateBase); - - const uint32_t cOriginalBaseId = apOriginalBase ? apOriginalBase->formID : 0; - // ExtraDataList::SetLeveledCreature stores this pointer directly in templateBase. - const uint32_t cTemplateBaseId = apTemplateBase ? apTemplateBase->formID : 0; - - TESForm* pResult = apThis->baseForm; - spdlog::debug( - "SetLeveledCreature: ref {:X}, original base {:X}, template base {:X}, current base {:X}", apThis->formID, cOriginalBaseId, cTemplateBaseId, pResult ? pResult->formID : 0); -} - static TiltedPhoques::Initializer s_objectReferencesHooks( []() { diff --git a/Code/client/Games/Skyrim/TESObjectREFR.h b/Code/client/Games/Skyrim/TESObjectREFR.h index 44c2d8f99..2ebd9b219 100644 --- a/Code/client/Games/Skyrim/TESObjectREFR.h +++ b/Code/client/Games/Skyrim/TESObjectREFR.h @@ -16,6 +16,7 @@ struct AnimationVariables; struct TESWorldSpace; struct TESBoundObject; +struct TESActorBase; struct TESContainer; enum class ITEM_REMOVE_REASON @@ -136,7 +137,7 @@ struct TESObjectREFR : TESForm virtual void sub_81(); virtual void sub_82(); virtual void sub_83(); - virtual void SetBaseForm(TESBoundObject* apForm); // "void SetObjectReference(...)"? + virtual void SetObjectReference(TESBoundObject* apObject); virtual void sub_85(); virtual void sub_86(); virtual void sub_87(); @@ -164,6 +165,7 @@ struct TESObjectREFR : TESForm virtual void sub_9B(); void SetRotation(float aX, float aY, float aZ) noexcept; + void SetLeveledCreature(TESActorBase* apOriginalBase, TESActorBase* apTemplateA) noexcept; BSPointerHandle GetHandle() const noexcept; uint32_t GetCellId() const noexcept; diff --git a/Code/client/Services/Generic/CharacterService.cpp b/Code/client/Services/Generic/CharacterService.cpp index 1b4da1051..5641275b3 100644 --- a/Code/client/Services/Generic/CharacterService.cpp +++ b/Code/client/Services/Generic/CharacterService.cpp @@ -1556,19 +1556,12 @@ void CharacterService::ApplyLeveledNpcPick(Actor* apActor, const GameId& acPickI if (!pBase) return; - if (!pBase->IsTemporary()) - { - // Conforming a shell is exactly what resolution would have done; any - // other static base is an already conformed actor. - if (!LeveledNpcSystem::IsUnresolvedLeveledShell(pBase)) + if (!LeveledNpcSystem::GetOriginalBase(apActor)) { - spdlog::info("Leveled pick {:x}:{:x} received for actor {:X} whose base is not a leveled temp, skipping", acPickId.ModId, acPickId.BaseId, apActor->formID); + spdlog::warn("Leveled pick {:x}:{:x} received for actor {:X} without an original leveled base, keeping local base", acPickId.ModId, acPickId.BaseId, apActor->formID); return; } - spdlog::debug("Actor {:X} still carries unresolved shell base {:X}, conforming to owner's pick", apActor->formID, pBase->formID); - } - const uint32_t cPickId = World::Get().GetModSystem().GetGameId(acPickId); if (cPickId == 0) { @@ -1586,7 +1579,8 @@ void CharacterService::ApplyLeveledNpcPick(Actor* apActor, const GameId& acPickI const TESNPC* pLocalPick = apActor->GetLeveledPick(); const uint32_t localPickId = pLocalPick ? pLocalPick->formID : 0; - if (localPickId == cPickId) + // Even a pick matching the current base must supersede pending work. + if (pBase->IsTemporary() && localPickId == cPickId && m_pendingLeveledConforms.find(apActor->formID) == m_pendingLeveledConforms.end()) { spdlog::info("Leveled actor {:X} already matches owner's pick {:X}", apActor->formID, cPickId); return; @@ -1648,9 +1642,9 @@ void CharacterService::ProcessLeveledConforms() noexcept continue; } - if (pActor->baseForm == pPick) + if (pActor->baseForm && pActor->baseForm->IsTemporary() && pActor->GetLeveledPick() == pPick) { - spdlog::info("Completed leveled NPC reconciliation for actor {:X}, base: {:X}", it->first, cPickFormId); + spdlog::info("Completed leveled NPC reconciliation for actor {:X}, base: {:X}, pick: {:X}", it->first, pActor->baseForm->formID, cPickFormId); stage = ReconciliationStage::None; it = m_pendingLeveledConforms.erase(it); continue; @@ -1670,21 +1664,28 @@ void CharacterService::ProcessLeveledConforms() noexcept continue; } - // Disable and 3D teardown have completed; rebuild from the pick. - pActor->baseForm = pPick; + if (!LeveledNpcSystem::ApplyPick(pActor, pPick)) + { + spdlog::warn("Could not rebuild leveled actor {:X} from its original base and pick {:X}, keeping local base", it->first, cPickFormId); pActor->EnableImpl(); + stage = ReconciliationStage::None; + it = m_pendingLeveledConforms.erase(it); + continue; + } // Recompute the graph descriptor after changing picks; stale variable indices can cause out-of-bounds writes. pActor->GetExtension()->GraphDescriptorHash = 0; // Enable can return before the rebuilt 3D is available to discovery. stage = ReconciliationStage::WaitingFor3D; - spdlog::info("Re-enabled conformed leveled actor {:X}, base: {:X}, waiting for 3D", it->first, cPickFormId); + pActor->EnableImpl(); + spdlog::info("Re-enabled conformed leveled actor {:X}, base: {:X}, pick: {:X}, waiting for 3D", + it->first, pActor->baseForm->formID, cPickFormId); ++it; continue; } - if (!pActor->loadedState && !LeveledNpcSystem::IsUnresolvedLeveledShell(Cast(pActor->baseForm))) + if (!pActor->loadedState && !LeveledNpcSystem::IsLeveledNpcBase(Cast(pActor->baseForm))) { // Wait for distant actors to load 3D; newer picks replace pending work and disconnects clear it. // Unresolved shells bypass this wait because they need a pick before they can load a model. diff --git a/Code/client/Systems/LeveledNpcSystem.cpp b/Code/client/Systems/LeveledNpcSystem.cpp index 075c20b23..b883459e9 100644 --- a/Code/client/Systems/LeveledNpcSystem.cpp +++ b/Code/client/Systems/LeveledNpcSystem.cpp @@ -1,13 +1,56 @@ #include #include +#include +#include #include +#include -bool LeveledNpcSystem::IsUnresolvedLeveledShell(const TESNPC* apBase) noexcept +bool LeveledNpcSystem::IsLeveledNpcBase(const TESNPC* apBase) noexcept { if (!apBase || apBase->IsTemporary()) return false; - const TESNPC* pTemplate = apBase->faceNPC; + const TESForm* pTemplate = apBase->actorData.baseTemplateForm; return pTemplate && pTemplate->formType == FormType::LeveledCharacter; } + +TESNPC* LeveledNpcSystem::GetOriginalBase(const Actor* apActor) noexcept +{ + if (!apActor) + return nullptr; + + const auto* pExtra = static_cast(apActor->extraData.GetByType(ExtraDataType::LeveledCreature)); + if (pExtra && pExtra->originalBase) + return Cast(pExtra->originalBase); + + auto* pBase = Cast(apActor->baseForm); + return IsLeveledNpcBase(pBase) ? pBase : nullptr; +} + +bool LeveledNpcSystem::ApplyPick(Actor* apActor, TESNPC* apPick) noexcept +{ + if (!apPick) + return false; + + // Skyrim resolves a leveled NPC by copying the original base, then + // applying the pick according to that base's template flags. Using + // the pick itself discards data such as a hold guard's name/outfit. + auto* pOriginalBase = GetOriginalBase(apActor); + auto* pResolvedBase = pOriginalBase ? TESActorBaseData::CreateTemplateActorBase(pOriginalBase, apPick) : nullptr; + if (!pResolvedBase) + return false; + + auto* pOldBase = Cast(apActor->baseForm); + apActor->SetLeveledCreature(pOriginalBase, apPick); + apActor->SetObjectReference(pResolvedBase); + + // Match RecalcLeveledActor's disposal policy, but never dispose of + // a static pick left by the old reconciliation implementation. + if (pOldBase && pOldBase->IsTemporary() && pOldBase != pOriginalBase && pOldBase != apPick) + GarbageCollector::Get()->Add(pOldBase); + + spdlog::info("Applied leveled NPC pick for actor {:X}, original base: {:X}, base: {:X}, pick: {:X}", + apActor->formID, pOriginalBase->formID, pResolvedBase->formID, apPick->formID); + return true; +} diff --git a/Code/client/Systems/LeveledNpcSystem.h b/Code/client/Systems/LeveledNpcSystem.h index 0431c793c..75073501f 100644 --- a/Code/client/Systems/LeveledNpcSystem.h +++ b/Code/client/Systems/LeveledNpcSystem.h @@ -1,10 +1,21 @@ #pragma once struct TESNPC; +struct Actor; // Provides helpers for synchronizing leveled NPC identities. struct LeveledNpcSystem { - // Returns whether a static NPC base still awaits a leveled-list pick. - static bool IsUnresolvedLeveledShell(const TESNPC* apBase) noexcept; + // Returns whether a non-temporary NPC base uses a leveled-character template. + static bool IsLeveledNpcBase(const TESNPC* apBase) noexcept; + + // The placed NPC owns the template flags and all non-inherited data. + // A resolved actor retains it in ExtraLeveledCreature::originalBase. + static TESNPC* GetOriginalBase(const Actor* apActor) noexcept; + + // Rebuilds the actor's base from its original NPC and the selected template, + // updates leveled-creature metadata, and disposes of the old temporary base. + // Requires an actor that has finished disabling and has no 3D. + // Returns false if the base cannot be rebuilt, leaving the actor unchanged. + static bool ApplyPick(Actor* apActor, TESNPC* apPick) noexcept; }; From 59669245742eec942fc1d480d6d45c3d617f895b Mon Sep 17 00:00:00 2001 From: Sergio Rayo <87398970+sfedev@users.noreply.github.com> Date: Sat, 19 Sep 2026 22:03:02 +0200 Subject: [PATCH 2/2] Exclude faction jail containers from object sync (#893) * Exclude faction jail containers from object sync (#700) When a player is sent to jail, the game moves their whole inventory into the faction's prisoner belongings container and stolen goods into its evidence chest. Both were synced like any other container, so the first player to enter the jail cell registered their stash on the server and the next player's local chest was overwritten with it; on release they walked out with someone else's inventory. Read the player inventory and stolen goods containers from every loaded faction's crime data and exclude them from object sync, so each client keeps its own copy. This replaces the need for per-chest hard-coded exclusions and also covers jails added by mods. The two quest chests already excluded by form ID stay as they are, since they are quest-owned rather than faction containers. Verified: client builds, TPTests pass. In-game check with two arrested party members pending. * Fix `TESFaction.h` layout, shorten the comment --------- Co-authored-by: Daniil Zakharov --- Code/client/Games/Skyrim/Forms/TESFaction.h | 50 +++++++++++++++++++ Code/client/Games/TES.h | 8 ++- .../client/Services/Generic/ObjectService.cpp | 38 +++++++++++++- 3 files changed, 93 insertions(+), 3 deletions(-) diff --git a/Code/client/Games/Skyrim/Forms/TESFaction.h b/Code/client/Games/Skyrim/Forms/TESFaction.h index b54126435..6f5d3f777 100644 --- a/Code/client/Games/Skyrim/Forms/TESFaction.h +++ b/Code/client/Games/Skyrim/Forms/TESFaction.h @@ -2,8 +2,58 @@ #include #include +#include +#include + +struct TESObjectREFR; +struct BGSListForm; +struct BGSOutfit; +struct TESNPC; + +struct TESReactionForm : BaseFormComponent +{ + GameValueList reactions; + uint8_t groupFormType; + uint8_t pad19; + uint16_t pad1A; + uint32_t pad1C; +}; + +static_assert(sizeof(TESReactionForm) == 0x20); +static_assert(offsetof(TESReactionForm, reactions) == 0x08); +static_assert(offsetof(TESReactionForm, groupFormType) == 0x18); struct TESFaction : TESForm { + // Where the game sends the player's belongings when they are arrested by this faction. + struct CrimeData + { + TESObjectREFR* jailMarker; // JAIL + TESObjectREFR* waitMarker; // WAIT + TESObjectREFR* stolenGoodsContainer; // STOL: stolen items taken on arrest + TESObjectREFR* playerInventoryContainer; // PLCN: the rest of the player's inventory, returned on release + BGSListForm* crimeGroup; // CRGR + BGSOutfit* jailOutfit; // JOUT + uint8_t crimeValues[0x14]; // CRVA + uint32_t pad44; + }; + TESFullName fullname; + TESReactionForm reactionForm; + creation::BSTHashMap* crimeGoldMap; + uint32_t factionFlags; // DATA + uint32_t pad5C; + CrimeData crimeData; + // Remaining vendor data, ranks, crime counts, and timestamps are not accessed here. + uint8_t padA8[0x100 - 0xA8]; }; + +static_assert(sizeof(TESFaction::CrimeData) == 0x48); +static_assert(sizeof(TESFaction) == 0x100); +static_assert(offsetof(TESFaction, fullname) == 0x20); +static_assert(offsetof(TESFaction, reactionForm) == 0x30); +static_assert(offsetof(TESFaction, crimeGoldMap) == 0x50); +static_assert(offsetof(TESFaction, factionFlags) == 0x58); +static_assert(offsetof(TESFaction, crimeData) == 0x60); +static_assert(offsetof(TESFaction, crimeData.stolenGoodsContainer) == 0x70); +static_assert(offsetof(TESFaction, crimeData.playerInventoryContainer) == 0x78); diff --git a/Code/client/Games/TES.h b/Code/client/Games/TES.h index 5ee1804ea..8c19d3a5c 100644 --- a/Code/client/Games/TES.h +++ b/Code/client/Games/TES.h @@ -5,6 +5,7 @@ struct TESObjectCELL; struct TESWorldSpace; struct NiPoint3; struct TESForm; +struct TESFaction; struct Actor; struct ImageSpaceModifierInstance; @@ -109,12 +110,17 @@ struct ModManager Mod* GetByName(const char* acpName) const noexcept; TESObjectCELL* GetCellFromCoordinates(int32_t aX, int32_t aY, TESWorldSpace* aWorldSpace, bool aSpawnCell) noexcept; - uint8_t pad0[0x748]; + // Form arrays start at 0x10 and are indexed by FormType, 0x18 bytes each. + uint8_t pad0[0x118]; + GameArray factions; + uint8_t pad130[0x748 - 0x130]; GameArray quests; uint8_t pad760[0xD60 - 0x760]; GameList mods; }; +static_assert(offsetof(ModManager, factions) == 0x118); +static_assert(offsetof(ModManager, quests) == 0x748); static_assert(offsetof(ModManager, mods) == 0xD60); struct Setting diff --git a/Code/client/Services/Generic/ObjectService.cpp b/Code/client/Services/Generic/ObjectService.cpp index 188dde477..b2140e2ed 100644 --- a/Code/client/Services/Generic/ObjectService.cpp +++ b/Code/client/Services/Generic/ObjectService.cpp @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include @@ -60,11 +62,41 @@ bool IsPlayerHome(const TESObjectCELL* pCell) noexcept return false; } -bool ShouldSyncObject(const TESObjectREFR* apObject) noexcept +// Find each loaded faction's containers for a jailed player's belongings and stolen items. +// Do not sync them: each player's items must stay separate (#700). +// Only compare the container pointers; never read through them. +Set GetPlayerStashContainers() noexcept +{ + Set containers{}; + + ModManager* pModManager = ModManager::Get(); + if (!pModManager) + return containers; + + for (const TESFaction* pFaction : pModManager->factions) + { + if (!pFaction) + continue; + + if (pFaction->crimeData.playerInventoryContainer) + containers.insert(pFaction->crimeData.playerInventoryContainer); + + if (pFaction->crimeData.stolenGoodsContainer) + containers.insert(pFaction->crimeData.stolenGoodsContainer); + } + + return containers; +} + +bool ShouldSyncObject(const TESObjectREFR* apObject, const Set& acPlayerStashContainers) noexcept { if (!apObject) return false; + if (acPlayerStashContainers.contains(apObject)) + return false; + + // Quest chests that take the player's whole inventory without going through faction crime data. switch (apObject->formID) { case 0x39CF1: // Don't sync the chest in the "Diplomatic Immunity" quest @@ -117,9 +149,11 @@ void ObjectService::OnCellChange(const CellChangeEvent& acEvent) noexcept AssignObjectsRequest request{}; + const Set playerStashContainers = GetPlayerStashContainers(); + for (TESObjectREFR* pObject : objects) { - if (!ShouldSyncObject(pObject)) + if (!ShouldSyncObject(pObject, playerStashContainers)) { spdlog::warn("Excluding sync for {:X}", pObject->formID); continue;