diff --git a/AcDream.slnx b/AcDream.slnx index 1cf8f24..c004e10 100644 --- a/AcDream.slnx +++ b/AcDream.slnx @@ -13,6 +13,7 @@ + diff --git a/CLAUDE.md b/CLAUDE.md index 2210334..b12d1d1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -126,12 +126,18 @@ ourselves". both radii and MSAA/anisotropic/A2C/completions-per-frame as a unit. Spec: `docs/superpowers/specs/2026-05-09-phase-a5-two-tier-streaming-design.md`. -**Execution phases:** R1→R8 in the architecture doc. Each phase has clear -goals, test criteria, and builds on the previous. Don't skip phases. +**Execution model:** the active source of truth is the **milestones doc** +(`docs/plans/2026-05-12-milestones.md`) for "what are we building right +now" and the **strategic roadmap** (`docs/plans/2026-04-11-roadmap.md`) +for the per-phase ledger of what's shipped, what's in flight, and what +comes next. **Ignore the old "R1→R8" sequence** — it was an early refactor +sketch that no longer matches reality (see the "Roadmap Model" section in +`docs/architecture/acdream-architecture.md`). Per-phase detailed specs +live under `docs/superpowers/specs/`. -The codebase is organized by layer (see architecture doc). Current phase -state lives in memory (`memory/project_*.md`), plans in `docs/plans/`, -research in `docs/research/`. +The codebase is organized by layer (see architecture doc + the **Code +Structure Rules** section below). Plans live in `docs/plans/`, +research in `docs/research/`, persistent project memory in `memory/`. **UI strategy:** three-layer split — swappable backend (ImGui.NET + `Silk.NET.OpenGL.Extensions.ImGui` for Phase D.2a, custom retail-look @@ -164,12 +170,74 @@ click-to-rebind. As of Phase K (2026-04-26), ALL keyboard / mouse input flows through the dispatcher — no IsKeyPressed polling outside the per-frame movement queries. +## Code Structure Rules + +These are the structural rules the project commits to. They are +**process rules** (where code goes, what depends on what), not style +rules (formatting / naming). They exist to keep the layer split honest +and to stop `GameWindow.cs` from continuing to grow into a 10k-line +god object. The full rationale + the extraction sequence we're +pursuing live in [`docs/architecture/code-structure.md`](docs/architecture/code-structure.md). + +1. **No new substantial feature bodies in `GameWindow.cs`.** It is + already over 10,000 lines and owns runtime wiring. New runtime + work goes into a dedicated controller / sink / orchestrator class + in `src/AcDream.App/` (or deeper in `AcDream.Core` when it's pure + logic). Adding a handful of fields and a one-paragraph method to + wire an extracted class in is fine; adding a new ~200-line feature + directly is not. When in doubt, file a small follow-up extraction + as part of the change. + +2. **`AcDream.Core` must not depend on the window / GL / backend + projects, except via documented interop seams.** The only + currently-allowed seams are `WorldBuilder.Shared` (stateless helpers: + `TerrainUtils`, `TerrainEntry`, `RegionInfo`) and + `Chorizite.OpenGLSDLBackend.Lib` (stateless helpers only: + `SceneryHelpers`, `TextureHelpers`). New Core code that needs a GL + surface must define an interface in Core and let `AcDream.App` + implement it — never the reverse. If you need to add a project + reference to Core, the change must come with an inventory-doc + update explaining why. + +3. **UI panels target `AcDream.UI.Abstractions` only.** No panel may + import `AcDream.UI.ImGui` or any backend namespace. ViewModels, + commands, and the `IPanel` / `IPanelRenderer` contract are the + surface; everything else is backend. This is what lets us swap + D.2a (ImGui) for D.2b (retail-look) later without rewriting + panels. + +4. **Startup environment variables enter through a typed options + object.** `AcDream.App.RuntimeOptions` is the single source of + truth for startup configuration. `Program.cs` reads the + environment once into `RuntimeOptions` and passes it into + `GameWindow`. Don't sprinkle `Environment.GetEnvironmentVariable` + reads through new code paths; add a field to `RuntimeOptions` and + pipe it through. + +5. **Runtime probes (diagnostic toggles) belong in diagnostic owner + classes.** Today `AcDream.Core.Physics.PhysicsDiagnostics` owns the + `ACDREAM_PROBE_*` family. The pattern: one static class per + subsystem, exposing typed bool/int properties read from env vars + once at startup (with optional runtime-toggleable counterparts for + the DebugPanel). Per-call-site `Environment.GetEnvironmentVariable` + reads in new code are a process smell — if a flag survives one + phase, promote it to a diagnostic owner. The dozens of existing + `ACDREAM_DUMP_*` reads scattered through `GameWindow` are tech + debt; do not add more. + +6. **Tests live in the project matching the layer under test.** Core + tests in `tests/AcDream.Core.Tests/`, UI tests in + `tests/AcDream.UI.Abstractions.Tests/`, network tests in + `tests/AcDream.Core.Net.Tests/`. App-layer tests (RuntimeOptions + parsing, etc.) belong in `tests/AcDream.App.Tests/`. When adding a + new test project, register it in `AcDream.slnx`. + ## How to operate **You are the lead engineer AND architect on this project at all times.** You own the architecture (`docs/architecture/acdream-architecture.md`), -the execution plan (phases R1–R8), the development workflow, and all -technical decisions. Stop as little as possible. Drive work autonomously and continuously through full phases and +the execution plan (milestones doc + strategic roadmap), the development +workflow, and all technical decisions. Stop as little as possible. Drive work autonomously and continuously through full phases and across commit boundaries. Do not stop mid-phase for routine progress check-ins, permission asks on low-stakes design calls, or "should I continue?" confirmations. The user has repeatedly authorized direct-to-main commits, multi-commit sessions, @@ -1095,13 +1163,19 @@ already-running ACE session via the handshake race. stop transitions, and keep their visual position tracked smoothly between the 5–10 Hz UpdatePosition bursts (dead-reckoning). -## Reference repos: check ALL FOUR, not just one +## Reference repos: cross-check the relevant ones -When researching a protocol detail, dat format, rendering algorithm, or -any "how does AC do X" question, **check all four of the vendored -references in `references/`** before committing to an approach. Do not -settle on the first hit and move on — cross-reference at least two of -these, ideally all four: +The `references/` tree holds **six** vendored projects (ACE, ACViewer, +WorldBuilder, Chorizite.ACProtocol, holtburger, AC2D). They overlap in +some areas and disagree in others. Before committing to an approach, +**cross-reference at least two of them** for the domain you're working +in — the per-domain hierarchy in the next section tells you which to +read first. A single reference can be misleading; the intersection of +the relevant references is almost always the truth. The user has +repeatedly had to remind me about this when I narrowly searched one ref +and missed obvious answers in another. + +The six references: - **`references/ACE/`** — ACEmulator server. Authority on the wire protocol (packet framing, ISAAC, game message opcodes, serialization @@ -1113,17 +1187,21 @@ these, ideally all four: `ACViewer/Render/TextureCache.cs::IndexToColor` for the canonical subpalette overlay algorithm. - **`references/WorldBuilder/`** — **acdream's rendering + dat-handling - BASE (not just a reference).** As of 2026-05-08 acdream is moving to - fork WorldBuilder upstream and depend on the fork for terrain, - scenery, static objects, EnvCells, portals, sky, particles, texture - decoding, mesh extraction, visibility/culling. WorldBuilder is - MIT-licensed, exact-stack match (Silk.NET + .NET), and verified to - render the world correctly. **Before re-porting any rendering or - dat-handling algorithm from retail decomp, check - `docs/architecture/worldbuilder-inventory.md` first.** If WB has it, - use WB's port. If WB doesn't have it (network, physics, animation, - movement, UI, plugin, audio, chat), port from retail decomp as - before. + base.** WorldBuilder is not just a reference: as of Phase N.4 (shipped + 2026-05-08), `ObjectMeshManager` is the production mesh pipeline, + `WbMeshAdapter` is the seam, and `WbDrawDispatcher` is the production + draw path. The modern path (`N.5`) is **mandatory** — missing bindless + throws at startup, there is no legacy fallback. **Before re-porting + any rendering or dat-handling algorithm from retail decomp, read + `docs/architecture/worldbuilder-inventory.md` first.** The inventory + tells you what WB covers (terrain, scenery, static objects, EnvCells, + portals, sky, particles, texture decoding, mesh extraction, + visibility/culling) and what we still write ourselves (the 🔴 list: + network, physics, animation, movement, UI, plugin, audio, chat). + WorldBuilder is MIT-licensed and exact-stack with us (Silk.NET + + .NET); the divergences we've documented (e.g. WB's terrain split + formula vs retail's `FSplitNESW`) are called out in the inventory + doc. - **`references/Chorizite.ACProtocol/`** — clean-room C# protocol library generated from a protocol XML description. Useful sanity check on field order, packed-dword conventions, type-prefix handling. The diff --git a/docs/architecture/acdream-architecture.md b/docs/architecture/acdream-architecture.md index 5f134da..2ada332 100644 --- a/docs/architecture/acdream-architecture.md +++ b/docs/architecture/acdream-architecture.md @@ -95,8 +95,8 @@ designed 2026-04-24. Full design: `docs/plans/2026-04-24-ui-framework.md`. The backend is pluggable; ViewModels / Commands / `IPanelRenderer` are stable across the swap. ImGui persists forever as the `ACDREAM_DEVTOOLS=1` devtools overlay regardless of which backend owns -the game UI. See `memory/project_ui_architecture.md` for the session -crib-sheet version. +the game UI. The full UI design lives in +`docs/plans/2026-04-24-ui-framework.md`. --- diff --git a/docs/architecture/code-structure.md b/docs/architecture/code-structure.md new file mode 100644 index 0000000..288efd0 --- /dev/null +++ b/docs/architecture/code-structure.md @@ -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//` (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. diff --git a/src/AcDream.App/Program.cs b/src/AcDream.App/Program.cs index e989b6e..bc43997 100644 --- a/src/AcDream.App/Program.cs +++ b/src/AcDream.App/Program.cs @@ -1,3 +1,4 @@ +using AcDream.App; using AcDream.App.Plugins; using AcDream.App.Rendering; using AcDream.Core.Plugins; @@ -15,6 +16,11 @@ if (string.IsNullOrWhiteSpace(datDir)) return 2; } +// Single read of the startup-time process environment. Every downstream +// consumer (GameWindow + collaborators) reads the typed bundle, not the +// raw env vars. See docs/architecture/code-structure.md §2 Rule 4. +var runtimeOptions = RuntimeOptions.FromEnvironment(datDir); + var worldGameState = new AcDream.Core.Plugins.WorldGameState(); var worldEvents = new AcDream.Core.Plugins.WorldEvents(); var host = new AppPluginHost(new SerilogAdapter(Log.Logger), worldGameState, worldEvents); @@ -50,7 +56,7 @@ try catch (Exception ex) { Log.Error(ex, "plugin enable failed: {Id}", plugin.Manifest.Id); } } - using var window = new GameWindow(datDir, worldGameState, worldEvents); + using var window = new GameWindow(runtimeOptions, worldGameState, worldEvents); window.Run(); } finally diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 471149f..6f5b9af 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -12,6 +12,7 @@ public sealed class GameWindow : IDisposable { private readonly record struct SkyPesKey(int ObjectIndex, uint PesObjectId, bool PostScene); + private readonly AcDream.App.RuntimeOptions _options; private readonly string _datDir; private readonly WorldGameState _worldGameState; private readonly WorldEvents _worldEvents; @@ -219,12 +220,10 @@ public sealed class GameWindow : IDisposable // Retail GameSky copies SkyObject.PesObjectId into CelestialPosition but // never consumes it in CreateDeletePhysicsObjects/MakeObject/UseTime. // Keep the experimental path available for DAT archaeology only. - private readonly bool _enableSkyPesDebug = - string.Equals(Environment.GetEnvironmentVariable("ACDREAM_ENABLE_SKY_PES"), "1", StringComparison.Ordinal); + // Backed by ACDREAM_ENABLE_SKY_PES via RuntimeOptions.EnableSkyPesDebug. // Diagnostic: hide a specific humanoid part (>=10 parts) at render. - private static readonly int s_hidePartIndex = - int.TryParse(Environment.GetEnvironmentVariable("ACDREAM_HIDE_PART"), out var hp) ? hp : -1; + // Backed by ACDREAM_HIDE_PART via RuntimeOptions.HidePartIndex. // Issue #47 — use retail's close-detail GfxObj selection on // humanoid setups. When enabled, every per-part GfxObj id (after @@ -233,8 +232,7 @@ public sealed class GameWindow : IDisposable // for the full retail-decomp citation. Default-on after visual // confirmation; set ACDREAM_RETAIL_CLOSE_DEGRADES=0 only for // diagnostic before/after comparisons. - private static readonly bool s_retailCloseDegrades = - !string.Equals(Environment.GetEnvironmentVariable("ACDREAM_RETAIL_CLOSE_DEGRADES"), "0", StringComparison.Ordinal); + // Backed by ACDREAM_RETAIL_CLOSE_DEGRADES via RuntimeOptions.RetailCloseDegrades. // Issue #48 diagnostic — dump per-scenery-spawn placement evidence // (rendered gfx id, sample source physics-vs-bilinear, ground/baseLoc/finalZ, @@ -242,8 +240,7 @@ public sealed class GameWindow : IDisposable // the user identify a floating tree by its world coordinates and tell // whether the cause is BaseLoc.Z addition (H1), bilinear-fallback drift // (H2), or DIDDegrade selection (H3). Diagnostic-first per CLAUDE.md. - private static readonly bool s_dumpSceneryZ = - string.Equals(Environment.GetEnvironmentVariable("ACDREAM_DUMP_SCENERY_Z"), "1", StringComparison.Ordinal); + // Backed by ACDREAM_DUMP_SCENERY_Z via RuntimeOptions.DumpSceneryZ. /// /// Issue #47 humanoid-setup detector. Matches Aluvian Male @@ -575,10 +572,11 @@ public sealed class GameWindow : IDisposable // _panelHost does. Self-subscribes to CombatState in its ctor, so // disposing isn't required (panel host holds the only ref). private AcDream.UI.Abstractions.Panels.Debug.DebugVM? _debugVm; - private static readonly bool DevToolsEnabled = - Environment.GetEnvironmentVariable("ACDREAM_DEVTOOLS") == "1"; - private static readonly bool DumpMoveTruthEnabled = - Environment.GetEnvironmentVariable("ACDREAM_DUMP_MOVE_TRUTH") == "1"; + // DevToolsEnabled + DumpMoveTruthEnabled now read through _options + // (RuntimeOptions.DevTools / DumpMoveTruth). Kept the same names for + // local readability via expression-bodied properties. + private bool DevToolsEnabled => _options.DevTools; + private bool DumpMoveTruthEnabled => _options.DumpMoveTruth; // Phase I.3 — real ICommandBus for live sessions. Constructed when // the live session spins up (so SendChatCmd handlers can close over @@ -748,8 +746,8 @@ public sealed class GameWindow : IDisposable // K-fix1 (2026-04-26): cached at startup so per-frame branches are // single-flag reads instead of env-var lookups. True iff // ACDREAM_LIVE=1 was set when the window came up. - private static readonly bool LiveModeEnabled = - Environment.GetEnvironmentVariable("ACDREAM_LIVE") == "1"; + // Backed by RuntimeOptions.LiveMode via the _options field. + private bool LiveModeEnabled => _options.LiveMode; /// /// K-fix1 (2026-04-26): true iff live mode is configured AND we have @@ -814,9 +812,10 @@ public sealed class GameWindow : IDisposable private int _liveAnimRejectSingleFrame; private int _liveAnimRejectPartFrames; - public GameWindow(string datDir, WorldGameState worldGameState, WorldEvents worldEvents) + public GameWindow(AcDream.App.RuntimeOptions options, WorldGameState worldGameState, WorldEvents worldEvents) { - _datDir = datDir; + _options = options ?? throw new System.ArgumentNullException(nameof(options)); + _datDir = options.DatDir; _worldGameState = worldGameState; _worldEvents = worldEvents; SpellBook = new AcDream.Core.Spells.Spellbook(SpellTable); @@ -1104,7 +1103,7 @@ public sealed class GameWindow : IDisposable // Phase E.2 audio: init OpenAL + hook sink. Suppressible via // ACDREAM_NO_AUDIO=1 for headless tests / broken audio drivers. - if (Environment.GetEnvironmentVariable("ACDREAM_NO_AUDIO") != "1") + if (!_options.NoAudio) { try { @@ -1749,15 +1748,13 @@ public sealed class GameWindow : IDisposable _farRadius = _resolvedQuality.FarRadius; // Legacy override: ACDREAM_STREAM_RADIUS acts as nearRadius and - // ensures farRadius >= streamRadius. + // ensures farRadius >= streamRadius. Parsed once into + // RuntimeOptions.LegacyStreamRadius (null when unset or invalid). + if (_options.LegacyStreamRadius is { } sr) { - var legacyEnv = Environment.GetEnvironmentVariable("ACDREAM_STREAM_RADIUS"); - if (int.TryParse(legacyEnv, out var sr) && sr >= 0) - { - _nearRadius = sr; - _streamingRadius = sr; // keep debug overlay in sync - _farRadius = System.Math.Max(sr, _farRadius); - } + _nearRadius = sr; + _streamingRadius = sr; // keep debug overlay in sync + _farRadius = System.Math.Max(sr, _farRadius); } Console.WriteLine( $"streaming: nearRadius={_nearRadius} (window={2*_nearRadius+1}x{2*_nearRadius+1})" + @@ -1822,12 +1819,12 @@ public sealed class GameWindow : IDisposable private void TryStartLiveSession() { - if (Environment.GetEnvironmentVariable("ACDREAM_LIVE") != "1") return; + if (!_options.LiveMode) return; - var host = Environment.GetEnvironmentVariable("ACDREAM_TEST_HOST") ?? "127.0.0.1"; - var portStr = Environment.GetEnvironmentVariable("ACDREAM_TEST_PORT") ?? "9000"; - var user = Environment.GetEnvironmentVariable("ACDREAM_TEST_USER"); - var pass = Environment.GetEnvironmentVariable("ACDREAM_TEST_PASS"); + var host = _options.LiveHost; + var port = _options.LivePort; + var user = _options.LiveUser; + var pass = _options.LivePass; if (string.IsNullOrEmpty(user) || string.IsNullOrEmpty(pass)) { Console.WriteLine("live: ACDREAM_LIVE set but TEST_USER/TEST_PASS missing; skipping"); @@ -1850,7 +1847,7 @@ public sealed class GameWindow : IDisposable $"DNS resolved no addresses for '{host}'")); Console.WriteLine($"live: resolved {host} → {ip}"); } - var endpoint = new System.Net.IPEndPoint(ip, int.Parse(portStr)); + var endpoint = new System.Net.IPEndPoint(ip, port); Console.WriteLine($"live: connecting to {endpoint} as {user}"); _liveSession = new AcDream.Core.Net.WorldSession(endpoint); _liveSession.EntitySpawned += OnLiveEntitySpawned; @@ -2123,7 +2120,7 @@ public sealed class GameWindow : IDisposable _liveSession.VitalCurrentUpdated += v => LocalPlayer.OnVitalCurrent(v.VitalId, v.Current); - Chat.OnSystemMessage($"connecting to {host}:{portStr} as {user}", chatType: 1); + Chat.OnSystemMessage($"connecting to {host}:{port} as {user}", chatType: 1); _liveSession.Connect(user, pass); Chat.OnSystemMessage("connected — character list received", chatType: 1); @@ -2480,7 +2477,7 @@ public sealed class GameWindow : IDisposable // changes resolve (which match against the resolved mesh's // surfaces) and BEFORE the GfxObjMesh.Build / texture upload // path consumes the part list. - if (s_retailCloseDegrades && IsIssue47HumanoidSetup(setup)) + if (_options.RetailCloseDegrades && IsIssue47HumanoidSetup(setup)) { for (int partIdx = 0; partIdx < parts.Count; partIdx++) { @@ -5210,7 +5207,7 @@ public sealed class GameWindow : IDisposable // (physics-vs-bilinear sampler drift), and H3 (DIDDegrade slot 0). // User identifies a floating tree visually, finds the matching // line by world coords + gfx id, the data picks the hypothesis. - if (s_dumpSceneryZ) + if (_options.DumpSceneryZ) { string source = maybePhysicsZ.HasValue ? "physics" : "bilinear"; foreach (var mr in meshRefs) @@ -6798,7 +6795,7 @@ public sealed class GameWindow : IDisposable // retail decomp confirms SkyObject.PesObjectId is copied by // SkyDesc::GetSky but ignored by GameSky, so the sky-PES path is // debug-only and disabled for normal retail rendering. - if (_enableSkyPesDebug) + if (_options.EnableSkyPesDebug) UpdateSkyPes((float)WorldTime.DayFraction, _activeDayGroup, camPos, cameraInsideCell); _scriptRunner?.Tick((float)deltaSeconds); _particleSystem?.Tick((float)deltaSeconds); @@ -7978,7 +7975,7 @@ public sealed class GameWindow : IDisposable partTransform = partTransform * scaleMat; var template = ae.PartTemplate[i]; - if (s_hidePartIndex >= 0 && i == s_hidePartIndex && partCount >= 10) + if (_options.HidePartIndex >= 0 && i == _options.HidePartIndex && partCount >= 10) { continue; } diff --git a/src/AcDream.App/RuntimeOptions.cs b/src/AcDream.App/RuntimeOptions.cs new file mode 100644 index 0000000..7450640 --- /dev/null +++ b/src/AcDream.App/RuntimeOptions.cs @@ -0,0 +1,100 @@ +using System; +using System.Globalization; + +namespace AcDream.App; + +/// +/// Typed bundle of startup-time configuration read from the process +/// environment. Built once in Program.cs and passed to +/// GameWindow so the rest of the app reads its config through +/// strongly-typed fields instead of scattered +/// Environment.GetEnvironmentVariable calls. +/// +/// +/// +/// Scope: startup-time only — values that don't change +/// once the window is up. Runtime diagnostic toggles +/// (e.g. ACDREAM_DUMP_MOTION, ACDREAM_PROBE_*) belong in +/// diagnostic owner classes (see AcDream.Core.Physics.PhysicsDiagnostics +/// for the template), not here. +/// +/// +/// See docs/architecture/code-structure.md §2 Rule 4 for the +/// rule that drove this extraction, and §4 Step 1 for the broader +/// extraction sequence this is the first cut of. +/// +/// +public sealed record RuntimeOptions( + string DatDir, + bool LiveMode, + string LiveHost, + int LivePort, + string? LiveUser, + string? LivePass, + bool DevTools, + bool DumpMoveTruth, + bool NoAudio, + bool EnableSkyPesDebug, + int HidePartIndex, + bool RetailCloseDegrades, + bool DumpSceneryZ, + int? LegacyStreamRadius) +{ + /// + /// Build options from the process environment. Used by + /// Program.cs at startup. + /// + public static RuntimeOptions FromEnvironment(string datDir) + => Parse(datDir, Environment.GetEnvironmentVariable); + + /// + /// Build options from a custom environment getter. Used by tests to + /// inject controlled env values without touching the process + /// environment. + /// + /// Resolved dat-file directory. + /// Function returning the value for an env-var + /// name, or null when unset. + public static RuntimeOptions Parse(string datDir, Func env) + { + if (datDir is null) throw new ArgumentNullException(nameof(datDir)); + if (env is null) throw new ArgumentNullException(nameof(env)); + + return new RuntimeOptions( + DatDir: datDir, + LiveMode: IsExactlyOne(env("ACDREAM_LIVE")), + LiveHost: env("ACDREAM_TEST_HOST") ?? "127.0.0.1", + LivePort: TryParseInt(env("ACDREAM_TEST_PORT")) ?? 9000, + LiveUser: NullIfEmpty(env("ACDREAM_TEST_USER")), + LivePass: NullIfEmpty(env("ACDREAM_TEST_PASS")), + DevTools: IsExactlyOne(env("ACDREAM_DEVTOOLS")), + DumpMoveTruth: IsExactlyOne(env("ACDREAM_DUMP_MOVE_TRUTH")), + NoAudio: IsExactlyOne(env("ACDREAM_NO_AUDIO")), + EnableSkyPesDebug: IsExactlyOne(env("ACDREAM_ENABLE_SKY_PES")), + HidePartIndex: TryParseInt(env("ACDREAM_HIDE_PART")) ?? -1, + // Default-on: any value other than the literal string "0" enables + // retail close-detail degrades. Set ACDREAM_RETAIL_CLOSE_DEGRADES=0 + // only for before/after diagnostic comparisons. + RetailCloseDegrades: !string.Equals(env("ACDREAM_RETAIL_CLOSE_DEGRADES"), "0", StringComparison.Ordinal), + DumpSceneryZ: IsExactlyOne(env("ACDREAM_DUMP_SCENERY_Z")), + // Legacy override for ACDREAM_STREAM_RADIUS. Caller applies it on + // top of the quality preset's radii. Null when unset or invalid. + LegacyStreamRadius: TryParseNonNegativeInt(env("ACDREAM_STREAM_RADIUS"))); + } + + /// True iff live-mode credentials are present and valid for connecting. + public bool HasLiveCredentials => + LiveMode && !string.IsNullOrEmpty(LiveUser) && !string.IsNullOrEmpty(LivePass); + + private static bool IsExactlyOne(string? s) + => string.Equals(s, "1", StringComparison.Ordinal); + + private static string? NullIfEmpty(string? s) + => string.IsNullOrEmpty(s) ? null : s; + + private static int? TryParseInt(string? s) + => int.TryParse(s, NumberStyles.Integer, CultureInfo.InvariantCulture, out var v) ? v : null; + + private static int? TryParseNonNegativeInt(string? s) + => TryParseInt(s) is { } v && v >= 0 ? v : null; +} diff --git a/tests/AcDream.App.Tests/AcDream.App.Tests.csproj b/tests/AcDream.App.Tests/AcDream.App.Tests.csproj new file mode 100644 index 0000000..5ab7992 --- /dev/null +++ b/tests/AcDream.App.Tests/AcDream.App.Tests.csproj @@ -0,0 +1,25 @@ + + + + net10.0 + enable + enable + false + + + + + + + + + + + + + + + + + + diff --git a/tests/AcDream.App.Tests/RuntimeOptionsTests.cs b/tests/AcDream.App.Tests/RuntimeOptionsTests.cs new file mode 100644 index 0000000..a6e0619 --- /dev/null +++ b/tests/AcDream.App.Tests/RuntimeOptionsTests.cs @@ -0,0 +1,217 @@ +using System.Collections.Generic; +using AcDream.App; + +namespace AcDream.App.Tests; + +/// +/// Unit tests for startup-time parsing. +/// Behavior-preservation only: every assertion locks in the exact +/// boolean / numeric / nullability semantics from the env reads that +/// previously lived in GameWindow.cs. +/// +public sealed class RuntimeOptionsTests +{ + private const string AnyDatDir = "C:/Users/test/dats"; + + private static Func Env(Dictionary values) + => name => values.TryGetValue(name, out var v) ? v : null; + + private static Func EmptyEnv() => _ => null; + + [Fact] + public void Defaults_AllSafeOff_WhenEnvironmentIsEmpty() + { + var opts = RuntimeOptions.Parse(AnyDatDir, EmptyEnv()); + + Assert.Equal(AnyDatDir, opts.DatDir); + Assert.False(opts.LiveMode); + Assert.Equal("127.0.0.1", opts.LiveHost); + Assert.Equal(9000, opts.LivePort); + Assert.Null(opts.LiveUser); + Assert.Null(opts.LivePass); + Assert.False(opts.DevTools); + Assert.False(opts.DumpMoveTruth); + Assert.False(opts.NoAudio); + Assert.False(opts.EnableSkyPesDebug); + Assert.Equal(-1, opts.HidePartIndex); + // Default-on: RetailCloseDegrades is true unless explicitly disabled. + Assert.True(opts.RetailCloseDegrades); + Assert.False(opts.DumpSceneryZ); + Assert.Null(opts.LegacyStreamRadius); + Assert.False(opts.HasLiveCredentials); + } + + [Fact] + public void LiveMode_Set_ExactlyByValue1() + { + Assert.True(RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_LIVE"] = "1" })).LiveMode); + Assert.False(RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_LIVE"] = "0" })).LiveMode); + Assert.False(RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_LIVE"] = "true" })).LiveMode); + Assert.False(RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_LIVE"] = "" })).LiveMode); + } + + [Fact] + public void LiveHostAndPort_FallBackToDefaults_WhenUnsetOrInvalid() + { + var withDefaults = RuntimeOptions.Parse(AnyDatDir, EmptyEnv()); + Assert.Equal("127.0.0.1", withDefaults.LiveHost); + Assert.Equal(9000, withDefaults.LivePort); + + var withOverrides = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_TEST_HOST"] = "play.example.com", + ["ACDREAM_TEST_PORT"] = "9123", + })); + Assert.Equal("play.example.com", withOverrides.LiveHost); + Assert.Equal(9123, withOverrides.LivePort); + + // Non-numeric port falls back to default; we don't throw at parse time. + var withBadPort = RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_TEST_PORT"] = "abc" })); + Assert.Equal(9000, withBadPort.LivePort); + } + + [Fact] + public void LiveUserPass_NullWhenEmptyOrUnset() + { + var emptyValues = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_TEST_USER"] = "", + ["ACDREAM_TEST_PASS"] = "", + })); + Assert.Null(emptyValues.LiveUser); + Assert.Null(emptyValues.LivePass); + + var realValues = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_TEST_USER"] = "testaccount", + ["ACDREAM_TEST_PASS"] = "testpassword", + })); + Assert.Equal("testaccount", realValues.LiveUser); + Assert.Equal("testpassword", realValues.LivePass); + } + + [Fact] + public void HasLiveCredentials_RequiresLiveModeAndBothUserAndPass() + { + // Live mode off → no credentials regardless of user/pass. + var noLive = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_TEST_USER"] = "u", + ["ACDREAM_TEST_PASS"] = "p", + })); + Assert.False(noLive.HasLiveCredentials); + + // Live mode on but missing user → no credentials. + var missingUser = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_LIVE"] = "1", + ["ACDREAM_TEST_PASS"] = "p", + })); + Assert.False(missingUser.HasLiveCredentials); + + // Live mode on but missing pass → no credentials. + var missingPass = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_LIVE"] = "1", + ["ACDREAM_TEST_USER"] = "u", + })); + Assert.False(missingPass.HasLiveCredentials); + + // All three present → credentials available. + var ok = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_LIVE"] = "1", + ["ACDREAM_TEST_USER"] = "u", + ["ACDREAM_TEST_PASS"] = "p", + })); + Assert.True(ok.HasLiveCredentials); + } + + [Fact] + public void HidePartIndex_MinusOneWhenUnset_ParsesIntegers() + { + Assert.Equal(-1, RuntimeOptions.Parse(AnyDatDir, EmptyEnv()).HidePartIndex); + Assert.Equal(7, RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_HIDE_PART"] = "7" })).HidePartIndex); + // Invalid → fall back to -1 (preserves the int.TryParse failure semantics). + Assert.Equal(-1, RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_HIDE_PART"] = "abc" })).HidePartIndex); + } + + [Fact] + public void RetailCloseDegrades_DefaultOn_ExceptWhenValueIsExactlyZero() + { + // Unset → on. + Assert.True(RuntimeOptions.Parse(AnyDatDir, EmptyEnv()).RetailCloseDegrades); + + // Exactly "0" → off. Matches the pre-refactor semantics: + // !string.Equals(env, "0", StringComparison.Ordinal) + Assert.False(RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_RETAIL_CLOSE_DEGRADES"] = "0", + })).RetailCloseDegrades); + + // Any other value → on (including "1", "false", "True"). The original + // code only checked for the literal "0"; preserve that. + Assert.True(RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_RETAIL_CLOSE_DEGRADES"] = "1", + })).RetailCloseDegrades); + Assert.True(RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_RETAIL_CLOSE_DEGRADES"] = "false", + })).RetailCloseDegrades); + } + + [Fact] + public void LegacyStreamRadius_NullWhenUnsetOrInvalid_ParsesNonNegativeIntegers() + { + Assert.Null(RuntimeOptions.Parse(AnyDatDir, EmptyEnv()).LegacyStreamRadius); + Assert.Null(RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_STREAM_RADIUS"] = "abc" })).LegacyStreamRadius); + // Negative values are filtered out by the pre-refactor `sr >= 0` guard. + Assert.Null(RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_STREAM_RADIUS"] = "-3" })).LegacyStreamRadius); + + Assert.Equal(0, RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_STREAM_RADIUS"] = "0" })).LegacyStreamRadius); + Assert.Equal(5, RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_STREAM_RADIUS"] = "5" })).LegacyStreamRadius); + Assert.Equal(12, RuntimeOptions.Parse(AnyDatDir, Env(new() { ["ACDREAM_STREAM_RADIUS"] = "12" })).LegacyStreamRadius); + } + + [Fact] + public void DiagnosticFlags_RespectExactValueOne() + { + var allOn = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_DEVTOOLS"] = "1", + ["ACDREAM_DUMP_MOVE_TRUTH"] = "1", + ["ACDREAM_NO_AUDIO"] = "1", + ["ACDREAM_ENABLE_SKY_PES"] = "1", + ["ACDREAM_DUMP_SCENERY_Z"] = "1", + })); + Assert.True(allOn.DevTools); + Assert.True(allOn.DumpMoveTruth); + Assert.True(allOn.NoAudio); + Assert.True(allOn.EnableSkyPesDebug); + Assert.True(allOn.DumpSceneryZ); + + // Any non-"1" value leaves them off, matching the + // string.Equals(env, "1", StringComparison.Ordinal) check. + var anyOther = RuntimeOptions.Parse(AnyDatDir, Env(new() + { + ["ACDREAM_DEVTOOLS"] = "true", + ["ACDREAM_DUMP_MOVE_TRUTH"] = "yes", + ["ACDREAM_NO_AUDIO"] = "2", + ["ACDREAM_ENABLE_SKY_PES"] = "on", + ["ACDREAM_DUMP_SCENERY_Z"] = " 1", + })); + Assert.False(anyOther.DevTools); + Assert.False(anyOther.DumpMoveTruth); + Assert.False(anyOther.NoAudio); + Assert.False(anyOther.EnableSkyPesDebug); + Assert.False(anyOther.DumpSceneryZ); + } + + [Fact] + public void Parse_RejectsNullDatDirOrEnv() + { + Assert.Throws(() => RuntimeOptions.Parse(null!, EmptyEnv())); + Assert.Throws(() => RuntimeOptions.Parse(AnyDatDir, null!)); + } +}