Log damage and health sync events to find doubled or missing party damage

.github/workflows/linux.yml

@@ -43,10 +43,10 @@ jobs:
           git submodule sync --recursive
           git submodule update --init --force --recursive --depth=1
 
-      - name: Checkout master on tag push
+      - name: Prepare tagged release checkout
         if: github.ref_type == 'tag'
         run: |
-          test "$(git rev-parse HEAD)" = "$(git rev-parse origin/master)"
+          # Build the tagged commit; the local branch name enables release packaging.
           git checkout -B master HEAD
 
       - name: Cache xmake dependencies

.github/workflows/release.yml

@@ -16,6 +16,8 @@ jobs:
       upload-build-for-next-job: true
 
   publish:
+    # Retagging builds CI artifacts without changing an existing release or its assets.
+    if: ${{ github.event.created }}
     needs: [build-windows, build-linux]
     runs-on: ubuntu-24.04
     permissions:

.github/workflows/windows.yml

@@ -48,11 +48,16 @@ jobs:
           git submodule sync --recursive
           git submodule update --init --force --recursive --depth=1
 
-      - name: Checkout master on tag push
+      - name: Prepare tagged release checkout
         if: github.ref_type == 'tag'
         run: |
-          test "$(git rev-parse HEAD)" = "$(git rev-parse origin/master)"
-          git checkout -B master HEAD
+          # Build the tagged commit. On master the local branch name makes it a release build (IS_MASTER: no
+          # debug keys or console). A tag on any other branch builds like that branch, keeping F6/F7 and the console.
+          if git merge-base --is-ancestor HEAD origin/master 2>/dev/null; then
+            git checkout -B master HEAD
+          else
+            git checkout -B "tag/${GITHUB_REF_NAME}" HEAD
+          fi
 
       - name: Resolve artifact version
         if: ${{ inputs.upload-build-for-next-job }}

Code/client/Components.h

@@ -15,6 +15,7 @@
 #include <Components/FaceGenComponent.h>
 #include <Components/CacheComponent.h>
 #include <Components/WaitingFor3D.h>
+#include <Components/PendingInventoryComponent.h>
 #include <Components/ActorValuesComponent.h>
 #include <Components/ObjectComponent.h>
 #include <Components/PlayerComponent.h>

Code/client/Components/PendingInventoryComponent.h

@@ -0,0 +1,22 @@
+#pragma once
+
+#ifndef TP_INTERNAL_COMPONENTS_GUARD
+#error Include Components.h instead
+#endif
+
+#include <Structs/Inventory.h>
+#include <Messages/NotifyInventoryChanges.h>
+#include <Messages/NotifyEquipmentChanges.h>
+#include <variant>
+
+struct PendingInventoryComponent
+{
+    PendingInventoryComponent(const Inventory& acInventory, uint32_t aOwnershipEpoch)
+        : Content(acInventory), OwnershipEpoch(aOwnershipEpoch)
+    {
+    }
+
+    Inventory Content;
+    uint32_t OwnershipEpoch;
+    Vector<std::variant<NotifyInventoryChanges, NotifyEquipmentChanges>> Updates;
+};

Code/client/Components/RemoteComponent.h

@@ -18,4 +18,9 @@ struct RemoteComponent
     uint32_t Id;
     uint32_t CachedRefId;
     uint32_t OwnershipEpoch;
+    // Canonical death state survives loading and prevents living animation replay on corpses.
+    bool IsDead{false};
+    // Retries of a death the game did not take, see CharacterService::RunRemoteUpdates.
+    uint8_t KillAttempts{0};
+    double NextKillAttempt{0.0};
 };

Code/client/Components/WaitingFor3D.h

@@ -5,6 +5,7 @@
 #endif
 
 #include <Messages/CharacterSpawnRequest.h>
+#include <Messages/NotifyEquipmentChanges.h>
 
 struct WaitingFor3D
 {
@@ -14,4 +15,6 @@ struct WaitingFor3D
     }
 
     CharacterSpawnRequest SpawnRequest{};
+    // Equipment changes relayed before the 3D loaded, replayed after the spawn request's inventory is applied
+    Vector<NotifyEquipmentChanges> PendingEquipmentChanges{};
 };

Code/client/CrashHandler.cpp

@@ -2,6 +2,7 @@
 #include "CrashHandler.h"
 #include <DbgHelp.h>
 #include <Windows.h>
+#include <atomic>
 #include <chrono>
 #include <filesystem>
 #include <iomanip>
