fix(A.5 T13-T16): canonicalize ids; init-only radii; demote/promote tests
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>
This commit is contained in:
parent
31d312add3
commit
19b4465257
3 changed files with 137 additions and 12 deletions
|
|
@ -343,14 +343,29 @@ public sealed class GpuWorldState
|
||||||
/// Drop all entities from a landblock without removing the terrain. Used
|
/// Drop all entities from a landblock without removing the terrain. Used
|
||||||
/// by two-tier streaming when a landblock crosses Near→Far hysteresis.
|
/// by two-tier streaming when a landblock crosses Near→Far hysteresis.
|
||||||
/// Per Phase A.5 spec §4.4.
|
/// Per Phase A.5 spec §4.4.
|
||||||
|
///
|
||||||
|
/// <para>
|
||||||
|
/// <b>Persistent-entity rescue is intentionally omitted</b> (unlike
|
||||||
|
/// <see cref="RemoveLandblock"/>): demote-tier entities are atlas-tier
|
||||||
|
/// only (procedural scenery, dat-static stabs/buildings) — they never
|
||||||
|
/// have <c>ServerGuid != 0</c> and so can never be in <see cref="_persistentGuids"/>.
|
||||||
|
/// The local player and other live server-spawned entities live in their
|
||||||
|
/// landblock via <c>RelocateEntity</c> per frame and are not affected
|
||||||
|
/// by Near→Far demotion of dat-static landblock layers.
|
||||||
|
/// </para>
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public void RemoveEntitiesFromLandblock(uint landblockId)
|
public void RemoveEntitiesFromLandblock(uint landblockId)
|
||||||
{
|
{
|
||||||
if (!_loaded.TryGetValue(landblockId, out var lb)) return;
|
// A.5 T14 follow-up: canonicalize for symmetry with AppendLiveEntity.
|
||||||
|
// Streaming callers always pass canonical (0xAAAA0xFFFF) ids; this
|
||||||
|
// protects against future callers that mirror AppendLiveEntity's
|
||||||
|
// cell-resolved-id pattern.
|
||||||
|
uint canonical = (landblockId & 0xFFFF0000u) | 0xFFFFu;
|
||||||
|
if (!_loaded.TryGetValue(canonical, out var lb)) return;
|
||||||
if (_wbSpawnAdapter is not null)
|
if (_wbSpawnAdapter is not null)
|
||||||
_wbSpawnAdapter.OnLandblockUnloaded(landblockId);
|
_wbSpawnAdapter.OnLandblockUnloaded(canonical);
|
||||||
_loaded[landblockId] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, System.Array.Empty<WorldEntity>());
|
_loaded[canonical] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, System.Array.Empty<WorldEntity>());
|
||||||
_pendingByLandblock.Remove(landblockId);
|
_pendingByLandblock.Remove(canonical);
|
||||||
RebuildFlatView();
|
RebuildFlatView();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -361,16 +376,23 @@ public sealed class GpuWorldState
|
||||||
/// landblock isn't loaded yet (handles the rare "promote arrives before
|
/// landblock isn't loaded yet (handles the rare "promote arrives before
|
||||||
/// far load completes" race).
|
/// far load completes" race).
|
||||||
/// Per Phase A.5 spec §4.4.
|
/// Per Phase A.5 spec §4.4.
|
||||||
|
///
|
||||||
|
/// <para>
|
||||||
|
/// <b>Landblock id is canonicalized</b> (low 16 bits forced to 0xFFFF) —
|
||||||
|
/// callers may pass cell-resolved ids and they will key correctly.
|
||||||
|
/// </para>
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public void AddEntitiesToExistingLandblock(uint landblockId, System.Collections.Generic.IReadOnlyList<WorldEntity> entities)
|
public void AddEntitiesToExistingLandblock(uint landblockId, IReadOnlyList<WorldEntity> entities)
|
||||||
{
|
{
|
||||||
if (!_loaded.TryGetValue(landblockId, out var lb))
|
// A.5 T14 follow-up: canonicalize for symmetry with AppendLiveEntity.
|
||||||
|
uint canonical = (landblockId & 0xFFFF0000u) | 0xFFFFu;
|
||||||
|
if (!_loaded.TryGetValue(canonical, out var lb))
|
||||||
{
|
{
|
||||||
// Park as pending — same pattern as AppendLiveEntity for not-yet-loaded LBs.
|
// Park as pending — same pattern as AppendLiveEntity for not-yet-loaded LBs.
|
||||||
if (!_pendingByLandblock.TryGetValue(landblockId, out var bucket))
|
if (!_pendingByLandblock.TryGetValue(canonical, out var bucket))
|
||||||
{
|
{
|
||||||
bucket = new List<WorldEntity>();
|
bucket = new List<WorldEntity>();
|
||||||
_pendingByLandblock[landblockId] = bucket;
|
_pendingByLandblock[canonical] = bucket;
|
||||||
}
|
}
|
||||||
bucket.AddRange(entities);
|
bucket.AddRange(entities);
|
||||||
return;
|
return;
|
||||||
|
|
@ -378,9 +400,9 @@ public sealed class GpuWorldState
|
||||||
var merged = new List<WorldEntity>(lb.Entities.Count + entities.Count);
|
var merged = new List<WorldEntity>(lb.Entities.Count + entities.Count);
|
||||||
merged.AddRange(lb.Entities);
|
merged.AddRange(lb.Entities);
|
||||||
merged.AddRange(entities);
|
merged.AddRange(entities);
|
||||||
_loaded[landblockId] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, merged);
|
_loaded[canonical] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, merged);
|
||||||
if (_wbSpawnAdapter is not null)
|
if (_wbSpawnAdapter is not null)
|
||||||
_wbSpawnAdapter.OnLandblockLoaded(_loaded[landblockId]);
|
_wbSpawnAdapter.OnLandblockLoaded(_loaded[canonical]);
|
||||||
RebuildFlatView();
|
RebuildFlatView();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -25,8 +25,25 @@ public sealed class StreamingController
|
||||||
private readonly GpuWorldState _state;
|
private readonly GpuWorldState _state;
|
||||||
private StreamingRegion? _region;
|
private StreamingRegion? _region;
|
||||||
|
|
||||||
public int NearRadius { get; set; }
|
/// <summary>
|
||||||
public int FarRadius { get; set; }
|
/// 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>
|
/// <summary>
|
||||||
/// Cap on completions drained per <see cref="Tick"/> call. The cap is
|
/// Cap on completions drained per <see cref="Tick"/> call. The cap is
|
||||||
|
|
|
||||||
|
|
@ -35,4 +35,90 @@ public class StreamingControllerTwoTierTests
|
||||||
Assert.Equal(9, nearCount); // 3x3 inner ring (radius=1)
|
Assert.Equal(9, nearCount); // 3x3 inner ring (radius=1)
|
||||||
Assert.Equal(40, farCount); // 7x7 - 3x3 outer ring (radius=3)
|
Assert.Equal(40, farCount); // 7x7 - 3x3 outer ring (radius=3)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Tick_PlayerWalksOutOfNear_ToDemoteRoutesToRemoveEntities()
|
||||||
|
{
|
||||||
|
// Setup: bootstrap region at (100,100) with near=1, far=3.
|
||||||
|
// The bootstrap puts LB (100,100) in the near tier.
|
||||||
|
// Walking 4+ east drops LB (100,100) past the near-hysteresis
|
||||||
|
// threshold (NearRadius+2 = 3); ToDemote should fire.
|
||||||
|
|
||||||
|
var loads = new List<(uint, LandblockStreamJobKind)>();
|
||||||
|
var unloads = new List<uint>();
|
||||||
|
var state = new GpuWorldState();
|
||||||
|
|
||||||
|
// Pre-load LB (100,100) so RemoveEntitiesFromLandblock has something
|
||||||
|
// to find. The actual entity content doesn't matter for routing.
|
||||||
|
var lb100 = new LoadedLandblock(
|
||||||
|
(100u << 24) | (100u << 16) | 0xFFFFu,
|
||||||
|
Heightmap: null!,
|
||||||
|
Entities: new[] { new WorldEntity { Id = 1, MeshRefs = System.Array.Empty<MeshRef>() } });
|
||||||
|
state.AddLandblock(lb100);
|
||||||
|
Assert.Equal(1, state.Entities.Count);
|
||||||
|
|
||||||
|
var ctrl = new StreamingController(
|
||||||
|
enqueueLoad: (id, kind) => loads.Add((id, kind)),
|
||||||
|
enqueueUnload: unloads.Add,
|
||||||
|
drainCompletions: _ => System.Array.Empty<LandblockStreamResult>(),
|
||||||
|
applyTerrain: (_, _) => { },
|
||||||
|
state: state,
|
||||||
|
nearRadius: 1,
|
||||||
|
farRadius: 3);
|
||||||
|
|
||||||
|
ctrl.Tick(observerCx: 100, observerCy: 100); // bootstrap
|
||||||
|
loads.Clear();
|
||||||
|
|
||||||
|
// Walk 4 east — LB (100,100) is now Chebyshev distance 4 from new
|
||||||
|
// center (104,100). NearRadius+2 = 3, so 4 > 3 fires the demote.
|
||||||
|
ctrl.Tick(observerCx: 104, observerCy: 100);
|
||||||
|
|
||||||
|
// ToDemote runs synchronously on the render thread (no enqueue).
|
||||||
|
// The visible effect is RemoveEntitiesFromLandblock dropping the entity.
|
||||||
|
Assert.Empty(state.Entities);
|
||||||
|
// Terrain stays loaded (demote != unload).
|
||||||
|
Assert.True(state.IsLoaded((100u << 24) | (100u << 16) | 0xFFFFu));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Tick_DrainingPromoted_RoutesToAddEntitiesToExisting()
|
||||||
|
{
|
||||||
|
var loads = new List<(uint, LandblockStreamJobKind)>();
|
||||||
|
var unloads = new List<uint>();
|
||||||
|
var state = new GpuWorldState();
|
||||||
|
|
||||||
|
// Pre-load a far-tier-style LB record (terrain only, no entities).
|
||||||
|
uint lbId = 0x32320FFFu;
|
||||||
|
var lb = new LoadedLandblock(lbId, Heightmap: null!, Entities: System.Array.Empty<WorldEntity>());
|
||||||
|
state.AddLandblock(lb);
|
||||||
|
Assert.Empty(state.Entities);
|
||||||
|
|
||||||
|
// Streamer pushes a Promoted result carrying the entity layer.
|
||||||
|
var promoted = new LandblockStreamResult.Promoted(
|
||||||
|
lbId,
|
||||||
|
new[] { new WorldEntity { Id = 7, MeshRefs = System.Array.Empty<MeshRef>() } });
|
||||||
|
var queue = new Queue<LandblockStreamResult>();
|
||||||
|
queue.Enqueue(promoted);
|
||||||
|
|
||||||
|
var ctrl = new StreamingController(
|
||||||
|
enqueueLoad: (id, kind) => loads.Add((id, kind)),
|
||||||
|
enqueueUnload: unloads.Add,
|
||||||
|
drainCompletions: max =>
|
||||||
|
{
|
||||||
|
var batch = new List<LandblockStreamResult>();
|
||||||
|
while (batch.Count < max && queue.Count > 0) batch.Add(queue.Dequeue());
|
||||||
|
return batch;
|
||||||
|
},
|
||||||
|
applyTerrain: (_, _) => { },
|
||||||
|
state: state,
|
||||||
|
nearRadius: 2,
|
||||||
|
farRadius: 2);
|
||||||
|
|
||||||
|
ctrl.Tick(50, 50); // drains the Promoted result
|
||||||
|
|
||||||
|
// Promoted routes to AddEntitiesToExistingLandblock — the entity is now
|
||||||
|
// merged into the existing LB record.
|
||||||
|
Assert.Equal(1, state.Entities.Count);
|
||||||
|
Assert.Equal(7u, state.Entities[0].Id);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue