Skip to content

Commit 5391da2

Browse files
committed
fix(animated-static-reload): walk sequenceArray to avoid anim-thread race
NiControllerManager::activeSequences is mutated per-frame by the animation worker thread. HookShouldSaveAnimationOnSaving was reading it from the save thread with no lock, corrupting the engine's BSTScatterTable and tripping the "Scatter table has been trashed" assert mid-save. Walk the owned sequenceArray instead (populated only on AddSequence / RemoveSequence during model setup and teardown) with a small sanity bound on count and capacity. Per-sequence state and cycleType are 32-bit aligned reads so an unlocked walk at worst flips one ref's verdict. Split the hook into DoCheck plus a SafeCheck SEH shim so the GFx/Ni locals do not collide with __try (C2712).
1 parent d27b26a commit 5391da2

1 file changed

Lines changed: 29 additions & 24 deletions

File tree

Addictol/Source/Modules/AdModuleAnimatedStaticReload.cpp

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
#include <AdUtils.h>
33

44
#include <RE/B/BGSMovableStatic.h>
5-
#include <RE/B/BSTArray.h>
65
#include <RE/N/NiAVObject.h>
76
#include <RE/N/NiControllerManager.h>
87
#include <RE/N/NiControllerSequence.h>
@@ -29,30 +28,25 @@ namespace Addictol
2928
using TShouldSave = bool(__fastcall*)(const RE::TESObjectREFR*);
3029
static TShouldSave OriginalShouldSave = nullptr;
3130

32-
using ActiveSequences = RE::BSTArray<RE::NiPointer<RE::NiControllerSequence>>;
33-
3431
[[nodiscard]] static CycleType GetCycleType(const RE::NiControllerSequence* a_seq) noexcept
3532
{
3633
return *reinterpret_cast<const CycleType*>(a_seq->cycleType);
3734
}
3835

39-
[[nodiscard]] static const ActiveSequences* GetActiveSequences(const RE::NiControllerManager* a_mgr) noexcept
40-
{
41-
if (!a_mgr)
42-
return nullptr;
43-
return reinterpret_cast<const ActiveSequences*>(a_mgr->activeSequences);
44-
}
45-
46-
[[nodiscard]] static bool HasLoopingActiveSequence(const RE::NiAVObject* a_root) noexcept
36+
// activeSequences gets stomped by the anim thread every frame. Walk sequenceArray instead.
37+
[[nodiscard]] static bool HasLoopingSequence(const RE::NiAVObject* a_root) noexcept
4738
{
4839
if (!a_root)
4940
return false;
5041
auto* mgr = RE::NiControllerManager::GetNiControllerManager(a_root);
51-
const auto* sequences = GetActiveSequences(mgr);
52-
if (!sequences)
42+
if (!mgr)
43+
return false;
44+
const auto count = mgr->sequenceArray.size();
45+
const auto capacity = mgr->sequenceArray.capacity();
46+
if (count == 0 || count > capacity || capacity > 1024 || !mgr->sequenceArray.data())
5347
return false;
54-
for (const auto& seq : *sequences) {
55-
const auto* raw = seq.get();
48+
for (std::uint32_t i = 0; i < count; ++i) {
49+
const auto* raw = mgr->sequenceArray[i].get();
5650
if (!raw)
5751
continue;
5852
if (raw->state == RE::NiControllerSequence::AnimState::kAnimating &&
@@ -62,22 +56,33 @@ namespace Addictol
6256
return false;
6357
}
6458

65-
static bool __fastcall HookShouldSaveAnimationOnSaving(const RE::TESObjectREFR* a_ref) noexcept
59+
static bool DoCheck(const RE::TESObjectREFR* a_ref) noexcept
6660
{
67-
const bool original = OriginalShouldSave ? OriginalShouldSave(a_ref) : false;
68-
if (original || !a_ref)
69-
return original;
61+
const auto* base = a_ref->GetObjectReference();
62+
if (!base || !base->As<RE::BGSMovableStatic>())
63+
return false;
64+
return HasLoopingSequence(a_ref->Get3D());
65+
}
7066

67+
// DoCheck has GFx/Ni locals, so the __try has to live up here (C2712).
68+
static bool SafeCheck(const RE::TESObjectREFR* a_ref) noexcept
69+
{
7170
__try {
72-
const auto* base = a_ref->GetObjectReference();
73-
if (!base || !base->As<RE::BGSMovableStatic>())
74-
return false;
75-
return HasLoopingActiveSequence(a_ref->Get3D());
71+
return DoCheck(a_ref);
7672
}
7773
__except (1) {
78-
return original;
74+
return false;
7975
}
8076
}
77+
78+
static bool __fastcall HookShouldSaveAnimationOnSaving(const RE::TESObjectREFR* a_ref) noexcept
79+
{
80+
const bool original = OriginalShouldSave ? OriginalShouldSave(a_ref) : false;
81+
if (original || !a_ref)
82+
return original;
83+
84+
return SafeCheck(a_ref);
85+
}
8186
}
8287

8388
ModuleAnimatedStaticReload::ModuleAnimatedStaticReload() :

0 commit comments

Comments
 (0)