acdream/docs/architecture/code-structure.md
Erik b0c175afc0 fix(streaming): commit EnvCell landblocks atomically (#214)
Carry one complete cell payload with each streaming completion and publish visibility, physics, and render state on the render thread. Remove the obsolete post-readiness login reload that triggered a duplicate dungeon build.

Co-Authored-By: Codex <noreply@openai.com>
2026-07-13 18:45:24 +02:00

377 lines
16 KiB
Markdown

# acdream — code structure & extraction sequence
**Status:** Living document. Created 2026-05-16 as the companion to the
"Code Structure Rules" section in `CLAUDE.md`.
**Purpose:** Describe the desired structural state of the App layer,
explain the rules we've adopted, and lay out the safe extraction
sequence from today's reality (one 10,304-line `GameWindow.cs`) to the
target (thin `GameWindow`, small focused collaborators).
**Companion to:** [`acdream-architecture.md`](acdream-architecture.md)
(the layered architecture) and
[`worldbuilder-inventory.md`](worldbuilder-inventory.md) (what we take
from WB vs port ourselves).
---
## 1. The structural problem we're solving
The layered architecture works: `AcDream.Core` is GL-free, the network
layer is wire-compatible, the UI has a stable contract, plugins load.
The structural debt is concentrated in **one file**:
```
src/AcDream.App/Rendering/GameWindow.cs 10,304 lines
```
`GameWindow` is the single object that:
- Owns the GL context, the window, input, and shaders.
- Reads ~40 different environment variables across its lifetime.
- Hosts the live network session (`WorldSession`) and the offline
pre-login state.
- Owns parallel dictionaries for entity lookup (`_entitiesByServerGuid`,
the per-landblock entity lists in `GpuWorldState`, plus the player
controller's own state).
- Drives selection / interaction (`WorldPicker`, `SendUse`,
`SendPickUp`).
- Drives per-frame render orchestration (sky → terrain → opaque mesh →
transparent mesh → particles → debug lines → UI).
- Wires up every plugin hook sink, every diagnostic, every panel.
Almost every M1 / M2 bug touches this file. Every new feature adds a
field plus a method plus a wiring call. It is not getting better on its
own.
The fix is **not** "rewrite `GameWindow` in one pass" — that's a
high-risk change that would block M2. The fix is to **extract one
collaborator at a time**, verify behavior is unchanged, ship, and
move on. This document defines that sequence.
---
## 2. Code Structure Rules — the discipline
Recap of the rules from `CLAUDE.md` with the rationale:
### Rule 1: No new substantial feature bodies in `GameWindow.cs`
**Why:** Every line we add to `GameWindow` makes the eventual decomposition
harder. New features that "live in" `GameWindow` instead of being
extracted are the reason the file is 10k lines.
**How to apply:** A new feature gets its own class under
`src/AcDream.App/<Subsystem>/` (or deeper in `AcDream.Core` if it's pure
logic). `GameWindow` owns a field and a wiring call, nothing more. If
you find yourself adding a 200-line method to `GameWindow`, stop and
extract.
**Exemption:** Trivial wiring that *must* stay in `GameWindow` because
it touches GL state during `OnLoad` is acceptable, but should still
delegate to a collaborator for the substance.
### Rule 2: `AcDream.Core` must not depend on window / GL / backend projects
**Why:** Core is the GL-free, testable layer. The moment Core imports
a GL or windowing namespace, we've lost the ability to test it without
a graphics context, and the layer split becomes fiction.
**How to apply:** The only currently-allowed seams from Core into the
WB / GL world are:
- `WorldBuilder.Shared` — stateless helpers (`TerrainUtils`,
`TerrainEntry`, `RegionInfo`).
- `Chorizite.OpenGLSDLBackend.Lib` — stateless helpers
(`SceneryHelpers`, `TextureHelpers`).
Both are leaf namespaces with no GL state. If you need a new seam (e.g.
WB's `ObjectMeshManager` needs to be visible from Core), the change
**must** come with an inventory-doc update justifying it and ideally a
slim interface in Core that the App layer implements.
**Current reality:** `src/AcDream.Core/AcDream.Core.csproj` references
`Chorizite.OpenGLSDLBackend` (not just `OpenGLSDLBackend.Lib`). This is
historical. Two Core files import from it:
- `World/SceneryGenerator.cs``Chorizite.OpenGLSDLBackend.Lib`
- `Textures/SurfaceDecoder.cs``Chorizite.OpenGLSDLBackend.Lib`
Both use the stateless `Lib` namespace only. The full project reference
is wider than it needs to be; tightening it to just `WorldBuilder.Shared`
+ a stateless-helpers shim is a candidate future cut, but not urgent.
### Rule 3: UI panels target `AcDream.UI.Abstractions` only
**Why:** This is the one rule that keeps D.2b (the future retail-look
backend) viable. Every panel that imports `ImGuiNET` directly is a panel
we'd have to rewrite when the backend swaps.
**How to apply:** A panel's `using` block must mention
`AcDream.UI.Abstractions.*` and nothing from `AcDream.UI.ImGui`. The
panel writes against `IPanelRenderer`. The `ImGuiPanelRenderer`
translates those calls to ImGui at runtime. Plugin-facing UI follows the
same rule.
### Rule 4: Startup env vars enter through `RuntimeOptions`
**Why:** Environment variables are global mutable state. Reading them
at random call sites means (a) duplicated `Environment.GetEnvironmentVariable`
boilerplate, (b) no single place to see "what flags does the client
respond to?", (c) impossible to unit-test parsing.
**How to apply:** `src/AcDream.App/RuntimeOptions.cs` is the typed
options object. `Program.cs` builds it once from args + env and passes
it to `GameWindow`. New startup flags add a field to `RuntimeOptions`
and a parser in `RuntimeOptions.FromEnvironment`. They don't add
`Environment.GetEnvironmentVariable` reads.
**Scope:** `RuntimeOptions` is for **startup-time** configuration —
things that don't change once the window is up. Runtime diagnostic
toggles are Rule 5's domain.
### Rule 5: Runtime diagnostic toggles live in diagnostic owner classes
**Why:** Diagnostic flags (`ACDREAM_DUMP_MOTION`, `ACDREAM_PROBE_*`,
etc.) need to be both env-readable at startup *and* runtime-toggleable
from the DebugPanel. Per-call-site env reads can't be runtime-toggled.
**How to apply:** Today's template is
`src/AcDream.Core/Physics/PhysicsDiagnostics.cs` — one static class with
typed `Probe*` properties read from env vars once at startup, plus
runtime setters that the DebugPanel binds. New diagnostic flags follow
this shape, not the per-call-site pattern that dominates `GameWindow.cs`.
**Cleanup direction:** The dozens of existing `ACDREAM_DUMP_*` reads
inside `GameWindow.cs` are tech debt. We do NOT bulk-migrate them as
part of this refactor — they're working, they're scattered, and
moving them carries risk without a current acceptor. We migrate them
opportunistically: when a `GameWindow` extraction lands and a diagnostic
moves with it, route it through the new owner's diagnostic class.
### Rule 6: Tests live in the project matching the layer
**Why:** Test discoverability + dependency hygiene. A test for a Core
class belongs next to other Core tests; a test for an App class belongs
in an App test project. Co-locating tests across layers makes the
dependency graph dishonest.
**How to apply:** One test project per source project that has tests.
Today:
- `tests/AcDream.Core.Tests/``src/AcDream.Core/`
- `tests/AcDream.Core.Net.Tests/``src/AcDream.Core.Net/`
- `tests/AcDream.UI.Abstractions.Tests/``src/AcDream.UI.Abstractions/`
`AcDream.App` does **not** yet have a test project. The RuntimeOptions
extraction is the trigger to create `tests/AcDream.App.Tests/`. Future
App-layer tests (LiveSessionController, SelectionInteractionController,
etc.) go there.
---
## 3. Target structure of the App layer
The end state — not what we're shipping in one pass, but the shape
we're aiming at.
```
src/AcDream.App/
├── Program.cs # parse args + env → RuntimeOptions, build GameWindow
├── RuntimeOptions.cs # typed startup options (Rule 4)
├── Rendering/
│ ├── GameWindow.cs # thin: GL/window lifecycle + delegates per-frame to RenderFrameOrchestrator
│ ├── RenderFrameOrchestrator.cs # per-frame draw order (sky → terrain → opaque → trans → particles → debug → UI)
│ ├── TerrainModernRenderer.cs # (already exists)
│ ├── TextureCache.cs # (already exists)
│ ├── ParticleRenderer.cs # (already exists)
│ ├── Sky/ # (already exists)
│ ├── Wb/ # WB seam + EnvCellLandblockBuild transaction
│ └── Vfx/ # (already exists)
├── Net/
│ └── LiveSessionController.cs # owns WorldSession lifecycle, login/handshake, reconnect
├── World/
│ └── LiveEntityRuntime.cs # owns per-entity state dicts + ServerGuid↔entity.Id translation
├── Interaction/
│ └── SelectionInteractionController.cs # owns WorldPicker, selection state, Use/PickUp dispatch
├── Streaming/ # LandblockStreamer + immutable LandblockBuild completion
├── Input/ # (already exists)
├── Audio/ # (already exists)
└── Plugins/ # (already exists)
```
What `GameWindow` keeps:
- `IWindow` / `GL` / `IInputContext` lifecycle (constructor + `OnLoad` +
`Run` + `OnClosing`).
- `RuntimeOptions` reference (the typed startup config).
- One field per collaborator (`_liveSessionController`,
`_liveEntityRuntime`, `_selectionInteraction`,
`_renderFrameOrchestrator`).
- The Silk.NET event-handler stubs that delegate to collaborators.
What `GameWindow` loses:
- The 7 startup-time env var fields → moved into `RuntimeOptions`.
- `TryStartLiveSession` + the post-login network drain → moved into
`LiveSessionController`.
- `_entitiesByServerGuid` + per-entity dictionaries + ServerGuid↔Id
translation → moved into `LiveEntityRuntime`.
- `WorldPicker` + selection-driven `SendUse` / `SendPickUp` orchestration → moved
into `SelectionInteractionController`; Core `SelectionState` remains the
already-shipped session owner and is injected into that controller.
- Per-frame draw orchestration → moved into `RenderFrameOrchestrator`.
The eventual `GameEntity` aggregation (target state described in
`acdream-architecture.md` §"GameEntity: The Unified Entity") happens
**after** `LiveEntityRuntime` is the single owner of entity state.
Until then, the parallel-dicts problem is bounded inside one class
instead of spread across `GameWindow`.
---
## 4. Extraction sequence — safest first
Each step is **one PR-sized refactor**. Each must build clean, all
tests pass, and visual verification at Holtburg looks identical to
the previous step. Don't bundle two steps.
### Step 1 — `RuntimeOptions` (this PR)
**Scope:** Replace startup-time env var reads with a typed options
object built once in `Program.cs`.
**Behavior change:** None. Same env vars, same defaults, same effects.
**Risk:** Low. Mechanical substitution at ~10-15 call sites in
`GameWindow.cs` + one constructor signature change.
**Test:** Unit tests for `RuntimeOptions.FromEnvironment` parsing (the
new `tests/AcDream.App.Tests/` project).
**Verification:** `dotnet build` + `dotnet test` green. Visual launch
verifies live mode + dat dir resolution still work.
### Step 2 — `LiveSessionController`
**Scope:** Extract `TryStartLiveSession` + the WorldSession ownership +
the post-EnterWorld drain (`OnLiveStateUpdated`, `OnLiveEntityDeleted`,
etc.) into a controller class.
**Behavior change:** None. Same wire behavior, same handshake.
**Risk:** Medium. WorldSession lifecycle is load-bearing — every
session-state crash would surface here. The change is a class
extraction with the same event subscriptions, not a rewrite.
**Test:** Existing `AcDream.Core.Net.Tests` already cover the wire
layer. The controller itself gets a smoke test that verifies it can be
constructed without a live socket (offline mode).
**Verification:** Visual login + Holtburg traversal + door interaction
identical to pre-extraction.
### Step 3 — `LiveEntityRuntime` (or `EntityRuntimeRegistry`)
**Scope:** Centralize the parallel dictionaries — `_entitiesByServerGuid`,
the streaming entity lists, the player controller's player entity —
into one owner. Surface the ServerGuid↔entity.Id translation as a
single API (eliminating the trap from L.2g slice 1c).
**Behavior change:** None.
**Risk:** Medium-high. Entity lookup is in every hot path. The change
is structural (one owner instead of three) but the lookup semantics
must be byte-identical.
**Test:** Entity-spawn / despawn / lookup tests in
`tests/AcDream.App.Tests/`. Existing visual verification at Holtburg
catches any drift in interaction.
**Verification:** Walk Holtburg, click NPC, open door, pick up item.
All four M1 demo targets must still work.
### Step 4 — `SelectionInteractionController`
**Scope:** Extract `WorldPicker`, the Core `SelectionState` consumers, `SendUse`,
`SendPickUp`, and the `InputAction.Select*` / `UseSelected` /
`SelectionPickUp` switch cases into one controller. Depends on Step 3
(uses `LiveEntityRuntime`).
**Behavior change:** None.
**Risk:** Low-medium. Selection state is local to interactions; the
network outbound side is well-defined (`InteractRequests.BuildUse` /
`BuildPickUp`).
**Test:** Selection state machine tests in `tests/AcDream.App.Tests/`.
**Verification:** Click-to-select, double-click-to-Use, F-key pickup
all still work.
### Step 5 — `RenderFrameOrchestrator`
**Scope:** Extract the per-frame draw sequence (sky → terrain →
opaque mesh → translucent mesh → particles → debug → UI) into a
dedicated orchestrator that `GameWindow.OnRender` delegates to.
**Behavior change:** None. Same draw order, same GL state.
**Risk:** Medium. GL state management is touchy; the orchestrator
must hand the GL context to the same renderers in the same order with
the same per-pass state setup.
**Test:** Visual verification only. Render orchestration is hard to
unit-test without a GL context.
**Verification:** Holtburg at radius 4, radius 8, radius 12 looks
identical across all four quality presets.
### Step 6 — `GameEntity` aggregation (the big one)
**Scope:** Consolidate `WorldEntity` + `AnimatedEntity` + the per-entity
state in `LiveEntityRuntime` into one `GameEntity` class (the target
described in `acdream-architecture.md`). Every entity in the world —
player, NPC, monster, door, item — becomes a single `GameEntity`.
**Behavior change:** None at the wire / visual level; substantial at
the call-site level (everyone moves to the new entity API).
**Risk:** High. Touches every system that reads entity state.
**Test:** All existing tests + the new `AcDream.App.Tests` suite. Visual
verification at every M1 / M2 scenario.
**Verification:** Full M2 demo loop (equip sword, kill drudge, pick up
loot, open inventory) works identically.
---
## 5. Rules of the road during the extraction
1. **One step at a time.** A PR that ships Step 1 ships only Step 1.
Bundling steps makes failures hard to isolate.
2. **Behavior preservation is the acceptance criterion.** Every step
must build clean, all tests pass, and visual verification at the
appropriate M1 / M2 scenarios must succeed. We're moving code, not
changing it.
3. **No new features during an extraction step.** If you spot a real
bug while extracting, file it in `docs/ISSUES.md` and address it in
a separate commit (before or after the extraction, not folded into
it).
4. **Diagnostic toggle migrations are opportunistic.** When a method
moves to a new owner, the diagnostic flag inside it can move to a
diagnostic class as part of the same commit. We do not do a bulk
diagnostic-cleanup pass.
5. **Update this document when the plan changes.** If Step 3 turns out
to need a different shape than described above, update §4 in the
same session you discover the divergence.
---
## 6. What this document is **not**
- **Not a full rewrite plan.** The point is the *opposite* — small
steps, verified at each boundary.
- **Not blocking M2.** Step 1 is small enough to ship without
disrupting M2 work. Later steps interleave with M2 / M3 phases as
the corresponding code paths come into focus.
- **Not a substitute for the milestones / roadmap.** Those drive the
feature work. This drives the structural work that runs underneath.