erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(app): Phase A.1 — separate Visible from Resident in StreamingRegion

Browse source 
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>
This commit is contained in:
Erik 2026-04-11 22:08:17 +02:00
parent 11df7930fc
commit 449c2caf8b
3 changed files with 67 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

55
src/AcDream.App/Streaming/StreamingRegion.cs
View file

@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
namespace AcDream.App.Streaming;
@ -13,14 +14,27 @@ public sealed class StreamingRegion
public int CenterY { get; private set; }
public int Radius { get; }
// Strictly the (2r+1)×(2r+1) window (clamped to world bounds).
private readonly HashSet<uint> _visible = new();
// Everything currently loaded: window + hysteresis-retained landblocks.
private readonly HashSet<uint> _resident = new();
/// <summary>
/// Landblock IDs (8.8 coordinate form: <c>(lbX &lt;&lt; 24) | (lbY &lt;&lt; 16) | 0xFFFE</c>)
/// in the current visible window.
/// in the current visible window. This is strictly the (2r+1)×(2r+1) set;
/// it does NOT include hysteresis-retained landblocks outside the window.
/// Use <see cref="Resident"/> to enumerate everything actually loaded.
/// </summary>
public IReadOnlyCollection<uint> Visible => _visible;
/// <summary>
/// Every landblock currently loaded: the current visible window plus any
/// landblocks being held in memory by the hysteresis buffer (not yet past
/// the <c>Radius + 2</c> unload threshold).
/// </summary>
public IReadOnlyCollection<uint> Resident => _resident;
public StreamingRegion(int cx, int cy, int radius)
{
Radius = radius;
@ -43,6 +57,8 @@ public sealed class StreamingRegion
_visible.Add(EncodeLandblockId(nx, ny));
}
}
// On first construction, resident == visible.
_resident.UnionWith(_visible);
}
internal static uint EncodeLandblockId(int lbX, int lbY)
@ -51,43 +67,42 @@ public sealed class StreamingRegion
/// <summary>
/// Recompute the visible window around a new center and return the
/// delta vs. the previous state. Hysteresis: landblocks aren't unloaded
/// until they're further than <c>Radius + 1</c> from the new center,
/// until they're further than <c>Radius + 2</c> from the new center,
/// so boundary crossings don't thrash.
/// </summary>
public RegionDiff RecenterTo(int newCx, int newCy)
{
// Snapshot the current visible set so we can diff against it.
var oldVisible = new HashSet<uint>(_visible);
// Snapshot the old resident set so we can diff against it.
var oldResident = new HashSet<uint>(_resident);
// Recompute _visible strictly as the new window.
Recenter(newCx, newCy);
// Loads = everything newly in visible but not previously.
// Loads = entries in the new window not yet in the resident set.
var toLoad = new List<uint>();
foreach (var id in _visible)
if (!oldVisible.Contains(id))
if (!oldResident.Contains(id))
toLoad.Add(id);
// Unloads = everything previously visible AND now outside the
// hysteresis threshold (|dx| > r+2 OR |dy| > r+2).
// The extra +1 beyond the visible radius (+1) gives a full buffer
// cell of hysteresis before eviction, matching the test expectations.
// Unloads = resident entries outside the hysteresis threshold
// (|dx| > Radius+2 OR |dy| > Radius+2).
int unloadThreshold = Radius + 2;
var toUnload = new List<uint>();
foreach (var id in oldVisible)
foreach (var id in oldResident)
{
if (_visible.Contains(id)) continue; // still visible, not unloading
if (_visible.Contains(id)) continue; // still in window, keep
int lbX = (int)((id >> 24) & 0xFFu);
int lbY = (int)((id >> 16) & 0xFFu);
int dx = System.Math.Abs(lbX - newCx);
int dy = System.Math.Abs(lbY - newCy);
int dx = Math.Abs(lbX - newCx);
int dy = Math.Abs(lbY - newCy);
if (dx > unloadThreshold || dy > unloadThreshold)
toUnload.Add(id);
}
// Any "still loaded but outside visible" landblocks not in toUnload
// need to rejoin the visible set so we don't lose track of them.
foreach (var id in oldVisible)
if (!_visible.Contains(id) && !toUnload.Contains(id))
_visible.Add(id);
// Update resident: (oldResident ∪ newVisible) ∖ toUnload.
_resident.UnionWith(_visible);
foreach (var id in toUnload)
_resident.Remove(id);
return new RegionDiff(toLoad, toUnload);
}
@ -96,7 +111,7 @@ public sealed class StreamingRegion
/// <summary>
/// Output of <see cref="StreamingRegion.RecenterTo"/>: the landblocks to
/// start loading (newly entered the visible window) and the landblocks to
/// unload (fell outside the unload threshold, which is <c>radius + 1</c>).
/// unload (fell outside the unload threshold, which is <c>Radius + 2</c>).
/// Both lists are disjoint from the current <see cref="StreamingRegion.Visible"/>
/// set; the caller hands them to <c>LandblockStreamer</c> as jobs.
/// </summary>