diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 91bde7ea..f7612b9a 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -167,7 +167,7 @@ accepted-divergence entries (#96, #49, #50). --- -## 5. Unclear (UN) — 6 rows +## 5. Unclear (UN) — 5 rows These rows have a missing, contradictory, or never-argued justification. They are the highest-priority audits: each needs either a recorded @@ -176,7 +176,6 @@ equivalence argument (promote to AD/AP) or a fix. | # | Divergence | Where (file:line) | Recorded justification (deficient) | Risk if assumption breaks | Retail oracle | |---|---|---|---|---|---| | UN-1 | `CheckOtherCells` iterates the overlap set SORTED by cell id; retail walks the CELLARRAY in build order — and the loop halts on the first non-OK result, so order is behavior-bearing | `src/AcDream.Core/Physics/CellTransit.cs:1718` | Justified only as "deterministic order for greppable probe logs" — no equivalence argument vs retail's array order recorded | A sphere straddling two cells that would each return a different non-OK result halts on a different cell than retail — different collision normal / slide direction at multi-cell straddles | `CTransition::check_other_cells` pc:272717-272798 | -| UN-2 | `GetMaxSpeed`: XML doc asserts the bare run rate is retail-correct (~5.9 m/s catch-up; the ×RunAnimSpeed multiply "a misread" → ~23.5 m/s), yet the implementation multiplies by RunAnimSpeed citing ACE as retail-verified. The two recorded justifications CONTRADICT — one describes the current code as known-wrong | `src/AcDream.Core/Physics/MotionInterpreter.cs:972` | None coherent — doc and code disagree about which behavior is retail | If the bare-rate reading is right, remote-entity catch-up runs ~4× retail speed — the multi-second 1-Hz blip / racing-remote symptom the doc itself records | `CMotionInterp::get_max_speed` pc:305127; catch-up :353122 | | UN-3 | AdminEnvirons fog-override RGB tints hardcoded with no retail constant cited (RedFog 0.60/0.05/0.05 etc.); Snapshot replaces fog COLOR only, keeping keyframe distances on an unverified assumption | `src/AcDream.Core/World/WeatherState.cs:350` | Enum semantics cite ACE EnvironChangeType + r12 §5.2; no source for the RGB values or the color-only override scope | A server-forced fog event renders the wrong hue and/or wrong density vs what retail clients showed for the same packet | AdminEnvirons 0xEA60; ACE EnvironChangeType.cs | | UN-4 | GfxObj double-sided/negative-surface handling keeps WB's legacy logic (cull-mode double-siding, no reversed-winding duplicate, different neg-surface predicate) while the CellStruct path follows the retail-cited `ConstructMesh` reading | `src/AcDream.App/Rendering/Wb/ObjectMeshManager.cs:1059` (CellStruct contrast :1396-1410) | No recorded justification on the GfxObj side — it is the unmodified WB extraction; the retail citation was added only to the CellStruct path | GfxObj models retail draws via duplicated-reversed-winding get wrong back-face lighting (normals not inverted) or missing/extra negative faces — dark or absent faces from behind | `D3DPolyRender::ConstructMesh` 0x0059dfa0 | | UN-5 | Run multiplier applied to backward (and strafe) speed while the wire reports speed 1.0; the 0.65 backward factor IS retail's, the runMul on top is justified only by feel ("~2.4× ratio felt wrong"); strafe cites holtburger, backward cites nothing | `src/AcDream.App/Input/PlayerMovementController.cs:909` | Feel fix (K-fix3); no retail citation for run-scaling backward movement | If retail does NOT run-scale backward, the local body moves up to ~2.4× faster backward than the wire declares — observers dead-reckon slower and see lag/teleport when backing up at run | adjust_motion FUN_00528010 (0.65 only); holtburger common.rs (sidestep) | @@ -192,20 +191,19 @@ phase-gated — they carry their trigger in their row and should land WITH that phase, not before. 1. **TS-20 — GfxObj DrawingBSP traversal (#113)** — phantom geometry is visible in Holtburg RIGHT NOW; the holistic port handoff already specs the fix; first diagnose the id filter against a door GfxObj. -2. **UN-2 — GetMaxSpeed contradiction** — the file argues against its own implementation; if the bare-rate reading is right, remote catch-up runs ~4× retail. Settle with one decomp re-read + a cdb catch-up trace; cheap to resolve, expensive to leave. -3. **TS-27 — Retransmit handling** — sole hard blocker for any non-loopback play; failure mode is silent permanent stalls (entities never spawn). Also fix the stale class-doc gap list while there. -4. **TS-4 — Path-6 steep slide-tangent shortcut** — landing/contact state diverges on every airborne-steep hit; the L.5+ retail-strict followup is already filed with the missing-ingredient analysis. -5. **UN-5 — Backward/strafe run multiplier** — potential ~2.4× local-vs-wire speed mismatch on a common input (S at run); one cdb session against retail answers it. -6. **UN-1 — CheckOtherCells iteration order** — behavior-bearing halt order with a log-cosmetics justification; trivial to fix (iterate CELLARRAY build order, sort only in probe output). -7. **TS-1 — PrecipiceSlide stop-at-edge** — visible movement mismatch at every cliff/roof edge; diagnostic already records which ingredient is missing. -8. **TS-22 — adjust_motion port** — active bug-class generator: any new `get_state_velocity` consumer during backward/strafe silently gets zero velocity. -9. **TS-26 — Position sequence freshness** — real-network correctness; pairs naturally with TS-27 in one transport-hardening pass. -10. **UN-6 — 200 ms ConnectResponse sleep** — unexplained constant on every login with an intermittent-failure shape; either find the ACE race and cite it, or replace with an acknowledged-ready check. -11. **UN-4 — GfxObj sides/negative-surface logic** — diagnose against the retail-cited CellStruct interpretation on a known double-sided GfxObj; promote to AP with a citation or align it. -12. **TS-8 — MagicUpdateEnchantment StatMod parse (#7/#12)** — vitals wrong for the whole session after any buff; parser shape is known from holtburger. -13. **TS-13 — CallPES/DefaultScript animation hooks** — the blocker comment is stale since C.1.5a shipped PhysicsScriptRunner; possibly a cheap wire-up now. -14. **UN-3 — AdminEnvirons tints** — invented RGB constants + unverified color-only scope; one decomp lookup against the 0xEA60 handler. -15. **TS-19 — Legacy ChaseCamera deletion** — already marked "pending the follow-up deletion commit"; its continued existence can mask or manufacture flap symptoms during debugging. +2. **TS-27 — Retransmit handling** — sole hard blocker for any non-loopback play; failure mode is silent permanent stalls (entities never spawn). Also fix the stale class-doc gap list while there. +3. **TS-4 — Path-6 steep slide-tangent shortcut** — landing/contact state diverges on every airborne-steep hit; the L.5+ retail-strict followup is already filed with the missing-ingredient analysis. +4. **UN-5 — Backward/strafe run multiplier** — potential ~2.4× local-vs-wire speed mismatch on a common input (S at run); one cdb session against retail answers it. +5. **UN-1 — CheckOtherCells iteration order** — behavior-bearing halt order with a log-cosmetics justification; trivial to fix (iterate CELLARRAY build order, sort only in probe output). +6. **TS-1 — PrecipiceSlide stop-at-edge** — visible movement mismatch at every cliff/roof edge; diagnostic already records which ingredient is missing. +7. **TS-22 — adjust_motion port** — active bug-class generator: any new `get_state_velocity` consumer during backward/strafe silently gets zero velocity. +8. **TS-26 — Position sequence freshness** — real-network correctness; pairs naturally with TS-27 in one transport-hardening pass. +9. **UN-6 — 200 ms ConnectResponse sleep** — unexplained constant on every login with an intermittent-failure shape; either find the ACE race and cite it, or replace with an acknowledged-ready check. +10. **UN-4 — GfxObj sides/negative-surface logic** — diagnose against the retail-cited CellStruct interpretation on a known double-sided GfxObj; promote to AP with a citation or align it. +11. **TS-8 — MagicUpdateEnchantment StatMod parse (#7/#12)** — vitals wrong for the whole session after any buff; parser shape is known from holtburger. +12. **TS-13 — CallPES/DefaultScript animation hooks** — the blocker comment is stale since C.1.5a shipped PhysicsScriptRunner; possibly a cheap wire-up now. +13. **UN-3 — AdminEnvirons tints** — invented RGB constants + unverified color-only scope; one decomp lookup against the 0xEA60 handler. +14. **TS-19 — Legacy ChaseCamera deletion** — already marked "pending the follow-up deletion commit"; its continued existence can mask or manufacture flap symptoms during debugging. **Phase-gated (do WITH the phase, flagged here so they aren't forgotten):** M2 combat must land TS-2 (BspOnlyDispatch terms), TS-5 (CanJump gating), diff --git a/src/AcDream.Core/Physics/MotionInterpreter.cs b/src/AcDream.Core/Physics/MotionInterpreter.cs index c82ce2b9..ec1006e1 100644 --- a/src/AcDream.Core/Physics/MotionInterpreter.cs +++ b/src/AcDream.Core/Physics/MotionInterpreter.cs @@ -935,52 +935,47 @@ public sealed class MotionInterpreter // ── CMotionInterp::get_max_speed (0x00527cb0) ───────────────────────────── /// - /// Return the run rate. Mirrors retail - /// CMotionInterp::get_max_speed at 0x00527cb0. + /// Return the maximum movement speed in m/s: run rate × RunAnimSpeed (4.0). + /// Mirrors retail CMotionInterp::get_max_speed at 0x00527cb0. /// /// - /// Decomp (named-retail/acclient_2013_pseudo_c.txt:305127): - /// - /// void get_max_speed(this) { - /// weenie_obj = this->weenie_obj; - /// this_1 = nullptr; - /// if (weenie_obj == 0) return; - /// if (weenie_obj->vtable->InqRunRate(&this_1) != 0) return; - /// this->my_run_rate; // x87 fld leaves my_run_rate on FPU stack - /// } - /// - /// Binary Ninja shows the return type as void because the float - /// return rides the x87 FPU stack rather than EAX. Both branches - /// emit an fld of either this_1 (the InqRunRate - /// out-param value) or my_run_rate, leaving the run rate on - /// ST0 as the return value. + /// The ×4.0 is byte-verified retail (UN-2 resolved 2026-06-12). + /// The Binary Ninja pseudo-C (named-retail/acclient_2013_pseudo_c.txt:305127) + /// renders this function as void with a bare this->my_run_rate; + /// statement because it drops x87 instructions — a known BN artifact class. + /// Disassembling the PDB-matched v11.4186 binary at VA 0x00527cb0 + /// shows all THREE return paths end with + /// fmul dword ptr [0x007C8918], and the .rdata dword at + /// 0x007C8918 is 0x40800000 = 4.0f (the sibling + /// get_adjusted_max_speed 0x00527d00 carries the same trailing + /// fmul). Re-derive with py tools/verify_un2_fmul.py. The three + /// retail paths: weenie_obj == null → 1.0×4; InqRunRate success → + /// queried×4; InqRunRate failure → my_run_rate×4. ACE's + /// MotionInterp.cs:665-676 ports it identically (RunAnimSpeed = 4.0f). /// /// /// - /// Critical: this returns the BARE run rate (typically 1.0 to - /// ~3.0), NOT a velocity in m/s. We previously multiplied by - /// RunAnimSpeed to get a m/s value, reasoning that - /// 2 × bare_rate would be too slow a catch-up speed for the - /// caller (InterpolationManager::adjust_offset). That was a - /// misread of the decomp — retail's catch-up IS that slow on purpose. - /// The multi-second 1-Hz blip the user reported when observing retail - /// remotes from acdream traced to body racing at the wrong (overshot) - /// catch-up speed (~23.5 m/s instead of the retail-correct ~5.9 m/s - /// for a run-skill-200 char). + /// Consequence: the dead-reckoning catch-up speed + /// (InterpolationManager::adjust_offset 0x00555d30, pc:353122) + /// is 2 × get_max_speed() ≈ 23.5 m/s for a run-rate-2.94 + /// (run-skill-200) character — that IS retail's value. An earlier + /// doc-comment here claimed the bare rate (~5.9 m/s catch-up) was + /// retail-correct and blamed the ×4 for the multi-second 1-Hz blip on + /// observed retail remotes; that reading trusted the BN x87 dropout + /// and is refuted by the binary. If the blip recurs, its root cause is + /// elsewhere (node-fail handling / progress-quantum abandonment / + /// position-queue feed — the #41 family), NOT this multiply. /// /// public float GetMaxSpeed() { - // Resolve current run rate: prefer WeenieObj.InqRunRate, fall back to MyRunRate. - // Then multiply by RunAnimSpeed (4.0). Matches ACE's MotionInterp.cs:670-678 - // which is verified against retail (the ACE MotionInterp file is a - // line-by-line port). Returns the maximum world-space velocity in m/s - // — for run skill 200 with rate ≈ 2.94, this is ≈ 11.76 m/s. Used by - // InterpolationManager.AdjustOffset to compute the catch-up speed - // (= 2 × maxSpeed). - float rate = MyRunRate; - if (WeenieObj is not null && WeenieObj.InqRunRate(out float queried)) - rate = queried; + // Retail 0x00527cb0: weenie null → 1.0; InqRunRate ok → queried; + // InqRunRate failed → my_run_rate. Every path × RunAnimSpeed (4.0, + // .rdata 0x007C8918). Note the weenie-null default is the LITERAL 1.0 + // (.rdata 0x007928B0), not my_run_rate. + float rate = 1.0f; + if (WeenieObj is not null && !WeenieObj.InqRunRate(out rate)) + rate = MyRunRate; return RunAnimSpeed * rate; } diff --git a/tests/AcDream.Core.Tests/Physics/MotionInterpreterTests.cs b/tests/AcDream.Core.Tests/Physics/MotionInterpreterTests.cs index 251b0570..e2c4f896 100644 --- a/tests/AcDream.Core.Tests/Physics/MotionInterpreterTests.cs +++ b/tests/AcDream.Core.Tests/Physics/MotionInterpreterTests.cs @@ -845,15 +845,14 @@ public sealed class MotionInterpreterTests [InlineData(MotionCommand.RunForward)] public void GetMaxSpeed_IgnoresForwardCommand_AlwaysReturnsRunRate(uint command) { - // GetMaxSpeed is the InterpolationManager.AdjustOffset catch-up speed — it deliberately - // returns RunAnimSpeed × run-rate REGARDLESS of the current ForwardCommand (see GetMaxSpeed's - // doc comment: the bare run rate × RunAnimSpeed, ACE MotionInterp.cs:670-678, retail-verified - // — the slow catch-up is intentional, it fixed the 1-Hz remote-blip). It does NOT branch - // per-command. These previously asserted a REMOVED command-branching design (WalkForward → - // WalkAnimSpeed, WalkBackward → ×0.65, Idle → 0); that contract no longer exists, so they are - // consolidated here to PIN the no-branch contract across commands (Phase W green-tests triage). - var interp = MakeInterp(); - interp.MyRunRate = 1.75f; + // GetMaxSpeed is the InterpolationManager.AdjustOffset catch-up speed — it + // returns RunAnimSpeed × run-rate REGARDLESS of the current ForwardCommand + // (retail 0x00527cb0 never reads interpreted_state; UN-2 byte verification + // 2026-06-12, tools/verify_un2_fmul.py). These previously asserted a REMOVED + // command-branching design (WalkForward → WalkAnimSpeed, WalkBackward → + // ×0.65, Idle → 0); they PIN the no-branch contract across commands. + var weenie = new FakeWeenie { RunRate = 1.75f }; + var interp = MakeInterp(weenie: weenie); interp.InterpretedState.ForwardCommand = command; float speed = interp.GetMaxSpeed(); @@ -862,17 +861,33 @@ public sealed class MotionInterpreterTests } [Fact] - public void GetMaxSpeed_RunForward_NoWeenie_FallsBackToMyRunRate() + public void GetMaxSpeed_NoWeenie_ReturnsLiteralOneTimesRunAnimSpeed() { - // WeenieObj is null (MakeInterp with no weenie argument); MyRunRate - // is set explicitly. GetMaxSpeed must use MyRunRate as the run-rate - // source when InqRunRate is unavailable. + // Retail 0x00527cb0 weenie_obj == null path: fld 1.0 (.rdata 0x007928B0), + // fmul 4.0 (.rdata 0x007C8918) — the LITERAL 1.0, NOT my_run_rate (UN-2 + // byte verification 2026-06-12). MyRunRate is set to a different value to + // prove it is not consulted on this path. var interp = MakeInterp(); interp.MyRunRate = 1.75f; interp.InterpretedState.ForwardCommand = MotionCommand.RunForward; float speed = interp.GetMaxSpeed(); + Assert.Equal(MotionInterpreter.RunAnimSpeed * 1.0f, speed, precision: 4); + } + + [Fact] + public void GetMaxSpeed_InqRunRateFails_FallsBackToMyRunRate() + { + // Retail 0x00527cb0 InqRunRate-failure path: fld [esi+0x7c] (my_run_rate), + // fmul 4.0. The InqRunRate out-value is discarded on failure. + var weenie = new FakeWeenie { RunRate = 9.9f, InqRunRateResult = false }; + var interp = MakeInterp(weenie: weenie); + interp.MyRunRate = 1.75f; + interp.InterpretedState.ForwardCommand = MotionCommand.RunForward; + + float speed = interp.GetMaxSpeed(); + Assert.Equal(MotionInterpreter.RunAnimSpeed * 1.75f, speed, precision: 4); } } diff --git a/tools/verify_un2_fmul.py b/tools/verify_un2_fmul.py new file mode 100644 index 00000000..d5b6eb8c --- /dev/null +++ b/tools/verify_un2_fmul.py @@ -0,0 +1,40 @@ +# UN-2 verification: prove/disprove that retail CMotionInterp::get_max_speed +# (VA 0x00527cb0) multiplies by the 4.0f constant at VA 0x007C8918 on its +# return paths (the fmul the BN pseudo-C drops). Throwaway apparatus. +import struct + +p = r"C:\Turbine\Asheron's Call\acclient.exe" +data = open(p, 'rb').read() + +pe_off = struct.unpack_from(' file', hex(off), 'sec', sec) +code = data[off:off + 0x50] +print('bytes:', code.hex()) +FMUL = bytes.fromhex('d80d18897c00') # fmul dword ptr [0x007C8918] +print('fmul [0x7C8918] count in get_max_speed:', code.count(FMUL)) + +off2, sec2 = va2off(0x007C8918) +print('dword @0x7C8918 sec', sec2, '=', struct.unpack_from('