From 7a823176147b95c6e669b9f0b6919692f15c5692 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 5 Jul 2026 09:34:25 +0200 Subject: [PATCH] =?UTF-8?q?fix(#171):=20gate=20NPC=20UP=20hard-snaps=20whi?= =?UTF-8?q?le=20stuck=20=E2=80=94=20the=20sticky/snap=20tug-of-war=20(gate?= =?UTF-8?q?-1=20residuals)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gate 1 (2026-07-04): "better in general" + three residuals — monsters pushed INTO the player, attacks with stale facing, position "flashing/flapping instead of gliding". All three are ONE mechanism: the legacy NPC UpdatePosition handler hard-snaps position, orientation, and velocity/cycle UNCONDITIONALLY, fighting the armed sticky every UP (ACE's authoritative rest pose sits ~0.6 m out and its server-side facing lags the strafing target; the client stick holds 0.3 m + live facing — oscillation at UP cadence). Retail is immune by architecture, not by tuning: UP corrections flow through the InterpolationManager into the SAME per-tick PositionManager::adjust_offset chain where StickyManager::adjust_offset OVERWRITES them while armed (0x00555190 chain order; 0x00555430 assigns m_fOrigin). A server correction can never fight an armed stick frame-by-frame. The remote-player branch already has exactly that (queue -> combiner -> sticky overwrite); the legacy NPC path snaps outside the chain. Fix: suppress the NPC UP position/orientation/velocity-adoption snaps while the entity is stuck (PositionManager.GetStickyObjectId() != 0) — the retail chain semantics translated to the snap architecture. LastServerPos/Time + cell bookkeeping still record; server truth reasserts on the first UP after unstick, bounded by the 1 s sticky lease. Register row TS-44 (same commit); retires with the S6/R6 interp-queue unification of the NPC path. Apparatus: ACDREAM_PROBE_STICKY=1 — per-guid [sticky] lifecycle lines (STICK / UNSTICK / LEASE-EXPIRE / TARGET-status teardown), per-armed- tick steer lines (signed gap dist, applied delta, heading delta, live resolve), and [sticky-snap-skip] at the suppressed-snap site. PhysicsDiagnostics.ProbeStickyEnabled owns the flag (rule #5). Full suite 4038 green. Awaiting gate 2 (pack melee vs retail). Co-Authored-By: Claude Fable 5 --- docs/ISSUES.md | 14 +++++++ .../retail-divergence-register.md | 3 +- src/AcDream.App/Rendering/GameWindow.cs | 41 +++++++++++++++++-- .../Physics/Motion/StickyManager.cs | 21 ++++++++++ .../Physics/PhysicsDiagnostics.cs | 15 +++++++ 5 files changed, 90 insertions(+), 4 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index e834d9b6..ea971260 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -100,6 +100,20 @@ MoveTo distance math. **SSOT:** `docs/research/2026-07-04-171-sticky-melee-hando **Acceptance:** side-by-side vs retail in a scamp pack: attackers hold separation + face the player while it strafes; user visual gate. +**Gate 1 result (2026-07-04):** "in general it is better" but three residuals: +(1) monsters sometimes pushed INTO the player; (2) monsters sometimes attack +while facing the wrong way; (3) "flashing/flapping instead of gliding". All +three = ONE mechanism: the legacy NPC UP handler hard-snaps position (5678) + +orientation (5714) + velocity/cycle UNCONDITIONALLY, fighting the armed sticky +per tick (ACE's authoritative rest pose sits ~0.6 m out + lags the strafing +target's bearing; sticky pulls to 0.3 m + live facing → oscillation at UP +cadence). Retail is immune BY ARCHITECTURE: UPs flow through the +InterpolationManager into the same adjust_offset chain where sticky OVERWRITES +them while armed. **Residual fix:** suppress the NPC UP snaps while stuck +(register TS-44 — the retail chain semantics translated to the snap path) + +`ACDREAM_PROBE_STICKY=1` apparatus (`[sticky]` lifecycle/steer lines + +`[sticky-snap-skip]`, all per-guid). Awaiting gate 2. + ## #170 — Remote creature chase+attack renders wrong vs retail (glide, over-frequency, uniform attack anims) **Status:** DONE (2026-07-04) — **user visual gate PASSED** ("looks good, as close diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 6ff2adce..6760c7de 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -180,7 +180,7 @@ accepted-divergence entries (#96, #49, #50). | AP-80 | **PlanFromVelocity survives for velocity-only NPC cycles** (M16): UpdatePosition-derived speed picks Ready/Walk/Run cycles for server-controlled creatures whose UMs never arrive (scripted-path NPCs); retail derives every cycle from motion messages through the motion tables (R4-V4 note; pre-existing mechanism, row added per the V4 plan) | `src/AcDream.Core/Physics/ServerControlledLocomotion.cs` (`PlanFromVelocity`); consumer `GameWindow.ApplyServerControlledVelocityCycle` | Some ACE entities move by position updates alone — without this, they slide in T-pose; constants (StopSpeed 0.2, RunThreshold 1.25) tuned against live ACE traffic | Cycle-pick thresholds are acdream inventions — a creature intended to walk fast may show run legs near the threshold | retire in R6 (root motion + full per-tick order) | | AP-81 | **Remote-DR gravity toggled via the Gravity STATE bit**: the jump handler sets `Body.State \|= Gravity` at VectorUpdate and both landing blocks clear it after `HitGround()`; retail keeps GRAVITY set for the object's whole life and gates gravity ACCELERATION on the Contact transient (`calc_acceleration`) (pre-existing K-fix9/K-fix15 mechanism, row added during #161 — which also fixed the ordering so `Motion.HitGround()`'s verbatim `state&0x400` gate runs BEFORE the clear) | `src/AcDream.App/Rendering/GameWindow.cs` (VectorUpdate jump handler + the two landing blocks) | The DR tick integrates gravity only for airborne remotes; the flag dance delivers exactly that without porting the full contact-gated `calc_acceleration` chain; the #161 ordering fix keeps the retail HitGround contract satisfied | Any NEW call into `Motion.HitGround`/`LeaveGround` placed after the clear silently no-ops on the gravity gate (the #161 leg-2 class); grounded remotes carry a non-retail state word (probes comparing state bits vs retail mislead) | `CPhysicsObj::calc_acceleration` (contact-gated); `set_on_walkable` 0x00511310; retire in R6 (contact-gated accel + persistent GRAVITY) | -## 4. Temporary stopgap (TS) — 38 rows (TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded) +## 4. Temporary stopgap (TS) — 39 rows (TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded) | # | Divergence | Where (file:line) | Why it is safe / justified | Risk if assumption breaks | Retail oracle | |---|---|---|---|---|---| @@ -222,6 +222,7 @@ accepted-divergence entries (#96, #49, #50). | TS-38 | `MotionInterpreter.Initted` defaults to `true` in both constructors, not retail's `false` — retail's `CMotionInterp` is never observed pre-`enter_default_state` (every real construction path calls it before exposing the interpreter); acdream's constructors are used directly by ~40 pre-existing tests and both App call sites as complete, immediately-usable objects with no separate "enter default state" step | `src/AcDream.Core/Physics/MotionInterpreter.cs` (`Initted` property + both constructors) | Defaulting `true` is the C# equivalent of "the constructor already did what `enter_default_state` would have done to this flag" — `EnterDefaultState()` remains available, verbatim, for the REST of retail's reset semantics (state defaults, sentinel enqueue, `LeaveGround` tail) when a caller wants them | None observed: no code path needs `apply_current_movement`/`ReportExhaustion` to no-op before an explicit `EnterDefaultState()` call, since nothing constructs a `MotionInterpreter` and defers initialization today. If a future caller DOES need staged construction (build now, `EnterDefaultState()` later), it must explicitly set `Initted = false` first | `CMotionInterp::enter_default_state` 0x00528c80 @306124 sets `initted = 1`; retire if/when construction is staged through `EnterDefaultState()` uniformly | | TS-41 | Grounded remote NPCs WITHOUT an armed moveto are body-driven by UP-synthesized server velocity (`HasServerVelocity` → the SERVERVEL per-tick leg: `Body.Velocity = ServerVelocity`, `MoveToManager.UseTime` SKIPPED, stale-decay stop via `ApplyServerControlledVelocityCycle(Zero)`); retail has no wire-velocity leg-driver anywhere — `MovementManager::UseTime` runs UNCONDITIONALLY per tick and between-UP translation comes from the motion state (`get_state_velocity`), UPs only hard-snap. **#170 residual fix narrowed this branch: an ARMED moveto (`MovementTypeState != Invalid`) now always takes the MOVETO leg** — the old arbitration starved the verbatim MoveToManager for exactly the duration of a server-side chase (UPs flowing → UseTime never ran → legs stayed Ready while the body glided = the #170 slide; live funnel 16 arms → 1 run install). **R5-V3 (#171) narrowed it again: a STUCK entity (`PositionManager.GetStickyObjectId() != 0`) also takes the MOVETO leg** — after the sticky arrival the moveto is cleaned (Invalid) but `StickyManager::adjust_offset` owns the between-snap translation; SERVERVEL would glide the body against the sticky steer (same starvation class) | `src/AcDream.App/Rendering/GameWindow.cs` (`TickAnimations` grounded NPC branch, `moveToArmed`+`stickyArmed` gate) | ACE moves some entities by position updates alone (scripted paths, missiles) with no UM/moveto stream — without a velocity fallback they freeze between UPs; entities WITH a moveto now get the retail drive | An entity class that carries BOTH wire velocity and an armed moveto with conflicting truths follows the moveto; UP hard-snaps bound the drift. Non-moveto entities keep the non-retail stale-stop heuristics (AP-80 thresholds) | `CPhysicsObj::UpdateObjectInternal` 0x005156b0 (`MovementManager::UseTime` call @0x00515998, unconditional); retire in R6 (full per-tick order) | | TS-42 | Per-tick DRAIN ORDER inverted vs retail: acdream's `TickAnimations` runs `HandleTargetting` → `MoveTo.UseTime` FIRST and the animation-completion drain (Sequencer.Advance → AnimDone hooks → `MotionTableManager.AnimationDone`/`UseTime` → `CMotionInterp.MotionDone` pops) LAST, so every motion-completion-gated decision (`BeginTurnToHeading`'s `motions_pending` wait) sees a queue that is one frame STALE — the unblock after a stop/swing lands one frame later than retail. Retail order (pinned from the named decomp this session): `UpdatePositionInternal` (CPartArray::Update + `process_hooks` @0x00512d3d — the drain) runs BEFORE `TargetManager::HandleTargetting` @0x00515989 → `MovementManager::UseTime` @0x00515998 → `CPartArray::HandleMovement` @0x005159a4 (zero-tick sweep) in `UpdateObjectInternal` | `src/AcDream.App/Rendering/GameWindow.cs` (`TickAnimations` per-entity phase order; the R2-Q4 comment already marks the placement "provisional until R6") | Bounded to exactly ONE frame (~16 ms) of extra latency per completion-gated event; every queue eventually drains identically (RemoteChaseEndToEndHarnessTests conformance) | Motion-completion-gated transitions (chase turn start, post-swing re-arm) systematically lag retail by one frame; under compound churn the lag can cost an extra retry cycle | `CPhysicsObj::UpdateObjectInternal` 0x005156b0 + `UpdatePositionInternal` 0x00512c30 (`process_hooks` @0x00512d3d); retire in R6 (retail UpdateObjectInternal order) | +| TS-44 | NPC UpdatePosition hard-snaps (position @`OnLivePositionUpdated` + orientation + velocity/cycle adoption) are SUPPRESSED while the entity is stuck (`PositionManager.GetStickyObjectId() != 0`) — an adaptation of retail's chain semantics to the legacy snap path: retail routes UP corrections through the InterpolationManager into the SAME per-tick `PositionManager::adjust_offset` chain where `StickyManager::adjust_offset` OVERWRITES them while armed (0x00555190 order, 0x00555430 assigns m_fOrigin), so a server correction can never fight an armed stick; the legacy NPC path snaps OUTSIDE the chain, producing snap-out/steer-back position flapping + stale-facing stomps (the 2026-07-04 #171 gate residuals). Bookkeeping (`LastServerPos/Time`, cell) still records; server truth reasserts on the first UP after unstick, bounded by the 1 s sticky lease | `src/AcDream.App/Rendering/GameWindow.cs` (`OnLivePositionUpdated` NPC section, `snapSuppressedByStick` gate) | Retail's mechanism (sticky-overwrites-interp) is unreachable until the NPC path gets the interp-queue architecture (the player-remote branch already has it — the R5-V3 combiner→sticky chain); the gate reproduces the retail-observable behavior on the snap architecture | A stick that stays armed while ACE moves the monster far (shouldn't happen — sticks follow the target by construction) would drift until unstick+next UP; worst case bounded by the 1 s lease + the next UM re-arm | `PositionManager::adjust_offset` 0x00555190; `InterpolationManager` UP routing (`CPhysicsObj::MoveOrTeleport`); retire when the NPC path unifies onto the interp queue (S6/R6) | | TS-43 | Remote teleport has no `teleport_hook` equivalent: retail tears down the position managers on every teleport (`CPhysicsObj::teleport_hook` 0x00514ed0 — `CancelMoveTo(0x3c)` @0x00514edf, `PositionManager::UnStick` @0x00514eee, `StopInterpolating`/`UnConstrain`); acdream's remote teleport is a bare UP hard-snap, so a stuck/chasing remote that the server teleports keeps its stick/moveto for up to the 1 s sticky lease / next UM. The LOCAL player side IS wired (R5-V3: `PlayerMovementController.SetPosition` → `PositionManager.UnStick`; the moveto cancel was already there via `StopCompletely`; the teleport-arrival site also fires the hook's tail — `EntityPhysicsHost.NotifyTeleported` = `TargetManager::ClearTarget` + `NotifyVoyeurOfEvent(Teleported)` @0x00514f1b-0x00514f28, which is what makes mobs stuck to the player drop their sticks on a recall) | `src/AcDream.App/Rendering/GameWindow.cs` (`OnLivePositionUpdated` — no teleport-flag manager teardown for remotes) | Remote teleports are rare (recalls/summons); the sticky 1 s lease + UP hard-snaps self-correct within a second; wiring it properly wants the UP teleport-stamp plumbing (TS-26's stamp work) | A teleported-away attacker briefly steers toward its pre-teleport target from the new location (≤1 s) before the lease/next-UM corrects it | `CPhysicsObj::teleport_hook` 0x00514ed0; retire with the TS-26 UP-stamp port | --- diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 5f523cf2..5cd5787a 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -5675,7 +5675,34 @@ public sealed class GameWindow : IDisposable rmState.ServerVelocity = System.Numerics.Vector3.Zero; rmState.HasServerVelocity = false; } - rmState.Body.Position = worldPos; + // R5-V3 #171 residual (2026-07-04 gate: "flashing/flapping", + // stale facing, pushed-into-player): while an entity is STUCK, + // the sticky steer owns its frame — retail's UP corrections flow + // through the InterpolationManager into the SAME adjust_offset + // chain where StickyManager OVERWRITES them while armed + // (PositionManager::adjust_offset 0x00555190 order; sticky + // assigns m_fOrigin 0x00555430), so a server correction can + // never fight an armed stick frame-by-frame. This legacy NPC + // path hard-snaps OUTSIDE that chain, producing a visible + // snap-out/steer-back oscillation at UP cadence (position) and + // a stale-facing stomp (orientation). Faithful translation to + // the snap architecture: suppress the position/orientation/ + // velocity snaps while stuck. LastServerPos/Time bookkeeping + // still records below — server truth reasserts on the first UP + // after unstick (bounded by the 1 s sticky lease). Register + // row with TS-41/TS-44. + bool snapSuppressedByStick = + (rmState.Host?.PositionManager.GetStickyObjectId() ?? 0u) != 0u; + if (snapSuppressedByStick + && AcDream.Core.Physics.PhysicsDiagnostics.ProbeStickyEnabled) + { + float snapDist = System.Numerics.Vector3.Distance( + worldPos, rmState.Body.Position); + Console.WriteLine(FormattableString.Invariant( + $"[sticky-snap-skip] guid=0x{update.Guid:X8} d={snapDist:F3} srv=({worldPos.X:F2},{worldPos.Y:F2}) body=({rmState.Body.Position.X:F2},{rmState.Body.Position.Y:F2})")); + } + if (!snapSuppressedByStick) + rmState.Body.Position = worldPos; // K-fix15 (2026-04-26): DON'T auto-clear airborne on UP. // ACE broadcasts UPs during the arc (peak / mid-fall / land) // at ~5-10 Hz. The previous K-fix9 logic cleared Airborne on @@ -5711,7 +5738,13 @@ public sealed class GameWindow : IDisposable // turn starts incorporates the pre-turn interval and produces // a halved "observed" rate → visible slow-start. Formula-only // is stable and simple; hard-snap fixes any drift. - rmState.Body.Orientation = rot; + // R5-V3 #171 residual: gated on the stick (see the position-snap + // comment above) — ACE's server-side facing lags the strafing + // target; stomping it over the sticky's per-tick face-tracking + // was the "monster attacking while facing a different direction" + // gate report. + if (!snapSuppressedByStick) + rmState.Body.Orientation = rot; rmState.LastServerPos = worldPos; rmState.LastServerPosTime = nowSec; // Align the body's physics clock with our clock so update_object @@ -5750,12 +5783,14 @@ public sealed class GameWindow : IDisposable } } } - else if (!IsPlayerGuid(update.Guid) && rmState.HasServerVelocity) + else if (!IsPlayerGuid(update.Guid) && rmState.HasServerVelocity + && !snapSuppressedByStick) { rmState.Body.Velocity = rmState.ServerVelocity; } if (rmState.HasServerVelocity + && !snapSuppressedByStick && _animatedEntities.TryGetValue(entity.Id, out var aeForVelocity)) { // NPC/monster remotes: PlanFromVelocity cycle selection from diff --git a/src/AcDream.Core/Physics/Motion/StickyManager.cs b/src/AcDream.Core/Physics/Motion/StickyManager.cs index 0d96e388..6db8d250 100644 --- a/src/AcDream.Core/Physics/Motion/StickyManager.cs +++ b/src/AcDream.Core/Physics/Motion/StickyManager.cs @@ -82,6 +82,10 @@ public sealed class StickyManager if (TargetId == 0) return; + if (PhysicsDiagnostics.ProbeStickyEnabled) + Console.WriteLine(FormattableString.Invariant( + $"[sticky] guid=0x{_host.Id:X8} UNSTICK target=0x{TargetId:X8}")); + TargetId = 0; Initialized = false; _host.ClearTarget(); @@ -120,6 +124,11 @@ public sealed class StickyManager TargetId = objectId; Initialized = false; StickyTimeoutTime = _host.CurTime + StickyTime; + + if (PhysicsDiagnostics.ProbeStickyEnabled) + Console.WriteLine(FormattableString.Invariant( + $"[sticky] guid=0x{_host.Id:X8} STICK target=0x{objectId:X8} tgtR={targetRadius:F2} ownR={_host.Radius:F2} lease={StickyTime:F1}s")); + // set_target(context_id=0, objectId, radius=0.5, quantum=0.5). _host.SetTarget(0, objectId, 0.5f, 0.5); } @@ -140,6 +149,10 @@ public sealed class StickyManager // C0|C3 clear = cur_time > timeout; ACE `>` too), not >=. if (_host.CurTime > StickyTimeoutTime) { + if (PhysicsDiagnostics.ProbeStickyEnabled) + Console.WriteLine(FormattableString.Invariant( + $"[sticky] guid=0x{_host.Id:X8} LEASE-EXPIRE target=0x{TargetId:X8}")); + TargetId = 0; Initialized = false; _host.ClearTarget(); @@ -170,6 +183,10 @@ public sealed class StickyManager if (TargetId != 0) { + if (PhysicsDiagnostics.ProbeStickyEnabled) + Console.WriteLine(FormattableString.Invariant( + $"[sticky] guid=0x{_host.Id:X8} TARGET-{info.Status} teardown target=0x{TargetId:X8}")); + TargetId = 0; Initialized = false; _host.ClearTarget(); @@ -233,5 +250,9 @@ public sealed class StickyManager if (heading < -MoveToMath.Epsilon) heading += 360f; offset.SetHeading(heading); + + if (PhysicsDiagnostics.ProbeStickyEnabled) + Console.WriteLine(FormattableString.Invariant( + $"[sticky] guid=0x{_host.Id:X8} ADJ dist={dist:F3} delta={delta:F3} speed={speed:F1} hdgDelta={heading:F1} live={(target is not null ? 1 : 0)}")); } } diff --git a/src/AcDream.Core/Physics/PhysicsDiagnostics.cs b/src/AcDream.Core/Physics/PhysicsDiagnostics.cs index 1412249c..c8a1ba3f 100644 --- a/src/AcDream.Core/Physics/PhysicsDiagnostics.cs +++ b/src/AcDream.Core/Physics/PhysicsDiagnostics.cs @@ -80,6 +80,20 @@ public static class PhysicsDiagnostics public static bool ProbeCellSetEnabled { get; set; } = Environment.GetEnvironmentVariable("ACDREAM_PROBE_CELLSET") == "1"; + /// + /// R5-V3 #171 residuals (2026-07-04) — sticky-melee timeline probe. + /// One [sticky] line per StickyManager lifecycle event (STICK / + /// UNSTICK / LEASE-EXPIRE / TARGET-status teardown) and per armed + /// AdjustOffset tick (guid, signed gap distance, applied delta, + /// heading delta), plus [sticky-snap-skip] lines at the NPC + /// UpdatePosition handler when a server hard-snap is suppressed because + /// the entity is stuck. Heavy while a pack is stuck (~60 Hz × stuck + /// count); capture-session only. All lines carry the guid + /// (feedback_probe_identity_attribution). + /// + public static bool ProbeStickyEnabled { get; set; } + = Environment.GetEnvironmentVariable("ACDREAM_PROBE_STICKY") == "1"; + public static void LogCellSetBuild( uint seedCellId, System.Numerics.Vector3 sphereCenter, @@ -562,6 +576,7 @@ public static class PhysicsDiagnostics ProbeCellEnabled = false; ProbeBuildingEnabled = false; ProbeCellSetEnabled = false; + ProbeStickyEnabled = false; ProbeAutoWalkEnabled = false; ProbeUseabilityFallbackEnabled= false; DumpSteepRoofEnabled = false;