Fix/issues#810 Where enemies going to local from remote got stuck in sheathe unequip state by absol89 · Pull Request #881 · tiltedphoques/TiltedEvolution

absol89 · 2026-07-18T16:48:33Z

Fixes #810 after testing the following: Video of fix in action

Code Review Summary — `fix/issues-810`

Problem

When a party leader leaves a cell, remote NPCs get their AI localized on another
client (SetRemote(false)). Two bugs followed:

The engine replays a burst of reset/stance actions that got spammed over the
network, freezing the NPC so it refused to aggro.
Even once unfrozen, a transferred hostile NPC would follow/detect a player but
never draw or attack — it had no valid combat target and its combat controller
was never started.

Net change

5 files, +151 / −15. Two independent fixes.

1. Anti-freeze: dedupe the post-localize action burst

ActorExtension.h — adds LocalizedTick (grace-window timestamp),
BroadcastedUnequip / BroadcastedCombatStanceStop flags, and
kAnimReconcileGraceTicks = 60 (~1s).
Animation.cpp — HookPerformAction now performs every reset action
locally (visuals stay correct) but broadcasts each reset type only once
within the grace window, instead of the old spam. IsResetAction() filters the
relevant events (Unequip, combatStance*, Sheathe/Draw, Idle).
CharacterService.cpp — stamps LocalizedTick and resets the broadcast
flags at each localize site.

2. Combat engagement after transfer

CombatService.cpp / Actor.cpp — on the engine's own detection hook
(HookUpdateDetectionState — the vanilla "NPC noticed you" moment), if a
local, non-player NPC detects a player (local OR remote) and it
arrived hostile, it calls StartCombatEx(player) to boot the combat
controller.
ActorExtension.h / CharacterService.cpp — ArrivedHostile is captured
at localize from actorState.IsWeaponDrawn() (transferred hostiles arrive
weapon-drawn; peaceful NPCs arrive sheathed), used as the safety gate so
friendly-area NPCs are never force-aggro'd.

Design decisions worth a reviewer's eye

"Any player" not "remote player": the target check accepts local or remote
players. After transfer the NPC localizes on the other machine, where the
discovered player is the local player — a remote-only check would miss the
common case.
Detection, not IsInCombat(), is the trigger: a transferred NPC's combat
flag is never engine-initialized, so gating on it was dead. The engine's
detection hook is the vanilla trigger and is used instead.
ArrivedHostile may be slightly strict — in one playtest a sheathed bandit
still engaged, hinting the gate could be looser. Left as-is since it works;
first thing to loosen if an idle hostile ever refuses to aggro.

Validation

Playtested: hidden → no premature discovery; approach after localize → bandit
draws and hits correctly; no interference with other enemies. Video of fix in action

Logs:
clientremote.txt
clientleader.txt

…or NPCs When an NPC is (re)localized via SetRemote(false) (e.g. party leader leaves a cell, ownership transferred to another player), the engine replays a burst of reset/stance actions (Unequip, combatStanceStop, etc.) through ActorMediator. These are broadcast over the network in a loop, which prevents the NPC from finishing an attack windup -- it stands in non-combat pose and refuses to aggro (issue tiltedphoques#810). - Add ActorExtension::LocalizedTick + kAnimReconcileGraceTicks (~1s window). - Stamp LocalizedTick at all SetRemote(false) NPC-ownership sites. - In HookPerformAction, swallow the NETWORK BROADCAST (not the local engine action) of reset/stance actions for freshly-localized NPC actors only, so visuals stay synced but the reconciling spam is never propagated. - Log each swallowed action so the guard's effect is verifiable.

action.EventName is a TiltedPhoques::String (CachedString), not std::string, so IsResetAction must take that type. Compared directly against const char* literals, matching the existing pattern in OverlayClient.cpp.

…ing NPCs) Previous approach suppressed the NETWORK BROADCAST of reset/stance actions for freshly-localized NPCs. That starved the server of the (un)equip/combat state it needs to unlock the NPC, so NPCs could not draw or move until clients disconnected (forced server resync). Now the engine action is ALWAYS performed locally (visuals + AI state stay correct), but each distinct reset action is forwarded over the network at most ONCE during the ~1s grace window. The server gets a single clean transition instead of the spam loop, and is never starved -- so NPCs can draw, move and aggro normally. - ActorExtension: add BroadcastedUnequip / BroadcastedCombatStanceStop dedupe flags. - Animation.cpp: collapse broadcast by action type; perform locally always. - CharacterService: reset dedupe flags at all SetRemote(false) localization sites.

