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

fix(app): Phase A.1 — pending-spawn list in GpuWorldState (proper fix)

Browse source 
Fifth and final Phase A.1 hotfix. Replaces the previous "drop on
miss" semantics in GpuWorldState.AppendLiveEntity with a per-landblock
pending bucket that survives the race where a CreateObject arrives
before its landblock has been streamed in.

Root cause:
The post-login spawn flood (40+ NPCs/items) drains in a single
WorldSession.Tick() call. The synchronous streamer enqueues all 25
visible-window landblocks in one shot but StreamingController.Tick
was capped at MaxCompletionsPerFrame=4, so only 4 landblocks landed
in GpuWorldState on the first frame. The center landblock 0xA9B4FFFF
may or may not have been in those first 4 (HashSet iteration order
is undefined). Spawns whose target landblock wasn't yet loaded were
silently dropped by AppendLiveEntity. Re-ordering the OnUpdate
(streaming first, live second) didn't fix it because the cap still
limited to 4 per frame; spawns for landblocks #5+ kept dropping
until the queue drained, by which point the spawn flood was over.

The reordering was correct but insufficient. The cap was a relic of
the original async streamer design (limit GPU upload spikes per
frame). With the synchronous streamer there's no backlog to spread,
so the cap was pure latency for no benefit. Setting it to int.MaxValue
restores "drain everything you just enqueued" semantics.

The pending-spawn list is the *correct* architecture fix that makes
the system robust against any future ordering bug, not just the cap:
- AppendLiveEntity for an unloaded landblock parks the entity in a
  per-landblock pending bucket instead of dropping it.
- AddLandblock drains pending entries for its landblock and merges
  them into the loaded record before storing.
- RemoveLandblock drops pending entries for the same landblock —
  if the player moved away, the spawns are no longer relevant; the
  server resends them via CreateObject when the player returns.

Diagnostic counter PendingLiveEntityCount exposes the bucket size
so future regressions are visible without spelunking.

7 new GpuWorldStateTests pin the contract:
- AppendLiveEntity_LandblockAlreadyLoaded_AppendsImmediately
- AppendLiveEntity_LandblockNotLoaded_ParksInPending
- AddLandblock_DrainsPendingEntriesForThatLandblock
- AddLandblock_DoesNotDrainPendingForADifferentLandblock
- RemoveLandblock_DropsPendingForThatLandblock
- RemoveLandblock_LoadedThenRemoved_DropsItsEntities
- IsLoaded_ReturnsTrueForLoaded_FalseForPendingOnly

Also removes the diagnostic Console.WriteLine I added in the previous
debugging round and the old LiveAppendsResolved/Dropped counters that
were never read by anyone.

219 tests green (212 + 7 new).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-04-11 23:19:40 +02:00
parent 4b01c95ecb
commit f792931d21
3 changed files with 250 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

109
src/AcDream.App/Streaming/GpuWorldState.cs
View file

