Adversarially-verified review findings 7 and 8:
(7) The DatCollection->IDatReaderWriter adapter existed as THREE
near-identical copies (App-internal original, Bake's copy, Content.
Tests' copy) — a structure where adapter drift is exactly what the
live-vs-pak equivalence suite cannot detect (both sides would only
drift together if they shared one implementation). Now ONE public
AcDream.Content.DatCollectionAdapter next to IDatReaderWriter (GL-free
home established in MP1a), carrying App's FULL behavior including the
[dat-miss] TryGet tripwire log (which now also covers the bake tool
and the equivalence suite) and the caching/locking. All three copies
deleted; WbMeshAdapter (App), BakeRunner (Bake), and
PakEquivalenceTests (Content.Tests) resolve the shared class.
Iteration properties return the REAL dat iterations — the App copy's
hardcoded 0 was a stub nothing read; the unification intentionally
keeps truth (noted in the doc comment). Verified post-move: no
Silk.NET anywhere in Content / Bake / Content.Tests / Bake.Tests
resolved dependency graphs.
(8) Two test gaps closed in PakRoundTripTests: (a) direct on-disk TOC
sortedness — blobs added in DESCENDING key order, then the raw file
bytes parsed (not through the reader) and every TOC entry asserted
strictly ascending; (b) corrupt-blob logging — five repeated reads
through both public paths (TryReadObjectMeshData + ContainsKey) with
stderr captured, asserting exactly ONE [pak-corrupt] line for the
victim key.
Full suite: 4120 tests, 0 failures (Content.Tests 56, Bake.Tests 1,
plus the pre-existing 4 skips).
Adversarially-verified review findings 2,3,4,5,6,9:
(2) PakFormat.CurrentFormatVersion=1 constant; PakWriter stamps it
unconditionally (caller header templates can no longer produce a
version-0 pak — the default-0 footgun); PakReader refuses any other
version with a message naming found/expected. Tests: versions 0 and 2
both rejected; writer stamps even when the template omits the field.
(3) Reader robustness, all under the documented corrupt-=-missing
CONTRACT (external-file input — surface loudly once, then behave as
absent; never garbage, never throw from a lookup): (a) per-TOC-entry
bounds validation at open (offset/length outside [header, toc) or
past EOF -> logged once, entry missing, siblings unaffected); (b)
TryReadObjectMeshData catches deserialization failures (malformed
structure behind a matching CRC) -> logged once per entry, false; (c)
structurally unopenable files throw at OPEN with a clear message:
unfinalized header (tocOffset < header size — the placeholder-header
crash signature) and TOC-past-EOF truncation. Tests: corrupt TOC
entry, truncated pak, half-written pak, malformed-blob-behind-valid-
CRC (tamper + CRC recompute) — each verifying sibling blobs still read.
(4) Single-pass read: TryReadObjectMeshData now does ONE ReadArray out
of the map; CRC and deserialization run over the same buffer (was: a
separate VerifyCrc traversal + a second ReadArray + a ToArray copy
inside Serializer.Read). New Serializer.Read(byte[]) overload avoids
the defensive copy. Verdict set stays a ConcurrentDictionary (MP1c
calls this from 4 decode workers). True span-over-mmap zero-copy is
deferred to MP1c profiling per the class doc comment.
(5) PakWriter.Dispose restored to try/finally: Finish() does real I/O
and can throw (disk full) — the stream must ALWAYS close so no file
handle leaks mid-unwind. (Reverts the a5926ebc simplification, which
was wrong about this.) Test: dispose after an AddBlob exception leaves
the file deletable.
(6) CRC-32 known-answer vectors: "123456789" -> 0xCBF43926, empty ->
0x00000000, single zero byte -> 0xD202EF8D. The suite was previously
blind to a self-consistent-but-wrong CRC.
(9) Equality comparator floats switched to bit-equality
(SingleToInt32Bits/DoubleToInt64Bits, incl. Vector2/3 + Matrix4x4
components): for byte-identity round-trip purposes `==` was both too
strict (NaN==NaN false — a surviving NaN payload would wrongly FAIL)
and too lax (-0.0==+0.0 true — a sign-bit flip would wrongly PASS).
Content.Tests: 54/54 green (was 43).
Self-review finding: Dispose()'s best-effort Finish() call was wrapped
in a try/catch for InvalidOperationException that could never fire —
the surrounding `if (!_finished)` guard already excludes the only
condition under which Finish() throws that exception. Dead defensive
code that LOOKS like it might be swallowing a real failure is exactly
the kind of thing the no-workarounds policy asks to avoid; removed with
a comment explaining why no catch is needed. Behavior unchanged; 43/43
Content.Tests still green.
TDD: PakRoundTripTests written first, confirmed a compile failure
against the not-yet-existing PakWriter/PakReader types.
PakWriter streams [header placeholder][64-byte-aligned blobs][TOC sorted
by key], then seeks back and finalizes the header once TocOffset/TocCount
are known (plan: "TOC last" so the writer doesn't need blob count
up front). PakReader mmaps the whole file once, loads the TOC into a
sorted array for O(log n) binary-search lookup, and verifies each blob's
CRC-32 LAZILY on first access (cached per-index so a corrupt blob logs
exactly once and is thereafter always treated as missing rather than
handing back garbage bytes). No locks anywhere — the mapped memory is
immutable for the reader's lifetime by construction.
CRC-32 implemented directly (standard IEEE 802.3 table-driven variant)
rather than adding a System.IO.Hashing package reference, keeping
AcDream.Content's dependency surface minimal per its existing
"no GL binaries" intent.
9 tests green: header round-trip, every-key-found, blob deep-equality
via the Task 3 comparator, missing-key returns false, corrupted-blob
(single flipped byte) is detected and treated as missing while leaving
sibling blobs unaffected, 64-byte blob alignment, and TOC binary search
cross-checked against a linear scan for both present and absent keys.
TDD: ObjectMeshDataSerializerTests + the shared ObjectMeshDataEquality
field-by-field comparator (reused by Task 6's equivalence suite) written
first, confirmed a compile failure against the not-yet-existing
ObjectMeshDataSerializer type.
Serializes EVERY field of the ObjectMeshData family per the plan's
normative layout: primitives raw LE, arrays as count:i32+payload,
blittable arrays (VertexPositionNormalTexture[], ushort[], byte[]) via
MemoryMarshal.AsBytes bulk copy, TextureBatches written sorted by the
(Width, Height, Format) key tuple for run-to-run determinism regardless
of dictionary insertion order, nullable fields as present:byte+value.
EnvCellGeometry nests recursively (MeshExtractor can populate one level
today; the serializer supports arbitrary depth rather than assuming it).
Namespace-trap finding: StagedEmitter.Emitter resolves to
DatReaderWriter.DBObjs.ParticleEmitter (the dat DBObj, verified via
reflection against the pinned Chorizite.DatReaderWriter 2.1.7 package
and confirmed live by MeshExtractor's `emitter.HwGfxObjId.DataId` call
site compiling), NOT AcDream.Core.Vfx.ParticleEmitter (the runtime
particle-simulation type with a live Particle[] pool that would NOT be
serializable asset data). All ~31 of its fields are written explicitly
rather than delegating to its own Pack/Unpack, which require a live
DatBinWriter/DatBinReader bound to a DatDatabase — coupling our pak's
determinism to a third-party wire-format helper we don't control the
versioning of.
33 tests green: 9 round-trip fixtures (empty/vertices+indices/multi
texture-batch-groups/setup-parts/emitters/nullable-present/nullable-
absent/edge-lines/nested-EnvCellGeometry), same-instance-twice byte-
identity, and dictionary-insertion-order-independence (two orders ->
identical bytes) plus a key-sort-order assertion on the raw bytes.
TDD: PakKeyTests + PakFormatTests written first (confirmed a compile
failure against the not-yet-existing AcDream.Content.Pak namespace),
then PakKey (64-bit type:u8|fileId:u32|reserved:u24 compose/decompose)
and PakFormat (64-byte PakHeader, 24-byte PakTocEntry) implemented to
the normative layout in the MP1b plan. 21 tests green, including a key-
ordering test proving ascending numeric key order equals ascending
(type, fileId) tuple order (the TOC binary-search precondition) and an
explicit byte-offset test for both structs.
Coordinator-directed final cleanup before the user gate; none behavioral:
1. MeshExtractor public surface narrowed to the cross-assembly entry
points App actually calls (PrepareMeshData, PrepareCellStructMeshData,
CollectParts, ComputeBounds); PrepareSetupMeshData,
CollectEmittersFromScript, PrepareGfxObjMeshData,
PrepareEnvCellMeshData, PrepareCellStructEdgeLineData back to private
(internal dispatch, only reached via PrepareMeshData).
2. sideStagedSink constructor parameter is now REQUIRED (no default;
type stays nullable for a conscious null): a bake tool that forgot
the sink would silently lose particle-preload meshes.
3. AcDream.Content.csproj gains TreatWarningsAsErrors + LangVersion
latest (parity with AcDream.Core.csproj). Surfaced zero warnings.
4. Dead usings removed from ObjectMeshManager.cs (BCnEncoder.*,
SixLabors.*) — the inline decode moved out in Task 4.
5. Doc fixes: ObjectMeshData.cs cross-assembly <see cref> ->
plain text (Content can't resolve App types); IDatReaderWriter.cs
stale Phase O-T7 'both in this namespace' sentence rewritten.
6. Stale test doc comments updated to MeshExtractor.PrepareGfxObjMeshData
(StipplingSurfaceEquivalenceTests, Issue119UpNullGfxObjDumpTests) —
comments only, no code/assertion changes.
dotnet build green (0 warnings in Content under warnings-as-errors);
full test suite 4059 passed / 0 failed / 4 skipped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Coordinator-directed follow-up. The buffer-and-drain seam diverged from
the original on the exception path: pre-MP1a, CollectEmittersFromScript
enqueued particle-preload meshes DIRECTLY into _stagedMeshData
mid-Prepare, so preloads staged before a later throw in the same
Prepare* call (reachable via PrepareEnvCellMeshData side-staging during
its StaticObjects loop, then PrepareCellStructMeshData throwing on a
malformed-dat texture decode) were already safely enqueued. The drain
version only flushed after a successful return — on throw, entries
stranded on the shared extractor until an unrelated successful call
flushed them, and were silently dropped on dispose.
Fix: MeshExtractor takes an Action<ObjectMeshData>? sideStagedSink
constructor parameter; the two CollectEmittersFromScript sites become
_sideStagedSink?.Invoke(meshData) — the original code shape (immediate
hand-off) at those exact lines. ObjectMeshManager wires the sink to
_stagedMeshData.Enqueue, restoring the original immediate-enqueue
semantics including on mid-Prepare throw. _sideStaged buffer,
DrainSideStaged(), and the ProcessQueueAsync drain loop are deleted.
The MP1b bake tool passes its own collector.
Inventory doc updated: MP1a note now records the sink seam and the
Content-owned upload enums, so its no-behavior-change claim is accurate.
dotnet build green; full test suite 4059 passed / 0 failed / 4 skipped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Coordinator-directed follow-up: AcDream.Content must stay Silk.NET-free
(the MP1b bake tool must not ship GL binaries). The Silk.NET.OpenGL
PackageReference added for the PixelFormat?/PixelType? upload hints is
replaced by Content-owned UploadPixelFormat/UploadPixelType enums
(UploadFormats.cs) whose underlying values are the GL ABI constants
(Rgba = 0x1908, UnsignedByte = 0x1401), verified numerically identical
to the Silk.NET.OpenGL members against 2.23.0. This is the one
sanctioned edit to the verbatim-moved Prepare* bodies: enum literal for
enum literal, numeric value identical, behavior unchanged. App casts at
the single upload boundary (AddTexture call in UploadGfxObjMeshData)
via lifted nullable enum conversion — value- and null-preserving.
Also hardens the MP1a _sideStaged hand-off seam: List -> ConcurrentQueue.
One MeshExtractor is shared by up to MaxParallelLoads (4) decode workers;
the original code enqueued to the thread-safe _stagedMeshData directly,
so the hand-off buffer must be thread-safe too. Drain ordering verified:
side-staged entries enqueue BEFORE the top-level result, preserving the
original mid-Prepare FIFO order.
Verified: grep -i silk on the csproj -> no matches; deps.json has zero
Silk entries; dotnet build 0 errors; full test suite green (4059 passed).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
