review(physics): P1 Opus review APPROVE - UN-8 retired by byte decode; PK-timer semantics recorded for P3

All seven review lenses pass. CanJump's polarity is upgraded from
plausibility to proof: raw bytes of 0x00591b50 show fld load / fcomp
[0x007c5e24 = 2.0f] / test ah,5 / jp -> return 0, i.e. return 1 iff
load < 2.0 with unordered refusing - exactly the shipped code, NaN edge
included. UN-8 deleted. CACQualities::JumpStaminaCost's pk flag decoded
for P3: PlayerKillerStatus in {4,0x40} AND PropertyFloat 0x91 + 20 s >=
now.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-07-30 08:32:06 +02:00
parent f30b90f5c1
commit 001e466d42
3 changed files with 47 additions and 5 deletions

View file

@ -284,7 +284,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps.
---
## 5. Unclear (UN) — 5 rows (UN-8 filed 2026-07-30 at Campaign P Slice P1)
## 5. Unclear (UN) — 4 rows (UN-8 retired 2026-07-30 by the P1 Opus review: CanJump polarity byte-proven `load < 2.0` from the PDB-paired binary — fld/fcomp [0x007c5e24=2.0f]/test ah,5/jp; unordered refuses. Evidence: stat-coupled pseudocode doc §12)
These rows have a missing, contradictory, or never-argued justification.
They are the highest-priority audits: each needs either a recorded
@ -296,7 +296,6 @@ equivalence argument (promote to AD/AP) or a fix.
| 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-6 | Fixed 200 ms sleep between ConnectRequest and ConnectResponse; retail inserts no delay. Annotated only as "with 200ms race delay"; the 2026-06-04 audit flagged it, the follow-up refuted "forbidden workaround" but wrote no fuller rationale back | `src/AcDream.Core.Net/WorldSession.cs:484` | Presumed ACE port+1 listener race guard — four words, no citation | Every login eats a flat 200 ms; if the race needs longer on a loaded server, the handshake fails intermittently (ConnectResponse ignored → CharacterList never arrives, exit-29 shape) with no retry — a timing constant masking an unconfirmed root cause | (none recorded) |
| UN-7 | Outdoor OBJECT point lighting uses `calc_point_light` (wrap/norm + per-channel cap, `~1/d²`) for ALL meshes including static buildings, but retail's object path is unconfirmed — `config_hardware_light` (0x0059ad30) sets D3D-FF point lights (`Diffuse=color×intensity`, `Attenuation=(0,1,0)``1/d`, `Range=falloff×1.5`, `material.diffuse=white`) yet that math would blow walls WHITE while retail stays DIM, so static buildings may instead use the `SetStaticLightingVertexColors` bake. Model + the brightness-scaling factor both UNRESOLVED (issue #140 / Fix D) | `src/AcDream.App/Rendering/Shaders/mesh_modern.vert` (`pointContribution`); `src/AcDream.Core/Lighting/LightManager.cs` (`SelectForObject`) | Fix A/B ported calc_point_light + per-object selection for objects without confirming retail uses that model for static buildings; cdb captured the D3D-FF path but it contradicts the observed dim result | Outdoor buildings blow out warm near torches (the #140 meeting-hall symptom); whichever model is wrong, the object torch contribution is too strong | `config_hardware_light` 0x0059ad30; `SetStaticLightingVertexColors` 0x0059cfe0; `rangeAdjust=1.5` 0x00820cc4 — see docs/research/2026-06-18-lighting-a7-fixABC-shipped-fixD-handoff.md |
| UN-8 | `CACQualities::CanJump`'s (0x00591b50) x87-mush comparison against the 2.0-load threshold was resolved by DOMAIN PLAUSIBILITY, not a literal BN read — Campaign P Slice P1 (2026-07-30) ported `load < 2.0` (can jump under 200% burden), the polarity a normal AC player's experience requires and that coincides with `LoadMod`'s own floor, over BN's literal (backwards) reading. ACE gives no tiebreaker (its `WeenieObject.CanJump` is an unconditional `true` stub); Ghidra MCP was unavailable this slice | `src/AcDream.Core/Physics/PlayerWeenie.cs` (`CanJump`, `CanJumpLoadThreshold`) | Plausibility argument recorded in the pseudocode doc §3, not a verified decompile; the exact x87 flag-synthesis for the FOLLOWING `test ah,mask` interpretation is the documented BN "bitfield mush" artifact class | If the polarity is actually backwards, CanJump refuses jumps under 200% load (breaking ordinary play) instead of only refusing severe overload — would surface immediately in the P1 visual matrix scenario 1 (barely moves/jumps near 200%) | `CACQualities::CanJump` 0x00591b50 pc 412907; `docs/research/2026-07-30-stat-coupled-movement-pseudocode.md` §3 |
---

