Merge pull request #123 from tiltedphoques/stability

Crash fixes and general stability improvements

Code/client/Components/RemoteComponent.h

@@ -15,7 +15,4 @@ struct RemoteComponent
     uint32_t Id;
     uint32_t CachedRefId;
     CharacterSpawnRequest SpawnRequest{};
-    // TODO: why is IsDead here (and by extension, IsWeaponDrawn)?
-    bool IsDead = false;
-    bool IsWeaponDrawn = false;
 };

Code/client/Events/InventoryChangeEvent.h

@@ -10,12 +10,12 @@ struct InventoryChangeEvent
     {
     }
 
-    InventoryChangeEvent(const uint32_t aFormId, Inventory::Entry&& arItem, bool aDropOrPickUp) 
-        : FormId(aFormId), Item(std::move(arItem)), DropOrPickUp(aDropOrPickUp)
+    InventoryChangeEvent(const uint32_t aFormId, Inventory::Entry&& arItem, bool aDrop) 
+        : FormId(aFormId), Item(std::move(arItem)), Drop(aDrop)
     {
     }
 
     uint32_t FormId{};
     Inventory::Entry Item{};
-    bool DropOrPickUp = false;
+    bool Drop = false;
 };

Code/client/Games/ExtraDataList.cpp

@@ -1,5 +1,7 @@
 #include "ExtraDataList.h"
 
+#include <Games/Overrides.h>
+
 ExtraDataList* ExtraDataList::New() noexcept
 {
     ExtraDataList* pExtraDataList = Memory::Allocate<ExtraDataList>();
@@ -27,10 +29,15 @@ bool ExtraDataList::Contains(ExtraData aType) const
 
 BSExtraData* ExtraDataList::GetByType(ExtraData aType) const
 {
-    BSScopedLock<BSRecursiveLock> _(lock);
+    if (!ScopedExtraDataOverride::IsOverriden())
+        lock.Lock();
 
     if (!Contains(aType))
+    {
+        if (!ScopedExtraDataOverride::IsOverriden())
+            lock.Unlock();
         return nullptr;
+    }
 
     auto pEntry = data;
 #if TP_SKYRIM
@@ -42,6 +49,9 @@ BSExtraData* ExtraDataList::GetByType(ExtraData aType) const
         pEntry = pEntry->next;
     }
 
+    if (!ScopedExtraDataOverride::IsOverriden())
+        lock.Unlock();
+
     return pEntry;
 }
 

Code/client/Games/Fallout4/Actor.h

@@ -102,7 +102,7 @@ struct Actor : TESObjectREFR
     Factions GetFactions() const noexcept;
     ActorValues GetEssentialActorValues() noexcept;
     float GetActorValue(uint32_t aId) const noexcept;
-    float GetActorMaxValue(uint32_t aId) const noexcept;
+    float GetActorPermanentValue(uint32_t aId) const noexcept;
     void* GetCurrentWeapon(void* apResult, uint32_t aEquipIndex) noexcept;
 
     // Setters

Code/client/Games/Overrides.cpp

@@ -8,3 +8,4 @@ thread_local uint32_t ScopedSaveLoadOverride::s_refCount = 0;
 thread_local uint32_t ScopedExperienceOverride::s_refCount = 0;
 thread_local uint32_t ScopedActivateOverride::s_refCount = 0;
 thread_local uint32_t ScopedInventoryOverride::s_refCount = 0;
+thread_local uint32_t ScopedExtraDataOverride::s_refCount = 0;

Code/client/Games/Overrides.h

@@ -33,6 +33,7 @@ namespace details
     struct Experience {};
     struct Activate {};
     struct Inventory {};
+    struct ExtraData {};
 }
 
 using ScopedEquipOverride = ScopedOverride<details::Equip>;
@@ -41,3 +42,4 @@ using ScopedSaveLoadOverride = ScopedOverride<details::SaveLoad>;
 using ScopedExperienceOverride = ScopedOverride<details::Experience>;
 using ScopedActivateOverride = ScopedOverride<details::Activate>;
 using ScopedInventoryOverride = ScopedOverride<details::Inventory>;
+using ScopedExtraDataOverride = ScopedOverride<details::ExtraData>;

Code/client/Games/Skyrim/Actor.cpp

@@ -242,7 +242,7 @@ ActorValues Actor::GetEssentialActorValues() const noexcept
     {
         float value = actorValueOwner.GetValue(i);
         actorValues.ActorValuesList.insert({i, value});
-        float maxValue = actorValueOwner.GetMaxValue(i);
+        float maxValue = actorValueOwner.GetPermanentValue(i);
         actorValues.ActorMaxValuesList.insert({i, maxValue});
     }
 
