diff --git a/CLAUDE.md b/CLAUDE.md index f462630..15fd61b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -698,20 +698,34 @@ inn door, click NPC, pick up item. Freeze list active — M1's phases are off-limits until M7 polish. Writeup at top of M1 block in `docs/plans/2026-05-12-milestones.md`. -**Currently working toward: M1.5 — "Indoor world feels right."** Opened -2026-05-20 after continued indoor testing surfaced a deep family of -physics + lighting bugs that span buildings AND dungeons. Demo -scenario: enter the Holtburg Sewer through the in-town portal, -navigate through (walls block, stairs work, items block, lighting -reads correctly), exit back to town. Phases to ship: A6 (Indoor -physics fidelity, cdb-driven) + A7 (Indoor lighting fidelity, -RenderDoc + retail-decomp driven). Issues in scope: #80, #81, #83, -#88, #90 (workaround removal), L-indoor, L-spotlight, stairs, -2nd-floor, cellar, and the `TryFindIndoorWalkablePlane` synthesis -removal. Estimated 3–5 weeks calendar work. **M2 ("Kill a drudge") -is deferred until M1.5 lands** — drudges live in dungeons and the -M2 demo target requires solid indoor navigation. Full M1.5 writeup -at the corresponding block in `docs/plans/2026-05-12-milestones.md`. +**Currently working toward: Phase O — DatPath Unification** (active as +of 2026-05-21, by user direction — pre-empts M1.5). Tagline: "ONE +thing touches the DATs." We currently run two dat readers in process +(our `DatCollection` + WorldBuilder's `DefaultDatReaderWriter`); this +phase extracts the WB pieces we actually use into +`src/AcDream.Core/Rendering/Wb/`, swaps their dat dependency to our +`DatCollection`, and drops the `WorldBuilder.Shared` + +`Chorizite.OpenGLSDLBackend` project references. WB stays in +`references/` as a read-reference, not as a project dependency. +Verbatim copy first, no "improvements" while extracting. Spec: +[`docs/superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md`](docs/superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md). +Estimated 7-8 working days, ships as one merge window. + +**M1.5 — PAUSED for Phase O.** Indoor walking work is held at the +2026-05-20 baseline (described below) and resumes once Phase O lands. +M1.5's Phase A6/A7 don't touch dat infrastructure, so no rework will +be needed. Demo scenario: enter the Holtburg Sewer through the +in-town portal, navigate through (walls block, stairs work, items +block, lighting reads correctly), exit back to town. Phases to ship +after Phase O: A6 (Indoor physics fidelity, cdb-driven) + A7 (Indoor +lighting fidelity, RenderDoc + retail-decomp driven). Issues in +scope: #80, #81, #83, #88, #90 (workaround removal), L-indoor, +L-spotlight, stairs, 2nd-floor, cellar, and the +`TryFindIndoorWalkablePlane` synthesis removal. **M2 ("Kill a +drudge") is deferred until M1.5 lands** — drudges live in dungeons +and the M2 demo target requires solid indoor navigation. Full M1.5 +writeup at the corresponding block in +`docs/plans/2026-05-12-milestones.md`. **Today's pre-M1.5 baseline (2026-05-20).** Five surgical fixes shipped to close the user-reported "logged in inside the inn, ran diff --git a/docs/plans/2026-04-11-roadmap.md b/docs/plans/2026-04-11-roadmap.md index ebe7ce4..547699f 100644 --- a/docs/plans/2026-04-11-roadmap.md +++ b/docs/plans/2026-04-11-roadmap.md @@ -85,7 +85,63 @@ Plus polish that doesn't get its own phase number: ## Phases ahead — agreed order -### Milestone M1.5 — "Indoor world feels right" (active, opened 2026-05-20) +### Phase O — DatPath Unification (ACTIVE — pre-empts M1.5 by user direction 2026-05-21) + +**Tagline:** ONE thing touches the DATs. + +**Filed:** 2026-05-21. **Status:** active. **Spec:** [`docs/superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md`](../superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md). + +**Problem:** As of Phase N.4 (2026-05-08) we run TWO dat readers in +process — our `DatCollection` and WorldBuilder's +`DefaultDatReaderWriter` (constructed inside `WbMeshAdapter.cs:79`). +Both open the same four `.dat` files independently. Costs: +~50-100 MB duplicated index cache, double seeks on shared blocks, and +the cross-check awkwardness in `WbMeshAdapter.cs:224-229`. AC community +feedback flagged WB's dat-touching as "built for tools, not runtime"; +the double-reader is the most concrete manifestation. + +**Approach:** Extract the WB pieces we actually use (mesh pipeline, +texture decode, GL state, scenery, terrain blending, EnvCell/portal +decode — roughly 3-5K LOC) into `src/AcDream.Core/Rendering/Wb/`. +Verbatim copy first, no "improvements" while extracting. Adapt +constructors to consume our `DatCollection` instead of +`IDatReaderWriter`. Drop the two project references +(`WorldBuilder.Shared` + `Chorizite.OpenGLSDLBackend`) at the end. +WB is MIT-licensed; attribute in `NOTICE.md`. Keep `references/WorldBuilder/` +in-tree for read-reference (not as a project dependency). + +**Why pre-empt M1.5:** User direction 2026-05-21 — we want this +foundational decoupling done before further milestone work compounds +the WB dependency. M1.5's Phase A6/A7 indoor work doesn't touch dat +infrastructure, so it can resume cleanly after Phase O lands. + +**Tasks** (full breakdown in spec §5; ~7-8 working days): +- **O-T1** — WB call-graph audit. Produce the exact "must extract" list. +- **O-T2** — Create `src/AcDream.Core/Rendering/Wb/` + `NOTICE.md` attribution. +- **O-T3** — Extract texture / GL infrastructure (no dat dependency, low risk). +- **O-T4** — Extract mesh pipeline (`ObjectMeshManager` + types) + `DatCollection` swap. +- **O-T5** — Extract scenery + terrain blending pipelines + `DatCollection` swap. +- **O-T6** — Extract EnvCell + portal renderers + `DatCollection` swap. +- **O-T7** — Drop the two project references. Drop `_wbDats` and cross-check code. +- **O-T8** — Verification gate: three-scene visual side-by-side with main (Holtburg, inn, dungeon). +- **O-T9** — Ship. Update CLAUDE.md + `worldbuilder-inventory.md` to reflect new ownership. + +**Acceptance** (full list in spec §6): +- `dotnet build` + `dotnet test` green; ~1147 test count maintained. +- Zero `using WorldBuilder.*` or `Chorizite.OpenGLSDLBackend.*` in `src/AcDream.*`. +- `DefaultDatReaderWriter` referenced in zero places in our source. +- Resident memory at `radius=4 + 50 entities visible`: **≥ 50 MB reduction** vs. pre-O main. +- Visual side-by-side identical for the three reference scenes; user confirms. +- `NOTICE.md` includes WB MIT attribution. + +**Non-goals** (explicit, spec §8): re-porting from retail decomp; +performance optimization of extracted code; API cleanup. Verbatim +copy + swap to `DatCollection` only. Refactors in follow-up phases. + +**After Phase O ships:** M1.5 resumes from its 2026-05-20 baseline +with no code changes lost — M1.5 doesn't touch WB-extracted territory. + +### Milestone M1.5 — "Indoor world feels right" (PAUSED for Phase O — see above) The current top of the work order. Two phases (A6 + A7) inside one milestone. M2 ("kill a drudge") is deferred until M1.5 lands — diff --git a/docs/superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md b/docs/superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md new file mode 100644 index 0000000..37ab084 --- /dev/null +++ b/docs/superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md @@ -0,0 +1,237 @@ +# Phase O — DatPath Unification — Design Spec + +**Filed:** 2026-05-21 +**Status:** DRAFT (not yet scheduled) +**Owner:** TBD +**Estimated effort:** 1-2 focused weeks of work; one ship-window, no slicing across multiple weeks. + +> **Tagline**: ONE thing touches the DATs. Today we have two readers in +> process (acdream's `DatCollection` + WorldBuilder's +> `DefaultDatReaderWriter`) reading the same files independently. Phase O +> collapses that to one. + +--- + +## 1. Problem statement + +As of Phase N.4 ship (2026-05-08), WorldBuilder is integrated as our +rendering + dat-handling base. Concretely: + +- `WbMeshAdapter.cs:79` constructs **`_wbDats = new DefaultDatReaderWriter(datDir)`** — WB's own dat reader. +- Our **`DatCollection`** is constructed independently at startup for the rest of the client (network, physics, animation, clothing, audio, UI, etc.). + +**Both readers open the same four files** (`client_portal.dat`, +`client_cell_1.dat`, `client_highres.dat`, `client_local_English.dat`) +with independent file handles and independent in-memory index caches. + +**Costs**: + +- **~50-100 MB duplicated index cache memory** (per `WbMeshAdapter.cs:27` comment). +- **Double seek cost** when the same dat block is read by both pipelines (mesh path via WB; surface metadata side-table via our `DatCollection`). +- **Cross-check awkwardness** — `WbMeshAdapter.cs:224-229` has explicit "if WE find the cell but WB doesn't" diagnostic code, born from the divergence. +- **Architectural smell** — a third-party feedback signal (AC community comment, 2026-05-21) flagged WB's dat-touching as "built for tools, not runtime." The double-reader is the most concrete manifestation. + +**WB is MIT-licensed**, so the path to fixing this is not to "wrap" WB but to **extract its load-bearing code into our repo** and route it through our `DatCollection`. This is the long-term clean architecture: one reader, one cache, one source of dat truth. + +--- + +## 2. Decisions log + +| # | Decision | Why | +|---|---|---| +| O-D1 | **Extract WB code verbatim into our repo.** No re-port from retail decomp. No "improvements" while extracting. | CLAUDE.md is explicit: "Re-porting from retail decomp when WB already has a tested port is how subtle bugs (scenery edge-vertex, triangle-Z) keep slipping in." Verbatim copy preserves all the corner-case fixes WB already has. | +| O-D2 | **Make extracted code consume `DatCollection`**, not `IDatReaderWriter`. | Single source of dat truth. The whole point of the phase. | +| O-D3 | **Drop the `WorldBuilder.Shared` + `Chorizite.OpenGLSDLBackend` project references** at the end of the phase. | If we still reference WB after extraction, we haven't actually finished the work. | +| O-D4 | **Keep WB in `references/` for reading/comparison**. Don't delete the vendored directory. | We'll still want to grep WB during ports of NEW pieces (e.g., minimap renderer if/when we add it). | +| O-D5 | **MIT attribution per WB convention**. Add `NOTICE.md` entry crediting WorldBuilder for the extracted code. | License compliance. | +| O-D6 | **One ship-window, not sliced**. Either the whole extraction lands and `references/WorldBuilder` is dropped, or we roll back the entire phase. | Half-extracted state (some WB code in our repo, some still referenced) is worse than either endpoint. | + +--- + +## 3. Architecture overview + +### Today (Phase N.4–A.5 state) + +``` +┌─────────────────────────┐ ┌──────────────────────────────────┐ +│ acdream subsystems │ │ WorldBuilder (referenced project)│ +│ - Network │ │ ┌────────────────────────────┐ │ +│ - Physics │ ───→ │ │ DefaultDatReaderWriter │ │ +│ - Animation │ │ │ (opens 4 .dat files) │ │ +│ - Clothing/Audio/UI │ │ └────────────┬───────────────┘ │ +│ - Surface metadata │ │ │ │ +└──────────┬──────────────┘ │ ┌────────────▼───────────────┐ │ + │ │ │ ObjectMeshManager │ │ + │ DatCollection │ │ TextureHelpers │ │ + │ (opens same 4 files) │ │ SceneryHelpers │ │ + │ │ │ LandSurfaceManager │ │ + ▼ │ │ OpenGLGraphicsDevice │ │ + ┌──────────────┐ │ └────────────────────────────┘ │ + │ Dat files │ └──────────────────────────────────┘ + │ (same files, │ ▲ + │ two readers)│──────────────┘ + └──────────────┘ +``` + +### Phase O target + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ acdream subsystems │ +│ - Network / Physics / Animation / Clothing / Audio / UI │ +│ - Mesh pipeline (extracted from WB.ObjectMeshManager) │ +│ - Texture decode (extracted from WB.TextureHelpers) │ +│ - Scenery (extracted from WB.SceneryHelpers etc.) │ +│ - Terrain blend (extracted from WB.LandSurfaceManager) │ +│ - GL state (extracted from WB.OpenGLGraphicsDevice) │ +└──────────────────────────┬──────────────────────────────────────┘ + │ + │ DatCollection (the ONLY reader) + ▼ + ┌──────────────┐ + │ Dat files │ + └──────────────┘ +``` + +### What stays acdream-original (unchanged by this phase) + +- `TerrainModernRenderer` (Phase N.5b — already ours, uses `LandblockMesh.Build` with retail's `FSplitNESW`) +- Network, physics, animation, movement, UI, audio, chat, streaming controller, plugin API +- Our `DatCollection` itself (becomes the **only** dat reader) + +--- + +## 4. Component changes (the actual extract list) + +### 4.1 Mesh pipeline (highest-value extraction) + +| WB source | New acdream home | Adaptation | +|---|---|---| +| `Chorizite.OpenGLSDLBackend.Lib.ObjectMeshManager` | `src/AcDream.Core/Rendering/Wb/ObjectMeshManager.cs` | Replace `IDatReaderWriter` field with `DatCollection`. Constructor takes our `DatCollection`. All `_dats.Get()` calls preserved. | +| `Chorizite.OpenGLSDLBackend.Lib.ObjectRenderBatch` + supporting types (`TextureKey`, etc.) | Same directory | Verbatim copy. No interface changes. | +| `Chorizite.OpenGLSDLBackend.Lib.SceneryHelpers` | `src/AcDream.Core/Rendering/Wb/SceneryHelpers.cs` | Verbatim. | +| `Chorizite.OpenGLSDLBackend.Lib.SceneryRenderManager` | Same | Verbatim + DatCollection swap. | + +### 4.2 Texture pipeline + +| WB source | New acdream home | Adaptation | +|---|---|---| +| `Chorizite.OpenGLSDLBackend.Lib.TextureHelpers` | `src/AcDream.Core/Rendering/Wb/TextureHelpers.cs` | Verbatim copy. Pure functions — no dat dependency. | +| `Chorizite.OpenGLSDLBackend.Lib.LandSurfaceManager` | `src/AcDream.Core/Rendering/Wb/LandSurfaceManager.cs` | Verbatim + DatCollection swap. | + +### 4.3 GL infrastructure + +| WB source | New acdream home | Adaptation | +|---|---|---| +| `Chorizite.OpenGLSDLBackend.OpenGLGraphicsDevice` | `src/AcDream.App/Rendering/Wb/OpenGLGraphicsDevice.cs` | Verbatim. No dat dependency. | +| `Chorizite.OpenGLSDLBackend.Frustum` + `VisibilityManager` | Same | Verbatim. | + +### 4.4 EnvCell decode + +| WB source | New acdream home | Adaptation | +|---|---|---| +| `Chorizite.OpenGLSDLBackend.Lib.EnvCellRenderManager` (if used at runtime) | `src/AcDream.Core/Rendering/Wb/EnvCellRenderManager.cs` | Verbatim + DatCollection swap. | +| `Chorizite.OpenGLSDLBackend.Lib.PortalRenderManager` | Same | Verbatim. | + +### 4.5 What we DROP + +- `_wbDats = new DefaultDatReaderWriter(datDir)` in `WbMeshAdapter.cs:79` — replaced with the existing `_dats: DatCollection` field. +- Cross-check code in `WbMeshAdapter.cs:224-229` (`if WE find the cell but WB doesn't`) — no longer relevant after unification. +- The two project references in `AcDream.App.csproj` and `AcDream.Core.csproj` to `WorldBuilder.Shared` + `Chorizite.OpenGLSDLBackend`. + +--- + +## 5. Task breakdown + +| Task | Description | Estimated effort | +|---|---|---| +| O-T1 | Audit WB call graph — produce a closure of every WB type/file we transitively use. Generate a "must extract" list with line counts. | 0.5d | +| O-T2 | Create `src/AcDream.Core/Rendering/Wb/` directory structure. Add `NOTICE.md` entry with MIT attribution to WB. | 0.25d | +| O-T3 | **Extract texture / GL infrastructure** (`TextureHelpers`, `OpenGLGraphicsDevice`, `Frustum`, `VisibilityManager`) — verbatim copy, no dat dependency means low risk. Build green. | 1d | +| O-T4 | **Extract mesh pipeline** (`ObjectMeshManager`, `ObjectRenderBatch`, `TextureKey`) — verbatim copy. Replace `IDatReaderWriter` with `DatCollection`. Build green. Existing tests green. | 2d | +| O-T5 | **Extract scenery + terrain pipelines** (`SceneryHelpers`, `SceneryRenderManager`, `LandSurfaceManager`) — verbatim + DatCollection swap. Build green. Visual verification on outdoor terrain. | 1.5d | +| O-T6 | **Extract EnvCell + portal renderers** if used at runtime (`EnvCellRenderManager`, `PortalRenderManager`) — verbatim + DatCollection swap. Visual verification on indoor scenes. | 1d | +| O-T7 | **Drop project references** from `AcDream.App.csproj` and `AcDream.Core.csproj`. Drop `_wbDats` and the cross-check code from `WbMeshAdapter`. Build green. | 0.5d | +| O-T8 | **Verification gate** — visual side-by-side with main against retail in three scenes: Holtburg town (outdoor + scenery), inn interior (EnvCell), and a dungeon (portals). User confirms "looks identical." | 1d (incl. user time) | +| O-T9 | **Ship** — single merge to main with one descriptive commit per task (T3..T7), then a "drop WB references" final commit. Update CLAUDE.md to remove the "WB is referenced as projects" language and replace with "extracted into src/AcDream.Core/Rendering/Wb/." Update `docs/architecture/worldbuilder-inventory.md`. | 0.5d | +| **Total** | | **~7-8 working days** + visual verification cycles | + +--- + +## 6. Acceptance criteria + +- [ ] `dotnet build` green across the solution. +- [ ] `dotnet test` green; no test count regression (currently ~1147). +- [ ] **Zero references to `WorldBuilder.*` or `Chorizite.OpenGLSDLBackend.*` namespaces** in `AcDream.App.csproj` and `AcDream.Core.csproj` (`PackageReference` for `Chorizite.DatReaderWriter` stays — that's the NuGet DatReaderWriter lib, not WB). +- [ ] **Zero `using WorldBuilder.*`** in `src/AcDream.*` (the extracted code lives in our namespace now). +- [ ] **`DefaultDatReaderWriter` referenced in exactly zero places** in our source. `DatCollection` is the only dat reader. +- [ ] Resident memory measured at `streaming: radius=4` + 50 entities visible: **>50 MB reduction** vs. pre-Phase-O main (validates the duplication is actually gone). +- [ ] **Visual side-by-side with main**: Holtburg town, an inn interior, a dungeon — all render identically. User confirms. +- [ ] `NOTICE.md` includes WB attribution per its MIT license. +- [ ] `references/WorldBuilder/` directory remains in the repo for read-reference, but is not referenced by any csproj. +- [ ] `CLAUDE.md` updated: WB is now a *read reference*, not a *project dependency*. The "WB integration cribs" section is rewritten to point at our extracted code. +- [ ] `docs/architecture/worldbuilder-inventory.md` updated to reflect the new ownership. + +--- + +## 7. Risks + mitigations + +| Risk | Likelihood | Severity | Mitigation | +|---|---|---|---| +| Re-introduce bugs WB had already fixed (edge-vertex, triangle-Z) | Medium | High | **Discipline: verbatim copy first.** No "improvements." A separate post-Phase-O ticket can refactor. | +| WB has implicit assumptions about `DefaultDatReaderWriter` semantics our `DatCollection` doesn't match (caching, thread safety) | Medium | High | Task O-T1 must audit the actual call surface used. If `DatCollection` diverges, add a thin adapter layer rather than monkey-patch WB code. | +| Hidden transitive WB dependencies (some helper class we didn't notice we use) | Low | Medium | Task O-T1 closure pass should catch this. If it slips, build will fail loudly. | +| Visual regression we don't catch in side-by-side | Low | Medium | Take screenshots of three reference scenes BEFORE starting; compare AFTER. Don't trust eyeballs alone. | +| User upstream-tracks WB for some reason | Low | Low | We keep `references/WorldBuilder/` in-tree. Re-syncing diffs is a manual port if needed (same as today). | +| Phase O is scoped too tightly and a piece of WB we use is missed | Medium | Medium | T1's audit is the safety. If we miss one, build fails before T7's reference-drop. | + +--- + +## 8. Out of scope (explicitly) + +- **Re-porting from retail decomp** anywhere in the extracted code. We copy WB verbatim. If a retail-faithfulness audit is needed later, file a separate phase. +- **Performance optimization** of extracted code. Even if WB's `ObjectMeshManager` has a 30% improvement waiting in it, this phase ships it as-is. +- **API cleanup** of the extracted code. If the constructor has 8 parameters and it's ugly, ugly it stays. Refactor in a follow-up. +- **Refactoring `WbMeshAdapter` itself.** Phase O drops the second reader; the adapter shape stays. +- **`tools/InspectCoatTex` or other tools that use the NuGet `Chorizite.DatReaderWriter`**. Those keep working — they use the NuGet `DatReaderWriter`, not the WB project reference. + +--- + +## 9. Open questions + +1. **Does WB's `ObjectMeshManager` thread-model match `LandblockStreamer`'s worker thread?** + - Phase O-T1 must answer. If WB assumes single-threaded mesh-pipeline access and we call into it from the worker, we have a real bug today that gets exposed louder after extraction. + - Fallback: if WB's code is not thread-safe, route the mesh-prepare call through a dispatcher to the render thread (extra latency, but correct). + +2. **Should we extract WB's `TerrainRenderManager`** even though we already wrote `TerrainModernRenderer`? + - Today: we use OUR terrain renderer (Phase N.5b). WB's `TerrainRenderManager` is *not* in our render path. + - But: `LandSurfaceManager` (terrain texture blending) IS WB code we use, and it might pull on `TerrainRenderManager` types. + - Recommendation: extract `LandSurfaceManager` only; leave `TerrainRenderManager` in `references/` since we don't use it. + +3. **What's the right namespace for extracted code?** + - Option A: `AcDream.Core.Rendering.Wb.*` — preserves "this came from WB" context. + - Option B: `AcDream.Core.Rendering.Meshes.*`, `AcDream.Core.Rendering.Textures.*` — clean namespaces, drops the WB association. + - Recommendation: **Option A** for now (keeps the audit trail), with a follow-up phase to fold into Option B once dust settles. + +4. **Do we extract the `Chorizite.DatReaderWriter` NuGet package code itself?** + - **No.** That's a separate, well-maintained library on NuGet. We keep depending on it via `PackageReference`. The phase is specifically about the WB project references (`WorldBuilder.Shared` + `Chorizite.OpenGLSDLBackend`) — the *application* code, not the *protocol* library. + +--- + +## 10. Naming + filing + +- **Phase name**: O (next free letter — A-N + R are in use). +- **Phase tagline**: "ONE thing touches the DATs." +- **Roadmap entry**: add to `docs/plans/2026-04-11-roadmap.md` under "Phases ahead" with cross-reference to this spec. +- **Issue tracking**: file as a Phase, not an issue (multi-commit, multi-day, infrastructural). + +--- + +## 11. After Phase O ships + +Things that become unblocked or cheaper: + +- **Single-cache memory pressure budget.** Easier to size streaming radii, MSAA, anisotropic budget, etc. +- **Audit a single dat-touching code path** instead of two when investigating bugs like #37 (the coat-stub palette overlay). +- **Future N.6+ rendering work** doesn't have to ask "is this WB's concern or ours?" — it's ours. +- **AC community-friendly architecture** — the "WB is for tools" criticism is addressed at the structural level, not just papered over.