docs(O): O-T1 audit shipped + Phase O spec amended
O-T1 audit (REPORT-ONLY) maps acdream's transitive closure on WorldBuilder: 33 files / ~7.7K LOC across Chorizite.OpenGLSDLBackend (28 files) and WorldBuilder.Shared (5 files). Verdict on O-Q1 (thread-model): SAFE — adapters run render-thread only; no worker-thread access to WB code. Spec amendments incorporated via brainstorm: - O-D7: Refactor ObjectMeshManager to take DatCollection directly (not via adapter). T4 safety check — fall back to thin adapter if call-site count >20. - O-D8: Drop LandSurfaceManager, EnvCellRenderManager, PortalRenderManager, TerrainRenderManager from the extract list — audit confirmed not reachable (we have our own ports or never used them). - O-D9: Promote 3 internal types in Chorizite to public on extraction (EmbeddedResourceReader, TextureFormatExtensions, BufferUsageExtensions). - O-D10: Strip [MemoryPackable] from TerrainEntry (we don't serialize). - O-D11: Namespace AcDream.Core.Rendering.Wb.* for extracted code. - O-D12: Drop ResolveId + [indoor-upload] NULL_RESULT diagnostic block. Task breakdown: T6 (EnvCell/portal) eliminated; T5 (stateless helpers) shrinks to 0.5d; T4 (mesh + refactor) grows to 2.5d. Net effort estimate holds at ~7.75d. All originally-open spec questions are now closed (Q1/Q2/Q3/Q4) or deferred to T3 with an explicit verify step (Q5: SixLabors.ImageSharp reachability). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e702dec7a3
commit
b9c111b80d
2 changed files with 602 additions and 93 deletions
456
docs/research/2026-05-21-phase-o-t1-wb-audit.md
Normal file
456
docs/research/2026-05-21-phase-o-t1-wb-audit.md
Normal file
|
|
@ -0,0 +1,456 @@
|
|||
# Phase O — Task O-T1 — WB Usage Audit
|
||||
|
||||
**Status:** REPORT-ONLY (no source edits, no project changes).
|
||||
**Filed:** 2026-05-21.
|
||||
**Purpose:** Closure of every WorldBuilder type and file acdream transitively
|
||||
uses, so T2–T7 don't miss a dependency at the "drop project references" step.
|
||||
|
||||
> **Headline:** Reachable closure from acdream is **33 files / ~7.7 K LOC**
|
||||
> (~6.8 K from `Chorizite.OpenGLSDLBackend`, ~0.9 K from `WorldBuilder.Shared`).
|
||||
> Substantially smaller than the spec's §4 component list anticipated, because
|
||||
> three of the spec's named components (`TerrainRenderManager`,
|
||||
> `LandSurfaceManager`, `EnvCellRenderManager` / `PortalRenderManager`) are
|
||||
> **not in our actual call graph** — we already replaced or never used them.
|
||||
> The 7-8 day estimate in spec §5 looks **reasonable**, possibly conservative.
|
||||
> O-Q1 (thread-model) verdict: **SAFE** — no worker-thread access to WB code.
|
||||
|
||||
Subreports (long-tail per-file tables) by parallel agents:
|
||||
- `2026-05-21-phase-o-t1-wb-audit-chorizite-closure.md` — Chorizite full per-file table
|
||||
- `2026-05-21-phase-o-t1-wb-audit-shared-closure.md` — WB.Shared per-file table
|
||||
- `2026-05-21-phase-o-t1-wb-audit-thread-and-hidden.md` — thread model + hidden deps
|
||||
- `2026-05-21-phase-o-t1-wb-audit-extensions-and-rest.md` — broad reachability sweep
|
||||
|
||||
> Note: those subreport file *names* are referenced in the parent agents'
|
||||
> output but the Explore agent type lacks `Write`, so the files were not
|
||||
> persisted to disk. The relevant content has been pulled into the summary
|
||||
> tables and inventory below.
|
||||
|
||||
---
|
||||
|
||||
## 1. Direct use surface
|
||||
|
||||
Files in `src/` and `tests/` that actually import a WorldBuilder type
|
||||
(`using WorldBuilder.*` or `using Chorizite.OpenGLSDLBackend*` or fully-
|
||||
qualified). Comment-only WB references (six files in src/ — TerrainBlending,
|
||||
SurfaceInfo, GfxObjMesh, SkyRenderer, SamplerCache, GameWindow) are
|
||||
excluded; they describe ports done by hand and don't carry a project
|
||||
dependency.
|
||||
|
||||
### Production source — `src/`
|
||||
|
||||
| File | LOC | WB types used | Notes |
|
||||
|---|---|---|---|
|
||||
| [`src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs`](src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs) | 349 | `OpenGLGraphicsDevice`, `DefaultDatReaderWriter`, `ObjectMeshManager`, `ObjectRenderData`, `DebugRenderSettings` | Single seam; constructs WB pipeline; **also constructs `_wbDats = new DefaultDatReaderWriter(datDir)` at line 79** (the second-reader smell Phase O closes) |
|
||||
| [`src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs`](src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs) | ~1500 | `ObjectRenderData`, `ObjectRenderBatch` | Production draw path (modern MDI); uses our own `DrawElementsIndirectCommand` struct + our own `mesh_modern.{vert,frag}` shaders |
|
||||
| [`src/AcDream.Core/World/WbSceneryAdapter.cs`](src/AcDream.Core/World/WbSceneryAdapter.cs) | 55 | `TerrainEntry` | Bridges `LandBlock` → `TerrainEntry[81]` for WB scenery helpers |
|
||||
| [`src/AcDream.Core/World/SceneryGenerator.cs`](src/AcDream.Core/World/SceneryGenerator.cs) | 196 | `SceneryHelpers.Displace/CheckSlope/ObjAlign/RotateObj/ScaleObj`, `TerrainUtils.OnRoad/GetNormal` | Procedural scenery placement; calls WB stateless helpers |
|
||||
| [`src/AcDream.Core/Textures/SurfaceDecoder.cs`](src/AcDream.Core/Textures/SurfaceDecoder.cs) | 219 | `TextureHelpers.FillIndex16/FillP8/FillA8/FillA8Additive/FillA8R8G8B8/FillR8G8B8/FillR5G6B5/FillA4R4G4B4` | Pure pixel-format decoders |
|
||||
|
||||
### Test source — `tests/`
|
||||
|
||||
| File | WB types used | Notes |
|
||||
|---|---|---|
|
||||
| [`tests/AcDream.Core.Tests/Textures/TextureDecodeConformanceTests.cs`](tests/AcDream.Core.Tests/Textures/TextureDecodeConformanceTests.cs) | `TextureHelpers.Fill*` | Byte-identity tests vs WB; should stay after extraction (validates our copy matches the original) |
|
||||
| [`tests/AcDream.Core.Tests/Terrain/SplitFormulaDivergenceTest.cs`](tests/AcDream.Core.Tests/Terrain/SplitFormulaDivergenceTest.cs) | `TerrainUtils.CalculateSplitDirection`, `CellSplitDirection` | One-time sweep test that compared WB's formula with retail's; **safely deletable after extraction** (test already informed the N.5b decision — its job is done) |
|
||||
|
||||
### Project files
|
||||
|
||||
| File | Lines | What |
|
||||
|---|---|---|
|
||||
| [`src/AcDream.App/AcDream.App.csproj`](src/AcDream.App/AcDream.App.csproj) | 38-39 | `<ProjectReference>` to `WorldBuilder.Shared` + `Chorizite.OpenGLSDLBackend` |
|
||||
| [`src/AcDream.Core/AcDream.Core.csproj`](src/AcDream.Core/AcDream.Core.csproj) | 27-28 | same |
|
||||
|
||||
These two csproj entries are what T7 deletes after extraction.
|
||||
|
||||
---
|
||||
|
||||
## 2. Transitive closure (summary)
|
||||
|
||||
Closure walked from the direct-use surface above, following only WB
|
||||
project-internal types. NuGet packages (`Chorizite.Core`,
|
||||
`Chorizite.DatReaderWriter`, `Silk.NET.*`, `BCnEncoder.Net`,
|
||||
`SixLabors.ImageSharp`, `MemoryPack`) stay as `<PackageReference>` and are
|
||||
**not** part of the extraction scope.
|
||||
|
||||
| Source project | Files reachable | LOC | Notes |
|
||||
|---|---|---|---|
|
||||
| `Chorizite.OpenGLSDLBackend` | **28** | **~6,829** | Entry points: `ObjectMeshManager`, `OpenGLGraphicsDevice`, `TextureHelpers`, `SceneryHelpers`, `ObjectRenderData/Batch`, `DebugRenderSettings` |
|
||||
| `WorldBuilder.Shared` | **5** | **876** | Entry points: `TerrainEntry`, `TerrainUtils`, `CellSplitDirection`, `DefaultDatReaderWriter`, `IDatReaderWriter` |
|
||||
| **Total reachable** | **33** | **~7,705** | |
|
||||
|
||||
Files in WB **not** in our reachable closure (informational; do NOT
|
||||
extract): all of `WorldBuilder/`, `WorldBuilder.Server/`, the platform
|
||||
projects (`WorldBuilder.Windows/Linux/Mac`); inside `WorldBuilder.Shared/`:
|
||||
`Hubs/` (SignalR), `Migrations/` (EF Core), `Repositories/`, the editor
|
||||
`Services/` (DocumentManager, SyncService, etc.), `Modules/Landscape/Tools/`
|
||||
(brush/painting), `Modules/Landscape/Commands/` (undo/redo); inside
|
||||
`Chorizite.OpenGLSDLBackend/`: `FontRenderer`, `AudioPlaybackEngine`,
|
||||
`MinimapRenderer`, `BackendGizmoDrawer`, the entire editor scene/camera
|
||||
subsystems, and **critically: `TerrainRenderManager`, `LandSurfaceManager`,
|
||||
`EnvCellRenderManager`, `PortalRenderManager`** — see §4 below.
|
||||
|
||||
---
|
||||
|
||||
## 3. Per-file inventory (top 30 by LOC)
|
||||
|
||||
DAT-TOUCH = file references `IDatReaderWriter` / `_dats.Get<T>()` / opens
|
||||
dat data. GL-TOUCH = file calls Silk.NET.OpenGL directly or writes shader
|
||||
binds / texture uploads. Bucket per Phase O spec (T3–T6, REPLACE for the
|
||||
dat-readers we drop, NOT for files in the spec's §4 list that turn out to
|
||||
not be reachable).
|
||||
|
||||
### From `WorldBuilder.Shared/` (5 files, 876 LOC) — ALL EXTRACTED
|
||||
|
||||
| Path (rel. `references/WorldBuilder/`) | LOC | Role | DAT? | GL? | Bucket |
|
||||
|---|---|---|---|---|---|
|
||||
| `WorldBuilder.Shared/Modules/Landscape/Lib/TerrainUtils.cs` | 258 | Static helpers: `GetHeight`, `GetNormal`, `OnRoad`, `CalculateSplitDirection` | indirect (caller passes Region) | no | **T5** |
|
||||
| `WorldBuilder.Shared/Modules/Landscape/Models/TerrainEntry.cs` | 248 | Packed per-vertex terrain struct (uses `[MemoryPackable]`) | no | no | **T5** |
|
||||
| `WorldBuilder.Shared/Services/DefaultDatReaderWriter.cs` | 215 | Concrete dat reader (4 dbs, cache, file handles) | yes | no | **REPLACE** (delete; route through DatCollection) |
|
||||
| `WorldBuilder.Shared/Services/IDatReaderWriter.cs` | 137 | Dat access interface + `IdResolution` record | yes | no | **REPLACE** (delete) |
|
||||
| `WorldBuilder.Shared/Modules/Landscape/Models/CellSplitDirection.cs` | 18 | `SWtoNE` / `SEtoNW` enum | no | no | **T5** |
|
||||
|
||||
### From `Chorizite.OpenGLSDLBackend/` (28 files, ~6,829 LOC reachable) — TOP 10 by LOC
|
||||
|
||||
(Full table in the chorizite-closure subreport; only the top by LOC and a
|
||||
few key smaller files are inlined here so this doc stays under 500 lines.)
|
||||
|
||||
| Path (rel. `references/WorldBuilder/`) | LOC | Role | DAT? | GL? | Bucket |
|
||||
|---|---|---|---|---|---|
|
||||
| `Chorizite.OpenGLSDLBackend/Lib/ObjectMeshManager.cs` | ~2079 | The hub: reads GfxObj/Setup/Palette from dats; decodes textures; manages GPU resources; modern-rendering global buffers | **yes** | yes | **T4** |
|
||||
| `Chorizite.OpenGLSDLBackend/OpenGLGraphicsDevice.cs` | ~625 | Silk.NET wrapper: GL ctx, ProcessGLQueue, render-state | no | yes | **T3** |
|
||||
| `Chorizite.OpenGLSDLBackend/Lib/TextureHelpers.cs` | ~variable | INDEX16 / P8 / A8 / DXT decode helpers (pure functions) | no | no | **T3** |
|
||||
| `Chorizite.OpenGLSDLBackend/Lib/SceneryHelpers.cs` | small | `Displace` / `RotateObj` / `ScaleObj` / `ObjAlign` / `CheckSlope` (pure) | no | no | **T5** |
|
||||
| Particle emitter + batcher (T4 supporting) | ~combined 1,200 | Particle pipeline used by ObjectMeshManager | maybe | yes | **T4** |
|
||||
| Global mesh buffer (modern rendering) | ~500 | Single global VAO/VBO/IBO for modern path | no | yes | **T4** |
|
||||
| ManagedGL{Texture, TextureArray, VertexBuffer, IndexBuffer, VertexArray, FrameBuffer, UniformBuffer} | ~150-300 ea | Per-resource GL lifecycle wrappers | no | yes | **T3** |
|
||||
| `GLSLShader.cs`, `GLHelpers.cs`, `GLStateScope.cs` | ~100-300 ea | Shader compile + GL state utility | no | yes | **T3** |
|
||||
| `Lib/ObjectRenderBatch.cs`, `Lib/ObjectRenderData.cs` | small | Per-batch + per-object render data structs | no | no | **T4** |
|
||||
| `Lib/DebugRenderSettings.cs` | small | Settings shape passed to OpenGLGraphicsDevice ctor | no | no | **T3** |
|
||||
|
||||
**Approximate bucket totals across the full Chorizite closure** (28 files):
|
||||
|
||||
- **T3 (GL infra, no dat dep)**: 14 files, ~2,900 LOC
|
||||
- **T4 (mesh pipeline)**: 8 files, ~3,313 LOC (ObjectMeshManager dominates)
|
||||
- **T5 stateless (SceneryHelpers + helpers)**: ~2 files, ~200 LOC
|
||||
- **NOT-CORE leaves** (TextureHelpers, EdgeLineBuilder, extensions): 4 files, ~400 LOC — note these still get extracted, they're just leaves not in the deep call graph of ObjectMeshManager
|
||||
|
||||
---
|
||||
|
||||
## 4. Extraction-task mapping
|
||||
|
||||
Reconciling agent findings against spec §4 — **three of the spec's listed
|
||||
extractions are unnecessary because the files turned out not to be
|
||||
reachable**.
|
||||
|
||||
### Reachable, must extract
|
||||
|
||||
| Bucket | Files (count, LOC) | Spec §4 alignment |
|
||||
|---|---|---|
|
||||
| **T3 — Texture / GL infrastructure** | ~15 files, ~3,100 LOC (14 from Chorizite + `TextureHelpers`) | Matches spec §4.2/§4.3: `TextureHelpers`, `OpenGLGraphicsDevice`, plus all `ManagedGL*` wrappers, `GLSLShader`, `GLHelpers`, `GLStateScope` |
|
||||
| **T4 — Mesh pipeline** | 8 files, ~3,313 LOC | Matches spec §4.1: `ObjectMeshManager`, `ObjectRenderBatch`, `ObjectRenderData` + transitive supports (`GlobalMeshBuffer`, particle batcher, modern render data) |
|
||||
| **T5 — Scenery + terrain stateless** | 5 files, ~782 LOC (`SceneryHelpers`, `TerrainUtils`, `TerrainEntry`, `CellSplitDirection`, possibly `RegionInfo` if reachable) | **Smaller than spec §4.1+§4.2 anticipated** — `LandSurfaceManager` and `SceneryRenderManager` are NOT in our closure (we have our own ports / our own renderer) |
|
||||
|
||||
### Files spec listed but we do NOT need
|
||||
|
||||
| File (spec §4 reference) | Status | Why |
|
||||
|---|---|---|
|
||||
| `Chorizite.OpenGLSDLBackend.Lib.LandSurfaceManager` (spec §4.2) | **NOT EXTRACTED** | Acdream has its own `src/AcDream.Core/Terrain/TerrainBlending.cs` (line 8: "Ported from WorldBuilder.LandSurfaceManager"). No `using` import of WB's class. Confirm in T2: kick or keep our port. |
|
||||
| `Chorizite.OpenGLSDLBackend.Lib.SceneryRenderManager` (spec §4.1) | **NOT EXTRACTED** | `SceneryGenerator.cs` uses only `SceneryHelpers` (stateless). No reference to `SceneryRenderManager` (the WB-internal pipeline class). |
|
||||
| `Chorizite.OpenGLSDLBackend.Lib.EnvCellRenderManager` (spec §4.4) | **NOT EXTRACTED** | Acdream renders EnvCells via `ObjectMeshManager` (its `PrepareEnvCellMeshData` path) + `WbDrawDispatcher`. WB's `EnvCellRenderManager` is the editor's separate render path and is not referenced. |
|
||||
| `Chorizite.OpenGLSDLBackend.Lib.PortalRenderManager` (spec §4.4) | **NOT EXTRACTED** | Acdream renders portals via the same mesh path; WB's `PortalRenderManager` is editor-only. |
|
||||
| `Chorizite.OpenGLSDLBackend.Lib.TerrainRenderManager` (spec §9.2 open Q) | **NOT EXTRACTED** | Confirmed: we use `src/AcDream.App/Rendering/TerrainModernRenderer.cs` (Phase N.5b ported retail's `FSplitNESW`). Spec §9.2's recommendation to leave WB's `TerrainRenderManager` in `references/` matches the closure finding. |
|
||||
|
||||
**This means the spec's T5 and T6 buckets shrink dramatically:**
|
||||
|
||||
- **Spec T5** was "scenery + terrain pipelines" (~3 named files: `SceneryHelpers`, `SceneryRenderManager`, `LandSurfaceManager`). Real T5 is just `SceneryHelpers` (stateless utility, ~100 LOC) + the 4 WB.Shared terrain helpers (`TerrainUtils`, `TerrainEntry`, `CellSplitDirection`).
|
||||
- **Spec T6** was "EnvCell + portal renderers". Real T6 is **empty**. Recommend dropping T6 from the task plan.
|
||||
|
||||
### Files we explicitly DROP (REPLACE bucket)
|
||||
|
||||
- `WorldBuilder.Shared/Services/IDatReaderWriter.cs` (137 LOC) — interface goes away.
|
||||
- `WorldBuilder.Shared/Services/DefaultDatReaderWriter.cs` (215 LOC) — concrete impl goes away.
|
||||
- The `_wbDats = new DefaultDatReaderWriter(datDir)` line in `WbMeshAdapter.cs:79` and its `Dispose()` partner at line 346 — removed in T7.
|
||||
- The cross-check diagnostic at `WbMeshAdapter.cs:224-262` (`[indoor-upload] NULL_RESULT`) — removed in T7; depends on `ResolveId` which goes away with `DefaultDatReaderWriter`.
|
||||
|
||||
---
|
||||
|
||||
## 5. Hidden-dependency risks
|
||||
|
||||
### 5.1 Internal types
|
||||
|
||||
**Three internal types need `internal` → `public` promotion** when copied
|
||||
across the project boundary (or copied with their owners and kept
|
||||
internal in the new project):
|
||||
|
||||
- `EmbeddedResourceReader` (Chorizite) — referenced by ObjectMeshManager
|
||||
for embedded shader / resource loads.
|
||||
- `TextureFormatExtensions` (Chorizite) — extension methods on
|
||||
`PixelFormat`.
|
||||
- `BufferUsageExtensions` (Chorizite) — extension methods on
|
||||
`BufferUsageARB`.
|
||||
|
||||
No internal types in `WorldBuilder.Shared`'s closure. Test mocks marked
|
||||
`internal` are not in scope (they live in test projects).
|
||||
|
||||
### 5.2 Source generators / build hooks
|
||||
|
||||
**None found.** Neither `Chorizite.OpenGLSDLBackend.csproj` nor
|
||||
`WorldBuilder.Shared.csproj` has `<Target>`, `<AnalyzerReference>`, or
|
||||
source-generator `<PackageReference>` entries. Verbatim file copy will
|
||||
transfer cleanly.
|
||||
|
||||
### 5.3 Resource files
|
||||
|
||||
**Shaders.** Reachable closure loads a minority of the 21 shaders in
|
||||
`Chorizite.OpenGLSDLBackend/Shaders/`. The Chorizite-closure agent
|
||||
identified only **2 shaders loaded by ObjectMeshManager's path**:
|
||||
|
||||
- `Shaders/Particle.vert`
|
||||
- `Shaders/Particle.frag`
|
||||
|
||||
(All other shaders — `Landscape`, `StaticObject*`, `PortalStencil`,
|
||||
`Outline`, `InstancedLine`, `Text`, `UI`, `Gizmo` — are loaded by render
|
||||
managers we don't use, e.g. `StaticObjectRenderManager`,
|
||||
`TerrainRenderManager`, `PortalRenderManager`. Those managers are NOT in
|
||||
our closure.) **Acdream's own entity shader is `mesh_modern.{vert,frag}`
|
||||
in `src/AcDream.App/Rendering/Shaders/` — not a WB asset.**
|
||||
|
||||
This contradicts the broader-sweep agent's count of "10 shader files
|
||||
actively referenced", which was including managers we don't use.
|
||||
**Verify in T4** by grepping for `LoadShader` string literals inside
|
||||
ObjectMeshManager + its transitive closure.
|
||||
|
||||
**Fonts.** Two `.ttf` files exist in `Chorizite.OpenGLSDLBackend/Fonts/`
|
||||
(DroidSans + DroidSans-Bold). They are used by `FontRenderer`, which is
|
||||
**NOT in our closure** (we use BitmapFont + StbTrueTypeSharp + ImGui).
|
||||
**Fonts do not need to be copied.**
|
||||
|
||||
**Other.** No PNG, JSON, XML, or other asset files referenced by the
|
||||
closure beyond shaders.
|
||||
|
||||
### 5.4 NuGet dependencies that transfer with the extraction
|
||||
|
||||
Files in the closure reference:
|
||||
|
||||
- `Silk.NET.OpenGL` (already a `<PackageReference>` in `AcDream.App.csproj`)
|
||||
- `BCnEncoder.Net` (already in `AcDream.Core.csproj` v2.2.1 — Chorizite
|
||||
uses v2.2.x, compatible)
|
||||
- `SixLabors.ImageSharp` (NOT currently in acdream — needs adding for
|
||||
some of WB's texture-load paths; **verify in T4 whether our extraction
|
||||
closure actually touches ImageSharp** — if only for the editor scene
|
||||
loads, we can drop)
|
||||
- `MemoryPack` — `TerrainEntry` uses `[MemoryPackable]`. **Add as
|
||||
`<PackageReference>` in `AcDream.Core.csproj` at T5**, or strip the
|
||||
attribute (it's used only by WB's editor save/load; acdream doesn't
|
||||
serialize `TerrainEntry`). Stripping is the simpler call.
|
||||
|
||||
### 5.5 DefaultDatReaderWriter ↔ DatCollection — the real risk
|
||||
|
||||
This is the only material risk in the whole audit. WB's interface and
|
||||
acdream's `DatCollection` diverge in several ways:
|
||||
|
||||
| Concern | WB | Acdream's `DatCollection` (per probe at `WbMeshAdapter.cs:228-260`) |
|
||||
|---|---|---|
|
||||
| **Database surface** | Returns `IDatDatabase` (abstract interface) for `Portal`, `HighRes`, `Language`, `Cell` | Concrete types (`PortalDatabase`, `CellDatabase`, `LocalDatabase`) |
|
||||
| **Cell databases** | `CellRegions` dictionary (multi-region cell support; auto-discovers at construction) | Single `Cell` property |
|
||||
| **Cross-database lookup** | `ResolveId(uint id)` returns `IEnumerable<(database, type)>` (HighRes → Portal → Language → all Cells) | **No equivalent.** WbMeshAdapter's diagnostic code uses it; production mesh path may or may not |
|
||||
| **Per-type caching** | `ConcurrentDictionary<(Type, uint), IDBObj>` per database; thread-safe by design | Documented as **NOT thread-safe** (per `memory/feedback_phase_a1_hotfix_saga.md`) — but with the O-Q1 verdict (render-thread only), this may be fine |
|
||||
| **Iteration tracking** | Per-database iteration counters | Verify in T4 |
|
||||
| **Write support** | `TrySave` overloads | Acdream is read-only (correct for a client) |
|
||||
| **File-handle sharing** | Today each reader opens its own 4 files (~50-100 MB index duplication, the smell Phase O closes) | The DatCollection's file handles are reused; T7 ends the duplication |
|
||||
|
||||
**Pre-T4 verification checklist:**
|
||||
|
||||
1. Grep `ObjectMeshManager.cs` for every `_dats.X` call site. Catalog
|
||||
which methods/properties of `IDatReaderWriter` it actually uses.
|
||||
2. For each, confirm `DatCollection` has the equivalent (or design the
|
||||
shim).
|
||||
3. Decide whether `ResolveId` is needed in production code or is purely
|
||||
diagnostic (the `[indoor-upload]` probe is the only known caller).
|
||||
If diagnostic-only: drop it at T7.
|
||||
4. Confirm the multi-region `CellRegions` story — does WB's
|
||||
`ObjectMeshManager` actually iterate `CellRegions`, or always call
|
||||
`Cell` singular? (If singular, the dict→single mapping is trivial.)
|
||||
5. Confirm thread-safety: with O-Q1 settled (render-thread only), do we
|
||||
still need `ConcurrentDictionary` semantics, or can our existing
|
||||
single-thread `DatCollection` serve?
|
||||
|
||||
---
|
||||
|
||||
## 6. Thread-model finding (Open Question O-Q1)
|
||||
|
||||
**Verdict: SAFE — verified by code inspection.** No worker-thread access
|
||||
to WB code today, so extraction does not expose a latent race.
|
||||
|
||||
Evidence (paths from the worktree root):
|
||||
|
||||
- `WbMeshAdapter.Tick()` is documented in the source as render-thread only
|
||||
([src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs:275-278](src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs:275)).
|
||||
- `LandblockSpawnAdapter` and `EntitySpawnAdapter` are called only from
|
||||
`GpuWorldState` methods (`AddLandblock`, `RemoveLandblock`,
|
||||
`AddEntitiesToExistingLandblock`, `RemoveEntitiesFromLandblock`,
|
||||
`OnCreate`, `OnRemove`). `GpuWorldState` is the render-thread entity
|
||||
state manager (per `CLAUDE.md` two-tier streaming arch); the streaming
|
||||
worker thread (`LandblockStreamer`) never touches WB.
|
||||
- The two-tier streaming spec
|
||||
(`docs/superpowers/specs/2026-05-09-phase-a5-two-tier-streaming-design.md`)
|
||||
is explicit that the worker thread builds dat-decoded `LandblockState`
|
||||
objects and hands them to the render thread via a completion queue;
|
||||
WB's `ObjectMeshManager` is never invoked from the worker.
|
||||
- WB's own `ObjectMeshManager` uses `ConcurrentDictionary` for
|
||||
`_usageCount` and an explicit `lock(_lruList)` — defense in depth that
|
||||
doesn't matter for us today, but keeps the door open if a future
|
||||
acdream design hands streaming work to a worker.
|
||||
|
||||
**Action for the spec:** §9.1 (O-Q1) can be marked closed with
|
||||
"Verified safe; render-thread-only access; ConcurrentDictionary in WB
|
||||
adds belt-and-braces."
|
||||
|
||||
---
|
||||
|
||||
## 7. Surprises / sharp edges
|
||||
|
||||
1. **Spec §4 over-specifies the extraction.** Three named files
|
||||
(`LandSurfaceManager`, `EnvCellRenderManager`, `PortalRenderManager`)
|
||||
and one open question (§9.2's `TerrainRenderManager`) are NOT in the
|
||||
actual closure. The spec should be amended: T5 shrinks to "stateless
|
||||
scenery + terrain helpers" (5 files, ~800 LOC); T6 disappears entirely.
|
||||
|
||||
2. **The biggest risk is API-shape mismatch, not file count.**
|
||||
`IDatReaderWriter` returns interface types and has multi-region cell
|
||||
support + a `ResolveId` cross-DB search; `DatCollection` returns
|
||||
concrete types and has a single cell property. T4 must design the
|
||||
shim layer (or refactor `ObjectMeshManager` slightly to use our
|
||||
shape — slightly violates "verbatim copy" but is one-call narrow).
|
||||
|
||||
3. **`SixLabors.ImageSharp` is a possibly-unnecessary new NuGet
|
||||
dependency.** If our closure actually touches it (some texture-load
|
||||
helpers in Chorizite use ImageSharp for PNG/JPG), T4 needs to add it.
|
||||
If it's only in editor-load paths we don't use, T4 can drop the
|
||||
imports. **Verify before T2.**
|
||||
|
||||
4. **`MemoryPack` is a small new dep for one attribute on
|
||||
`TerrainEntry`.** Cheapest answer: strip the attribute when copying
|
||||
`TerrainEntry.cs` into our tree (we don't serialize the struct).
|
||||
|
||||
5. **The `[indoor-upload]` diagnostic in `WbMeshAdapter.cs:192-263`
|
||||
becomes vestigial after T7.** It depends on `ResolveId` (which goes
|
||||
away), it compares `_wbDats` (which goes away) against `_dats`. The
|
||||
whole `if (RenderingDiagnostics.IsEnvCellId(id) && ...)` block + its
|
||||
`_pendingEnvCellRequests` companion can be deleted in T7.
|
||||
|
||||
6. **`SplitFormulaDivergenceTest.cs` is safely deletable** after T5
|
||||
ships. It was a one-time data-collection test that informed the
|
||||
N.5b path-C decision; its findings are baked into the codebase. We
|
||||
can drop the WB-types import by deleting the test.
|
||||
|
||||
7. **Total `LOC of extracted code: ~6.0-6.5K`** (excluding the 350 LOC
|
||||
of `IDatReaderWriter`/`DefaultDatReaderWriter` which are deleted, and
|
||||
excluding the 600 LOC of NOT-CORE leaves the broader agent
|
||||
classified as "optional" but on review still need to come along).
|
||||
This is somewhat **larger than the spec's implicit ~5K assumption**
|
||||
but only marginally; the 7-8 day estimate still looks right.
|
||||
|
||||
---
|
||||
|
||||
## 8. Open questions for the user (to call before T2 starts)
|
||||
|
||||
1. **Spec §4 amendment.** Confirm that the three not-reachable
|
||||
components (`LandSurfaceManager`, `EnvCellRenderManager`,
|
||||
`PortalRenderManager`) are dropped from the extraction task list. If
|
||||
yes, T6 disappears entirely and T5 narrows to stateless helpers.
|
||||
*Recommendation: drop them. Saves ~1.5 days of T6 work that would
|
||||
otherwise be a no-op anyway.*
|
||||
|
||||
2. **NuGet additions.** Are we OK adding `MemoryPack` and (potentially)
|
||||
`SixLabors.ImageSharp` as new `<PackageReference>` lines in
|
||||
`AcDream.Core.csproj`? Or do we prefer stripping the `[MemoryPackable]`
|
||||
attribute from `TerrainEntry` and dropping ImageSharp-touching code
|
||||
from the extraction?
|
||||
*Recommendation: strip `MemoryPack` (one-line change). On ImageSharp,
|
||||
wait until T4 confirms whether the closure actually touches it.*
|
||||
|
||||
3. **`DatCollection` design call.** Two paths for the dat-swap:
|
||||
- **A) Adapter shim**: write a thin `DatCollectionAsIDatReaderWriter`
|
||||
adapter so `ObjectMeshManager` keeps its current dat field type
|
||||
and we preserve "verbatim copy" discipline.
|
||||
- **B) Refactor**: change `ObjectMeshManager`'s field type to
|
||||
`DatCollection` and rewrite ~5-20 call sites inside it.
|
||||
*Recommendation: A) for the first pass (preserves "verbatim copy"
|
||||
discipline; spec O-D1 says no improvements). A follow-up phase can
|
||||
refactor (B) once the extraction is settled.*
|
||||
|
||||
4. **`ResolveId` fate.** Drop it (diagnostic-only) or implement an
|
||||
equivalent on `DatCollection`?
|
||||
*Recommendation: drop. The `[indoor-upload]` diagnostic was a Phase-2
|
||||
investigation tool that has done its job.*
|
||||
|
||||
5. **Test deletion.** OK to delete `SplitFormulaDivergenceTest.cs` (the
|
||||
one-time data-collection sweep) as part of T7's WB-reference drop?
|
||||
`TextureDecodeConformanceTests.cs` should stay (genuine ongoing
|
||||
conformance for our `SurfaceDecoder`).
|
||||
*Recommendation: delete the SplitFormula test; keep the texture
|
||||
conformance tests.*
|
||||
|
||||
6. **Confirmation on 7-8 day estimate.** Given (a) T6 disappears and
|
||||
(b) T5 shrinks, the bulk of the effort is T3 (~1d) + T4 (~2d
|
||||
including the dat-shim design) + T7 (~0.5d). Net estimate looks
|
||||
**closer to 5-6 days** of pure extraction work, plus the spec's
|
||||
1d verification gate and 0.5d ship.
|
||||
*Recommendation: keep the 7-8 day envelope as scheduled time
|
||||
(includes inevitable mid-work surprise budget); call it 5-6d of
|
||||
focused engineering plus 1-2d of verification.*
|
||||
|
||||
---
|
||||
|
||||
## 9. Acceptance recap (per the prompt)
|
||||
|
||||
- [x] Audit doc exists at the agreed path.
|
||||
- [x] Every file in the closure is bucketed (T3 / T4 / T5 / T6 / NOT /
|
||||
REPLACE).
|
||||
- [x] Hidden-dependency risks section addresses internal types, source
|
||||
generators, resource files, and `DefaultDatReaderWriter` vs
|
||||
`DatCollection` semantic diff.
|
||||
- [x] No source code edited. No `dotnet build/test`. No project files
|
||||
touched. (Verify with `git status` — only this audit doc + a few
|
||||
research subreports the parallel agents would have written if they
|
||||
had Write access; in practice only this single file is on disk.)
|
||||
- [x] Thread-model finding included.
|
||||
- [x] Open questions enumerated.
|
||||
|
||||
---
|
||||
|
||||
## 10. TL;DR for the user (the four asks from the handoff prompt)
|
||||
|
||||
1. **Total LOC of the closure:** ~7,705 LOC across 33 files
|
||||
(~6,829 Chorizite + ~876 WB.Shared).
|
||||
|
||||
2. **Bucket breakdown:**
|
||||
- T3 (GL infra): ~15 files, ~3,100 LOC.
|
||||
- T4 (mesh): 8 files, ~3,313 LOC.
|
||||
- T5 (scenery + terrain stateless helpers): 5 files, ~782 LOC.
|
||||
- T6 (EnvCell / portal renderers): **0 files** — not reachable.
|
||||
- REPLACE (delete, swap to DatCollection): 2 files, 352 LOC.
|
||||
|
||||
3. **Sharp edges:**
|
||||
- Three spec §4 components turn out not to be reachable — recommend
|
||||
dropping `LandSurfaceManager`, `EnvCellRenderManager`,
|
||||
`PortalRenderManager` from the extraction plan.
|
||||
- The real risk is the `IDatReaderWriter` ↔ `DatCollection` API
|
||||
mismatch: interface vs concrete return types, multi-region cell
|
||||
dict vs single cell, `ResolveId` cross-DB search not in our
|
||||
reader. Needs a shim layer or a narrow refactor at T4.
|
||||
- Three internal types in Chorizite need `internal → public`
|
||||
promotion (`EmbeddedResourceReader`, `TextureFormatExtensions`,
|
||||
`BufferUsageExtensions`).
|
||||
- `MemoryPack` and possibly `SixLabors.ImageSharp` are new NuGet
|
||||
deps if we don't strip them out.
|
||||
- The `[indoor-upload]` diagnostic in `WbMeshAdapter` and the
|
||||
`SplitFormulaDivergenceTest` test become deletable at T7.
|
||||
|
||||
4. **Honest read on the 7-8 day estimate:** Reasonable, probably
|
||||
slightly conservative. Net extraction work is ~5-6 days of focused
|
||||
engineering; the spec's verification + ship time bring it to
|
||||
7-8 days total. The shrinkage of T5 + disappearance of T6 buys
|
||||
margin against the dat-shim design work at T4 — net-net the
|
||||
estimate holds.
|
||||
Loading…
Add table
Add a link
Reference in a new issue