Transferred local NPCs have no valid combat-target handle to a remote player, so they draw on detection but never attack or chase (issues tiltedphoques#810/tiltedphoques#741). - HookUpdateDetectionState: when a local NPC detects a remote player and is already in combat, point its combat controller at that player. Gated by IsInCombat() so it never forces aggro at localize time (player undetected). - OnHitEvent: re-enable (was #if 0) and accept a remote player as the hitter, so a local NPC retaliates when hit by a player.

A transferred NPC's cached weapon-drawn flag (from the previous owner's client) is desynced from the real animation graph, so combat AI issues a draw that no-ops — the NPC follows/aggros but never draws or attacks. Force a clean sheathed baseline through the existing 3D-safe m_weaponDrawUpdates path at localize. SetWeaponDrawnEx's flip-first-if-equal logic then makes the next real draw produce a genuine transition. Skipped for NPCs already in combat to avoid disturbing a live fight.

SetCombatTargetEx only sets the controller target field; a dormant transferred NPC's combat AI was never started with that target, so it followed but never drew/attacked/chased. Use StartCombatEx (boots the combat controller) at detection and on hit, gated on IsInCombat() so it only fires after the NPC has aggro'd (never at localize time). Detection: HookUpdateDetectionState (local NPC detecting remote player). Retaliation: OnHitEvent (local NPC hit by a player). Log evidence (party-member remote, 2026-07-18 17:18): the detection hook fired (target re-acquired) but the NPC still would not draw -- proving the target field alone is insufficient and the combat must be started.

…of remote player The IsInCombat() gate was dead: a transferred hostile NPC's combat flag is never initialized by the engine, so it stayed false forever and the reactive hook never fired (bandit followed but never drew/attacked, even when hit). The detection hook only runs when the engine is already evaluating this NPC vs that target -- i.e. the vanilla 'this NPC noticed you' moment -- so engaging here mirrors vanilla's own trigger. Now: local NPC detecting a remote player -> StartCombatEx (boots the combat controller against the player). No IsInCombat() requirement. StartCombatEx's GetCombatTarget()!=target guard prevents re-trigger flicker. Friendly areas stay safe because detection of a remote player is itself the aggression trigger (peaceful NPCs that never detect a remote player are untouched).

…(not just remote) The previous IsRemotePlayer() target check was backwards for the common tiltedphoques#810 flow: when the leader leaves, the NPC's AI localizes on the OTHER player's machine, where the NPC is a LOCAL actor and the discovered player is the LOCAL player (formID 14). The detected target is never remote in that direction, so the hook never fired. Now engage when a local, non-player NPC detects a player of EITHER kind (local or remote). Gated by ArrivedHostile (captured at localize from IsWeaponDrawn): transferred hostiles arrive weapon-drawn (true), peaceful NPCs arrive sheathed (false). So combat only starts for NPCs that were ALREADY hostile -- friendly areas are never force-aggro'd. Detection hook (HookUpdateDetectionState) is the vanilla 'noticed you' trigger; StartCombatEx boots the combat controller against the player. StartCombatEx's GetCombatTarget()!=target guard prevents re-trigger flicker.

The force-sheathed baseline was a best-guess from cosideci's note, but the real blocker was the combat controller never starting (fixed by StartCombat in 97ddb74). The bandit arrived DRAWN (isweapondrawn: true) yet still would not attack until StartCombat, proving the draw-flag was a side issue, not the cause. Keeping the reconcile could even contradict the real fix (drawn -> forced sheathed), so remove it. ArrivedHostile (captured from IsWeaponDrawn) is retained as the hostile-only gate.

IsWeaponDrawn() is not a member of Actor; the flag lives on actorState (as everywhere else: InventoryService, ActorValueService, SetWeaponDrawnEx). Fixes C2039 build error.

…rs combat The ArrivedHostile (weapon-drawn) gate filtered out exactly the tiltedphoques#810 repro: an idle bandit standing its post arrives SHEATHED, so ArrivedHostile was false and combat never started. Logs proved it -- always-drawn creatures (wolves) engaged via the hook every time, while sheathed transferred bandits never did, despite identical code. Also, ArrivedHostile was only ever set in OnAssignCharacter, never in TakeOwnership (the leader-leaves localize path), so the primary tiltedphoques#810 flow could not pass the gate regardless of weapon state. Remove the gate entirely. The detection hook firing is itself the engine's own valid-threat-pair signal (it only evaluates this NPC vs that target when the engine considers them a real detection pair), so a genuinely peaceful NPC is never reached. Removes the ArrivedHostile field, its OnAssignCharacter assignment, and the dead TakeOwnership consideration.

