The community loves Members being able to advance quests (instead of only Party Leader), but it introduced a bunch of problems with duplicate quest triggers. This rework deals with it (hopefully).

The most fundamental problem is that ScopedQuestOverride() doesn't work, because quest progress is executed on a different thread; the ScopedQuestOverride() is destructed before the quest OnEvent...() routines are invoked. So all the Members send their updates to the server even when told not to. The server just ignored them all before, but now with Member quest progression enabled, it ends up causing replicated quest updates. Some quests ignore it, many break.

The engine is in control of the content of events passed to the OnEvent routines. I could not find a reliable way to pass any context to the OnEvent routines through that barrier (it runs async, so matching up was nigh-on impossible). So ended up implementing a dedup history server-side, where there is enough context to figure this out.

The worst-case flow (when a Member triggers progress), is:
1) Member sends update.
2) Update is forwarded to Leader, only.
3) Leader OnEvent hooks send the Leader update to the server.
4) Leader SendsToParty
5) Party members reflect the updates to the Server, but are dropped by the dedup history because the Leader has already sent it.
6) Entries in the dedup history time out in seconds (needs some tuning), so quests can rewind and stages can be triggered multiple times to evaluate.

Partial support for Remote actors to run AI packages when in-Scene

Author: rfortier

Code/client/Games/Animation.cpp

@@ -4,6 +4,7 @@
 
 #include <Forms/BGSAction.h>
 #include <Forms/TESIdleForm.h>
+#include <Forms/TESQuest.h>
 
 #include <Structs/ActionEvent.h>
 
@@ -20,13 +21,21 @@ static TPerformAction* RealPerformAction;
 // TODO: make scoped override
 thread_local bool g_forceAnimation = false;
 
