Triage step from the plan at C:\Users\erikn\.claude\plans\ i-did-some-work-sharded-acorn.md. Four sessions on issue #98 left the worktree dirty with ~1352 LOC of mixed work. This commit splits the work into "keep" (defensible + diagnostic) and "drop" (failed experiments), then commits the keep set with the drops removed. Plan asked for three commits (diag / fix / revert); consolidated to one because the diagnostic emits in TransitionTypes.cs are tightly interleaved with the multi-sphere CellTransit calls and the CellId switch. Hunk-level splitting in those files for marginal bisect granularity didn't justify the misclick risk. Reverted entirely (failed experiments per slice 7 handoff): - src/AcDream.Core/Physics/PhysicsDataCache.cs — neg-poly storage fields (Stippling, PosSurface, NegSurface, HasNegativeSide, IsNegativeSide, NegativeSide). - src/AcDream.Core/Physics/ShadowObjectRegistry.cs — isBuilding flag propagation through Register / ShadowEntry. - tests/AcDream.Core.Tests/Physics/BSPQueryTests.cs — 165 lines of PolygonWithNegativeSide_* tests. - tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryTests.cs — isBuilding propagation tests. - src/AcDream.Core/World/WorldEntity.cs — IsLandblockBuilding field (no consumer once ShadowObjectRegistry.isBuilding is gone). - src/AcDream.Core/World/LandblockLoader.cs — IsLandblockBuilding=true setter on building entities (kept BuildBuildingTerrainCells). - src/AcDream.App/Rendering/GameWindow.cs — isBuilding: arg passed to ShadowObjects.Register. - src/AcDream.Core/Physics/BSPQuery.cs — TryAdjustWalkableSide / IsWalkableAt helpers, their callers, the Path 5 / Path 6 neg-poly branch split, the BldgCheck-tied clearCell conditional, and the neg-poly ResolveCellPolygons writes. - src/AcDream.Core/Physics/PhysicsDiagnostics.cs — neg-poly fields in the poly-dump format. - src/AcDream.Core/Physics/TransitionTypes.cs — SpherePath.BldgCheck + SpherePath.HitsInteriorCell fields and every consumer, the savedBldgCheck try/finally around FindCollisions, and the neg-poly format additions to the dump-on-error helper. - src/AcDream.Core/Physics/CellTransit.cs — FindCellSet overloads with hitsInteriorCell out-param and the BuildCellSetAndPickContaining out-param threading. Kept (defensible correctness fixes + diagnostic infrastructure): - src/AcDream.App/Rendering/GameWindow.cs — render-vs-physics cell origin split: the 0.02m render lift no longer leaks into physics BSP caching. lb.BuildingTerrainCells threaded into LandblockMesh.Build. - src/AcDream.Core/World/LoadedLandblock.cs — BuildingTerrainCells record field. - src/AcDream.Core/World/LandblockLoader.cs — BuildBuildingTerrainCells (cy*8+cx from LandBlockInfo.Buildings). - src/AcDream.Core/Terrain/LandblockMesh.cs — hiddenTerrainCells param that collapses owned-cell triangles to a zero-area degenerate. - src/AcDream.App/Streaming/{GpuWorldState,LandblockStreamer}.cs — mechanical BuildingTerrainCells threading through LoadedLandblock reconstructions. - src/AcDream.Core/Physics/CellTransit.cs — multi-sphere FindTransitCellsSphere variant + multi-sphere AddAllOutsideCells + FindCellSet(IReadOnlyList<Sphere>, …) overload + the BSPQuery.SphereIntersectsCellBsp call for loaded neighbours. Matches retail CObjCell::find_cell_list / CEnvCell::find_transit_cells. - src/AcDream.Core/Physics/TransitionTypes.cs — multi-sphere FindCellSet call site, retail-faithful CellId switch after CheckOtherCells, the outdoor-landcell terrain-walkable fallback in CheckOtherCells, and the full diagnostic suite ([step-walk], [walkable-nearest], [issue98-walkable-detail], [cell-set-summary], LastBspHitPoly emits). - src/AcDream.Core/Physics/PhysicsDiagnostics.cs — ProbeStepWalkEnabled gate (ACDREAM_PROBE_STEP_WALK=1) + LogStepWalk helper + FormatVector / FormatPlane utilities. All emit-gated. - src/AcDream.Core/Physics/BSPQuery.cs — diagnostic emits to LastBspHitPoly at four sites in SphereIntersectsPolyInternal / the placement adjustment path. - Test files for the kept work: CellTransitFindCellSetTests, CellTransitFindTransitCellsSphereTests, PhysicsDiagnosticsTests, TransitionCheckOtherCellsTests, LandblockMeshTests, LandblockLoaderTests. Verification: - dotnet build: green, 0 errors, 3 pre-existing warnings. - dotnet test: 1156 passed + 8 failed (baseline was 1148 + 8 pre- existing; the +8 passing are the new tests for the kept defensible work). Same 8 pre-existing failures, no new regressions. Backup of pre-triage worktree state in stash@{0}. A6.P3 #98 is still open; this is the apparatus-prep step, not a fix. Next: cell-dump probe (Step 2 of the plan).
261 lines
11 KiB
C#
261 lines
11 KiB
C#
using System;
|
|
using System.Collections.Generic;
|
|
using System.Threading;
|
|
using System.Threading.Channels;
|
|
using System.Threading.Tasks;
|
|
using AcDream.Core.World;
|
|
|
|
namespace AcDream.App.Streaming;
|
|
|
|
/// <summary>
|
|
/// Services landblock load/unload requests by invoking caller-supplied
|
|
/// factory delegates (the production instance wraps
|
|
/// <see cref="LandblockLoader.Load"/> for loading and
|
|
/// <see cref="AcDream.Core.Terrain.LandblockMesh.Build"/> for the terrain
|
|
/// mesh) and posting results to an outbox the render thread drains once
|
|
/// per OnUpdate.
|
|
///
|
|
/// <para>
|
|
/// <b>Thread model (Phase A.5 T11+):</b> <see cref="Start"/> spawns a
|
|
/// dedicated background worker thread. <see cref="EnqueueLoad"/> and
|
|
/// <see cref="EnqueueUnload"/> write non-blocking to the inbox
|
|
/// <see cref="Channel{T}"/>; the worker drains it and posts
|
|
/// <see cref="LandblockStreamResult"/> records to the outbox.
|
|
/// </para>
|
|
///
|
|
/// <para>
|
|
/// <b>DatCollection thread safety</b> is provided by the caller:
|
|
/// GameWindow's <c>_datLock</c> (Phase A.5 T10) serialises all
|
|
/// <c>DatCollection.Get<T></c> calls. Both factory closures passed at
|
|
/// construction acquire that lock before reading dats. The worker never
|
|
/// touches <c>DatCollection</c> directly — it only calls the factories.
|
|
/// </para>
|
|
///
|
|
/// <para>
|
|
/// Unloads pass through the outbox as <see cref="LandblockStreamResult.Unloaded"/>
|
|
/// records so the render thread can release GPU state on the next drain —
|
|
/// the streamer never touches GPU resources directly.
|
|
/// </para>
|
|
///
|
|
/// <remarks>
|
|
/// Threading: <see cref="DrainCompletions"/> must be called from a single
|
|
/// consumer thread (the render thread in production). All other public
|
|
/// methods are thread-safe.
|
|
/// </remarks>
|
|
/// </summary>
|
|
public sealed class LandblockStreamer : IDisposable
|
|
{
|
|
/// <summary>
|
|
/// Default drain batch size. Tuned to cap GPU upload work the render
|
|
/// thread does per frame while still draining a moderate backlog in a
|
|
/// few frames. Callers can override on a per-call basis.
|
|
/// </summary>
|
|
public const int DefaultDrainBatchSize = 4;
|
|
|
|
private readonly Func<uint, LandblockStreamJobKind, LoadedLandblock?> _loadLandblock;
|
|
private readonly Func<uint, LoadedLandblock?, AcDream.Core.Terrain.LandblockMeshData?> _buildMeshOrNull;
|
|
private readonly Channel<LandblockStreamJob> _inbox;
|
|
private readonly Channel<LandblockStreamResult> _outbox;
|
|
private readonly CancellationTokenSource _cancel = new();
|
|
private Thread? _worker;
|
|
private int _disposed;
|
|
|
|
/// <summary>
|
|
/// Primary ctor — the factory takes the job's <see cref="LandblockStreamJobKind"/>
|
|
/// so it can branch on far-tier vs near-tier and skip entity hydration on far-tier
|
|
/// loads (heightmap-only). See ISSUE #54: prior to this signature the worker always
|
|
/// called the full-load path and stripped entities at the output, wasting per-LB
|
|
/// <c>LandBlockInfo</c> + <c>SceneryGenerator</c> work.
|
|
/// </summary>
|
|
public LandblockStreamer(
|
|
Func<uint, LandblockStreamJobKind, LoadedLandblock?> loadLandblock,
|
|
Func<uint, LoadedLandblock?, AcDream.Core.Terrain.LandblockMeshData?>? buildMeshOrNull = null)
|
|
{
|
|
_loadLandblock = loadLandblock;
|
|
// Default: no mesh build (returns null → Failed result). Production
|
|
// wires in LandblockMesh.Build via the T12 construction site.
|
|
_buildMeshOrNull = buildMeshOrNull ?? ((_, _) => null);
|
|
_inbox = Channel.CreateUnbounded<LandblockStreamJob>(
|
|
new UnboundedChannelOptions { SingleReader = true, SingleWriter = false });
|
|
_outbox = Channel.CreateUnbounded<LandblockStreamResult>(
|
|
new UnboundedChannelOptions { SingleReader = true, SingleWriter = true });
|
|
}
|
|
|
|
/// <summary>
|
|
/// Back-compat overload — wraps a kind-agnostic factory so existing test code
|
|
/// that doesn't care about the JobKind branch keeps compiling. The wrapper
|
|
/// ignores the kind and calls the factory once per LB regardless of tier.
|
|
/// New production code should use the primary 2-arg ctor.
|
|
/// </summary>
|
|
public LandblockStreamer(
|
|
Func<uint, LoadedLandblock?> loadLandblock,
|
|
Func<uint, LoadedLandblock?, AcDream.Core.Terrain.LandblockMeshData?>? buildMeshOrNull = null)
|
|
: this((id, _) => loadLandblock(id), buildMeshOrNull)
|
|
{
|
|
}
|
|
|
|
/// <summary>
|
|
/// Activate the dedicated background worker thread. Idempotent and
|
|
/// thread-safe: concurrent callers will only spawn one worker; subsequent
|
|
/// calls are no-ops. Atomic via <see cref="Interlocked.CompareExchange{T}(ref T, T, T)"/>.
|
|
/// </summary>
|
|
public void Start()
|
|
{
|
|
if (System.Threading.Volatile.Read(ref _disposed) != 0)
|
|
throw new ObjectDisposedException(nameof(LandblockStreamer));
|
|
|
|
// A.5 T10-T12 follow-up: atomically install the worker so concurrent
|
|
// Start() callers don't both pass the null check and spawn duplicate
|
|
// threads. Construct the candidate; CAS it into _worker; if we lost
|
|
// the race, the candidate goes unstarted and is GCed.
|
|
var candidate = new Thread(WorkerLoop)
|
|
{
|
|
IsBackground = true,
|
|
Name = "acdream.streaming.worker",
|
|
};
|
|
if (Interlocked.CompareExchange(ref _worker, candidate, null) == null)
|
|
candidate.Start();
|
|
// else: another caller won the race; their thread is running.
|
|
}
|
|
|
|
/// <summary>
|
|
/// Non-blocking enqueue. The worker drains the inbox and posts a
|
|
/// <see cref="LandblockStreamResult.Loaded"/> (or
|
|
/// <see cref="LandblockStreamResult.Failed"/>) to the outbox.
|
|
/// </summary>
|
|
public void EnqueueLoad(uint landblockId, LandblockStreamJobKind kind = LandblockStreamJobKind.LoadNear)
|
|
{
|
|
if (System.Threading.Volatile.Read(ref _disposed) != 0)
|
|
throw new ObjectDisposedException(nameof(LandblockStreamer));
|
|
_inbox.Writer.TryWrite(new LandblockStreamJob.Load(landblockId, kind));
|
|
}
|
|
|
|
/// <summary>
|
|
/// Non-blocking enqueue. The worker posts a
|
|
/// <see cref="LandblockStreamResult.Unloaded"/> to the outbox.
|
|
/// </summary>
|
|
public void EnqueueUnload(uint landblockId)
|
|
{
|
|
if (System.Threading.Volatile.Read(ref _disposed) != 0)
|
|
throw new ObjectDisposedException(nameof(LandblockStreamer));
|
|
_inbox.Writer.TryWrite(new LandblockStreamJob.Unload(landblockId));
|
|
}
|
|
|
|
/// <summary>
|
|
/// Drain up to <paramref name="maxBatchSize"/> completed results.
|
|
/// Non-blocking. Call from the render thread once per OnUpdate.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// Must be called from a single consumer thread. The outbox channel is
|
|
/// configured with SingleReader = true and will throw on concurrent reads.
|
|
/// </remarks>
|
|
public IReadOnlyList<LandblockStreamResult> DrainCompletions(int maxBatchSize = DefaultDrainBatchSize)
|
|
{
|
|
var batch = new List<LandblockStreamResult>(maxBatchSize);
|
|
while (batch.Count < maxBatchSize && _outbox.Reader.TryRead(out var result))
|
|
batch.Add(result);
|
|
return batch;
|
|
}
|
|
|
|
private void WorkerLoop()
|
|
{
|
|
try
|
|
{
|
|
// Safe to block: this is a dedicated worker thread with no
|
|
// SynchronizationContext, so .Result/.GetResult cannot deadlock
|
|
// against any captured continuation. Using the sync pattern
|
|
// here keeps the loop linear; an async-enumerable alternative
|
|
// would force WorkerLoop to be async Task and lose the
|
|
// simple thread-start shape.
|
|
while (!_cancel.Token.IsCancellationRequested)
|
|
{
|
|
if (!_inbox.Reader.WaitToReadAsync(_cancel.Token).AsTask().GetAwaiter().GetResult())
|
|
break;
|
|
|
|
while (_inbox.Reader.TryRead(out var job))
|
|
{
|
|
if (_cancel.Token.IsCancellationRequested) return;
|
|
HandleJob(job);
|
|
}
|
|
}
|
|
}
|
|
catch (OperationCanceledException) { /* graceful shutdown */ }
|
|
catch (Exception ex)
|
|
{
|
|
// Last-ditch: surface via outbox so the caller at least sees
|
|
// something. We never retry a crashed worker.
|
|
_outbox.Writer.TryWrite(new LandblockStreamResult.WorkerCrashed(ex.ToString()));
|
|
}
|
|
finally
|
|
{
|
|
_outbox.Writer.TryComplete();
|
|
}
|
|
}
|
|
|
|
private void HandleJob(LandblockStreamJob job)
|
|
{
|
|
switch (job)
|
|
{
|
|
case LandblockStreamJob.Load load:
|
|
// ISSUE #54 (post-A.5): JobKind is now plumbed through to the
|
|
// factory, so far-tier loads can skip LandBlockInfo + scenery
|
|
// + interior hydration on the worker thread (heightmap-only).
|
|
// The post-load entity-strip below is retained as a Debug
|
|
// assertion + Release safety net for the case where a buggy
|
|
// factory returns far-tier with entities anyway.
|
|
try
|
|
{
|
|
var lb = _loadLandblock(load.LandblockId, load.Kind);
|
|
if (lb is null)
|
|
{
|
|
_outbox.Writer.TryWrite(new LandblockStreamResult.Failed(
|
|
load.LandblockId, "LandblockLoader.Load returned null"));
|
|
break;
|
|
}
|
|
var mesh = _buildMeshOrNull(load.LandblockId, lb);
|
|
if (mesh is null)
|
|
{
|
|
_outbox.Writer.TryWrite(new LandblockStreamResult.Failed(
|
|
load.LandblockId, "buildMeshOrNull returned null"));
|
|
break;
|
|
}
|
|
var tier = load.Kind == LandblockStreamJobKind.LoadFar
|
|
? LandblockStreamTier.Far : LandblockStreamTier.Near;
|
|
if (tier == LandblockStreamTier.Far && lb.Entities.Count > 0)
|
|
{
|
|
// Belt-and-suspenders: factory should have skipped
|
|
// entity hydration for LoadFar. If it didn't, fail
|
|
// loud in Debug builds and strip in Release.
|
|
System.Diagnostics.Debug.Assert(false,
|
|
$"Far-tier factory should skip entity hydration; got {lb.Entities.Count} entities for LB 0x{load.LandblockId:X8}");
|
|
lb = new LoadedLandblock(
|
|
lb.LandblockId,
|
|
lb.Heightmap,
|
|
System.Array.Empty<AcDream.Core.World.WorldEntity>(),
|
|
lb.BuildingTerrainCells);
|
|
}
|
|
_outbox.Writer.TryWrite(new LandblockStreamResult.Loaded(
|
|
load.LandblockId, tier, lb, mesh));
|
|
}
|
|
catch (Exception ex)
|
|
{
|
|
_outbox.Writer.TryWrite(new LandblockStreamResult.Failed(
|
|
load.LandblockId, ex.ToString()));
|
|
}
|
|
break;
|
|
|
|
case LandblockStreamJob.Unload unload:
|
|
_outbox.Writer.TryWrite(new LandblockStreamResult.Unloaded(unload.LandblockId));
|
|
break;
|
|
}
|
|
}
|
|
|
|
public void Dispose()
|
|
{
|
|
if (System.Threading.Interlocked.Exchange(ref _disposed, 1) != 0) return;
|
|
_cancel.Cancel();
|
|
_inbox.Writer.TryComplete();
|
|
_worker?.Join(TimeSpan.FromSeconds(2));
|
|
_cancel.Dispose();
|
|
}
|
|
}
|