View file

@ -385,3 +385,43 @@ every pre-P1 `PlayerWeenieTests.cs` expectation unchanged.
- `LiveSessionEventRouterTests` (Runtime, if a harness exists) or a focused
new test: ObjectTable burden-trigger events recompute and push burden;
Stamina vital change pushes CurrentStamina.
## 12. P1 Opus-review addenda (2026-07-30, post-implementation)
### 12a. UN-8 RETIRED — CanJump polarity byte-proven
Raw bytes of `CACQualities::CanJump @ 0x00591b50` in the PDB-paired
v11.4186 binary (technique: `reference_pe_byte_decode`):
```
e8 ca d5 ff ff call InqLoad (0x0058f130)
85 c0 / 74 1a test eax,eax; jz return0 ; load unknowable -> 0
d9 44 24 00 fld dword [esp] ; st0 = load
d8 1d 24 5e 7c 00 fcomp dword [0x007c5e24] ; vs 2.0f (verified read)
df e0 fnstsw ax
f6 c4 05 test ah, 0x05 ; C0|C2
7a 09 jp return0 ; PF=1 on {neither, both}
b8 01 00 00 00 mov eax, 1 ; fall-through: C0 only
```
`test ah,5` result parity: `0x00` (load ≥ 2.0, incl. ==) → PF=1 → 0;
`0x01` (load < 2.0) PF=0 1; `0x05` (unordered) PF=1 0.
**`CanJump = (load < 2.0f)`; NaN/unordered refuses.** The shipped
`_burden < CanJumpLoadThreshold` matches exactly, including the NaN edge.
### 12b. PK-timer jump-cost semantics (for Slice P3 / TS-23)
`CACQualities::JumpStaminaCost @ 0x00591b90` (pc 412934-412968), fully
readable: the `pk` flag passed to `MovementSystem::JumpStaminaCost` is
```
pk = InqInt(0x86 /*134 PlayerKillerStatus*/, default 8) in {4 /*PK*/, 0x40 /*PKLite*/}
&& InqFloat(0x91 /*145*/) succeeded
&& (that_float + 20.0) >= Timer::cur_time
```
i.e. PK/PKLite status AND a 20-second recency window on PropertyFloat
0x91. The P3 implementer should plumb exactly this pair alongside the
mover-flag work; `MovementSystem.JumpStaminaCost`'s pk branch
(`(int)((power + 1) * 100)`, ACE-derived — the retail branch is an
elided `_ftol2` tailcall) is already in place.

View file

@ -31,9 +31,12 @@ public sealed class PlayerWeenie : IWeenieObject
{
/// <summary>
/// Retail <c>CACQualities::CanJump</c>'s hard burden gate (0x00591b50,
/// pc 412907) — x87 mush, polarity resolved by domain plausibility
/// (register row UN-8; Ghidra MCP was unavailable to confirm). Chosen
/// to coincide with <see cref="EncumbranceSystem.LoadMod"/>'s own floor.
/// pc 412907) — polarity PROVEN by raw byte decode of the PDB-paired
/// binary (P1 Opus review, 2026-07-30): <c>fld load; fcomp [0x007c5e24
/// = 2.0f]; fnstsw; test ah,5; jp → return 0</c>, i.e. return 1 iff
/// load &lt; 2.0 (≥ 2.0 or unordered/NaN refuses). Former register row
/// UN-8 retired on this evidence; see the byte decode in
/// <c>docs/research/2026-07-30-stat-coupled-movement-pseudocode.md</c> §12.
/// </summary>
public const float CanJumpLoadThreshold = 2.0f;