…e triggers combat" This reverts commit f9217c8.

…the loop Baseline (418e213) looped when a hostile NPC's ownership bounced between two nearby players (Embershard bandits B6FEE/B6DBB/B6FE6 seen cycling TakeOwnership<->Relinquish; leader bandit B6FE4 fired the hook 223x). Cause: the detection hook re-kicked StartCombatEx on every detection re-evaluation, and StartCombatEx does StopCombat()+ StartCombat() -- the StopCombat() sheathes and clears the combat target, so the GetCombatTarget()!=target guard re-qualified next tick => visible draw/sheathe oscillation. Add EngagedFromDetection latch on ActorExtension: fire StartCombatEx exactly once per actor, then hand all further combat/sheathe control to the engine. Not reset on re-localize, so a contested NPC cannot re-trigger the burst when it flips back to local. ArrivedHostile gate unchanged (peaceful/sheathed NPCs still never engaged).

absol89 · 2026-07-18T22:02:52Z

Note that it's a bit hilarious in its eccd890 state, if you walk into certain areas you need to sheathe your weapons to make NPC abandon hostility. This can be seen when you enter the basement of Jorrvaskr the first time, or after fighting the vampires outside fort Dawnguard. Presumably this is because they were having weapons out, and fighting npc did StartCombat(player). Checking this to make a more thorough consequence analysis for the main quest. This quirk can be reprod in a solo game.

I will harden the checks to only apply to Aggression >=2 npc, and not trigger more than once onHit so it won't get intrusive

Test results on 64122b2 concluded that 3 wolves and 2 out of 3 bandits attacked party member after leader left the area. Jorrvaskr no longer triggered the startcombat(player) because it now requires the enemies to have an Agression AV of >=2

…king allies Playtest exposed our hook force-attacking the player's own follower (Lydia, A2C94): she walked out of Dragonsreach weapon-drawn, so ArrivedHostile latched true and StartCombatEx turned her hostile. ArrivedHostile (weapon-drawn-at-transfer) is only a hostility proxy and catches any drawn NPC including allies. Add an Aggression actor-value gate: only force-engage NPCs with GetActorValue(kAggression) >= 2 ('Very Aggressive' -- attacks on sight). Followers/guards/townsfolk (aggression 0-1) are never turned hostile, while bandits/vampires/predators (aggression 2+) still engage. Read via the vanilla ActorValue already vendored (Actor::GetActorValue, ActorValueInfo::kAggression) -- no new address-library pointer.

… draw/sheathe loop Enemies burning in fire looped unequip/equip: fire/DoT delivers a HitEvent every damage tick, and OnHitEvent re-ran StartCombatEx on each one. StartCombatEx does StopCombat()+StartCombat() -- the StopCombat() sheathes and clears the combat target, so GetCombatTarget()!=hitter re-qualified on the next tick => draw/sheathe oscillation (log showed Combat started (hit) x801, Forsworn FF000EC9 alone x125). Add EngagedFromHit latch: retaliate once, then let the engine own combat. Latch is cleared when the NPC leaves combat (IsInCombat()==false) so a later separate fight still triggers. Mirrors EngagedFromDetection.

…nt !IsInCombat) The OnHit retaliation latch was reset whenever IsInCombat()==false, but our OWN StartCombatEx does StopCombat() which momentarily drops IsInCombat() and sheathes the weapon. That one-frame !IsInCombat() right after our engage cleared the latch -> next player hit re-qualified the guard and re-fired StartCombatEx -> sheathe again. Result: Vampire Nightstalker un-aggroed and walked casually between hits. Remove the reset; the latch now persists for the actor's lifetime, same as EngagedFromDetection. The engine re-triggers normal combat on genuine re-engagement; our hook only seeds combat for a target-less transferred NPC.

miredirex

Vibecoding bug fixes is not the way. If you want to delegate a bug fix to an LLM, you need to understand why the bug occurs in the first place

miredirex · 2026-07-19T13:07:56Z

                return;
            }
+
+            // Issue #810 / #741: a local NPC whose ownership was transferred has no valid


