ROOT CAUSE of the "giant ball with spikes" terrain corruption that the previous two hotfix attempts (lock + synchronous loading) failed to address. Threading was a red herring all along. AC dat conventions: 0xAAAA0xFFFF — LandBlock dat (terrain heightmap) 0xAAAA0xFFFE — LandBlockInfo dat (static-object metadata) WorldView.NeighborLandblockIds correctly uses 0xFFFF. My StreamingRegion.EncodeLandblockId from Phase A.1 Task 1 used 0xFFFE by mistake. Every streaming load was therefore calling LandblockLoader.Load with the LandBlockInfo id, which makes DatCollection ask DatBinReader to read a LandBlock from the LandBlockInfo file. The reader's internal buffer position lands in the middle of the wrong file's bytes, ReadBytesInternal asks for an out-of-range slice, throws ArgumentOutOfRangeException, and the landblocks that DON'T throw return half-populated LandBlock objects whose Height[] arrays contain garbage. Garbage Z values render as the spike pattern. The kicker: my Task-1 review fix added a test (Constructor_SmallRadius_IDsMatchEncodingRule) that asserted Assert.Contains(0x1234FFFEu, region.Visible). The test was passing because it pinned the wrong value. I literally codified the bug. Fix: change EncodeLandblockId's terminator from 0xFFFEu to 0xFFFFu and update the test to assert 0x1234FFFFu. The XML doc on Visible now explicitly explains the 0xFFFF/0xFFFE distinction so this can't recur. The previous two hotfixes (_datLock inc991fb2, synchronous streamer in531c9f9) stay in place — _datLock is defensive belt-and-suspenders that documents which entry points read dats, and synchronous loading is correct-by-default until we decide whether to reintroduce background loading (Phase A.3 may make it unnecessary anyway). 212 tests green. With this fix the streaming should actually work. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
137 lines
5.3 KiB
C#
137 lines
5.3 KiB
C#
using System;
|
||
using System.Collections.Generic;
|
||
|
||
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; }
|
||
|
||
// 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 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 cx, int cy, int radius)
|
||
{
|
||
Radius = radius;
|
||
Recenter(cx, cy);
|
||
}
|
||
|
||
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>
|
||
/// 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 RecenterTo(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.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 + 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);
|