UN-2 RESOLVED: GetMaxSpeed x4 is byte-verified retail; doc-comment was the misread

The register's UN-2 row recorded a contradiction: the GetMaxSpeed XML doc
claimed the bare run rate was retail-correct (~5.9 m/s catch-up, calling
the xRunAnimSpeed multiply a misread), while the implementation multiplied
by RunAnimSpeed citing ACE. Settled against the binary, not the pseudo-C:

- BN pseudo-C (acclient_2013_pseudo_c.txt:305127) renders get_max_speed as
  void with a bare `this->my_run_rate;` because it DROPS x87 instructions.
- Disassembling the PDB-matched v11.4186 binary at VA 0x00527cb0: all THREE
  return paths end `fld <rate>; fmul dword ptr [0x007C8918]; ret`, and the
  .rdata dword at 0x007C8918 is 4.0f. Sibling get_adjusted_max_speed
  (0x00527d00) carries the same trailing fmul. Verifier committed at
  tools/verify_un2_fmul.py (PE parse + byte decode, rerunnable).
- Retail paths: weenie null -> 1.0 x4; InqRunRate ok -> queried x4;
  InqRunRate failed -> my_run_rate x4. ACE MotionInterp.cs:665-676 matches.

Changes:
- Doc-comment rewritten: the implementation is retail-correct; the catch-up
  speed 2 x get_max_speed ~= 23.5 m/s at run 200 IS retail. The 1-Hz
  remote-blip symptom the old comment attributed to this multiply is
  therefore UNEXPLAINED by it (if it recurs: #41 family, not this).
- Weenie-null path aligned to retail's LITERAL 1.0 default (was MyRunRate).
- Tests re-pinned to the three retail paths (the old NoWeenie test pinned
  the non-retail fallback).
- Register: UN-2 row deleted per the retire rule (6 -> 5 UN rows);
  shortlist renumbered.

This is the 2nd confirmed instance of the BN x87-dropout artifact class
(memory: feedback_bn_decomp_field_names) deciding a register row.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-06-12 13:17:50 +02:00
parent f6a30f4aae
commit 0cb97aa594
4 changed files with 114 additions and 66 deletions

View file

@ -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),

View file

@ -935,52 +935,47 @@ public sealed class MotionInterpreter
// ── CMotionInterp::get_max_speed (0x00527cb0) ─────────────────────────────
/// <summary>
/// Return the run rate. Mirrors retail
/// <c>CMotionInterp::get_max_speed</c> at <c>0x00527cb0</c>.
/// Return the maximum movement speed in m/s: run rate × RunAnimSpeed (4.0).
/// Mirrors retail <c>CMotionInterp::get_max_speed</c> at <c>0x00527cb0</c>.
///
/// <para>
/// <b>Decomp (named-retail/acclient_2013_pseudo_c.txt:305127):</b>
/// <code>
/// 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
/// }
/// </code>
/// Binary Ninja shows the return type as <c>void</c> because the float
/// return rides the x87 FPU stack rather than EAX. Both branches
/// emit an <c>fld</c> of either <c>this_1</c> (the InqRunRate
/// out-param value) or <c>my_run_rate</c>, leaving the run rate on
/// ST0 as the return value.
/// <b>The ×4.0 is byte-verified retail (UN-2 resolved 2026-06-12).</b>
/// The Binary Ninja pseudo-C (named-retail/acclient_2013_pseudo_c.txt:305127)
/// renders this function as <c>void</c> with a bare <c>this->my_run_rate;</c>
/// statement because it drops x87 instructions — a known BN artifact class.
/// Disassembling the PDB-matched v11.4186 binary at VA <c>0x00527cb0</c>
/// shows all THREE return paths end with
/// <c>fmul dword ptr [0x007C8918]</c>, and the .rdata dword at
/// <c>0x007C8918</c> is <c>0x40800000</c> = 4.0f (the sibling
/// <c>get_adjusted_max_speed</c> 0x00527d00 carries the same trailing
/// fmul). Re-derive with <c>py tools/verify_un2_fmul.py</c>. 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).
/// </para>
///
/// <para>
/// <b>Critical:</b> this returns the BARE run rate (typically 1.0 to
/// ~3.0), NOT a velocity in m/s. We previously multiplied by
/// <c>RunAnimSpeed</c> to get a m/s value, reasoning that
/// <c>2 × bare_rate</c> would be too slow a catch-up speed for the
/// caller (<c>InterpolationManager::adjust_offset</c>). 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
/// (<c>InterpolationManager::adjust_offset</c> 0x00555d30, pc:353122)
/// is <c>2 × get_max_speed()</c> ≈ 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.
/// </para>
/// </summary>
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;
}

View file

@ -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);
}
}

40
tools/verify_un2_fmul.py Normal file
View file

@ -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('<I', data, 0x3C)[0]
nsec = struct.unpack_from('<H', data, pe_off + 6)[0]
opt_size = struct.unpack_from('<H', data, pe_off + 20)[0]
sec0 = pe_off + 24 + opt_size
imgbase = struct.unpack_from('<I', data, pe_off + 24 + 28)[0]
def va2off(va):
rva = va - imgbase
for i in range(nsec):
o = sec0 + i * 40
name = data[o:o + 8].rstrip(b'\x00').decode()
vsz, vaddr, rsz, roff = struct.unpack_from('<IIII', data, o + 8)
if vaddr <= rva < vaddr + max(vsz, rsz):
return roff + (rva - vaddr), name
return None, None
print('imgbase', hex(imgbase))
off, sec = va2off(0x00527CB0)
print('get_max_speed VA 0x527cb0 -> 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('<f', data, off2)[0])
off3, sec3 = va2off(0x007928B0)
print('dword @0x7928B0 sec', sec3, '=', struct.unpack_from('<f', data, off3)[0])
off4, _ = va2off(0x00527D00)
code4 = data[off4:off4 + 0x70]
print('get_adjusted_max_speed fmul count:', code4.count(FMUL))