+// This is where the Actors AI is enabled/disabled: almost all of NPC AI/behavior is
+// determined by Actions that are run on them.
+
 uint8_t TP_MAKE_THISCALL(HookPerformAction, ActorMediator, TESActionData* apAction)
 {
     auto pActor = apAction->actor;
     const auto pExtension = pActor->GetExtension();
+    const auto pScene = pActor->GetCurrentScene();
+    const bool isPlaying = pScene && pScene->isPlaying;
 
-    if (!pExtension->IsRemote() || g_forceAnimation)
+    if (pExtension->IsLocal() || isPlaying || g_forceAnimation)
     {
+        if (pExtension->IsRemote())
+            spdlog::warn(__FUNCTION__ ": performing actions for remote Actor formId {:X}, name {}", pActor->formID, pActor->baseForm->GetName());
+
         ActionEvent action;
         action.State1 = pActor->actorState.flags1;
         action.State2 = pActor->actorState.flags2;

Code/client/Games/Skyrim/AI/Movement/PlayerControls.cpp

@@ -19,6 +19,13 @@ void PlayerControls::SetCamSwitch(bool aSet) noexcept
     Data.remapMode = aSet;
 }
 
+bool PlayerControls::IsMovementControlsEnabled() noexcept
+{
+    using TIsMovementControlsEnabled = bool();
+    POINTER_SKYRIMSE(TIsMovementControlsEnabled, s_isMovementControlsEnabled, 55485);
+    return s_isMovementControlsEnabled.Get()();
+}
+
 BSInputEnableManager* BSInputEnableManager::Get()
 {
     POINTER_SKYRIMSE(BSInputEnableManager*, s_instance, 400863);

Code/client/Games/Skyrim/AI/Movement/PlayerControls.h

@@ -42,6 +42,8 @@ struct PlayerControls
 
     void SetCamSwitch(bool aSet) noexcept;
 
+    static bool IsMovementControlsEnabled() noexcept;
+
 public:
     char pad0[0x20];
     PlayerControlsData Data;

Code/client/Games/Skyrim/Actor.cpp

@@ -1267,11 +1267,16 @@ void Actor::SpeakSound(const char* pFile)
 
 char TP_MAKE_THISCALL(HookActorProcess, Actor, float a2)
 {
-    // Don't process AI if we own the actor
+    // Don't process AI unless we own the actor, or they are playing Scene packages
+    const auto pScene = apThis->GetCurrentScene();
+    const auto isPlaying = pScene && pScene->isPlaying;
 
-    if (apThis->GetExtension()->IsRemote())
+    if (apThis->GetExtension()->IsRemote() && !isPlaying)
             return 0;
 
+    if (apThis->GetExtension()->IsRemote() && isPlaying)
+        spdlog::warn(__FUNCTION__ ": enabling for remote Actor in Scene formId {:X}, name {}", apThis->formID, apThis->baseForm->GetName());
+
     return TiltedPhoques::ThisCall(RealActorProcess, apThis, a2);
 }
 

Code/client/Games/Skyrim/Events/EventDispatcher.h

@@ -188,16 +188,31 @@ struct TESResolveNPCTemplatesEvent
 {
 };
 
+// The RE'd fields in TESSceneEvent, TESSceneActionEvent and TESScenePhaseEvent may be incorrect
+
 struct TESSceneEvent
 {
+    void* ref;
+    uint32_t sceneFormId;
+    uint32_t sceneType; // BEGIN (0) or END (1)
 };
 
 struct TESSceneActionEvent
 {
+    void* ref;
+    uint32_t sceneFormId;
+    uint32_t actionIndex;
+    uint32_t questFormId;
+    uint32_t actorAliasId;
 };
 
 struct TESScenePhaseEvent
 {
+    void* callback;
+    uint32_t sceneFormId;
+    uint16_t phaseIndex;   
+    uint32_t sceneType;     // BEGIN (0) or END (1)
+    uint32_t questStageId; 
 };
 
 struct TESSellEvent

Code/client/Games/Skyrim/Forms/TESQuest.cpp

@@ -54,13 +54,9 @@ void TESQuest::SetActive(bool toggle)
 
 bool TESQuest::IsStageDone(uint16_t stageIndex)
 {
-    for (Stage* it : stages)
-    {
-        if (it->stageIndex == stageIndex)
-            return it->IsDone();
-    }
-
-    return false;
+    TP_THIS_FUNCTION(TIsStageDone, bool, TESQuest, uint16_t);
+    POINTER_SKYRIMSE(TIsStageDone, IsStageDone, 25011);
+    return IsStageDone(this, stageIndex);
 }
 
 bool TESQuest::Kill()
@@ -88,34 +84,112 @@ bool TESQuest::EnsureQuestStarted(bool& success, bool force)
     return SetRunning(this, &success, force);
 }
 
-bool TESQuest::SetStage(uint16_t newStage)
+bool TESQuest::SetStage(uint16_t stageIndex)
 {
-    ScopedQuestOverride _;
-
+    // According to wiki, calling newStage == currentStage does nothing.
+    // Calling with newStage < currentStage, will rerun the stage actions
+    // IFF the target newStage is not marked IsCompleted(). Regardless,
+    // will not reduce currentStage (it stays the same).
+    // Actually reducing currentStage rquires reset() to be called first.
     TP_THIS_FUNCTION(TSetStage, bool, TESQuest, uint16_t);
     POINTER_SKYRIMSE(TSetStage, SetStage, 25004);
-    return SetStage(this, newStage);
+    bool bSuccess = SetStage(this, stageIndex);
+    if (!bSuccess)
+    {
+        spdlog::warn(__FUNCTION__ ": returned false quest formId {:X}, currentStage {}, newStage {}, name {}", 
+                     formID, currentStage, stageIndex, fullName.value.AsAscii());
+    }
+    return bSuccess;
 }
 
-void TESQuest::ScriptSetStage(uint16_t stageIndex)
+bool TESQuest::ScriptSetStage(uint16_t stageIndex, bool bForce)
 {
-    if (currentStage == stageIndex || IsStageDone(stageIndex))
-        return;
+    // According to wiki, calling with stageIndex == currentStage does nothing.
+    // Calling with stageIndex < currentStage will rerun the stageIndex actions
+    // IFF the target stageIndex is not marked IsCompleted(). Regardless,
+    // will not reduce currentStage (it stays the same).
+    // Actually reducing currentStage rquires reset() to be called first.
+    // Since this is not well-known and hooks may be confused, filter rewind
+    // according to TESQuest::SetStage rules.
+    bool bSuccess = stageIndex > currentStage || stageIndex != currentStage && !IsStageDone(stageIndex) || bForce;
+
+    if (bSuccess)
+    {
+        using Quest = TESQuest;
+        PAPYRUS_FUNCTION(bool, Quest, SetCurrentStageID, int);
+        bSuccess = s_pSetCurrentStageID(this, stageIndex);
+    }
+
+    if (!bSuccess)
+    {
+        spdlog::warn(__FUNCTION__ ": returned false quest formId {:X}, currentStage {}, newStage {}, name {}", formID, currentStage, stageIndex, fullName.value.AsAscii());
+    }
 
+    return bSuccess;
+}
+
+void TESQuest::ScriptReset()
+{
     using Quest = TESQuest;
-    PAPYRUS_FUNCTION(void, Quest, SetCurrentStageID, int);
-    s_pSetCurrentStageID(this, stageIndex);
+    PAPYRUS_FUNCTION(void, Quest, Reset);
+    s_pReset(this);
 }
 
+void TESQuest::ScriptResetAndUpdate()
+{
+    ScriptReset();
+
+    if (!IsEnabled())
+    {
+        if (!IsStopped())
+            SetStopped();
+    }
+
+    else
+    {
+        bool isStarted;
+        if (flags & StartsEnabled && !EnsureQuestStarted(isStarted, false))
+            spdlog::warn(__FUNCTION__ ": EnsureQuestStarted failed questId {:X}, is Started {}", formID, isStarted);
+    }
+}
+
+
 void TESQuest::SetStopped()
 {
     flags &= 0xFFFE;
     MarkChanged(2);
 }
 
+bool TESQuest::IsAnyCutscenePlaying()
+{
+    for (const auto& scene : scenes)
+    {
+        if (scene->isPlaying)
+            return true;
+    }
+    return false;
+}
+
+void BGSScene::ScriptForceStart()
+{
+    spdlog::info(__FUNCTION__ ": force starting scene questId {:X}, sceneId: {:X}, isPlaying? {}", owningQuest->formID, formID, isPlaying);
+    using Scene = BGSScene;
+    PAPYRUS_FUNCTION(void, Scene, ForceStart);
+    s_pForceStart(this);
+}
+
+void BGSScene::ScriptStop()
+{
+    spdlog::info(__FUNCTION__ ": stopping scene questId {:X}, sceneId: {:X}, isPlaying? {}", owningQuest->formID, formID, isPlaying);
+    using Scene = BGSScene;
+    PAPYRUS_FUNCTION(void, Scene, Stop);
+    s_pStop(this);
+}
+
 static TiltedPhoques::Initializer s_questInitHooks(
-    []()
+    []() 
     {
-        // kill quest init in cold blood
-        // TiltedPhoques::Write<uint8_t>(25003, 0xC3);
+    // kill quest init in cold blood
+    // TiltedPhoques::Write<uint8_t>(25003, 0xC3);
     });
+

Code/client/Games/Skyrim/Forms/TESQuest.h

@@ -5,6 +5,23 @@
 #include <Components/TESFullName.h>
 #include <Forms/BGSStoryManagerTree.h>
 
+struct BGSSceneAction
+{
+    virtual ~BGSSceneAction();
+
+    uint32_t actorID;
+    uint16_t startPhase;
+    uint16_t endPhase;
+    uint32_t flags;
+    uint8_t status;
+
+    void Start()
+    {
+        this->status |= 1u;
+    }
+};
+
+static_assert(offsetof(BGSSceneAction, flags) == 0x10);
 struct BGSScene : TESForm
 {
     GameArray<void*> phases;
@@ -89,6 +106,10 @@ struct TESQuest : BGSStoryManagerTreeForm
         uint16_t stageIndex;
         uint8_t flags;
 
+        operator bool() const
+        {
+            return *reinterpret_cast<const std::uintptr_t*>(this) != 0;
+        }
         inline bool IsDone() { return flags & 1; }
     };
 
@@ -103,11 +124,8 @@ struct TESQuest : BGSStoryManagerTreeForm
     Type type;            // 0x00DF
     int32_t scopedStatus; // 0x00E0 default init: -1, if not -1 outside of story manager scope
     uint32_t padE4;
-    GameList<Stage> stages;
-    /*
-    GameList<Stage>* pExecutedStages;  // 0x00E8
-    GameList<Stage>* pWaitingStages;   // 0x00F0
-    */
+    GameValueList<Stage>* pExecutedStages; // 0x00E8
+    GameValueList<Stage*>* pWaitingStages; // 0x00F0
     GameList<Objective> objectives; // 0x00F8
     char pad108[0x100];             // 0x0108
     GameArray<BGSScene*> scenes;    // 0x0208
@@ -139,15 +157,18 @@ struct TESQuest : BGSStoryManagerTreeForm
 
     bool EnsureQuestStarted(bool& succeded, bool force);
 
-    bool SetStage(uint16_t stage);
-    void ScriptSetStage(uint16_t stage);
+    bool SetStage(uint16_t stageIndex);
+    bool ScriptSetStage(uint16_t stage, bool bForce = false);
+    void ScriptReset();
+    void ScriptResetAndUpdate();
     void SetStopped();
+    bool IsAnyCutscenePlaying();    
 };
 
 static_assert(sizeof(TESQuest) == 0x268);
 static_assert(offsetof(TESQuest, fullName) == 0x28);
 static_assert(offsetof(TESQuest, flags) == 0xDC);