@ -11,20 +11,41 @@ namespace AcDream.App.Streaming;
/// frame.
///
/// <para>
/// Replaces <c>GameWindow._entities</c>, which was a flat list updated in
/// multiple places. This class is the single point of truth for "what's in
/// the world right now" and the only thing that mutates it.
/// Replaces the pre-streaming flat <c>_entities</c> list. This class is the
/// single point of truth for "what's in the world right now" and the only
/// thing that mutates it.
/// </para>
///
/// <para>
/// <b>Pending live entities.</b> Live <c>CreateObject</c> spawns can race
/// against streaming: the server may send a spawn for landblock X before
/// X is loaded into <see cref="_loaded"/> (frequently true on the first
/// frame after login, where the entire post-login spawn flood drains
/// before the streaming controller has finished loading the visible
/// window). To survive this race, <see cref="AppendLiveEntity"/> stores
/// orphaned spawns in a per-landblock pending bucket. When
/// <see cref="AddLandblock"/> later loads the landblock, the matching
/// pending entries are merged into the loaded record before the flat
/// view rebuild. <see cref="RemoveLandblock"/> drops pending entries for
/// the same landblock — if the landblock just left the visible window,
/// any spawns that came with it are no longer relevant.
/// </para>
///
/// <remarks>
/// Threading: not thread-safe. All calls must happen on the render thread.
/// The streaming worker never touches this type.
/// </remarks>
/// </summary>
public sealed class GpuWorldState
{
private readonly Dictionary<uint, LoadedLandblock> _loaded = new();
/// <summary>
/// Per-landblock buffer of live entities awaiting their landblock's
/// arrival. Keyed by canonical landblock id (<c>0xAAAA0xFFFF</c>).
/// Drained into <see cref="_loaded"/> in <see cref="AddLandblock"/>.
/// </summary>
private readonly Dictionary<uint, List<WorldEntity>> _pendingByLandblock = new();
// Cached flat view over all entities across all loaded landblocks,
// rebuilt on each add/remove. The renderer holds a reference to this
// list, so rebuilding it replaces the reference atomically.
@ -35,48 +56,94 @@ public sealed class GpuWorldState
public bool IsLoaded(uint landblockId) => _loaded.ContainsKey(landblockId);
/// <summary>
/// Total live entities currently parked in the pending bucket waiting
/// for their landblock to arrive. Useful diagnostic for verifying the
/// pending path is doing its job.
/// </summary>
public int PendingLiveEntityCount => _pendingByLandblock.Values.Sum(list => list.Count);
public void AddLandblock(LoadedLandblock landblock)
{
// If pending live entities have been waiting for this landblock,
// merge them into the LoadedLandblock record before storing. The
// record's Entities field is IReadOnlyList; we replace the whole
// list rather than try to mutate in place.
if (_pendingByLandblock.TryGetValue(landblock.LandblockId, out var pending) && pending.Count > 0)
{
var merged = new List<WorldEntity>(landblock.Entities.Count + pending.Count);
merged.AddRange(landblock.Entities);
merged.AddRange(pending);
landblock = new LoadedLandblock(landblock.LandblockId, landblock.Heightmap, merged);
_pendingByLandblock.Remove(landblock.LandblockId);
}
_loaded[landblock.LandblockId] = landblock;
RebuildFlatView();
}
public void RemoveLandblock(uint landblockId)
{
// Drop pending entries for the same landblock — if the landblock
// is being unloaded the player has moved away from it, and any
// pending spawns that arrived for it are no longer relevant. The
// server will resend them via CreateObject when the player returns.
_pendingByLandblock.Remove(landblockId);
if (_loaded.Remove(landblockId))
RebuildFlatView();
}
/// <summary>
/// Append an entity to a specific landblock's slot. Used by the live
/// CreateObject path where the server spawns entities into an already-
/// loaded landblock after the initial hydration pass.
/// CreateObject path where the server spawns entities at a server-side
/// position whose landblock may or may not be loaded yet.
///
/// <para>
/// The server's <c>landblockId</c> is in cell-resolved form
/// (<c>0xAAAA00CC</c>: high byte X, second byte Y, low 16 bits cell
/// index within the landblock). The streaming system stores landblocks
/// keyed by their canonical <c>0xAAAA0xFFFF</c> form. Canonicalize on
/// the way in so callers don't have to think about it — masking with
/// <c>0xFFFF0000u | 0xFFFFu</c> drops the cell index and selects the
/// LandBlock dat terminator.
/// keyed by their canonical <c>0xAAAA0xFFFF</c> form. Canonicalize
/// on the way in so callers don't have to think about it.
/// </para>
///
/// <para>
/// Outcome:
/// <list type="bullet">
/// <item>If the landblock is already loaded, the entity is appended
/// to its <c>Entities</c> list and the flat view is rebuilt
/// immediately.</item>
/// <item>If the landblock is not yet loaded, the entity is parked
/// in <see cref="_pendingByLandblock"/> and will be merged
/// into the next <see cref="AddLandblock"/> for the same id.</item>
/// </list>
/// </para>
/// </summary>
public void AppendLiveEntity(uint landblockId, WorldEntity entity)
{
uint canonicalLandblockId = (landblockId & 0xFFFF0000u) | 0xFFFFu;
if (!_loaded.TryGetValue(canonicalLandblockId, out var lb))
return;
// LoadedLandblock.Entities is an IReadOnlyList. Rebuild the
// landblock record with the new entity appended. We accept the
// allocation here because live spawns are rare compared to frame
// iteration.
var newEntities = new List<WorldEntity>(lb.Entities.Count + 1);
newEntities.AddRange(lb.Entities);
newEntities.Add(entity);
_loaded[canonicalLandblockId] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, newEntities);
RebuildFlatView();
if (_loaded.TryGetValue(canonicalLandblockId, out var lb))
{
// Hot path — landblock is already loaded. Rebuild the record
// with the new entity appended.
var newEntities = new List<WorldEntity>(lb.Entities.Count + 1);
newEntities.AddRange(lb.Entities);
newEntities.Add(entity);
_loaded[canonicalLandblockId] = new LoadedLandblock(lb.LandblockId, lb.Heightmap, newEntities);
RebuildFlatView();
return;
}
// Cold path — landblock not yet loaded. Park the entity in the
// pending bucket; AddLandblock will pick it up when the streamer
// delivers the matching landblock.
if (!_pendingByLandblock.TryGetValue(canonicalLandblockId, out var bucket))
{
bucket = new List<WorldEntity>();
_pendingByLandblock[canonicalLandblockId] = bucket;
}
bucket.Add(entity);
}
private void RebuildFlatView()

