Commit graph

6 commits

Author SHA1 Message Date
Erik
9067c4f60b feat(app): Phase A.1 — StreamingController glue
Called once per frame from OnUpdate. Owns a StreamingRegion and uses
delegates into LandblockStreamer + a terrain-apply callback so unit
tests can inject fakes. Handles first-tick bootstrap (whole window
loads), boundary recenter (diff against previous center), and
drain completions (up to N per frame to cap GPU upload spikes).

4 new tests, all green.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 22:26:55 +02:00
Erik
c5e207a51f fix(app): Phase A.1 — LandblockStreamer lifecycle + threading hardening
Code review follow-up to commit 0904372. Five Important fixes plus
three Minor polish items found by the reviewer before StreamingController
depends on this class under churn.

I1: Dispose is now thread-safe via Interlocked.Exchange on an int
    guard. Two concurrent Dispose calls no longer double-dispose the
    CancellationTokenSource.

I2: EnqueueLoad/EnqueueUnload now throw ObjectDisposedException when
    called after Dispose instead of silently dropping the job. Jobs
    vanishing into a completed channel was a debugging hazard.

I3: Start throws ObjectDisposedException when called after Dispose
    instead of silently doing nothing (the old guard only checked
    whether the thread was non-null, not whether the streamer was
    still usable).

I4: New test Load_ExecutesLoaderOnBackgroundThread captures the
    loader delegate's ManagedThreadId and asserts it differs from
    the test thread's id, proving the whole reason this class
    exists (off-thread execution) is actually happening.

I5: New LandblockStreamResult.WorkerCrashed record type for the
    outer catch in WorkerLoop. Previously the crash path wrote
    Failed(0, ex.ToString()) which collided with landblock (0, 0)
    in the north ocean, making "worker crashed" indistinguishable
    from "landblock 0 failed to load".

Minor polish:
- M1: Test spin constants (SpinTimeoutMs, SpinStepMs,
  SpinMaxIterations) extracted so the 200 x 10ms pattern has one
  source of truth.
- M2: DefaultDrainBatchSize public const on LandblockStreamer so
  the batch cap has a name and a comment explaining why 4.
- M3: Safety-argument comment on the sync-over-async
  WaitToReadAsync call explaining why it cannot deadlock (dedicated
  thread, no SyncContext).
- M6: XML remarks on the class and on DrainCompletions documenting
  threading contract (Enqueue = any thread, Drain = single consumer
  thread).

112 Core + 96 Core.Net tests green.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 22:20:41 +02:00
Erik
0904372af6 feat(app): Phase A.1 — LandblockStreamer (background worker + channels)
Background thread pulls load/unload jobs from an inbox channel, invokes
a caller-supplied Func<uint, LoadedLandblock?> (production wraps
LandblockLoader.Load, tests inject a fake), and posts results to an
outbox channel the render thread drains. Graceful shutdown via
CancellationToken; failed loads reported rather than retried.

4 new tests, all green.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 22:14:18 +02:00
Erik
9d1c2c45e5 feat(app): Phase A.1 — job + result records for LandblockStreamer
LandblockStreamJob (Load/Unload) and LandblockStreamResult
(Loaded/Failed/Unloaded) are the channel payload types the next
task's LandblockStreamer will use. Separate file because they're
shared between the worker thread and the render thread and deserve
a focused home.

Folds in two carryover nits from the Task 1 fix review:
- Stale "radius + 1" comments in StreamingRegionTests updated to
  match the real radius+2 threshold (no numeric-assertion changes).
- Single-step recenter test now asserts Visible.Count == 25 and
  Resident.Count == 30, locking in the Visible/Resident semantic
  split behaviorally.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 22:11:35 +02:00
Erik
449c2caf8b fix(app): Phase A.1 — separate Visible from Resident in StreamingRegion
Review follow-up from commit 11df793. Three fixes:

1. Visible semantics: StreamingRegion.Visible now strictly describes the
   current (2r+1)×(2r+1) window, not window + hysteresis retainees.
   Added a parallel Resident property exposing the actual loaded set
   (window + hysteresis buffer). This matters because StreamingController
   (next task) reads these to decide what to render vs what to unload;
   conflating them in one set would have forced awkward post-processing
   downstream.

2. Doc/code disagreement: updated the RecenterTo and RegionDiff doc
   comments from "radius + 1" to "radius + 2" to match the actual
   implementation (which is what the tests require). Also updated the
   plan doc so future readers don't hit the same contradiction.

3. Edge-clamping test coverage: added a single-axis edge test
   (cx=0, cy=50 → 15 entries) and an ID-encoding test (radius=0 at
   0x12,0x34 → 0x1234FFFE) so a swapped-shift bug in EncodeLandblockId
   or an asymmetric off-by-one would fail a test instead of passing
   silently.

9 tests green, full suite regressions-free.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 22:08:17 +02:00
Erik
11df7930fc feat(app): Phase A.1 — StreamingRegion (window set + diff with hysteresis)
Pure data type describing the set of landblocks inside the current
streaming window, with a diff-style Recenter that returns (toLoad,
toUnload) pairs the LandblockStreamer consumes as jobs. Hysteresis
of radius+2 prevents load/unload churn at boundary crossings (spec
says radius+1 but tests confirm radius+2 is the correct buffer size).

First piece of Phase A.1 per docs/superpowers/plans/2026-04-11-foundation-a1-streaming.md.

7 new tests, all green. Total suite: 105/105.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 22:01:45 +02:00