@@ -254,17 +254,17 @@ float Actor::GetActorValue(uint32_t aId) const noexcept
     return actorValueOwner.GetValue(aId);
 }
 
-float Actor::GetActorMaxValue(uint32_t aId) const noexcept
+float Actor::GetActorPermanentValue(uint32_t aId) const noexcept
 {
-    return actorValueOwner.GetMaxValue(aId);
+    return actorValueOwner.GetPermanentValue(aId);
 }
 
 void Actor::SetActorValue(uint32_t aId, float aValue) noexcept
 {
     actorValueOwner.SetValue(aId, aValue);
 }
 
-void Actor::ForceActorValue(uint32_t aMode, uint32_t aId, float aValue) noexcept
+void Actor::ForceActorValue(ActorValueOwner::ForceMode aMode, uint32_t aId, float aValue) noexcept
 {
     const float current = GetActorValue(aId);
     actorValueOwner.ForceCurrent(aMode, aId, aValue - current);
@@ -336,13 +336,13 @@ void Actor::SetActorValues(const ActorValues& acActorValues) noexcept
     for (auto& value : acActorValues.ActorMaxValuesList)
     {
         float current = actorValueOwner.GetValue(value.first);
-        actorValueOwner.ForceCurrent(0, value.first, value.second - current);
+        actorValueOwner.ForceCurrent(ActorValueOwner::ForceMode::PERMANENT, value.first, value.second - current);
     }
 
     for (auto& value : acActorValues.ActorValuesList)
     {
         float current = actorValueOwner.GetValue(value.first);
-        actorValueOwner.ForceCurrent(2, value.first, value.second - current);
+        actorValueOwner.ForceCurrent(ActorValueOwner::ForceMode::DAMAGE, value.first, value.second - current);
     }
 }
 

Code/client/Games/Skyrim/Actor.h

@@ -189,7 +189,7 @@ struct Actor : TESObjectREFR
     // in reality this is a BGSLocation
     TESForm *GetCurrentLocation();
     float GetActorValue(uint32_t aId) const noexcept;
-    float GetActorMaxValue(uint32_t aId) const noexcept;
+    float GetActorPermanentValue(uint32_t aId) const noexcept;
     Inventory GetActorInventory() const noexcept;
     MagicEquipment GetMagicEquipment() const noexcept;
 
@@ -200,7 +200,7 @@ struct Actor : TESObjectREFR
     void SetSpeed(float aSpeed) noexcept;
     void SetLevelMod(uint32_t aLevel) noexcept;
     void SetActorValue(uint32_t aId, float aValue) noexcept;
-    void ForceActorValue(uint32_t aMode, uint32_t aId, float aValue) noexcept;
+    void ForceActorValue(ActorValueOwner::ForceMode aMode, uint32_t aId, float aValue) noexcept;
     void SetActorValues(const ActorValues& acActorValues) noexcept;
     void SetFactions(const Factions& acFactions) noexcept;
     void SetFactionRank(const TESFaction* apFaction, int8_t aRank) noexcept;

Code/client/Games/Skyrim/EquipManager.cpp

@@ -74,6 +74,8 @@ void* EquipManager::Equip(Actor* apActor, TESForm* apItem, ExtraDataList* apExtr
 
     ScopedEquipOverride equipOverride;
 
+    spdlog::debug("Call Actor[{:X}]::Equip(), item id: {:X}, extra data? {}, count: {}", apActor->formID, apItem->formID, (bool)apExtraDataList, aCount);
+
     return ThisCall(s_equipFunc, this, apActor, apItem, apExtraDataList, aCount, apSlot, abQueueEquip, abForceEquip, abPlaySound, abApplyNow);
 }
 
@@ -84,6 +86,8 @@ void* EquipManager::UnEquip(Actor* apActor, TESForm* apItem, ExtraDataList* apEx
 
     ScopedEquipOverride equipOverride;
 
+    spdlog::debug("Call Actor[{:X}]::UnEquip(), item id: {:X}, extra data? {}, count: {}", apActor->formID, apItem->formID, (bool)apExtraDataList, aCount);
+
     return ThisCall(s_unequipFunc, this, apActor, apItem, apExtraDataList, aCount, apSlot, abQueueEquip, abForceEquip, abPlaySound, abApplyNow, apSlotToReplace);
 }
 
@@ -165,7 +169,7 @@ void* TP_MAKE_THISCALL(UnEquipHook, EquipManager, Actor* apActor, TESForm* apIte
     const auto pExtension = apActor->GetExtension();
     if (pExtension->IsRemote())
     {
-        spdlog::info("Actor[{:X}]::Unequip(), item form id: {:X}, IsOverridden: {}", apActor->formID, apItem->formID, ScopedEquipOverride::IsOverriden());
+        spdlog::info("Actor[{:X}]::Unequip(), item form id: {:X}, IsOverridden, equip: {}, inventory: {}", apActor->formID, apItem->formID, ScopedEquipOverride::IsOverriden(), ScopedInventoryOverride::IsOverriden());
         // The ScopedInventoryOverride check is here to allow the item to be unequipped if it is removed
         // Without this check, the game will not accept null as a return, and it'll keep trying to unequip infinitely
         if (!ScopedEquipOverride::IsOverriden() && !ScopedInventoryOverride::IsOverriden())

Code/client/Games/Skyrim/Interface/UI.cpp

@@ -78,6 +78,7 @@ static void UnfreezeMenu(IMenu* apEntry)
 
 static constexpr const char* kAllowList[] = {
     "Console", "TweenMenu", "MagicMenu", "StatsMenu", "InventoryMenu", "MessageBoxMenu",
+    "ContainerMenu", "BarterMenu", "Book Menu", "FavoritesMenu"
     //"MapMenu", // MapMenu is disabled till we find a proper fix for first person.
     //"Journal Menu", // Journal menu, aka pause menu, is disabled until we find a fix for manual save crashing while unpaused.
 };

Code/client/Games/Skyrim/Misc/ActorValueOwner.h

@@ -2,14 +2,22 @@
 
 struct ActorValueOwner
 {
+    enum class ForceMode : uint32_t
+    {
+        PERMANENT = 0,
+        TEMPORARY = 1,
+        DAMAGE = 2,
+        COUNT = 3,
+    };
+
     virtual ~ActorValueOwner();
 
     virtual float GetValue(uint32_t aId) const noexcept;
-    virtual float GetMaxValue(uint32_t aId) const noexcept;
-    virtual float GetBaseValue(uint32_t aId);
-    virtual void sub_4();
-    virtual void sub_5();
-    virtual void ForceCurrent(uint32_t aMode, uint32_t aId, float aValue); // Pass 0 for aUnk1 in force, 2 in restore
+    virtual float GetPermanentValue(uint32_t aId) const noexcept;
+    virtual float GetBaseValue(uint32_t aId) const noexcept;
+    virtual void SetBaseValue(uint32_t aId);
+    virtual void ModValue(uint32_t aId, float aValue);
+    virtual void ForceCurrent(ForceMode aMode, uint32_t aId, float aValue);
     virtual void SetValue(uint32_t aId, float aValue) noexcept;
-    virtual void sub_8();
+    virtual bool IsPlayerOwner();
 };

Code/client/Games/Skyrim/PlayerCharacter.cpp

@@ -54,7 +54,7 @@ char TP_MAKE_THISCALL(HookPickUpObject, PlayerCharacter, TESObjectREFR* apObject
         if (apObject->GetExtraDataList())
             apThis->GetItemFromExtraData(item, apObject->GetExtraDataList());
 
-        World::Get().GetRunner().Trigger(InventoryChangeEvent(apThis->formID, std::move(item), true));
+        World::Get().GetRunner().Trigger(InventoryChangeEvent(apThis->formID, std::move(item)));
     }
 
     ScopedInventoryOverride _;

Code/client/Games/Skyrim/TESObjectREFR.cpp

@@ -505,6 +505,7 @@ void TESObjectREFR::AddOrRemoveItem(const Inventory::Entry& arEntry) noexcept
             isWornLeft = pExtraDataList->Contains(ExtraData::WornLeft);
         }
 
+        spdlog::debug("Adding item {:X}, count {}", pObject->formID, arEntry.Count);
         AddObjectToContainer(pObject, pExtraDataList, arEntry.Count, nullptr);
 
         if (isWorn)
@@ -628,15 +629,20 @@ BSPointerHandle<TESObjectREFR>* TP_MAKE_THISCALL(HookRemoveInventoryItem, TESObj
         Inventory::Entry item{};
         modSystem.GetServerModId(apItem->formID, item.BaseId);
         item.Count = -aCount;
-        
+
         if (apExtraList)
+        {
+            ScopedExtraDataOverride _;
             apThis->GetItemFromExtraData(item, apExtraList);
+        }
 
         World::Get().GetRunner().Trigger(InventoryChangeEvent(apThis->formID, std::move(item)));
     }
 
     spdlog::debug("Removing inventory item {:X} from {:X}", apItem->formID, apThis->formID);
 
+    ScopedEquipOverride _;
+
     return ThisCall(RealRemoveInventoryItem, apThis, apResult, apItem, aCount, aReason, apExtraList, apMoveToRef, apDropLoc, apRotate);
 }
 

Code/client/Services/ActorValueService.h

@@ -21,7 +21,7 @@ struct Actor;
 
 struct ActorValueService
 {
-  public:
+public:
     ActorValueService(World& aWorld, entt::dispatcher& aDispatcher, TransportService& aTransport) noexcept;
     ~ActorValueService() noexcept = default;
 
@@ -34,13 +34,6 @@ struct ActorValueService
         kMaxValue
     };
 
-    World& m_world;
-    entt::dispatcher& m_dispatcher;
-    TransportService& m_transport;
-
-    Map<uint32_t, float> m_smallHealthChanges;
-    double m_timeSinceDiff = 1;
-
     void OnLocalComponentAdded(entt::registry& aRegistry, entt::entity aEntity) noexcept;
     void OnDisconnected(const DisconnectedEvent&) noexcept;
     void OnReferenceRemoved(const ReferenceRemovedEvent&) noexcept;
@@ -56,4 +49,11 @@ struct ActorValueService
     void RunDeathStateUpdates() noexcept;
     void CreateActorValuesComponent(entt::entity aEntity, Actor* apActor) noexcept;
     void BroadcastActorValues() noexcept;
+
+    World& m_world;
+    entt::dispatcher& m_dispatcher;
+    TransportService& m_transport;
+
+    Map<uint32_t, float> m_smallHealthChanges;
+    double m_timeSinceDiff = 1;
 };

Code/client/Services/Debug/TestService.cpp

@@ -40,6 +40,9 @@
 
 #include <Messages/RequestRespawn.h>
 
+#include <Interface/UI.h>
+#include <Interface/IMenu.h>
+
 #if TP_SKYRIM64
 #include <EquipManager.h>
 #include <Games/Skyrim/BSGraphics/BSGraphicsRenderer.h>
@@ -134,8 +137,6 @@ void TestService::OnUpdate(const UpdateEvent& acUpdateEvent) noexcept
         if (!s_f8Pressed)
         {
             s_f8Pressed = true;
-
-            PlaceActorInWorld();
         }
     }
     else
@@ -162,13 +163,16 @@ static bool g_enableFormsWindow{false};
 static bool g_enablePlayerWindow{false};
 static bool g_enableSkillsWindow{false};
 static bool g_enablePartyWindow{false};
+static bool g_enableActorValuesWindow{false};
 
 void TestService::OnDraw() noexcept
 {
     const auto view = m_world.view<FormIdComponent>();
     if (view.empty() || !m_showDebugStuff)
         return;
 
+    DrawEntitiesView();
+
     ImGui::BeginMainMenuBar();
     if (ImGui::BeginMenu("Server"))
     {
@@ -196,6 +200,16 @@ void TestService::OnDraw() noexcept
     if (ImGui::BeginMenu("UI"))
     {
         ImGui::MenuItem("Show build tag", nullptr, &m_showBuildTag);
+        if (ImGui::Button("Log all open windows"))
+        {
+            UI* pUI = UI::Get();
+            for (const auto& it : pUI->menuMap)
+            {
+                if (pUI->GetMenuOpen(it.key))
+                    spdlog::info("{}", it.key.AsAscii());
+            }
+        }
+
         if (ImGui::Button("Close all menus"))
         {
             UI::Get()->CloseAllMenus();
@@ -231,6 +245,8 @@ void TestService::OnDraw() noexcept
         DrawSkillView();
     if (g_enablePartyWindow)
         DrawPartyView();
+    if (g_enableActorValuesWindow)
+        DrawActorValuesView();
 
     if (m_toggleComponentWindow)
         DrawComponentDebugView();

Code/client/Services/Debug/Views/ActorValuesView.cpp

@@ -0,0 +1,20 @@
+#include <Services/TestService.h>
+
+#include <Forms/ActorValueInfo.h>
+
+#include <imgui.h>
+
+void TestService::DrawActorValuesView()
+{
+    Actor* pActor = RTTI_CAST(TESForm::GetById(m_formId), TESForm, Actor);
+    if (!pActor)
+        return;
+
+    //for (int i = 0; i < ActorValueInfo::kActorValueCount; i++)
+    {
+        ActorValueOwner& actorValueOwner = pActor->actorValueOwner;
+        float health[3] {actorValueOwner.GetValue(24), actorValueOwner.GetBaseValue(24),
+                        actorValueOwner.GetPermanentValue(24)};
+        ImGui::InputFloat3("Health (val/base/perm)", health, "%.3f", ImGuiInputTextFlags_ReadOnly);
+    }
+}