From 61e959169bb9a5ec733461a29c7b2f0b890484ab Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 30 Jul 2026 19:22:41 +0200 Subject: [PATCH] fix #266: retail run-rate 800 branch is exact-equality sentinel, not a cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raw byte decode of MovementSystem::GetRunRate (0x006b0950, PDB-paired binary): fild skill; fcom [800f]; fnstsw; test ah, 0x44; jp general — the C2/C3 parity idiom whose 18/4 fall-through executes ONLY at skill == 800 exactly. ACE read this as >= 800 ('max run speed?') and Campaign P P1 inherited that misread when BN dropped the arithmetic, flat-lining every maxed character at 4.5 (retail-true ~3.70, +21%) and erasing the vitae differential (both 10200 and 15225 sat above 800). The [stat-chain] live capture proved the enchant chain correct end to end (vitae 0.67 -> eff run 10200 -> controller), isolating the formula. General path byte-verified: (loadMod*(skill/(skill+200)*11)+4)/scaling/4. InqMaxRunRate's skill=9999 probe gets ~3.6961, not 4.5. Golden tests pin the 799/800/801 straddle and the maxed-skill vitae differential; pseudocode doc §6 carries the decode plus a do-not- reimport-ACE warning. Complete Release suite: 10,025 passed / 5 skips. Co-Authored-By: Claude Opus 5 --- docs/ISSUES.md | 34 ++++++++++++------- ...-07-30-stat-coupled-movement-pseudocode.md | 29 +++++++++++----- src/AcDream.Core/Physics/MovementSystem.cs | 23 ++++++++++--- src/AcDream.Core/Physics/PlayerWeenie.cs | 3 +- .../Physics/MovementSystemTests.cs | 24 +++++++++++-- 5 files changed, 85 insertions(+), 28 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 01cffdbe..585a0bf9 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -156,22 +156,30 @@ first, don't guess the format string. ## #266 — Local player faster than a comparable retail character -**Status:** OPEN — needs a controlled comparison capture +**Status:** CLOSED 2026-07-30 — root cause: ACE-inherited `>= 800` misread +of retail's exact-equality run-rate sentinel **Severity:** HIGH (matrix live gate 2026-07-30; core speed parity) -**Component:** movement / stat chain (Campaign P P1) / outbound speed +**Component:** movement / `MovementSystem.GetRunRate` -**Symptom (user report):** side-by-side, the acdream character runs -visibly faster than a character on retail. Confounds to rule out first: -(a) unequal run skills/vitae between the two characters; (b) whether the -comparison retail char had buffs. Diagnosis plan: log both chars' run -skill + vitae + computed runRate; capture our GetRunRate inputs -(effective skill after EnchantSkill, burden) at the moment of comparison; -verify ACE's echoed ForwardSpeed matches what we apply locally -(ApplyServerRunRate) and what retail applies for the same skill. Suspect -list: effective-skill vitae application on the LOCAL run path, -run-rate-cap difference, or an uneven comparison. +**Root cause:** retail `MovementSystem::GetRunRate` (0x006b0950) returns +18/4 = 4.5 ONLY when runSkill == 800 EXACTLY (byte-decoded `fcom [800f]; +test ah, 0x44; jp` — the C2/C3 parity equality idiom; <, >, and unordered +all take the general formula). ACE misread the same x87 mush as +`>= 800` ("max run speed?") and our P1 port inherited it via the +ACE cross-reference. Every maxed character therefore ran a flat 4.5 +(retail-true ~3.70) — ~21% too fast and completely vitae-independent, +because both the vitae-reduced and unreduced skill sat above 800. The +controlled comparison (33%-vitae +Acdream vs 5%-vitae +Je, both maxed) +showed acdream faster while retail runs them within ~0.4% — exactly the +formula's prediction. The vitae/enchantment chain itself was verified +intact end-to-end via [stat-chain] live capture (vitae 0.67 installed → +eff run 10200 → applied to controller). ---- +**Fix:** `==` restores the general formula for all non-800 skills; +golden tests pin 799/800/801 straddle + the maxed-skill vitae +differential; `docs/research/2026-07-30-stat-coupled-movement-pseudocode.md` +§6 corrected with the full byte decode and an explicit "do not re-import +ACE's >= reading" warning. ## #265 — Steep-slope response set: uphill-jump bounce, roof slides lost, edge wedge (TS-4 removal fallout — REVERTED) diff --git a/docs/research/2026-07-30-stat-coupled-movement-pseudocode.md b/docs/research/2026-07-30-stat-coupled-movement-pseudocode.md index 77b9cb65..1e0496c0 100644 --- a/docs/research/2026-07-30-stat-coupled-movement-pseudocode.md +++ b/docs/research/2026-07-30-stat-coupled-movement-pseudocode.md @@ -204,7 +204,7 @@ AP-127): `GetRunRate` (0x006b0950) and the `arg3!=0` (PK) branch of `JumpStaminaCost` (0x006b0a40) have their GENERAL-CASE arithmetic entirely dropped by BN (only -the `EncumbranceSystem::LoadMod`/`800`-skill-cap calls and the `arg3==0` +the `EncumbranceSystem::LoadMod`/`800`-skill-compare calls and the `arg3==0` ceil expression survive uncollapsed — the same information-loss class as the x87 mush, just total rather than partial). **ACE is the cross-reference tiebreaker for those two spots** (`references/ACE/Source/ACE.Server/Physics/ @@ -213,13 +213,26 @@ citation style (`PlayerWeenie.cs`'s pre-P1 doc comments already said "decompiled + ACE MovementSystem" for these two formulas — nothing new here, just now with a named-decomp address alongside): -- `GetRunRate(load, skill, scaling) = skill>=800 ? 18/4 : ((LoadMod(load) * (skill/(skill+200)*11) + 4) / scaling) / 4` — - matches acdream's pre-existing `PlayerWeenie.GetRunRate` exactly (which - hardcoded `scaling=1`); the true retail signature carries a 3rd - `scaling` arg (confirmed by the decomp's own function signature), and - every known call site (`InqMaxRunRate`, `InqRunRate`) passes `1f` — so - porting the full signature is free (no behavior change), just closer to - the retail surface for future callers. +- `GetRunRate(load, skill, scaling) = skill==800 ? 18/4 : ((LoadMod(load) * (skill/(skill+200)*11) + 4) / scaling) / 4` — + **§12c correction (#266, 2026-07-30): the 800 branch is EXACT EQUALITY, + not `>=`.** Raw byte decode of 0x006b0950 (PDB-paired binary): + `fild skill; fcom [0x00803b94 = 800f]; fnstsw ax; test ah, 0x44; jp + 0x6b097f` — the C2/C3 parity idiom in which `jp` (general path) fires + for `<`, `>`, AND unordered; the `fld [18f]; fdiv [4f]; ret` fall-through + executes only when C3=1/C2=0, i.e. skill == 800 exactly. The general + path decodes instruction-by-instruction to + `(LoadMod(load) * (skill/(skill+200)*11) + 4) / scaling / 4` + (constants 200f @0x00803b8c, 11f @0x00803b88, 4f @0x007c6174, /scaling + from `[esp+0xc]`, final /4f @0x00803b80). **ACE's `>= 800` "max run + speed?" reading is a misread of the same mush and must not be used as a + tiebreaker here** — it flat-lined every maxed character at 4.5 (retail + general formula gives ~3.70) and erased the vitae speed differential + (#266: 33%-vitae +Acdream visibly outran 5%-vitae +Je in acdream while + retail runs them within ~0.4%). `InqMaxRunRate`'s skill=9999 probe gets + the general formula (~3.6961), not 4.5. The true retail signature + carries a 3rd `scaling` arg (confirmed by the decomp's own function + signature), and every known call site (`InqMaxRunRate`, `InqRunRate`) + passes `1f`. - `GetJumpHeight(load, skill, extent, scaling)` — BN's extent-clamp micro-branch (pc 006b09b0-006b09ca) is the SAME x87-mush pattern as §3; ACE's `Math.Clamp(extent, 0, 1)` is the tiebreaker (matches the EXISTING diff --git a/src/AcDream.Core/Physics/MovementSystem.cs b/src/AcDream.Core/Physics/MovementSystem.cs index 6cf7e020..0b378cb3 100644 --- a/src/AcDream.Core/Physics/MovementSystem.cs +++ b/src/AcDream.Core/Physics/MovementSystem.cs @@ -25,13 +25,28 @@ namespace AcDream.Core.Physics; public static class MovementSystem { /// - /// MovementSystem::GetRunRate 0x006b0950. Retail-verified 800-skill - /// cap (InqMaxRunRate passes skill=9999 to reach this cap); general - /// case ACE-cross-referenced (BN dropped the arithmetic, see class doc). + /// MovementSystem::GetRunRate 0x006b0950. The 800 branch is an + /// EXACT-EQUALITY sentinel, not a cap: raw byte decode of the PDB-paired + /// binary (#266, 2026-07-30) shows fild skill; fcom [800f]; fnstsw; + /// test ah, 0x44; jp general — the C2/C3 parity idiom that falls + /// through to 18/4 ONLY when skill == 800 (<, >, and + /// unordered all take the general path). InqMaxRunRate (0x00591b20) + /// passes skill=9999 and therefore gets the general formula's ~3.6961, + /// NOT 4.5. The general-path arithmetic is byte-verified + /// instruction-by-instruction: (loadMod * (skill/(skill+200) * 11) + + /// 4) / scaling / 4. /// + /// + /// ACE's MovementSystem.GetRunRate reads this branch as + /// >= 800 ("max run speed?") — that is ACE's misread of the + /// same x87 mush, NOT a tiebreaker. Porting >= made every + /// maxed character run a flat 4.5 (vs retail's ~3.70) and erased the + /// vitae speed differential entirely (#266). Do not "fix" this back + /// from ACE. + /// public static float GetRunRate(float burden, int runSkill, float scaling = 1f) { - if (runSkill >= 800) + if (runSkill == 800) return 18f / 4f; float loadMod = EncumbranceSystem.LoadMod(burden); diff --git a/src/AcDream.Core/Physics/PlayerWeenie.cs b/src/AcDream.Core/Physics/PlayerWeenie.cs index 8a895a49..6740a8ef 100644 --- a/src/AcDream.Core/Physics/PlayerWeenie.cs +++ b/src/AcDream.Core/Physics/PlayerWeenie.cs @@ -200,7 +200,8 @@ public sealed class PlayerWeenie : IWeenieObject /// /// RunRate = (burdenMod * (runSkill / (runSkill + 200)) * 11 + 4) / 4. - /// Capped at 4.5 when runSkill >= 800. Thin forwarder to + /// Returns 4.5 only for the retail skill == 800 sentinel (byte-decoded + /// exact-equality branch, #266 — NOT a cap). Thin forwarder to /// — kept for source /// compatibility with existing golden-value tests. /// diff --git a/tests/AcDream.Core.Tests/Physics/MovementSystemTests.cs b/tests/AcDream.Core.Tests/Physics/MovementSystemTests.cs index cd9e6773..9b339a30 100644 --- a/tests/AcDream.Core.Tests/Physics/MovementSystemTests.cs +++ b/tests/AcDream.Core.Tests/Physics/MovementSystemTests.cs @@ -1,3 +1,4 @@ +using System; using AcDream.Core.Physics; using Xunit; @@ -11,10 +12,29 @@ namespace AcDream.Core.Tests.Physics; public class MovementSystemTests { [Fact] - public void GetRunRate_Skill800Cap_Returns4Point5() + public void GetRunRate_Skill800Sentinel_IsExactEqualityOnly() { + // Retail 0x006b0950 byte decode (#266): fcom [800f]; test ah, 0x44; + // jp — the 18/4 return executes ONLY when skill == 800 exactly. + // 799 and 801 take the general formula on either side of it. Assert.Equal(4.5f, MovementSystem.GetRunRate(0f, 800), precision: 5); - Assert.Equal(4.5f, MovementSystem.GetRunRate(0f, 999999), precision: 5); + Assert.Equal(3.1994f, MovementSystem.GetRunRate(0f, 799), precision: 3); + Assert.Equal(3.2005f, MovementSystem.GetRunRate(0f, 801), precision: 3); + } + + [Fact] + public void GetRunRate_MaxedSkills_UseGeneralFormula_VitaeDifferentiates() + { + // #266 regression: +Acdream at 33% vitae (eff 10200) vs +Je at 5% + // vitae (eff 14463) — retail runs them within ~0.4% of each other + // (~3.70 vs ~3.71), NOT at a flat 4.5. The old ">= 800 cap" + // misread made both saturate identically and erased vitae entirely. + // Tolerance asserts (not precision:) — 3.7125 straddles a 3-decimal + // rounding boundary, the same xunit footgun as the AP-7 case. + Assert.True(MathF.Abs(MovementSystem.GetRunRate(0f, 10200) - 3.6971f) < 1e-3f); + Assert.True(MathF.Abs(MovementSystem.GetRunRate(0f, 14463) - 3.7125f) < 1e-3f); + // InqMaxRunRate's skill=9999 probe: general formula, not 4.5. + Assert.True(MathF.Abs(MovementSystem.GetRunRate(0f, 9999) - 3.6961f) < 1e-3f); } [Fact]