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

chore(O-T7): code-review housekeeping after WB extraction

Browse source 
Five small post-cleanup items from T7 code review:

I1: Removed dead `datDir` parameter from WbMeshAdapter ctor (parameter
    was unused after _wbDats removal; ArgumentNullException.ThrowIfNull
    was misleading). Updated call sites in GameWindow.cs and
    WbMeshAdapterTests.cs.

I2: Updated stale GameWindow.cs comment that still described
    WbMeshAdapter as opening its own dat handles. Now reflects Phase O
    state: shared DatCollection via DatCollectionAdapter.

I3: Documented thread-safety contract on RenderStateCache (render-thread
    only — required for the mutable-static GL sentinel pattern).

M1: Added comment on IDatReaderWriter's write-path methods noting they
    are preserved for verbatim compatibility but unused in acdream.

M3: Added comment on Chorizite.Core PackageReference in Core.csproj
    explaining the previously-transitive dependency.

Also excluded SplitFormulaDivergenceTest.cs from the test build via
<Compile Remove>: this N.5b one-time data-collection test referenced
WorldBuilder.Shared types directly; after Phase O-T7 dropped that
project reference it no longer compiles. The sweep data it produced
already informed the N.5b Path-C decision and the file is retained
in the tree for historical reference.

Build green; tests green (1146 + 8 pre-existing failures baseline
maintained).

Spec: docs/superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-21 17:29:06 +02:00
parent dc722e70bd
commit 3e6f6ec858
8 changed files with 195 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

8
src/AcDream.App/Rendering/GameWindow.cs
View file

@ -1651,11 +1651,13 @@ public sealed class GameWindow : IDisposable
// Phase N.4+N.5 — WB rendering pipeline foundation. The modern path is
// mandatory as of N.5 ship amendment: WbMeshAdapter + WbDrawDispatcher
// always construct. WbMeshAdapter owns ObjectMeshManager and opens its
// own file handles for the dat files (independent of our DatCollection).
// always construct.
// Phase O (2026-05-21): WbMeshAdapter now consumes our DatCollection
// directly via DatCollectionAdapter. No second dat reader; index cache
// is shared with the rest of the client.
{
var wbLogger = Microsoft.Extensions.Logging.Abstractions.NullLogger<AcDream.App.Rendering.Wb.WbMeshAdapter>.Instance;
_wbMeshAdapter = new AcDream.App.Rendering.Wb.WbMeshAdapter(_gl, _datDir, _dats, wbLogger);
_wbMeshAdapter = new AcDream.App.Rendering.Wb.WbMeshAdapter(_gl, _dats, wbLogger);
Console.WriteLine("[N.4+N.5] WB foundation + modern path active — routing all content through ObjectMeshManager.");
}

2
src/AcDream.App/Rendering/Wb/IDatReaderWriter.cs
View file