23
src/AcDream.App/Streaming/StreamingController.cs
View file

@ -24,7 +24,28 @@ public sealed class StreamingController
private StreamingRegion? _region;
public int Radius { get; set; }
public int MaxCompletionsPerFrame { get; set; } = 4;
/// <summary>
/// Cap on completions drained per <see cref="Tick"/> call. Defaults to
/// effectively unlimited because the current <c>LandblockStreamer</c>
/// is synchronous — every <c>EnqueueLoad</c> writes to the outbox on
/// the same thread, so by the time we drain there's no backlog
/// to spread, and the cap only serves to *delay* applying landblocks
/// the user is already trying to look at.
///
/// <para>
/// The original async design used a small cap (4) to limit per-frame
/// GPU upload spikes. That reasoning becomes relevant again if/when
/// the streamer moves back to async loading; lower this knob then.
/// Crucially, dropping completions to a lower frame is what was
/// silently breaking live spawns: the post-login spawn flood would
/// arrive on a frame where only 4 of the 25 visible-window landblocks
/// had been applied, the spawns for the other 21 hit
/// <c>AppendLiveEntity</c> with no matching loaded slot, and got
/// dropped (now: parked in the pending bucket).
/// </para>
/// </summary>
public int MaxCompletionsPerFrame { get; set; } = int.MaxValue;
public StreamingController(
Action<uint> enqueueLoad,

140
tests/AcDream.Core.Tests/Streaming/GpuWorldStateTests.cs Normal file
View file

@ -0,0 +1,140 @@
using System;
using AcDream.App.Streaming;
using AcDream.Core.World;
using DatReaderWriter.DBObjs;
using Xunit;
namespace AcDream.Core.Tests.Streaming;
/// <summary>
/// Covers <see cref="GpuWorldState"/>'s pending-spawn behavior — the path
/// that survives the race where the server delivers a CreateObject for a
/// landblock that hasn't been streamed in yet. This was the bug that
/// silently dropped 40+ NPCs on the first frame after live login during
/// Phase A.1 visual verification.
/// </summary>
public class GpuWorldStateTests
{
private static LoadedLandblock MakeStubLandblock(uint canonicalId)
=> new(canonicalId, new LandBlock(), Array.Empty<WorldEntity>());
private static WorldEntity MakeStubEntity(uint id)
=> new()
{
Id = id,
SourceGfxObjOrSetupId = 0x01000001u,
Position = System.Numerics.Vector3.Zero,
Rotation = System.Numerics.Quaternion.Identity,
MeshRefs = Array.Empty<MeshRef>(),
};
[Fact]
public void AppendLiveEntity_LandblockAlreadyLoaded_AppendsImmediately()
{
var state = new GpuWorldState();
state.AddLandblock(MakeStubLandblock(0xA9B4FFFFu));
// Server sends a spawn at landblock 0xA9B40011 — same landblock,
// cell index 0x0011. Canonicalize drops the cell index, lookup
// succeeds, entity lands in the loaded landblock immediately.
state.AppendLiveEntity(0xA9B40011u, MakeStubEntity(42));
Assert.Single(state.Entities);
Assert.Equal(0u, (uint)state.PendingLiveEntityCount);
}
[Fact]
public void AppendLiveEntity_LandblockNotLoaded_ParksInPending()
{
var state = new GpuWorldState();
// No landblock loaded — the spawn must survive instead of being
// silently dropped (the bug from Phase A.1's first live run).
state.AppendLiveEntity(0xA9B40011u, MakeStubEntity(42));
Assert.Empty(state.Entities); // not visible yet
Assert.Equal(1, state.PendingLiveEntityCount);
}
[Fact]
public void AddLandblock_DrainsPendingEntriesForThatLandblock()
{
var state = new GpuWorldState();
// Three spawns arrive before the landblock loads.
state.AppendLiveEntity(0xA9B40011u, MakeStubEntity(1));
state.AppendLiveEntity(0xA9B40022u, MakeStubEntity(2)); // same landblock, different cell
state.AppendLiveEntity(0xA9B40033u, MakeStubEntity(3));
Assert.Equal(3, state.PendingLiveEntityCount);
Assert.Empty(state.Entities);
// Now the landblock streams in.
state.AddLandblock(MakeStubLandblock(0xA9B4FFFFu));
// The three pending entities are now visible, and the pending
// bucket for that landblock is empty.
Assert.Equal(3, state.Entities.Count);
Assert.Equal(0, state.PendingLiveEntityCount);
}
[Fact]
public void AddLandblock_DoesNotDrainPendingForADifferentLandblock()
{
var state = new GpuWorldState();
state.AppendLiveEntity(0xA9B40011u, MakeStubEntity(1)); // pending for 0xA9B4FFFF
state.AppendLiveEntity(0xAAAA0022u, MakeStubEntity(2)); // pending for 0xAAAAFFFF
// Loading 0xA9B4FFFF only drains its own bucket.
state.AddLandblock(MakeStubLandblock(0xA9B4FFFFu));
Assert.Single(state.Entities);
Assert.Equal(1, state.PendingLiveEntityCount); // 0xAAAAFFFF entry still parked
}
[Fact]
public void RemoveLandblock_DropsPendingForThatLandblock()
{
var state = new GpuWorldState();
// Spawns for landblock 0xA9B4FFFF arrive while it's pending.
state.AppendLiveEntity(0xA9B40011u, MakeStubEntity(1));
state.AppendLiveEntity(0xA9B40022u, MakeStubEntity(2));
Assert.Equal(2, state.PendingLiveEntityCount);
// Player moves away — the streamer says "this landblock is no
// longer in the visible window, drop it." The pending entries
// for it are dropped too because they came along with that
// landblock and are no longer relevant.
state.RemoveLandblock(0xA9B4FFFFu);
Assert.Equal(0, state.PendingLiveEntityCount);
Assert.Empty(state.Entities);
}
[Fact]
public void RemoveLandblock_LoadedThenRemoved_DropsItsEntities()
{
var state = new GpuWorldState();
state.AddLandblock(MakeStubLandblock(0xA9B4FFFFu));
state.AppendLiveEntity(0xA9B40011u, MakeStubEntity(1));
Assert.Single(state.Entities);
state.RemoveLandblock(0xA9B4FFFFu);
Assert.Empty(state.Entities);
}
[Fact]
public void IsLoaded_ReturnsTrueForLoaded_FalseForPendingOnly()
{
var state = new GpuWorldState();
state.AppendLiveEntity(0xA9B40011u, MakeStubEntity(1));
Assert.False(state.IsLoaded(0xA9B4FFFFu)); // pending doesn't count
state.AddLandblock(MakeStubLandblock(0xA9B4FFFFu));
Assert.True(state.IsLoaded(0xA9B4FFFFu));
}
}