-static_assert(offsetof(TESQuest, stages) == 0xE8);
+static_assert(offsetof(TESQuest, pExecutedStages) == 0xE8);
 static_assert(offsetof(TESQuest, objectives) == 0xF8);
 static_assert(offsetof(TESQuest, currentStage) == 0x228);
 static_assert(offsetof(TESQuest, unkFlags) == 0x248);

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

@@ -53,9 +53,11 @@ void DebugService::DrawQuestDebugView()
 
             if (ImGui::CollapsingHeader("Stages"))
             {
-                for (auto* pStage : pQuest->stages)
+                for (auto& stage : *pQuest->pExecutedStages)
                 {
-                    ImGui::TextColored({0.f, 255.f, 255.f, 255.f}, "Stage: %d, is done? %s", pStage->stageIndex, pStage->IsDone() ? "true" : "false");
+                    auto pStage = &stage;
+                    ImGui::TextColored({0.f, 255.f, 255.f, 255.f}, "Stage: %d, is done? %s", pStage->stageIndex,
+                                       pStage->IsDone() ? "true" : "false");
 
                     char setStage[64];
                     sprintf_s(setStage, std::size(setStage), "Set stage (%d)", pStage->stageIndex);
@@ -64,6 +66,38 @@ void DebugService::DrawQuestDebugView()
                         pQuest->ScriptSetStage(pStage->stageIndex);
                 }
             }