@@ -23,16 +24,12 @@ std::string SerializeTimePoint(const time_point& time, const std::string& format
 
 LONG WINAPI VectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo)
 {
-    static int alreadyCrashed = 0;
+    static std::atomic_bool s_alreadyCrashed = false;
     auto retval = EXCEPTION_CONTINUE_SEARCH;
 
-    // Serialize 
-    static std::mutex singleThreaded;
-    const std::lock_guard lock{singleThreaded};
-
-    // Check for severe, not continuable and not software-originated exception
+    // Handle only the first access violation. Later or nested ones pass through instead of blocking.
     if (pExceptionInfo->ExceptionRecord->ExceptionCode == EXCEPTION_ACCESS_VIOLATION &&
-        alreadyCrashed++ == 0)
+        !s_alreadyCrashed.exchange(true))
     {
         spdlog::critical (__FUNCTION__ ": crash occurred!"); 
 
@@ -115,13 +112,13 @@ LONG WINAPI VectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo)
         {
             spdlog::critical(__FUNCTION__ ": UnhandledExceptionFilter() workaround triggered.");
 
-            singleThreaded.unlock();        // Might reenter, but is safe at this point.
             if ((*pCurrentUnhandledExceptionFilter)(pExceptionInfo) == EXCEPTION_CONTINUE_EXECUTION)
                 retval = EXCEPTION_CONTINUE_EXECUTION;
-            singleThreaded.lock();
         }
 
-        spdlog::shutdown();
+        // The exception may still be handled, and logging after spdlog::shutdown() crashes
+        if (auto pLogger = spdlog::default_logger())
+            pLogger->flush();
     }
     return retval;
 }

Code/client/Events/ActivateEvent.h

@@ -6,12 +6,17 @@ struct TESBoundObject;
 
 /**
  * @brief In-game activation (i.e. interacting with an object).
+ *
+ * Handled after a queue, when the game may have deleted the object or the activator: check that ObjectId and
+ * ActivatorId still name them (TESForm::GetById) before using the pointers.
  */
 struct ActivateEvent
 {
     ActivateEvent(TESObjectREFR* apObject, Actor* apActivator, TESBoundObject* apObjectToGet, int32_t aCount, bool aDefaultProcessing, uint8_t aUnk1, TESObjectREFR::OpenState aPreActivationOpenState, bool aActivateFlag = false)
         : pObject(apObject)
         , pActivator(apActivator)
+        , ObjectId(apObject ? apObject->formID : 0)
+        , ActivatorId(apActivator ? apActivator->formID : 0)
         , pObjectToGet(apObjectToGet)
         , Count(aCount)
         , DefaultProcessing(aDefaultProcessing)
@@ -23,6 +28,8 @@ struct ActivateEvent
 
     TESObjectREFR* pObject;
     Actor* pActivator;
+    uint32_t ObjectId;
+    uint32_t ActivatorId;
     TESBoundObject* pObjectToGet;
     int32_t Count;
     bool DefaultProcessing;

Code/client/Events/AddTargetEvent.h

@@ -15,4 +15,6 @@ struct AddTargetEvent
     bool IsDualCasting{false};
     bool ApplyHealPerkBonus{false};
     bool ApplyStaminaPerkBonus{false};
+    // A hit on another player that the victim's client can't work out by itself, even for a concentration spell.
+    bool IsPvpForwarded{false};
 };

Code/client/Events/DropItemEvent.h

@@ -0,0 +1,19 @@
+#pragma once
+
+#include <Structs/Inventory.h>
+
+/**
+ * @brief Dispatched when the local player drops an item into the world.
+ */
+struct DropItemEvent
+{
+    DropItemEvent(const uint32_t aFormId, Inventory::Entry aItem)
+        : FormId(aFormId)
+        , Item(std::move(aItem))
+    {
+    }
+
+    // Form id of the reference the engine placed in the world.
+    uint32_t FormId{};
+    Inventory::Entry Item{};
+};

Code/client/Events/EquipmentChangeEvent.h

@@ -6,8 +6,6 @@
 struct EquipmentChangeEvent
 {
     uint32_t ActorId = 0;
-    uint32_t ServerId = 0;
-    uint32_t OwnershipEpoch = 0;
     uint32_t ItemId = 0;
     uint32_t EquipSlotId = 0;
     uint32_t Count = 0;

Code/client/Events/InventoryChangeEvent.h

@@ -5,8 +5,6 @@
 
 /**
  * @brief Dispatched when the contents of an object or actor inventory changes locally.
- *
- * The event has a Drop member variable, since dropped items need to be handled differently.
  */
 struct InventoryChangeEvent
 {
@@ -16,25 +14,16 @@ struct InventoryChangeEvent
     {
     }
 
-    InventoryChangeEvent(const uint32_t aFormId, Inventory::Entry arItem, bool aDrop)
+    InventoryChangeEvent(const uint32_t aFormId, Inventory::Entry arItem, bool aUpdateClients)
         : FormId(aFormId)
         , Item(std::move(arItem))
-        , Drop(aDrop)
-    {
-    }
-
-    InventoryChangeEvent(const uint32_t aFormId, Inventory::Entry arItem, bool aDrop, bool aUpdateClients)
-        : FormId(aFormId)
-        , Item(std::move(arItem))
-        , Drop(aDrop)
         , UpdateClients(aUpdateClients)
     {
     }
 
     uint32_t FormId{};
-    uint32_t ServerId{};
-    uint32_t OwnershipEpoch{};
     Inventory::Entry Item{};
-    bool Drop = false;
     bool UpdateClients = true;
+    // The item moved to or from the local player, e.g. looting or pickpocketing an actor owned by someone else
+    bool IsPlayerTransfer = false;
 };

Code/client/Events/ReferencePickUpEvent.h

@@ -0,0 +1,14 @@
+#pragma once
+
+/**
+ * @brief Dispatched when a local actor picks up a temporary reference, such as a dropped item.
+ */
+struct ReferencePickUpEvent
+{
+    explicit ReferencePickUpEvent(const uint32_t aFormId)
+        : FormId(aFormId)
+    {
+    }
+
+    uint32_t FormId{};
+};

Code/client/Events/ScriptAnimationEvent.h

@@ -5,14 +5,19 @@
  */
 struct ScriptAnimationEvent
 {
-    ScriptAnimationEvent(uint32_t aFormID, String aAnimation, String aEventName)
+    ScriptAnimationEvent(uint32_t aFormID, String aAnimation, String aEventName, bool aIsSubGraph = false)
         : FormID(aFormID)
         , Animation(aAnimation)
         , EventName(aEventName)
+        , IsSubGraph(aIsSubGraph)
     {
     }
 
+    // Local form id of the animating reference; ObjectService::OnScriptAnimationEvent
+    // translates it to a server id before it goes on the wire.
     uint32_t FormID;
     String Animation;
     String EventName;
+    // Sent to the graphs of what the actor wears (Actor.PlaySubGraphAnimation), not its own.
+    bool IsSubGraph;
 };

Code/client/Events/WaveCommandEvent.h

@@ -0,0 +1,8 @@
+#pragma once
+
+/**
+ * @brief Dispatched when the local player uses the /wave chat command.
+ */
+struct WaveCommandEvent
+{
+};

Code/client/Games/Animation.cpp

@@ -19,6 +19,8 @@ static TPerformAction* RealPerformAction;
 
 // TODO: make scoped override
 thread_local bool g_forceAnimation = false;
+// Result of the last action the game performed while g_forceAnimation was set (-1: none).
+thread_local int g_forcedActionResult = -1;
 
 uint8_t TP_MAKE_THISCALL(HookPerformAction, ActorMediator, TESActionData* apAction)
 {
@@ -40,6 +42,9 @@ uint8_t TP_MAKE_THISCALL(HookPerformAction, ActorMediator, TESActionData* apActi
 
         const auto res = TiltedPhoques::ThisCall(RealPerformAction, apThis, apAction);
 
+        if (g_forceAnimation)
+            g_forcedActionResult = res;
+
         // spdlog::debug("Action event name: {}, target name: {}", apAction->eventName.AsAscii(), apAction->targetEventName.AsAscii());
 
         // This is a weird case where it gets spammed and doesn't do much, not sure if it still needs to be sent over the network

Code/client/Games/Memory.cpp

@@ -130,9 +130,49 @@ void* Hook_aligned_malloc(size_t aSize, size_t aAlignment)
     return mi_malloc_aligned(aSize, aAlignment);
 }
 
+// Playtest builds, not meant for upstream. mimalloc's secure mode reports a double free (EAGAIN), a corrupted free list
+// (EFAULT) or an invalid pointer (EINVAL) where it is detected; the handler then crashes on the spot, so CrashLogger
+// records the stack of the code that freed the block instead of a hang somewhere later. Messages go to
+// logs\mimalloc_errors.log through the Windows file API, as the mimalloc heap cannot be trusted at that point.
+static constexpr DWORD kMimallocErrorException = 0xE04D494D; // "MIM"
+
+static void WriteMimallocLog(const char* acpText) noexcept
+{
+    const HANDLE cFile = CreateFileA("Data\\SkyrimTogetherReborn\\logs\\mimalloc_errors.log", FILE_APPEND_DATA, FILE_SHARE_READ | FILE_SHARE_WRITE,
+                                     nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
+    if (cFile == INVALID_HANDLE_VALUE)
+        return;
+
+    DWORD written = 0;
+    WriteFile(cFile, acpText, static_cast<DWORD>(strlen(acpText)), &written, nullptr);
+    CloseHandle(cFile);
+}
+
+static void OnMimallocOutput(const char* acpMessage, void*) noexcept
+{
+    OutputDebugStringA(acpMessage);
+    WriteMimallocLog(acpMessage);
+}
+
+static void OnMimallocError(int aError, void*) noexcept
+{
+    char line[96];
+    sprintf_s(line, "mimalloc error %d on thread %lu\r\n", aError, GetCurrentThreadId());
+    WriteMimallocLog(line);
+
+    // An invalid pointer is a free of memory mimalloc never handed out. No playtest session logged one by mistake, so it
+    // crashes too.
+    if (aError == EAGAIN || aError == EFAULT || aError == EINVAL)
+        RaiseException(kMimallocErrorException, EXCEPTION_NONCONTINUABLE, 0, nullptr);
+}
+
 static TiltedPhoques::Initializer s_memoryHooks(
     []()
     {
+        mi_register_output(&OnMimallocOutput, nullptr);
+        mi_register_error(&OnMimallocError, nullptr);
+        mi_option_enable(mi_option_show_errors);
+
         POINTER_SKYRIMSE(TFormAllocate, s_formAllocate, 68115);
 
         POINTER_SKYRIMSE(TFormFree, s_formFree, 68117);