Code review on commits 7bcabab/fb6b61e/326b698 flagged 2 Important + 4 Minor issues. Apply all fixes: Important: - Two-tier RecenterTo + MarkResidentFromBootstrap now throw InvalidOperationException on misuse — calling RecenterTo before the bootstrap silently emitted the entire window as fresh loads (no demotes/unloads since _tierResidence was empty), a correctness hazard that produced no exception. Calling MarkResidentFromBootstrap twice silently dropped accumulated tier state. Both now crash loudly via a _bootstrapped flag. - Dropped TierResidence.None from the enum — never assigned, never checked; absence from the dictionary already encodes "not resident." Minor: - Renamed test: RecenterTo_FirstTick_* → ComputeFirstTickDiff_FirstTick_* (the test calls ComputeFirstTickDiff, not RecenterTo). - Strengthened RecenterTo_PlayerWalks_NullToFar_* with assertions for ToPromote.Count==3 (the x=102 column promoting Far→Near) and ToUnload.Empty (everything within hysteresis). - Replaced System.Math.Abs with Math.Abs in new code to match the file's existing `using System;` convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
345 lines
14 KiB
C#
345 lines
14 KiB
C#
using System;
|
||
using System.Collections.Generic;
|
||
using System.Linq;
|
||
|
||
namespace AcDream.App.Streaming;
|
||
|
||
/// <summary>
|
||
/// Pure data type describing the set of landblocks currently considered
|
||
/// "visible" by the streaming system. Given a center landblock (x, y) and
|
||
/// a radius, builds the set of landblock IDs in the (2r+1)×(2r+1) window.
|
||
/// </summary>
|
||
public sealed class StreamingRegion
|
||
{
|
||
public int CenterX { get; private set; }
|
||
public int CenterY { get; private set; }
|
||
public int Radius { get; }
|
||
public int NearRadius { get; }
|
||
public int FarRadius { 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();
|
||
|
||
// Two-tier residence tracking: maps each resident LB to its current tier.
|
||
private readonly Dictionary<uint, TierResidence> _tierResidence = new();
|
||
|
||
// Set true after MarkResidentFromBootstrap. The two-tier RecenterTo
|
||
// requires this state to be seeded; calling RecenterTo before the
|
||
// bootstrap silently emits the entire window as fresh loads (no demotes,
|
||
// no unloads, since _tierResidence is empty), which is a correctness
|
||
// hazard. The flag converts that into a loud InvalidOperationException.
|
||
private bool _bootstrapped;
|
||
|
||
/// <summary>
|
||
/// Landblock IDs in the current visible window in the AC 8.8 coordinate
|
||
/// form: <c>(lbX << 24) | (lbY << 16) | 0xFFFF</c>. The trailing
|
||
/// <c>0xFFFF</c> selects the LandBlock dat (terrain heightmap); the
|
||
/// matching LandBlockInfo (static-object metadata) is at <c>0xFFFE</c>
|
||
/// on the same coordinates and is resolved internally by
|
||
/// <c>LandblockLoader</c>.
|
||
///
|
||
/// <para>
|
||
/// This set is strictly the (2r+1)×(2r+1) window; it does NOT include
|
||
/// hysteresis-retained landblocks outside the window. Use
|
||
/// <see cref="Resident"/> to enumerate everything actually loaded.
|
||
/// </para>
|
||
/// </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 centerX, int centerY, int nearRadius, int farRadius)
|
||
{
|
||
NearRadius = nearRadius;
|
||
FarRadius = farRadius;
|
||
Radius = farRadius; // outer ring drives Resident bookkeeping
|
||
Recenter(centerX, centerY);
|
||
}
|
||
|
||
public StreamingRegion(int cx, int cy, int radius) : this(cx, cy, radius, radius) { }
|
||
|
||
private void Recenter(int cx, int cy)
|
||
{
|
||
CenterX = cx;
|
||
CenterY = cy;
|
||
_visible.Clear();
|
||
for (int dx = -Radius; dx <= Radius; dx++)
|
||
{
|
||
for (int dy = -Radius; dy <= Radius; dy++)
|
||
{
|
||
int nx = cx + dx;
|
||
int ny = cy + dy;
|
||
if (nx < 0 || nx > 0xFF || ny < 0 || ny > 0xFF)
|
||
continue;
|
||
_visible.Add(EncodeLandblockId(nx, ny));
|
||
}
|
||
}
|
||
// On first construction, resident == visible.
|
||
_resident.UnionWith(_visible);
|
||
}
|
||
|
||
/// <summary>
|
||
/// Encode a landblock at (lbX, lbY) into the AC dat id form. Always uses
|
||
/// the <c>0xFFFF</c> terminator (LandBlock = terrain). The earlier
|
||
/// version of this method used <c>0xFFFE</c> by mistake — that's the
|
||
/// LandBlockInfo id, and asking <c>LandblockLoader.Load</c> to read a
|
||
/// LandBlock at the LandBlockInfo coords corrupts the dat reader's
|
||
/// buffer position, returning a half-populated <c>LandBlock.Height[]</c>
|
||
/// array which renders as wildly distorted "ball with spikes" terrain.
|
||
/// </summary>
|
||
internal static uint EncodeLandblockId(int lbX, int lbY)
|
||
=> ((uint)lbX << 24) | ((uint)lbY << 16) | 0xFFFFu;
|
||
|
||
/// <summary>
|
||
/// First-tick bootstrap: emit ToLoadNear for every LB in the inner ring,
|
||
/// ToLoadFar for every LB in the outer ring (between near and far). Used
|
||
/// by <see cref="StreamingController.Tick"/> on the first call before any
|
||
/// RecenterTo.
|
||
/// </summary>
|
||
public TwoTierDiff ComputeFirstTickDiff()
|
||
{
|
||
var near = new List<uint>();
|
||
var far = new List<uint>();
|
||
for (int dx = -FarRadius; dx <= FarRadius; dx++)
|
||
{
|
||
for (int dy = -FarRadius; dy <= FarRadius; dy++)
|
||
{
|
||
int nx = CenterX + dx;
|
||
int ny = CenterY + dy;
|
||
if (nx < 0 || nx > 0xFF || ny < 0 || ny > 0xFF) continue;
|
||
int absDx = System.Math.Abs(dx);
|
||
int absDy = System.Math.Abs(dy);
|
||
var id = EncodeLandblockId(nx, ny);
|
||
if (absDx <= NearRadius && absDy <= NearRadius)
|
||
near.Add(id);
|
||
else
|
||
far.Add(id);
|
||
}
|
||
}
|
||
return new TwoTierDiff(
|
||
ToLoadFar: far,
|
||
ToLoadNear: near,
|
||
ToPromote: System.Array.Empty<uint>(),
|
||
ToDemote: System.Array.Empty<uint>(),
|
||
ToUnload: System.Array.Empty<uint>());
|
||
}
|
||
|
||
/// <summary>
|
||
/// Call once after <see cref="ComputeFirstTickDiff"/> to seed
|
||
/// <c>_tierResidence</c> with the initial window. Every LB in the inner
|
||
/// ring (Chebyshev ≤ NearRadius) is marked Near; everything else Far.
|
||
/// </summary>
|
||
public void MarkResidentFromBootstrap()
|
||
{
|
||
if (_bootstrapped)
|
||
throw new InvalidOperationException(
|
||
"MarkResidentFromBootstrap was already called; calling it again would " +
|
||
"reset accumulated tier-residence state and silently drop differential " +
|
||
"data built up by interim RecenterTo calls.");
|
||
|
||
_tierResidence.Clear();
|
||
for (int dx = -FarRadius; dx <= FarRadius; dx++)
|
||
{
|
||
for (int dy = -FarRadius; dy <= FarRadius; dy++)
|
||
{
|
||
int nx = CenterX + dx;
|
||
int ny = CenterY + dy;
|
||
if (nx < 0 || nx > 0xFF || ny < 0 || ny > 0xFF) continue;
|
||
int absDx = Math.Abs(dx);
|
||
int absDy = Math.Abs(dy);
|
||
var id = EncodeLandblockId(nx, ny);
|
||
_tierResidence[id] = (absDx <= NearRadius && absDy <= NearRadius)
|
||
? TierResidence.Near
|
||
: TierResidence.Far;
|
||
}
|
||
}
|
||
_bootstrapped = true;
|
||
}
|
||
|
||
/// <summary>
|
||
/// Test-visible wrapper around <see cref="EncodeLandblockId"/> so test
|
||
/// assemblies can build expected IDs without duplicating the encoding rule.
|
||
/// </summary>
|
||
internal static uint EncodeLandblockIdForTest(int lbX, int lbY)
|
||
=> EncodeLandblockId(lbX, lbY);
|
||
|
||
/// <summary>
|
||
/// Two-tier recenter: computes the 5-list diff per Phase A.5 spec §4.2.
|
||
/// Hysteresis: NearRadius+2 for Near→Far demote; FarRadius+2 for Far→null
|
||
/// unload. Requires <see cref="MarkResidentFromBootstrap"/> (or a prior
|
||
/// call to this method) to have seeded <c>_tierResidence</c>.
|
||
/// </summary>
|
||
public TwoTierDiff RecenterTo(int newCx, int newCy)
|
||
{
|
||
if (!_bootstrapped)
|
||
throw new InvalidOperationException(
|
||
"Two-tier RecenterTo called before MarkResidentFromBootstrap. " +
|
||
"First call ComputeFirstTickDiff to enqueue the bootstrap loads, " +
|
||
"then MarkResidentFromBootstrap to seed _tierResidence, then RecenterTo " +
|
||
"for subsequent observer moves.");
|
||
|
||
int nearUnloadThreshold = NearRadius + 2;
|
||
int farUnloadThreshold = FarRadius + 2;
|
||
|
||
var toLoadFar = new List<uint>();
|
||
var toLoadNear = new List<uint>();
|
||
var toPromote = new List<uint>();
|
||
var toDemote = new List<uint>();
|
||
var toUnload = new List<uint>();
|
||
|
||
// Pass 1: walk new far window — emit ToLoadFar / ToLoadNear / ToPromote.
|
||
var newCenterIds = new HashSet<uint>();
|
||
for (int dx = -FarRadius; dx <= FarRadius; dx++)
|
||
{
|
||
for (int dy = -FarRadius; dy <= FarRadius; dy++)
|
||
{
|
||
int nx = newCx + dx;
|
||
int ny = newCy + dy;
|
||
if (nx < 0 || nx > 0xFF || ny < 0 || ny > 0xFF) continue;
|
||
int absDx = Math.Abs(dx);
|
||
int absDy = Math.Abs(dy);
|
||
bool inNear = absDx <= NearRadius && absDy <= NearRadius;
|
||
var id = EncodeLandblockId(nx, ny);
|
||
newCenterIds.Add(id);
|
||
|
||
if (!_tierResidence.TryGetValue(id, out var current))
|
||
{
|
||
// Not resident at all — fresh load.
|
||
if (inNear) toLoadNear.Add(id);
|
||
else toLoadFar.Add(id);
|
||
_tierResidence[id] = inNear ? TierResidence.Near : TierResidence.Far;
|
||
}
|
||
else if (current == TierResidence.Far && inNear)
|
||
{
|
||
// Was Far, now inside Near ring — promote.
|
||
toPromote.Add(id);
|
||
_tierResidence[id] = TierResidence.Near;
|
||
}
|
||
// Near→Near and Far→Far are no-ops.
|
||
}
|
||
}
|
||
|
||
// Pass 2: check previously-resident LBs for demote / unload.
|
||
foreach (var kvp in _tierResidence.ToArray())
|
||
{
|
||
var id = kvp.Key;
|
||
var current = kvp.Value;
|
||
int lbX = (int)((id >> 24) & 0xFFu);
|
||
int lbY = (int)((id >> 16) & 0xFFu);
|
||
int absDx = Math.Abs(lbX - newCx);
|
||
int absDy = Math.Abs(lbY - newCy);
|
||
int distance = Math.Max(absDx, absDy); // Chebyshev
|
||
|
||
if (newCenterIds.Contains(id))
|
||
{
|
||
// Still in the far window — only Near→Far demote possible here.
|
||
if (current == TierResidence.Near && (absDx > NearRadius || absDy > NearRadius))
|
||
{
|
||
if (distance > nearUnloadThreshold)
|
||
{
|
||
toDemote.Add(id);
|
||
_tierResidence[id] = TierResidence.Far;
|
||
}
|
||
}
|
||
continue;
|
||
}
|
||
|
||
// Outside the new window — demote / unload by threshold.
|
||
if (current == TierResidence.Near)
|
||
{
|
||
if (distance > nearUnloadThreshold)
|
||
{
|
||
toDemote.Add(id);
|
||
_tierResidence[id] = TierResidence.Far;
|
||
if (distance > farUnloadThreshold)
|
||
{
|
||
toUnload.Add(id);
|
||
_tierResidence.Remove(id);
|
||
}
|
||
}
|
||
}
|
||
else if (current == TierResidence.Far)
|
||
{
|
||
if (distance > farUnloadThreshold)
|
||
{
|
||
toUnload.Add(id);
|
||
_tierResidence.Remove(id);
|
||
}
|
||
}
|
||
}
|
||
|
||
CenterX = newCx;
|
||
CenterY = newCy;
|
||
|
||
return new TwoTierDiff(toLoadFar, toLoadNear, toPromote, toDemote, toUnload);
|
||
}
|
||
|
||
/// <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 + 2</c> from the new center,
|
||
/// so boundary crossings don't thrash.
|
||
/// </summary>
|
||
public RegionDiff RecenterToSingleTier(int newCx, int newCy)
|
||
{
|
||
// 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 = entries in the new window not yet in the resident set.
|
||
var toLoad = new List<uint>();
|
||
foreach (var id in _visible)
|
||
if (!oldResident.Contains(id))
|
||
toLoad.Add(id);
|
||
|
||
// 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 oldResident)
|
||
{
|
||
if (_visible.Contains(id)) continue; // still in window, keep
|
||
int lbX = (int)((id >> 24) & 0xFFu);
|
||
int lbY = (int)((id >> 16) & 0xFFu);
|
||
int dx = Math.Abs(lbX - newCx);
|
||
int dy = Math.Abs(lbY - newCy);
|
||
if (dx > unloadThreshold || dy > unloadThreshold)
|
||
toUnload.Add(id);
|
||
}
|
||
|
||
// Update resident: (oldResident ∪ newVisible) ∖ toUnload.
|
||
_resident.UnionWith(_visible);
|
||
foreach (var id in toUnload)
|
||
_resident.Remove(id);
|
||
|
||
return new RegionDiff(toLoad, toUnload);
|
||
}
|
||
}
|
||
|
||
/// <summary>
|
||
/// Output of <see cref="StreamingRegion.RecenterToSingleTier"/>: the landblocks to
|
||
/// start loading (newly entered the visible window) and the landblocks to
|
||
/// 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>
|
||
public readonly record struct RegionDiff(
|
||
IReadOnlyList<uint> ToLoad,
|
||
IReadOnlyList<uint> ToUnload);
|
||
|
||
/// <summary>
|
||
/// Tracks which tier a landblock currently occupies in the two-tier streaming
|
||
/// model. Absence from the dictionary encodes "not resident"; the enum has no
|
||
/// None member to avoid suggesting a third runtime state.
|
||
/// </summary>
|
||
internal enum TierResidence { Far, Near }
|