docs(research): sky/weather investigation handoff + diagnostic tools

Captures everything learned from a long worktree iteration on the
foreground-rain bug (ISSUES.md #1 / #26) plus a new star-rendering
bug observed in the same area. The code work from that worktree
(WeatherDispatcher, EmitterDescLoader.LoadFromDat, WeatherCellRenderer,
GameWindow integration) was reverted because it didn't visibly fix
the rain bug — but the research findings + diagnostic tools are
durable and should not have to be rediscovered.

What's added:
- docs/research/2026-04-26-sky-investigation-handoff.md
  Comprehensive seed prompt for the next session. Covers:
  * Bug A: foreground rain (#26) — what's open, what's confirmed,
    what's been tried
  * Bug B: stars rendering as square in corner (NEW, user-observed)
  * 40-agent decomp scan findings — retail rain is NOT camera-
    particles, NOT server-driven, NOT screen-space; the mesh IS
    a hollow octagonal tube; only 5 weather GfxObjs in Dereth
  * Things ruled out by trial (envelope, scaling, unlit, depth-
    always alone, Setup loading)
  * Things to try next (depth+zfar combined, full render-state
    audit, frame ordering, star UV bug as easier first target)
  * Acceptance criteria for "done"

- docs/research/2026-04-26-chorizite-pr-draft.md
  Upstream PR draft for Chorizite/DatReaderWriter. Five generated
  DBObj source files reference nonexistent enum values and are
  silently excluded from the NuGet build:
  ParticleEmitterInfo, Clothing, PaletteSet, DataIdMapper,
  DualDataIdMapper. Fix: delete the duplicates. Independent of
  the rain work — benefits the AC modding ecosystem broadly.

- docs/research/2026-04-26-datreaderwriter-reference.md
  Developer reference for our DatReaderWriter usage. Version,
  types we consume, known broken types, thread-safety caveats,
  upgrade procedure, NuGet-vs-vendored decision matrix.

- tools/PesChainAudit/
  Recursive PES walker — given a 0x33xxxxxx script id, walks all
  CallPES references and dumps every hook + every referenced
  ParticleEmitter's parameters. Used to prove no weather PES
  emits rain particles.

- tools/TextureDump/
  Dumps texture pixel statistics (alpha histogram, brightness,
  max) and saves as PNG for visual inspection.

- tools/WeatherEnumerator/
  Enumerates every DayGroup in a Region, lists weather SkyObjects
  (Properties & 0x04), dumps GfxObj bounding boxes.

- tools/WeatherSetupProbe/
  Loads a Setup id, dumps each part's GfxObj + frame + scale +
  surface. Used to prove weather Setups are 5cm dummy carriers.

Worktree feature/sky-fixes is being deleted in a follow-up step.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-04-26 21:40:34 +02:00
parent a060f4fc98
commit 8db7a9ec28
11 changed files with 1085 additions and 0 deletions

View file

@ -0,0 +1,158 @@
# DatReaderWriter (DRW) — developer reference
Status as of **2026-04-26**. Source for "what's safe to call, what's
broken, how to upgrade." Update this doc when DRW version bumps or when
we discover a new safety/coverage gap.
## 1. Current version
- **Package:** `Chorizite.DatReaderWriter`
- **Version:** `2.1.7` (NuGet)
- Pinned in `src/AcDream.Core/AcDream.Core.csproj` and
`src/AcDream.Cli/AcDream.Cli.csproj`. App + Core both transitively
use that one version.
- Some `tools/*` (WeatherSetupProbe, WeatherEnumerator, SkyObjectInspect,
dump-keymap, PesChainAudit) `ProjectReference` the vendored repo
directly instead of NuGet — see §7.
## 2. Source repo
- **Origin:** https://github.com/Chorizite/DatReaderWriter
- **Vendored at:** `references/DatReaderWriter/` (HEAD `63e84b6 "Bump
version"` — no git tag matching the NuGet version; the repo doesn't
tag releases, ship version is `GitVersion.yml`-driven).
- **License:** MIT, © 2024 ACClientLib (`references/DatReaderWriter/LICENSE.txt`).
- **Targets:** `net8.0`, `netstandard2.0`, `net48`. We consume from
`net10.0` projects; the `net8.0` TFM resolves cleanly via netstandard
forward-compat.
- **Project layout:** `DatReaderWriter/` (library), `…SourceGenerator/`
(Roslyn generator emits packing code), `…Tests/`, `…Benchmarks/`.
## 3. Types we use
Top-level entry points:
- `DatReaderWriter.DatCollection` — the four-database container
(`Portal`, `Cell`, `HighRes`, `Local`).
- `DatReaderWriter.Options.DatCollectionOptions` (caching mode, paths).
- `DatCollection.Get<T>(id)` / `Portal.Get<T>(id)` — generic typed read.
- `DatCollection.GetAllIdsOfType<T>()` — used by CLI enumerators.
`DatReaderWriter.DBObjs.*` consumed in `src/`:
- `LandBlock`, `LandBlockInfo`, `EnvCell`, `Region`
- `GfxObj`, `Setup`, `Surface`, `SurfaceTexture` (via SurfaceDecoder)
- `Animation`, `MotionTable`
- `ParticleEmitter`, `PhysicsScript`
- `Environment` (note: collides with `System.Environment`, must be FQN
in `AcDream.Cli/Program.cs`)
- `Wave` / sound DBObjs (via `DatSoundCache`, `SoundCookbook`)
`DatReaderWriter.Types.*` consumed:
- `AnimationFrame`, `MotionData`
- `Polygon`, `VertexArray`, `Sphere` (BSPQuery, collision)
- `SoundEntry`, `AnimationHook`, `CreateParticleHook`
- `BSPNode` / `BSPTree` / `PhysicsBSPNode` / `CellBSPNode` /
`DrawingBSPNode` (BSP traversal in `BSPQuery`, `PhysicsDataCache`)
`DatReaderWriter.Enums.*` consumed:
- `MotionCommand` (aliased `DRWMotionCommand` in MotionCommandResolver)
- `Sound` (aliased `DRWSound`)
- `StipplingType`, plus various surface/material flags
## 4. Known broken / missing types
- **`RenderMaterial`** — not implemented. Per upstream README "Known
Issues." We do not consume it.
- **`LayoutDesc`** — README marks it "supported but structure will need
cleanup." Treat fields as unstable across versions. We do not depend
on it directly.
- **`ParticleEmitterInfo`** — DRW exposes the **`ParticleEmitter`**
DBObj only. There is no separate `ParticleEmitterInfo` type. Our
`Vfx/VfxModel.cs` comment that mentions "ParticleEmitterInfo dat" is
conceptual (matches the retail header struct name); the actual lookup
in `EmitterDescLoader.cs` is `dats.Portal.Get<ParticleEmitter>(id)`
with a `using DatParticleEmitter = DatReaderWriter.DBObjs.ParticleEmitter;`
alias because we have our own `ParticleEmitter` runtime class in
`AcDream.Core.Vfx`.
- **Anything not in `DBObjs/`** — only 5 DBObj files exist
(`ActionMap`, `DBProperties`, `Iteration`, `LandBlock`,
`MasterProperty`) at the top level of that folder; the rest (GfxObj,
Setup, etc.) live elsewhere in the source tree. If a type doesn't
resolve, search the DRW source — don't assume it's missing.
## 5. Unsafe patterns
**`DatCollection` is NOT thread-safe.** `DatBinReader` holds a buffer
position field per database; concurrent `Get<T>` calls from two threads
corrupt each other and produce silently half-populated payloads
(half-empty `LandBlock.Height[]` in our case). See
`memory/feedback_phase_a1_hotfix_saga.md` rule #2 — Phase A.1 burned
~3 hotfixes mis-diagnosing this as something else.
**Rules:**
1. All `DatCollection.Get<T>()` calls run on the render thread.
2. `LandblockStreamer` runs synchronously (see header comment, lines
1732 of `src/AcDream.App/Streaming/LandblockStreamer.cs`). The
worker-thread design was reverted; the channel-based outbox is
retained so the moment a thread-safe wrapper exists we can swap
loaders back to async without touching call sites.
3. Don't claim "DRW is thread-safe per its docs." There are no such
docs. The README says nothing about concurrency.
4. The async surface area (`*Async` methods) is for I/O, not
concurrent reads — calling them from two threads is still unsafe.
**Caching:** `DatCollectionOptions` caching mode is `OnDemand`. Don't
flip to `None` without re-benchmarking — terrain mesh build hits the
same `LandBlock` repeatedly.
## 6. How to upgrade
1. Bump `Version="2.1.7"` in `src/AcDream.Core/AcDream.Core.csproj` and
`src/AcDream.Cli/AcDream.Cli.csproj` (and any `tools/*` using the
NuGet package, e.g. `TextureDump`).
2. Pull `references/DatReaderWriter/` to the matching commit so the
vendored copy stays in sync (used by tools that `ProjectReference`
it directly + as a search target for "what's in DRW today").
3. `dotnet restore && dotnet build` from repo root.
4. Run the full test suite (`dotnet test`). Watch for compile breaks
on type renames (DRW has historically renamed `DBObj` types between
minor versions).
5. Smoke-test live: launch against the local ACE server and walk
Holtburg → Foundry. Watch for half-loaded landblocks (missing
heights / null surfaces) — that's the canonical "thread-safety
regression slipped in" signature.
**Risks per area:**
- **DBObj field renames** — DRW source-generates packing; field renames
break every consumer. Grep before bumping.
- **Enum value reordering**`MotionCommand`, `Sound`,
`StipplingType` are enum-by-name in our code, so adds are safe but
removals will break.
- **BSP type changes**`BSPNode` family is consumed in `BSPQuery.cs`
(collision). Any signature change there is a high-risk diff that
needs the conformance sweep re-run.
## 7. NuGet vs vendored
- **Use NuGet** (`Chorizite.DatReaderWriter` package) for everything
shipped in `src/` and any `tools/` that doesn't need DRW internals.
This is the default. Currently `AcDream.Core`, `AcDream.Cli`, and
`tools/TextureDump`.
- **Vendor (`ProjectReference` to `references/DatReaderWriter/…`)** when:
- The tool needs a DRW type or method that isn't `public` in the
NuGet build (some helpers are `internal`).
- You're prototyping a DRW patch you intend to upstream — edit in
place, build, validate, then PR upstream.
- Diagnostic / probe tools (`WeatherSetupProbe`, `WeatherEnumerator`,
`SkyObjectInspect`, `dump-keymap`, `PesChainAudit`) where iteration
speed against DRW source matters more than ship discipline.
- **Never mix** within one assembly. A project either NuGet-references
DRW or `ProjectReference`s the vendored copy. Both produces type-
identity errors at the boundary.
- **Production code stays on NuGet.** When a vendored tool proves a
DRW change is needed, upstream it, wait for a release, and bump §6.