refactor(app): extract typed RuntimeOptions for startup env vars (Step 1)
Lifts 13 startup-time environment variables out of GameWindow.cs into a
single typed AcDream.App.RuntimeOptions record read once in Program.cs.
Behavior-preservation only — no live behavior change, no visual change.
Verified end-to-end against ACE on 127.0.0.1:9000: full M1 demo loop
(walk Holtburg, click door, click NPC, portal entry) plus DEVTOOLS
ImGui panels load cleanly.
Why: GameWindow.cs is 10,304 LOC and scattered Environment.GetEnvironmentVariable
calls were one of the structural smells called out in the new
"Code Structure Rules" doc. Typed options is the safest cut to make
first because the substitution is mechanical and parsing semantics
get pinned by unit tests.
What lands:
- CLAUDE.md: removed stale R1→R8 execution-phases line, replaced with
pointers to the milestones doc + strategic roadmap (the actual
source of truth). Tightened the "check ALL FOUR references"
section to describe WB as the production rendering base, not
just a reference. New "Code Structure Rules" section (6 rules)
captures the discipline we're committing to.
- docs/architecture/acdream-architecture.md: removed dangling link
to the deleted memory/project_ui_architecture.md.
- docs/architecture/code-structure.md (NEW, 376 LOC): rationale for
the 6 rules + 6-step extraction sequence
(RuntimeOptions → LiveSessionController → LiveEntityRuntime →
SelectionInteractionController → RenderFrameOrchestrator →
GameEntity aggregation). This PR is Step 1.
- src/AcDream.App/RuntimeOptions.cs (NEW, 100 LOC): typed record
with FromEnvironment(string) factory and Parse(datDir, env)
overload for testability. Covers ACDREAM_LIVE, _TEST_HOST/PORT/
USER/PASS, _DEVTOOLS, _DUMP_MOVE_TRUTH, _NO_AUDIO,
_ENABLE_SKY_PES, _HIDE_PART, _RETAIL_CLOSE_DEGRADES,
_DUMP_SCENERY_Z, _STREAM_RADIUS.
- src/AcDream.App/Program.cs: builds RuntimeOptions once, passes
to GameWindow.
- src/AcDream.App/Rendering/GameWindow.cs: ctor takes RuntimeOptions;
7 startup-cached env-var fields become expression-bodied
properties or direct _options.X reads; TryStartLiveSession,
audio init, legacy stream-radius branch all route through
_options.
- tests/AcDream.App.Tests/ (NEW project, 10 unit tests + csproj):
pins parser semantics — default-off bools, the literal "0"
gate for RETAIL_CLOSE_DEGRADES, the >=0 guard for
STREAM_RADIUS, null-vs-empty for user/pass, exact-"1" check
for diagnostic flags. Registered in AcDream.slnx.
Out of scope (per code-structure.md §4):
- Per-call-site ACDREAM_DUMP_* / _REMOTE_VEL_DIAG diagnostic reads
sprinkled through GameWindow (~40 sites). Rule 5 in CLAUDE.md
commits us to migrating these opportunistically as larger
extractions land, not in a bulk pass.
- AcDream.Core's project-reference to Chorizite.OpenGLSDLBackend.
Only the stateless .Lib namespace is used; tightening the project
reference is documented as future work in code-structure.md §2.
Build: green.
Tests: AcDream.App.Tests 10/10 ✓, Core.Net.Tests 294/294 ✓,
UI.Abstractions.Tests 419/419 ✓,
AcDream.Core.Tests 1073/1081 (8 pre-existing failures verified
against pre-refactor baseline by stash-and-rerun).
Visual verification: full M1 demo loop against ACE +Acdream login
including DEVTOOLS panel host load.
Next: Step 2 — extract LiveSessionController per code-structure.md §4.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
2950cd5740
commit
eda936dc4d
9 changed files with 863 additions and 63 deletions
376
docs/architecture/code-structure.md
Normal file
376
docs/architecture/code-structure.md
Normal file
|
|
@ -0,0 +1,376 @@
|
|||
# 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/ # (already exists — WB seam)
|
||||
│ └── 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/ # (already exists)
|
||||
├── 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` + `_selectedGuid` + `SendUse` / `SendPickUp` → moved
|
||||
into `SelectionInteractionController`.
|
||||
- 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`, `_selectedGuid`, `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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue