Code review on T13-T16 bundle (commits fb10c3f/aff35d2/b8d80fe/c4fd373/31d312a) flagged 3 Important + 2 test-coverage gaps. Apply all 5: Important #1: GpuWorldState.AddEntitiesToExistingLandblock didn't canonicalize landblockId. Streaming callers always pass canonical 0xAAAA0xFFFF ids, but the public API silently key-missed for callers that mirror AppendLiveEntity's cell-resolved-id pattern. Both new methods now canonicalize the id on entry. Important #2: RemoveEntitiesFromLandblock asymmetry with RemoveLandblock re: persistent-entity rescue. Documented as intentional — demote-tier entities are atlas-tier only (procedural scenery, dat-static stabs/ buildings; never ServerGuid != 0); the local player and live server spawns live in their LB via RelocateEntity per frame and aren't affected by atlas-layer demote. Important #3: StreamingController.NearRadius / FarRadius were { get; set; } but mutating them after the first Tick is a no-op (StreamingRegion snapshots the values). Switched to { get; } only with XML doc warning. Test gap #1: ToDemote routing through Tick — added test that walks the player past hysteresis and asserts entities drop while terrain stays. Test gap #2: Promoted result routing through Tick — added test that enqueues a Promoted and asserts AddEntitiesToExistingLandblock fires. Deferred Minor: dead _streamingRadius write + style consistency on fully-qualified IReadOnlyList — non-load-bearing, can roll into a later cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
149 lines
6.7 KiB
C#
149 lines
6.7 KiB
C#
using System;
|
||
using System.Collections.Generic;
|
||
using AcDream.Core.Terrain;
|
||
using AcDream.Core.World;
|
||
|
||
namespace AcDream.App.Streaming;
|
||
|
||
/// <summary>
|
||
/// Called once per frame from <c>GameWindow.OnUpdate</c>. Owns the
|
||
/// <see cref="StreamingRegion"/> and uses delegates into
|
||
/// <see cref="LandblockStreamer"/> so tests can inject fakes. All work
|
||
/// happens on the render thread; the streamer itself is background.
|
||
///
|
||
/// <remarks>
|
||
/// Threading: not thread-safe. All calls must happen on the render thread.
|
||
/// </remarks>
|
||
/// </summary>
|
||
public sealed class StreamingController
|
||
{
|
||
private readonly Action<uint, LandblockStreamJobKind> _enqueueLoad;
|
||
private readonly Action<uint> _enqueueUnload;
|
||
private readonly Func<int, IReadOnlyList<LandblockStreamResult>> _drainCompletions;
|
||
private readonly Action<LoadedLandblock, LandblockMeshData> _applyTerrain;
|
||
private readonly Action<uint>? _removeTerrain;
|
||
private readonly GpuWorldState _state;
|
||
private StreamingRegion? _region;
|
||
|
||
/// <summary>
|
||
/// Near-tier radius (LBs from observer that load full detail: terrain +
|
||
/// scenery + entities). Set at construction; readable thereafter.
|
||
/// </summary>
|
||
/// <remarks>
|
||
/// Mutating after the first <see cref="Tick"/> has no effect — the
|
||
/// internal <see cref="StreamingRegion"/> snapshots both radii on its
|
||
/// constructor. Treat as init-only post-Tick.
|
||
/// </remarks>
|
||
public int NearRadius { get; }
|
||
|
||
/// <summary>
|
||
/// Far-tier radius (LBs from observer that load terrain only). Set at
|
||
/// construction; readable thereafter.
|
||
/// </summary>
|
||
/// <remarks>
|
||
/// Mutating after the first <see cref="Tick"/> has no effect — see <see cref="NearRadius"/>.
|
||
/// </remarks>
|
||
public int FarRadius { get; }
|
||
|
||
/// <summary>
|
||
/// Cap on completions drained per <see cref="Tick"/> call. The cap is
|
||
/// the GPU upload budget for one frame: terrain mesh + per-entity GfxObj
|
||
/// sub-mesh uploads + texture uploads for one landblock take a few ms;
|
||
/// applying 25 of them in a single frame produces a memory spike
|
||
/// (observed: out-of-memory crash on the 5×5 first-frame load).
|
||
///
|
||
/// <para>
|
||
/// 4 is the original async-streamer value; it spreads a 5×5 first-frame
|
||
/// load over ~7 frames (~116ms at 60fps), which is below the human
|
||
/// perception threshold. Spawn races that previously dropped entities
|
||
/// while landblocks were in flight are now handled by
|
||
/// <see cref="GpuWorldState"/>'s pending-spawn list, so spreading
|
||
/// completions doesn't lose any data.
|
||
/// </para>
|
||
/// </summary>
|
||
public int MaxCompletionsPerFrame { get; set; } = 4;
|
||
|
||
public StreamingController(
|
||
Action<uint, LandblockStreamJobKind> enqueueLoad,
|
||
Action<uint> enqueueUnload,
|
||
Func<int, IReadOnlyList<LandblockStreamResult>> drainCompletions,
|
||
Action<LoadedLandblock, LandblockMeshData> applyTerrain,
|
||
GpuWorldState state,
|
||
int nearRadius,
|
||
int farRadius,
|
||
Action<uint>? removeTerrain = null)
|
||
{
|
||
_enqueueLoad = enqueueLoad;
|
||
_enqueueUnload = enqueueUnload;
|
||
_drainCompletions = drainCompletions;
|
||
_applyTerrain = applyTerrain;
|
||
_removeTerrain = removeTerrain;
|
||
_state = state;
|
||
NearRadius = nearRadius;
|
||
FarRadius = farRadius;
|
||
}
|
||
|
||
/// <summary>
|
||
/// Advance one frame. <paramref name="observerCx"/>/<paramref name="observerCy"/>
|
||
/// are landblock coordinates (0..255) of the current viewer — the camera
|
||
/// in offline mode, the server-sent player position in live.
|
||
///
|
||
/// <para>Two-tier model (Phase A.5 T13):</para>
|
||
/// <list type="bullet">
|
||
/// <item><see cref="TwoTierDiff.ToLoadFar"/> → enqueue LoadFar (terrain only, no entities)</item>
|
||
/// <item><see cref="TwoTierDiff.ToLoadNear"/> → enqueue LoadNear (terrain + entities)</item>
|
||
/// <item><see cref="TwoTierDiff.ToPromote"/> → enqueue PromoteToNear (entity layer for already-loaded terrain)</item>
|
||
/// <item><see cref="TwoTierDiff.ToDemote"/> → drop entities on render thread immediately (terrain stays)</item>
|
||
/// <item><see cref="TwoTierDiff.ToUnload"/> → enqueue full unload</item>
|
||
/// </list>
|
||
/// </summary>
|
||
public void Tick(int observerCx, int observerCy)
|
||
{
|
||
if (_region is null)
|
||
{
|
||
_region = new StreamingRegion(observerCx, observerCy, NearRadius, FarRadius);
|
||
var bootstrap = _region.ComputeFirstTickDiff();
|
||
foreach (var id in bootstrap.ToLoadFar) _enqueueLoad(id, LandblockStreamJobKind.LoadFar);
|
||
foreach (var id in bootstrap.ToLoadNear) _enqueueLoad(id, LandblockStreamJobKind.LoadNear);
|
||
_region.MarkResidentFromBootstrap();
|
||
}
|
||
else if (_region.CenterX != observerCx || _region.CenterY != observerCy)
|
||
{
|
||
var diff = _region.RecenterTo(observerCx, observerCy);
|
||
foreach (var id in diff.ToLoadFar) _enqueueLoad(id, LandblockStreamJobKind.LoadFar);
|
||
foreach (var id in diff.ToLoadNear) _enqueueLoad(id, LandblockStreamJobKind.LoadNear);
|
||
foreach (var id in diff.ToPromote) _enqueueLoad(id, LandblockStreamJobKind.PromoteToNear);
|
||
foreach (var id in diff.ToDemote) _state.RemoveEntitiesFromLandblock(id);
|
||
foreach (var id in diff.ToUnload) _enqueueUnload(id);
|
||
}
|
||
|
||
// Drain up to N completions per frame so a big diff doesn't spike
|
||
// GPU upload time. Remaining completions wait for the next frame.
|
||
var drained = _drainCompletions(MaxCompletionsPerFrame);
|
||
foreach (var result in drained)
|
||
{
|
||
switch (result)
|
||
{
|
||
case LandblockStreamResult.Loaded loaded:
|
||
_applyTerrain(loaded.Landblock, loaded.MeshData);
|
||
_state.AddLandblock(loaded.Landblock);
|
||
break;
|
||
case LandblockStreamResult.Promoted promoted:
|
||
_state.AddEntitiesToExistingLandblock(promoted.LandblockId, promoted.Entities);
|
||
break;
|
||
case LandblockStreamResult.Unloaded unloaded:
|
||
_state.RemoveLandblock(unloaded.LandblockId);
|
||
_removeTerrain?.Invoke(unloaded.LandblockId);
|
||
break;
|
||
case LandblockStreamResult.Failed failed:
|
||
Console.WriteLine(
|
||
$"streaming: load failed for 0x{failed.LandblockId:X8}: {failed.Error}");
|
||
break;
|
||
case LandblockStreamResult.WorkerCrashed crashed:
|
||
Console.WriteLine(
|
||
$"streaming: worker CRASHED: {crashed.Error}");
|
||
break;
|
||
}
|
||
}
|
||
}
|
||
}
|