+            if (ImGui::CollapsingHeader("Waiting Stages"))
+            {
+                for (auto& pStage : *pQuest->pWaitingStages)
+                {
+                    ImGui::TextColored({0.f, 255.f, 255.f, 255.f}, "Stage: %d, is done? %s", pStage->stageIndex,
+                                       pStage->IsDone() ? "true" : "false");
+
+                    char setStage[64];
+                    sprintf_s(setStage, std::size(setStage), "Set stage (%d)", pStage->stageIndex);
+
+                    if (ImGui::Button(setStage))
+                        pQuest->ScriptSetStage(pStage->stageIndex);
+                }
+            }
+
+            if (ImGui::CollapsingHeader("Scenes"))
+            {
+                for (auto& pScene : pQuest->scenes)
+                {
+                    ImGui::TextColored({0.f, 255.f, 255.f, 255.f}, "Scene Form ID: %x, is playing? %s", pScene->formID,
+                                       pScene->isPlaying ? "true" : "false");
+
+                    ImGui::Text("Scene actions:");
+                    for (int i = 0; i < pScene->actions.length; ++i)
+                    {
+                        char startAction[64];
+                        sprintf_s(startAction, std::size(startAction), "Start action %d", i);
+                        if (ImGui::Button(startAction))
+                            pScene->actions[i]->Start();
+                    }
+                }
+            }
 
             if (ImGui::CollapsingHeader("Actors"))
             {

Code/client/Services/Generic/MagicService.cpp

@@ -425,7 +425,13 @@ void MagicService::OnNotifyAddTarget(const NotifyAddTarget& acMessage) noexcept
 
     // This hack is here because slow time seems to be twice as slow when cast by an npc
     if (pEffect->IsSlowEffect())
-        pActor = PlayerCharacter::Get();
+    {
+        spdlog::debug(
+            __FUNCTION__ ": hacking IsSlowEffect() targetId {:X}, casterId {:X}, magnitued {}, IsDualCasting {}",
+            acMessage.TargetId, acMessage.CasterId, acMessage.Magnitude, acMessage.IsDualCasting);
+        acMessage.CasterId && (pCaster = PlayerCharacter::Get());
+    }
+
 
     pActor->magicTarget.AddTarget(data, acMessage.ApplyHealPerkBonus, acMessage.ApplyStaminaPerkBonus);
     spdlog::debug("Applied remote magic effect");