@ -71,6 +71,8 @@ public interface IDatReaderWriter : IDisposable {
/// </summary>
int LanguageIteration { get; }
// Write-path methods — preserved from WB's interface for verbatim
// compatibility but not exercised by ObjectMeshManager in acdream.
/// <summary>Attempts to save a database object to the appropriate DAT.</summary>
bool TrySave<T>(T obj, int iteration = 0) where T : IDBObj;

6
src/AcDream.App/Rendering/Wb/RenderStateCache.cs
View file

@ -12,6 +12,12 @@ namespace AcDream.App.Rendering.Wb;
/// <c>CurrentIBO</c> — OpenGL name of the currently bound index buffer object.
/// Sentinel value 0 means "no valid binding cached."
/// </summary>
/// <remarks>
/// All accesses must occur on the render thread. GL state binding is
/// not thread-safe; these sentinels are written immediately after the
/// corresponding glBind* call and read by the next dispatch on the
/// same thread.
/// </remarks>
public static class RenderStateCache
{
public static uint CurrentAtlas = 0;

5
src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs
View file

@ -43,17 +43,14 @@ public sealed class WbMeshAdapter : IDisposable, IWbMeshAdapter
/// </summary>
/// <param name="gl">Active Silk.NET GL context. Must be bound to the current
/// thread (construction runs GL queries; call from OnLoad).</param>
/// <param name="datDir">Path to the dat directory. Retained for API compatibility;
/// DatCollectionAdapter routes all DAT I/O through our shared DatCollection.</param>
/// <param name="dats">acdream's DatCollection, used to populate the surface
/// metadata side-table via <c>GfxObjMesh.Build</c>. Shares file handles with
/// the rest of the client; read-only access from the render thread.</param>
/// <param name="logger">Logger for the adapter; ObjectMeshManager uses
/// NullLogger internally.</param>
public WbMeshAdapter(GL gl, string datDir, DatCollection dats, ILogger<WbMeshAdapter> logger)
public WbMeshAdapter(GL gl, DatCollection dats, ILogger<WbMeshAdapter> logger)
{
ArgumentNullException.ThrowIfNull(gl);
ArgumentNullException.ThrowIfNull(datDir);
ArgumentNullException.ThrowIfNull(dats);
ArgumentNullException.ThrowIfNull(logger);

4
src/AcDream.Core/AcDream.Core.csproj
View file

@ -8,6 +8,10 @@
</PropertyGroup>
<ItemGroup>
<PackageReference Include="BCnEncoder.Net" Version="2.2.1" />
<!-- Phase O (2026-05-21): Chorizite.Core was previously transitive
via the WorldBuilder.Shared project reference. After T7 dropped
that, Core's TextureHelpers needs an explicit reference for
Chorizite.Core.Render.Enums.TextureFormat. -->
<PackageReference Include="Chorizite.Core" Version="0.0.18" />
<PackageReference Include="Chorizite.DatReaderWriter" Version="2.1.7" />
<PackageReference Include="Serilog" Version="4.0.2" />

8
tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj
View file

@ -32,4 +32,12 @@
</ProjectReference>
</ItemGroup>
<ItemGroup>
<!-- Phase O (2026-05-21): N.5b data-collection test referenced WorldBuilder.Shared
types directly. After Phase O dropped the WorldBuilder.Shared project reference,
this one-time data-collection test no longer compiles. Excluding from build;
the sweep data it produced already informed the N.5b Path-C decision. -->
<Compile Remove="Terrain\SplitFormulaDivergenceTest.cs" />
</ItemGroup>
</Project>

2
tests/AcDream.Core.Tests/Rendering/Wb/WbMeshAdapterTests.cs
View file

@ -14,7 +14,7 @@ public sealed class WbMeshAdapterTests
// We can't pass a real GL (no context in tests), so we verify only the
// null-GL guard. The real pipeline is tested via integration.
Assert.Throws<ArgumentNullException>(() =>
new WbMeshAdapter(gl: null!, datDir: "some/path", dats: null!, logger: NullLogger<WbMeshAdapter>.Instance));
new WbMeshAdapter(gl: null!, dats: null!, logger: NullLogger<WbMeshAdapter>.Instance));
}
[Fact]

168
tests/AcDream.Core.Tests/Terrain/SplitFormulaDivergenceTest.cs Normal file
View file

@ -0,0 +1,168 @@
using AcDream.Core.Terrain;
using Xunit;
using Xunit.Abstractions;
using WbTerrainUtils = WorldBuilder.Shared.Modules.Landscape.Lib.TerrainUtils;
using WbCellSplitDirection = WorldBuilder.Shared.Modules.Landscape.Models.CellSplitDirection;
namespace AcDream.Core.Tests.Terrain;
/// <summary>
/// Phase N.5b data-collection test: quantifies how often WB's
/// <c>TerrainUtils.CalculateSplitDirection</c> disagrees with acdream's
/// <c>TerrainBlending.CalculateSplitDirection</c> (which retail uses
/// per <c>CLandBlockStruct::ConstructPolygons</c> at retail address
/// <c>00531d10</c>; named-retail decomp lines 316042-316144 contain
/// the exact constants <c>0x0CCAC033 / 0x6C1AC587 / 0x421BE3BD /
/// 0x519B8F25</c>).
///
/// Sweeps every (lbX, lbY, cellX, cellY) tuple in the world map
/// (255 x 255 landblocks x 64 cells = ~4.16M cells) and reports the
/// disagreement rate, per-landblock worst case, and a few named
/// representative landblocks. The number drives the Path A/B/C
/// decision in the N.5b brainstorm:
/// - Low disagreement &lt;5% : Path A's risk is bounded
/// - Medium 5-20% : Path B (fork-patch WB) preferred
/// - High &gt;20% : Path B/C strongly preferred
/// </summary>
public class SplitFormulaDivergenceTest
{
private readonly ITestOutputHelper _out;
public SplitFormulaDivergenceTest(ITestOutputHelper output) => _out = output;
[Fact]
public void Quantify_RetailVsWb_DivergenceRate()
{
// Two divergence flavors are tracked simultaneously:
//
// rawDisagree : retail-enum != wb-enum (pure formula output)
// diagonalDisagree: retail-actually-paints-diagonal !=
// wb-actually-paints-diagonal (effective geometry)
//
// The two differ because the enums are SEMANTICALLY INVERTED:
// - acdream `CellSplitDirection.SWtoNE` -> renderer paints BL->TR
// (SW-NE diagonal). Matches retail per AC2D Landblocks.cpp:400-412
// where FSplitNESW=true wraps a TRIANGLE_FAN [BL, BR, TR, TL] =
// diagonal BL-TR.
// - WB `CellSplitDirection.SWtoNE` -> WB's TerrainGeometryGenerator
// emits triangles {BL,TL,BR}+{BR,TL,TR} which share the BR-TL
// diagonal (SE-NW direction). The enum name is misleading; what
// WB actually draws is the OTHER diagonal.
//
// So the question "would WB's pipeline produce the same diagonals as
// retail's pipeline?" is answered by `diagonalDisagree`, not
// `rawDisagree`. If diagonalDisagree is near 0%, WB's formula +
// renderer happen to compose into a correct pipeline (despite the
// confusing labels). If diagonalDisagree is ~50%, the two pipelines
// truly diverge and Path A would visibly break terrain on every
// landblock.
const int lbCount = 255;
const int cellsPerSide = 8;
long totalCells = 0;
long rawDisagree = 0;
long diagonalDisagree = 0;
int worstLbDiag = 0;
uint worstLbX = 0, worstLbY = 0;
int bestLbDiag = 64;
uint bestLbX = 0, bestLbY = 0;
for (uint lbX = 0; lbX < lbCount; lbX++)
for (uint lbY = 0; lbY < lbCount; lbY++)
{
int lbDiagDisagree = 0;
for (uint cx = 0; cx < cellsPerSide; cx++)
for (uint cy = 0; cy < cellsPerSide; cy++)
{
bool retailEnumSWtoNE =
TerrainBlending.CalculateSplitDirection(lbX, cx, lbY, cy)
== CellSplitDirection.SWtoNE;
bool wbEnumSWtoNE =
WbTerrainUtils.CalculateSplitDirection(lbX, cx, lbY, cy)
== WbCellSplitDirection.SWtoNE;
// What diagonal each pipeline actually paints.
bool retailPaintsBLtoTR = retailEnumSWtoNE; // direct mapping
bool wbPaintsBLtoTR = !wbEnumSWtoNE; // inverted mapping
totalCells++;
if (retailEnumSWtoNE != wbEnumSWtoNE) rawDisagree++;
if (retailPaintsBLtoTR != wbPaintsBLtoTR)
{
diagonalDisagree++;
lbDiagDisagree++;
}
}
if (lbDiagDisagree > worstLbDiag)
{
worstLbDiag = lbDiagDisagree;
worstLbX = lbX;
worstLbY = lbY;
}
if (lbDiagDisagree < bestLbDiag)
{
bestLbDiag = lbDiagDisagree;
bestLbX = lbX;
bestLbY = lbY;
}
}
double rawPct = 100.0 * rawDisagree / totalCells;
double diagPct = 100.0 * diagonalDisagree / totalCells;
_out.WriteLine($"=== Phase N.5b — terrain split formula divergence ===");
_out.WriteLine($"Sweep: {lbCount}x{lbCount} landblocks, {cellsPerSide*cellsPerSide} cells each");
_out.WriteLine($"Total cells: {totalCells:N0}");
_out.WriteLine("");
_out.WriteLine($"RAW enum-output disagreement : {rawDisagree,12:N0} ({rawPct:F2}%)");
_out.WriteLine($" (compares retail-enum vs wb-enum, NOT what each system actually draws)");
_out.WriteLine("");
_out.WriteLine($"DIAGONAL-actually-painted disagreement: {diagonalDisagree,12:N0} ({diagPct:F2}%)");
_out.WriteLine($" (compares retail-paints-BL->TR vs wb-paints-BL->TR; this is the");
_out.WriteLine($" number that determines whether Path A visibly works)");
_out.WriteLine("");
_out.WriteLine($"Worst landblock (diagonal): 0x{worstLbX:X2}{worstLbY:X2} disagrees on {worstLbDiag}/64 cells ({100.0*worstLbDiag/64:F1}%)");
_out.WriteLine($"Best landblock (diagonal): 0x{bestLbX:X2}{bestLbY:X2} disagrees on {bestLbDiag}/64 cells ({100.0*bestLbDiag/64:F1}%)");
// Specific landblocks of interest (per N.5b handoff representative set).
var representative = new (string name, uint lbX, uint lbY)[]
{
("Holtburg town", 0xA9, 0xB0),
("Holtburg LB 0xA9B1", 0xA9, 0xB1),
("Foundry-area", 0x80, 0x80),
("Cragstone", 0xCB, 0x99),
("Direlands sample", 0xC0, 0x40),
("MapOrigin 0x0000", 0x00, 0x00),
("MapCorner 0xFEFE", 0xFE, 0xFE),
("Mid-map 0x7F7F", 0x7F, 0x7F),
("Subway dungeon LB 0x0185 outdoor part", 0x01, 0x85),
};
_out.WriteLine("");
_out.WriteLine("Representative landblocks (diagonal-actually-painted disagreement):");
foreach (var (name, lbX, lbY) in representative)
{
int dis = 0;
for (uint cx = 0; cx < 8; cx++)
for (uint cy = 0; cy < 8; cy++)
{
bool retailEnum = TerrainBlending.CalculateSplitDirection(lbX, cx, lbY, cy) == CellSplitDirection.SWtoNE;
bool wbEnum = WbTerrainUtils.CalculateSplitDirection(lbX, cx, lbY, cy) == WbCellSplitDirection.SWtoNE;
bool retailPaintsBLtoTR = retailEnum;
bool wbPaintsBLtoTR = !wbEnum;
if (retailPaintsBLtoTR != wbPaintsBLtoTR) dis++;
}
_out.WriteLine($" 0x{lbX:X2}{lbY:X2} {dis,2}/64 cells disagree ({100.0*dis/64:F1}%) {name}");
}
// Soft-floor on the DIAGONAL comparison: if diagPct is near 0% the
// formulas are equivalent post-inversion (Path A would just work
// visually; the only "bug" is enum naming). If diagPct is well
// above 0%, Path A truly breaks terrain.
// Soft-ceiling: an inversion of inversion shouldn't push past ~70%.
Assert.True(diagPct >= 0 && diagPct <= 100,
$"Sanity: diagonal disagreement out of range (rate={diagPct:F2}%)");
}
}