Compact this comment

miredirex · 2026-07-19T13:09:15Z

ActorExtension should not be extended with new fields unless absolutely necessary - and this isn't the case. If you want to store data on NPCs, use entt

Also please compact the comments to 1-2 lines after you move the fields

miredirex · 2026-07-19T13:11:59Z

+static bool IsResetAction(const TiltedPhoques::String& aEventName) noexcept
+{
+    return aEventName == "Unequip"
+        || aEventName == "combatStanceStop"
+        || aEventName == "combatStanceGo"
+        || aEventName == "Sheathe"
+        || aEventName == "Draw"
+        || aEventName == "DrawEnd"
+        || aEventName == "Idle";
+}


Checking for exact event names and filtering them doesn't address the root cause of the bug

miredirex · 2026-07-19T13:16:00Z

 void CombatService::OnHitEvent(const HitEvent& acEvent) const noexcept
 {
-#if 0
    if (!m_transport.IsConnected())


This function was #if 0'd for a reason (#633), you essentially uncommented very old (and likely broken) combat code

miredirex · 2026-07-19T13:21:26Z

+                if (pOwnerActor->GetCombatTarget() != pTargetActor)
+                {
+                    spdlog::info("Combat started (detection): local NPC {:X} -> player {:X} (remote={})", pOwner->formID, pTarget->formID, pTargetEx->IsRemotePlayer());
+                    pOwnerActor->StartCombatEx(pTargetActor);


See Actor::StartCombatEx's fn comment

absol89 · 2026-07-21T20:39:30Z

I have experimented with other tweaks (more of a feature), and couldn't conclude that the commits in fix/issues-810 resolved the core issue. At least not without workarounds that deviate from vanilla agression behaviors. Closing the vibed bugfix pr.

absol89 added 10 commits July 18, 2026 12:43

fix(issues-810): use CachedString for reset-action check

fdabb57

action.EventName is a TiltedPhoques::String (CachedString), not std::string, so IsResetAction must take that type. Compared directly against const char* literals, matching the existing pattern in OverlayClient.cpp.

fix(issues-810): use actorState.IsWeaponDrawn() for ArrivedHostile

ccf1b99

IsWeaponDrawn() is not a member of Actor; the flag lives on actorState (as everywhere else: InventoryService, ActorValueService, SetWeaponDrawnEx). Fixes C2039 build error.

absol89 changed the title ~~Fix/issues#810 Where enemies going from local to remote got stuck in sheathe unequip state~~ Fix/issues#810 Where enemies going to local from remote got stuck in sheathe unequip state Jul 18, 2026

absol89 added 2 commits July 18, 2026 19:48

Revert "fix(issues-810/741): drop ArrivedHostile gate; detection alon…

20efed5

…e triggers combat" This reverts commit f9217c8.

absol89 marked this pull request as draft July 18, 2026 18:39

absol89 marked this pull request as ready for review July 18, 2026 21:41

absol89 added 3 commits July 19, 2026 11:25

miredirex requested changes Jul 19, 2026

View reviewed changes

absol89 closed this Jul 21, 2026

Navigation Menu

Uh oh!

Fix/issues#810 Where enemies going to local from remote got stuck in sheathe unequip state - #881

Fix/issues#810 Where enemies going to local from remote got stuck in sheathe unequip state#881
absol89 wants to merge 16 commits into
tiltedphoques:devfrom
absol89:fix/issues-810

absol89 commented Jul 18, 2026

Uh oh!

absol89 commented Jul 18, 2026 •

edited

Loading

Uh oh!

miredirex left a comment

Uh oh!

miredirex Jul 19, 2026

Uh oh!

miredirex Jul 19, 2026

Uh oh!

miredirex Jul 19, 2026

Uh oh!

miredirex Jul 19, 2026

Uh oh!

miredirex Jul 19, 2026

Uh oh!

absol89 commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

absol89 commented Jul 18, 2026

Code Review Summary — fix/issues-810

Problem

Net change

1. Anti-freeze: dedupe the post-localize action burst

2. Combat engagement after transfer

Design decisions worth a reviewer's eye

Validation

Uh oh!

absol89 commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miredirex left a comment

Choose a reason for hiding this comment

Uh oh!

miredirex Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

miredirex Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

miredirex Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

miredirex Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

miredirex Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

absol89 commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Code Review Summary — `fix/issues-810`

absol89 commented Jul 18, 2026 •

edited

Loading