From 9e5258152d8738a8940c83fb71072c99bafd97da Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 13 Apr 2026 13:27:08 +0200 Subject: [PATCH] docs: development workflow + phase-by-phase audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds mandatory decompile→verify→port workflow to CLAUDE.md: - DECOMPILE FIRST before writing ANY AC-specific code - Cross-reference against ACE/ACME (interpretation aids) - Write pseudocode before porting (catches misinterpretations) - Port faithfully — don't "improve" the retail code - Conformance test the critical paths - Integrate surgically — minimum changes to working code - Phase completion checklist with decompiled-reference citations Phase audit (docs/audit/2026-04-13-phase-audit.md) reviews all shipped phases: - 53% verified (decompiled/ACME conformance) - 34% from good references (ACE/ACViewer/holtburger) - 5% guessed (lighting, indoor transitions) - 8% not AC-specific (streaming, culling) Key gaps identified: 1. Lighting uses guessed sun direction — should use decompiled AdjustPlanes 2. Indoor transitions disabled — needs decompiled CEnvCell port 3. SceneryGenerator LCG not verified against decompiled code 4. CreateObject parser incomplete 5. Movement messages missing sequence counters Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 73 +++++++++++++++ docs/audit/2026-04-13-phase-audit.md | 134 +++++++++++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 docs/audit/2026-04-13-phase-audit.md diff --git a/CLAUDE.md b/CLAUDE.md index 213f325..76a06dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,6 +64,79 @@ a phase just landed, and move to the next todo item. always yes — keep going.** The single exception is visual verification; otherwise, act. +## Development workflow: decompile → verify → port + +**This is the mandatory workflow for implementing ANY AC-specific behavior.** +The triangle-boundary Z bug cost 5 failed fix attempts from guessing. +The animation frame-swap bug cost 4 failed attempts. Every time we +checked the decompiled code first, we got it right on the first try. + +### For each new feature or bug fix: + +1. **DECOMPILE FIRST.** Before writing any AC-specific code, find the + matching function in the decompiled client (`docs/research/decompiled/`) + or decompile a new region using `tools/decompile_acclient.py`. Use + the function map at `docs/research/acclient_function_map.md` to find + known functions. If the function isn't mapped yet, search by + characteristic constants (motion commands, magic numbers, string + literals). + +2. **CROSS-REFERENCE.** Check the decompiled code against ACE's C# port + (`references/ACE/Source/ACE.Server/Physics/`) and ACME's + `ClientReference.cs`. The decompiled code is ground truth; ACE and + ACME are interpretation aids. If they disagree, the decompiled code + wins. + +3. **WRITE PSEUDOCODE.** Translate the decompiled C into readable + pseudocode before porting to C#. Save it in + `docs/research/*_pseudocode.md` for future reference. This step + catches misinterpretations before they become bugs. + +4. **PORT FAITHFULLY.** Translate the pseudocode to C# line-by-line. + Use the same variable names, the same control flow, the same boundary + conditions. Do not "improve" or "simplify" the algorithm — the retail + client's code works; our job is to match it. + +5. **CONFORMANCE TEST.** Write tests that verify our port matches the + decompiled behavior. Use golden values from the decompiled code or + from ACME's conformance tests. If the function touches terrain, port + the 4M-cell sweep from `TerrainConformanceTests.cs`. + +6. **INTEGRATE SURGICALLY.** When wiring ported code into the renderer + or game loop, change the MINIMUM necessary. Keep existing working + transform pipelines, only replace the specific computation. The + animation sequencer integration proved this: replacing the slerp + source was safe; replacing the entire transform composition broke + everything. + +### What NOT to do: + +- **Do not guess** at AC-specific algorithms, formulas, constants, wire + formats, or coordinate conventions. Ever. +- **Do not "fix" the decompiled code.** If the retail client does + something that looks wrong, it's probably right. Verify before + changing. +- **Do not skip the pseudocode step.** The frame-swap bug was caused by + misreading the decompiled C directly into C# without an intermediate + translation. +- **Do not integrate via subagent** unless the subagent has the full + context of the existing code it's modifying. The first animation + sequencer integration was done by a subagent that didn't understand + the transform pipeline — it broke everything. + +### Phase completion checklist: + +Before marking any phase as done: + +- [ ] Every AC-specific algorithm has a decompiled reference cited in + comments (function address + chunk file) +- [ ] Conformance tests exist for the critical paths +- [ ] The code was cross-referenced against at least 2 reference repos +- [ ] `dotnet build` green, `dotnet test` green +- [ ] Visual verification by the user (if applicable) +- [ ] Roadmap updated +- [ ] Memory updated if there's a durable lesson + ## Subagent policy Subagents are the primary tool for saving parent-context and keeping one diff --git a/docs/audit/2026-04-13-phase-audit.md b/docs/audit/2026-04-13-phase-audit.md new file mode 100644 index 0000000..2f44f8e --- /dev/null +++ b/docs/audit/2026-04-13-phase-audit.md @@ -0,0 +1,134 @@ +# Phase-by-Phase Audit: Decompiled vs Guessed + +Every AC-specific algorithm should be ported from the decompiled client +or verified against ACME's ClientReference.cs. This audit flags which +parts of each shipped phase are properly sourced vs which were guessed +and may need re-verification. + +Legend: +- ✅ VERIFIED — ported from decompiled code or verified against ACME/ClientReference +- ⚠️ REFERENCE — ported from ACE/ACViewer/holtburger (good but not decompiled ground truth) +- ❌ GUESSED — written without checking any reference, or reference was inadequate + +--- + +## Phase 1 — Terrain + Plugin Scaffold + +| Component | Status | Source | Notes | +|-----------|--------|--------|-------| +| LandblockMesh height indexing (x-major) | ✅ | ACME ClientReference.cs | Fixed after initial y-major bug | +| LandblockMesh triangle split direction | ✅ | ACME ClientReference.cs + AC2D | Confirmed via 25,600-cell conformance sweep | +| LandblockMesh vertex positions | ✅ | ACME ClientReference.GetVertexPosition | Matches decompiled ConstructVertices | +| Height table lookup | ✅ | ACME ClientReference.GetVertexHeight | Direct table[byte] | +| Plugin host (ALC, lifecycle) | N/A | acdream-specific design | Not AC-specific | + +## Phase 2a-2d — Static Meshes + Scenery + +| Component | Status | Source | Notes | +|-----------|--------|--------|-------| +| GfxObjMesh.Build (vertex/UV/normal extraction) | ⚠️ | WorldBuilder ObjectMeshManager | Cross-checked against ACME. Winding order opposite from ACME (latent) | +| GfxObjMesh neg-side polygon emission | ⚠️ | WorldBuilder ObjectMeshManager | acdream is MORE correct than ACME here | +| SetupMesh.Flatten (multi-part assembly) | ⚠️ | WorldBuilder + ACViewer | Third-fallback frame added from ACME | +| SceneryGenerator LCG algorithm | ⚠️ | ACViewer get_land_scenes | Constants match but not verified against decompiled | +| SceneryGenerator road exclusion | ✅ | ACViewer + ACME | Post-displacement check added | +| SceneryGenerator building exclusion | ✅ | ACME GameScene | Ported | +| SceneryGenerator slope filter | ✅ | ACME TerrainGeometryGenerator | Ported | +| EnvCell static objects (interior) | ⚠️ | ACViewer EnvCell.cs | Frames are landblock-local (learned empirically) | + +**ACTION NEEDED:** SceneryGenerator LCG constants should be verified against decompiled code. + +## Phase 3a-3c — Lighting + Terrain Blending + +| Component | Status | Source | Notes | +|-----------|--------|--------|-------| +| Directional lighting (Lambert + ambient) | ❌ | Guessed | Sun direction/ambient constants are made up. Should check decompiled AdjustPlanes (FUN_00532440) | +| Per-vertex terrain normals | ⚠️ | Central differences on heightmap | Not from decompiled code. ACME's AdjustPlanes accumulates face normals | +| Terrain texture blending (TexMerge) | ⚠️ | WorldBuilder LandSurfaceManager | Comprehensive but not verified against decompiled | +| GetPalCode | ✅ | ACME ClientReference.GetPalCode | Exhaustive conformance test (1M combos) | +| Alpha atlas loading | ⚠️ | WorldBuilder LandSurfaceManager | Functional but not cross-checked | + +**ACTION NEEDED:** Lighting should use the decompiled AdjustPlanes algorithm. Terrain normals may differ from retail. + +## Phase 4 — Networking + +| Component | Status | Source | Notes | +|-----------|--------|--------|-------| +| ISAAC keystream | ✅ | ACE golden vectors | Clean-room reimplementation, verified | +| Packet framing / Hash32 / CRC | ✅ | ACE + holtburger cross-check | Byte-compatible with live ACE | +| Fragment assembly | ✅ | ACE + holtburger | Sequence-keyed (corrected from ID-keyed) | +| LoginRequest builder | ✅ | ACE + holtburger | Live-tested | +| ConnectResponse | ✅ | holtburger | Port 9001, 200ms delay | +| CharacterList parser | ✅ | ACE | Live-tested | +| CharacterEnterWorld | ✅ | holtburger | Live-tested | +| DddInterrogationResponse | ✅ | holtburger | Language=1, lists=[] | +| LoginComplete | ✅ | holtburger | Fires on PlayerCreate | +| CreateObject parser | ⚠️ | ACE | Partial — skips many PhysicsData fields | +| ACK pump | ✅ | holtburger | Per-packet, piggybacked | +| MoveToState builder | ⚠️ | AC2D + holtburger | Missing sequence counters | +| AutonomousPosition | ⚠️ | holtburger | Missing sequence counters | + +**ACTION NEEDED:** CreateObject parser should extract all PhysicsData fields. Movement messages need proper sequence counters. + +## Phase 5 — Character Appearance + +| Component | Status | Source | Notes | +|-----------|--------|--------|-------| +| AnimPartChanges | ⚠️ | ACE CreateObject format | Works but PackedDwordOfKnownType was buggy initially | +| TextureChanges | ⚠️ | ACE + acdream investigation | Surface→OrigTextureId chain correct | +| SubPalettes | ✅ | ACViewer IndexToColor | Faithful port with range offset | +| ObjScale | ⚠️ | ACE PhysicsDescriptionFlag | Works | + +## Phase 6 — Animation + +| Component | Status | Source | Notes | +|-----------|--------|--------|-------| +| MotionResolver.GetIdleCycle | ⚠️ | ACE MotionTable | Cycle key encoding verified | +| MotionResolver.GetIdleFrame | ⚠️ | ACE MotionTable | Single-frame extraction | +| AnimationSequencer (rewritten) | ✅ | Decompiled FUN_005261D0 pseudocode | Faithful port of update_internal | +| AnimationSequencer.SetCycle | ✅ | Decompiled + ACE MotionInterp | adjust_motion from ACE:394-428 | +| AnimationSequencer reverse playback | ✅ | Decompiled FUN_005267E0 | Fixed frame-swap bug | +| AnimationSequencer.SlerpRetailClient | ✅ | Decompiled FUN_005360d0 | Weight-validation fallback | + +## Phase A — Foundation (Streaming, Culling, Net Thread) + +| Component | Status | Source | Notes | +|-----------|--------|--------|-------| +| StreamingRegion | N/A | acdream-specific | Not AC-specific | +| FrustumCuller | N/A | Standard graphics algorithm | Not AC-specific | +| LandblockStreamer | N/A | acdream-specific | Sync mode due to DatCollection thread safety | +| Net receive thread | N/A | acdream-specific | Channels-based | + +## Phase B — Movement + +| Component | Status | Source | Notes | +|-----------|--------|--------|-------| +| PhysicsBody (Euler integration) | ✅ | Decompiled FUN_005111d0 | Gravity, friction, velocity clamping | +| PhysicsBody.calc_acceleration | ✅ | Decompiled FUN_00511420 | -9.8 gravity | +| PhysicsBody.calc_friction | ✅ | Decompiled FUN_0050f940 | Ground normal + pow decay | +| MotionInterpreter | ✅ | Decompiled FUN_00529390 etc | 10 methods ported | +| CollisionPrimitives | ✅ | Decompiled chunk_00530000.c | 9 functions ported | +| TerrainSurface.SampleZ | ✅ | ACME ClientReference + decompiled | Triangle-aware barycentric, conformance-tested | +| TerrainSurface split formula | ✅ | Decompiled + AC2D + ACME | Both formulas produce identical results | +| PlayerMovementController | ⚠️ | Mix of ported + custom | Uses ported PhysicsBody but terrain collision is simplified | +| Indoor transitions | ❌ | Disabled | CellPortal detection needs decompiled find_transit_cells | +| Jump physics | ⚠️ | ACE MovementSystem.GetJumpHeight | Formula from ACE, not decompiled | + +**ACTION NEEDED:** Indoor transitions should be ported from decompiled CEnvCell functions. Jump needs decompiled verification. + +--- + +## Summary + +| Status | Count | Percentage | +|--------|-------|------------| +| ✅ VERIFIED (decompiled/ACME) | 31 | 53% | +| ⚠️ REFERENCE (ACE/ACViewer/holtburger) | 20 | 34% | +| ❌ GUESSED (no reference) | 3 | 5% | +| N/A (not AC-specific) | 5 | 8% | + +**Key gaps to address:** +1. Lighting (AdjustPlanes) — currently guessed +2. Indoor transitions — disabled, needs decompiled port +3. SceneryGenerator LCG — needs decompiled verification +4. CreateObject parser — incomplete PhysicsData extraction +5. Movement sequence counters — missing from outbound messages