Code-quality review followup on Task 2 (becbde6) — addresses I1 (the
forward-looking concern that Tasks 3-9's inner-catch will leave partial
lists visible to callers with no signal) and M1 (silent inner catch).
Changes:
- Parsed gains a trailing `bool TrailerTruncated` field. Both
construction sites pass `false` by default; the trailer try/catch
flips a local `trailerTruncated` to `true` on FormatException and
feeds it into the final return.
- Inner catch logs `pos`/`payload.Length`/exception message under
ACDREAM_DUMP_VITALS=1, mirroring the outer catch's diagnostic
pattern.
- Task 2 test strengthened to assert defaults on Options2 /
SpellbookFilters / HotbarSpells / DesiredComps / GameplayOptions /
Equipped + TrailerTruncated=false (M2 followup — gives Tasks 3-9
a regression guard if they consume into the wrong field).
- New test `TryParse_TrailerAbsent_LessThan8BytesAfterEnchantments_*`
documents the contract that <8 bytes after enchantments means the
trailer is absent (not truncated): TrailerTruncated stays false,
upstream attribute data survives.
- Plan updated in lockstep so Tasks 3-11 implementers see the
`trailerTruncated` local and the new return-arg position.
271/271 AcDream.Core.Net.Tests pass.
46 KiB
Issue #13 — PlayerDescription Trailer Parser Implementation Plan
For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (
- [ ]) syntax for tracking.
Goal: Extend PlayerDescriptionParser past the enchantment block through the full trailer — Options1 / Shortcuts / HotbarSpells / DesiredComps / SpellbookFilters / Options2 / GameplayOptions blob / Inventory / Equipped — and route the parsed Inventory + Equipped lists into ItemRepository so ItemCount > 0 after login.
Architecture: Match holtburger's PlayerDescriptionEventData::unpack (references/holtburger/crates/holtburger-protocol/src/messages/player/events.rs:503-607) structure-for-structure. The trailer reads in a single forward walk except for the gameplay_options blob, which is opaque variable-length and uses a 4-byte-aligned forward heuristic search (find_inventory_start_after_gameplay_options) to locate the inventory-count+GUID-pair that follows it. The trailer parse is wrapped in its own try/catch so a malformed trailer does not lose the attribute/skill/spell/enchantment data already extracted upstream.
Tech Stack: C# 12 / .NET 10, System.Buffers.Binary, xUnit. No new dependencies.
Reference cross-walk:
- Holtburger trailer wire format:
references/holtburger/crates/holtburger-protocol/src/messages/player/events.rs:503-607(theunpackimpl after enchantments). - Holtburger inventory unpacker:
events.rs:143-218(unpack_inventory_and_equipped_strict+find_inventory_start_after_gameplay_options). - Holtburger Shortcut format:
references/holtburger/crates/holtburger-protocol/src/messages/player/shortcuts.rs:13-34(16 bytes: u32 index + Guid (4 B) + u16 spell_id + u16 layer). - Existing parser: src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs.
- Existing wiring: src/AcDream.Core.Net/GameEventWiring.cs:281-398.
ItemInstanceconstructor: object-initializer withObjectId+WeenieClassId(init-only), see src/AcDream.Core/Items/ItemInstance.cs:128.
Acceptance:
- All sections of a synthetic real-world-shaped PlayerDescription parse to completion without nulling out earlier fields.
- New tests cover each trailer section in isolation + a combined end-to-end fixture.
- After a PlayerDescription with non-empty Inventory is dispatched,
ItemRepository.ItemCount > 0. dotnet build+dotnet testgreen.
File Structure
| Path | Action | Responsibility |
|---|---|---|
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs |
Modify | Add CharacterOptionDataFlag enum + Shortcut record + EquippedEntry record + InventoryEntry record, extend Parsed with trailer fields, add trailer reader functions wrapped in their own try/catch. |
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs |
Modify | Add per-section trailer tests + heuristic gameplay_options test + end-to-end full-trailer fixture. |
src/AcDream.Core.Net/GameEventWiring.cs |
Modify | Extend the existing PlayerDescription handler (~line 281) to register each Inventory entry as a stub ItemInstance in ItemRepository. |
tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs (or similar — locate during Task 11) |
Modify or Add | Test: dispatching a PlayerDescription event with inventory entries grows ItemRepository.ItemCount. |
Wire Format (Reference)
After the enchantment block, all little-endian:
u32 option_flags // CharacterOptionDataFlag
u32 options1 // CharacterOptions1 bitfield (opaque uint to us)
if option_flags & SHORTCUT: // 0x01
u32 count
count × Shortcut(16 B) // u32 idx + u32 guid + u16 spell + u16 layer
if option_flags & SPELL_LISTS8: // 0x400
8 × { u32 count, count × u32 spell_id }
else:
u32 count, count × u32 spell_id // single legacy list
if option_flags & DESIRED_COMPS: // 0x08
u16 count, u16 _padding // (4-byte header — count is u16 + u16 ignored)
count × { u32 id, u32 amt }
u32 spellbook_filters // optional — defaults to 0 if no more bytes
if option_flags & CHARACTER_OPTIONS2: // 0x40
u32 options2
if option_flags & GAMEPLAY_OPTIONS: // 0x200
// opaque blob; heuristic find_inventory_start_after_gameplay_options
// walks forward in 4-byte steps from current pos and accepts the first
// candidate that parses inventory+equipped exactly to end-of-buffer.
blob_bytes
inventory + equipped (strict)
else:
inventory + equipped (strict)
// inventory + equipped strict format:
u32 inv_count // <= 10000
inv_count × { u32 guid, u32 weenieType (0..2) } // ContainerType validated
u32 eq_count // <= 10000
eq_count × { u32 guid, u32 loc, u32 prio }
Bite-Sized Tasks
Task 1: Extend Parsed record + add types
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Step 1: Add
CharacterOptionDataFlagenum,Shortcut,InventoryEntry,EquippedEntryrecords inside thePlayerDescriptionParserstatic class (just below the existingEnchantmentMaskenum, ~line 178).
[Flags]
public enum CharacterOptionDataFlag : uint
{
None = 0,
Shortcut = 0x00000001,
SquelchList = 0x00000002,
MultiSpellList = 0x00000004,
DesiredComps = 0x00000008,
ExtendedMultiSpellLists = 0x00000010,
SpellbookFilters = 0x00000020,
CharacterOptions2 = 0x00000040,
TimestampFormat = 0x00000080,
GenericQualitiesData = 0x00000100,
GameplayOptions = 0x00000200,
SpellLists8 = 0x00000400,
}
/// <summary>One shortcut bar entry. 16 bytes wire size.
/// holtburger <c>shortcuts.rs:13-34</c>. Named <c>ShortcutEntry</c>
/// (not <c>Shortcut</c>) to avoid a homograph with the
/// <see cref="CharacterOptionDataFlag.Shortcut"/> flag bit.</summary>
public readonly record struct ShortcutEntry(
uint Index,
uint ObjectGuid,
ushort SpellId,
ushort Layer);
/// <summary>One inventory entry — a guid plus a ContainerType discriminator
/// (0=NonContainer, 1=Container, 2=Foci).</summary>
public readonly record struct InventoryEntry(
uint Guid,
uint ContainerType);
/// <summary>One equipped object entry.</summary>
public readonly record struct EquippedEntry(
uint Guid,
uint EquipLocation,
uint Priority);
- Step 2: Extend the
Parsedrecord. Append new fields afterEnchantments, all defaulting to empty inBuildPartial.
Replace the existing Parsed record (~line 180) with:
public readonly record struct Parsed(
uint WeenieType,
DescriptionPropertyFlag PropertyFlags,
DescriptionVectorFlag VectorFlags,
bool HasHealth,
PropertyBundle Properties,
IReadOnlyDictionary<uint, WorldPosition> Positions,
IReadOnlyList<AttributeEntry> Attributes,
IReadOnlyList<SkillEntry> Skills,
IReadOnlyDictionary<uint, float> Spells,
IReadOnlyList<EnchantmentEntry> Enchantments,
CharacterOptionDataFlag OptionFlags,
uint Options1,
uint Options2,
IReadOnlyList<ShortcutEntry> Shortcuts,
IReadOnlyList<IReadOnlyList<uint>> HotbarSpells,
IReadOnlyList<(uint Id, uint Amount)> DesiredComps,
uint SpellbookFilters,
ReadOnlyMemory<byte> GameplayOptions,
IReadOnlyList<InventoryEntry> Inventory,
IReadOnlyList<EquippedEntry> Equipped,
bool TrailerTruncated);
Code-review followup (added after Task 2 review): the trailing
TrailerTruncatedflag was added to let callers distinguish a clean parse from one where the trailer try/catch swallowed aFormatExceptionmid-section (Tasks 3–9 will make this reachable). All construction sites passTrailerTruncated: falseby default; the trailer try/catch inTryParseflips a local totrueon catch.
- Step 3: Update
BuildPartialto fill the new fields with defaults.
Replace the body of BuildPartial (~line 275) with:
private static Parsed BuildPartial(
uint weenieType, DescriptionPropertyFlag pFlags, DescriptionVectorFlag vFlags,
bool hasHealth, PropertyBundle bundle,
Dictionary<uint, WorldPosition> positions,
List<AttributeEntry> attributes, List<SkillEntry> skills,
Dictionary<uint, float> spells)
{
return new Parsed(weenieType, pFlags, vFlags, hasHealth,
bundle, positions, attributes, skills, spells,
System.Array.Empty<EnchantmentEntry>(),
CharacterOptionDataFlag.None, 0u, 0u,
System.Array.Empty<ShortcutEntry>(),
System.Array.Empty<IReadOnlyList<uint>>(),
System.Array.Empty<(uint, uint)>(),
0u,
ReadOnlyMemory<byte>.Empty,
System.Array.Empty<InventoryEntry>(),
System.Array.Empty<EquippedEntry>(),
TrailerTruncated: false);
}
- Step 4: Update the existing
return new Parsed(...)(~line 261) at the end ofTryParseto also include the new fields with defaults.
Replace it with:
return new Parsed(
weenieType, propertyFlags, vectorFlags, hasHealth,
bundle, positions, attributes, skills, spells, enchantments,
CharacterOptionDataFlag.None, 0u, 0u,
System.Array.Empty<ShortcutEntry>(),
System.Array.Empty<IReadOnlyList<uint>>(),
System.Array.Empty<(uint, uint)>(),
0u,
ReadOnlyMemory<byte>.Empty,
System.Array.Empty<InventoryEntry>(),
System.Array.Empty<EquippedEntry>(),
TrailerTruncated: false);
- Step 5: Run the build + existing tests to verify no regressions.
Run: dotnet build and dotnet test --filter "FullyQualifiedName~PlayerDescriptionParserTests"
Expected: GREEN — all 5 existing tests still pass.
- Step 6: Commit.
git add src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs
git commit -m "feat(net): #13 scaffold trailer fields on PlayerDescriptionParser.Parsed
No behavior change yet — adds CharacterOptionDataFlag, Shortcut/Inventory/
EquippedEntry records, and extends Parsed with trailer fields filled with
empty defaults. Sets up the per-section TDD walk in subsequent commits."
Task 2: Read OptionFlags + Options1 (8 bytes after enchantments)
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write the failing test. Append to
PlayerDescriptionParserTests.cs:
[Fact]
public void TryParse_TrailerOptionFlagsAndOptions1_AreReadAfterEnchantments()
{
// ATTRIBUTE | ENCHANTMENT vector flag; empty enchantment mask (0).
// After mask, trailer adds u32 option_flags + u32 options1.
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // EnchantmentMask = empty
// Trailer header: option_flags + options1
writer.Write(0u); // option_flags = None — no further sections
writer.Write(0xDEADBEEFu); // options1 sentinel
// No more bytes — spellbook_filters is optional (defaults to 0).
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Equal(PlayerDescriptionParser.CharacterOptionDataFlag.None, parsed!.Value.OptionFlags);
Assert.Equal(0xDEADBEEFu, parsed.Value.Options1);
Assert.Empty(parsed.Value.Shortcuts);
Assert.Empty(parsed.Value.Inventory);
}
- Step 2: Run the test — expect FAIL (
OptionFlagsstill defaultNone,Options1still 0).
Run: dotnet test --filter "FullyQualifiedName~TryParse_TrailerOptionFlagsAndOptions1"
Expected: FAIL — assertion 0xDEADBEEFu != 0u for Options1.
- Step 3: Implement. In
PlayerDescriptionParser.TryParse, after the existing enchantments-read block (~line 259, just before the existingreturn new Parsed(...)), insert a trailer-walk block. Replace lines 258-273 (the enchantment read + final return) with:
// ── Enchantments (Issue #7 / #12) ───────────────────────────────
if (vectorFlags.HasFlag(DescriptionVectorFlag.Enchantment))
ReadEnchantmentBlock(payload, ref pos, enchantments);
// ── Trailer (Issue #13): options + shortcuts + hotbars + inventory ──
// Wrapped in its own try/catch — a malformed trailer must not destroy
// the attribute / skill / spell / enchantment data we already extracted.
CharacterOptionDataFlag optionFlags = CharacterOptionDataFlag.None;
uint options1 = 0;
uint options2 = 0;
uint spellbookFilters = 0;
List<ShortcutEntry> shortcuts = new();
List<IReadOnlyList<uint>> hotbarSpells = new();
List<(uint, uint)> desiredComps = new();
ReadOnlyMemory<byte> gameplayOptions = ReadOnlyMemory<byte>.Empty;
List<InventoryEntry> inventory = new();
List<EquippedEntry> equipped = new();
bool trailerTruncated = false;
try
{
if (payload.Length - pos >= 8)
{
optionFlags = (CharacterOptionDataFlag)ReadU32(payload, ref pos);
options1 = ReadU32(payload, ref pos);
}
}
catch (FormatException ex)
{
// Trailer corrupted — keep what we have and flag it. Tasks 3-9
// can leave partial lists in scope; TrailerTruncated lets callers
// ignore the trailer when they need all-or-nothing semantics.
trailerTruncated = true;
if (System.Environment.GetEnvironmentVariable("ACDREAM_DUMP_VITALS") == "1")
System.Console.WriteLine($"PlayerDescriptionParser: trailer FormatException at pos={pos}/{payload.Length}: {ex.Message}");
}
return new Parsed(
weenieType, propertyFlags, vectorFlags, hasHealth,
bundle, positions, attributes, skills, spells, enchantments,
optionFlags, options1, options2,
shortcuts, hotbarSpells, desiredComps, spellbookFilters,
gameplayOptions, inventory, equipped, trailerTruncated);
Tasks 3–9 note: every
return new Parsed(...)extension or rewrite in subsequent tasks must includetrailerTruncatedas the final positional argument, and any new try-blocks that read trailer sections should settrailerTruncated = true;in their catch.
- Step 4: Run the test — expect PASS.
Run: dotnet test --filter "FullyQualifiedName~TryParse_TrailerOptionFlagsAndOptions1"
Expected: PASS.
- Step 5: Run full PlayerDescription test suite to confirm no regressions.
Run: dotnet test --filter "FullyQualifiedName~PlayerDescriptionParserTests"
Expected: 6 tests pass (5 original + 1 new).
- Step 6: Commit.
git add src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs
git commit -m "feat(net): #13 read OptionFlags + Options1 after enchantments
First step of the PD trailer walk. Wraps trailer reads in their own
try/catch so a malformed trailer does not null out the upstream
attribute/skill/spell/enchantment data."
Task 3: Read Shortcuts list (gated on SHORTCUT bit)
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write the failing test.
[Fact]
public void TryParse_TrailerShortcuts_PopulatesList()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
writer.Write(0x01u); // option_flags = SHORTCUT
writer.Write(0xCAFEu); // options1 sentinel
// Shortcut count + 2 entries (16 B each).
writer.Write(2u);
writer.Write(0u); writer.Write(0xAABBCCDDu); writer.Write((ushort)0); writer.Write((ushort)0);
writer.Write(7u); writer.Write(0u); writer.Write((ushort)1234); writer.Write((ushort)5);
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Equal(2, parsed!.Value.Shortcuts.Count);
Assert.Equal(0u, parsed.Value.Shortcuts[0].Index);
Assert.Equal(0xAABBCCDDu, parsed.Value.Shortcuts[0].ObjectGuid);
Assert.Equal((ushort)0, parsed.Value.Shortcuts[0].SpellId);
Assert.Equal(7u, parsed.Value.Shortcuts[1].Index);
Assert.Equal((ushort)1234, parsed.Value.Shortcuts[1].SpellId);
Assert.Equal((ushort)5, parsed.Value.Shortcuts[1].Layer);
}
-
Step 2: Run the test — expect FAIL (
Shortcutsstill empty). -
Step 3: Implement. Inside the trailer try-block, after the
options1 = ReadU32(...)line, append:
if (optionFlags.HasFlag(CharacterOptionDataFlag.Shortcut))
{
uint count = ReadU32(payload, ref pos);
if (count > 10_000) throw new FormatException("unreasonable shortcut count");
for (uint i = 0; i < count; i++)
{
uint idx = ReadU32(payload, ref pos);
uint guid = ReadU32(payload, ref pos);
ushort spellId = ReadU16(payload, ref pos);
ushort layer = ReadU16(payload, ref pos);
shortcuts.Add(new ShortcutEntry(idx, guid, spellId, layer));
}
}
-
Step 4: Run the test — expect PASS.
-
Step 5: Run full suite — expect green.
-
Step 6: Commit.
git add -u
git commit -m "feat(net): #13 read shortcuts list (SHORTCUT bit) in PD trailer"
Task 4: Read HotbarSpells with SPELL_LISTS8 path
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write the failing test.
[Fact]
public void TryParse_TrailerHotbarSpells_SpellLists8_Reads8Lists()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
writer.Write(0x400u); // option_flags = SPELL_LISTS8
writer.Write(0u); // options1
// 8 hotbars: counts {2,1,0,0,0,0,0,3} — first list has 2 spells, second has 1, last has 3.
writer.Write(2u); writer.Write(11u); writer.Write(12u);
writer.Write(1u); writer.Write(21u);
writer.Write(0u);
writer.Write(0u);
writer.Write(0u);
writer.Write(0u);
writer.Write(0u);
writer.Write(3u); writer.Write(81u); writer.Write(82u); writer.Write(83u);
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Equal(8, parsed!.Value.HotbarSpells.Count);
Assert.Equal(new uint[] { 11u, 12u }, parsed.Value.HotbarSpells[0]);
Assert.Equal(new uint[] { 21u }, parsed.Value.HotbarSpells[1]);
Assert.Empty(parsed.Value.HotbarSpells[2]);
Assert.Equal(new uint[] { 81u, 82u, 83u }, parsed.Value.HotbarSpells[7]);
}
-
Step 2: Run the test — expect FAIL.
-
Step 3: Implement. After the shortcuts block in the trailer try-block, append:
if (optionFlags.HasFlag(CharacterOptionDataFlag.SpellLists8))
{
for (int b = 0; b < 8; b++)
{
uint count = ReadU32(payload, ref pos);
if (count > 10_000) throw new FormatException("unreasonable hotbar count");
var list = new List<uint>((int)count);
for (uint i = 0; i < count; i++)
list.Add(ReadU32(payload, ref pos));
hotbarSpells.Add(list);
}
}
else if (payload.Length - pos >= 4)
{
// Legacy single-list fallback (holtburger events.rs:544-556).
uint count = ReadU32(payload, ref pos);
if (count > 10_000) throw new FormatException("unreasonable hotbar count");
var list = new List<uint>((int)count);
for (uint i = 0; i < count; i++)
list.Add(ReadU32(payload, ref pos));
hotbarSpells.Add(list);
}
-
Step 4: Run the test — expect PASS.
-
Step 5: Add a second test for the legacy single-list path.
[Fact]
public void TryParse_TrailerHotbarSpells_NoSpellLists8_ReadsSingleLegacyList()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
writer.Write(0u); // option_flags = None (no SPELL_LISTS8)
writer.Write(0u); // options1
// Legacy single hotbar list: count=2, two spells.
writer.Write(2u); writer.Write(101u); writer.Write(102u);
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Single(parsed!.Value.HotbarSpells);
Assert.Equal(new uint[] { 101u, 102u }, parsed.Value.HotbarSpells[0]);
}
-
Step 6: Run both hotbar tests — expect PASS. Then full suite green.
-
Step 7: Commit.
git add -u
git commit -m "feat(net): #13 read hotbar spells (SPELL_LISTS8 + legacy path)"
Task 5: Read DesiredComps list
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write the failing test.
[Fact]
public void TryParse_TrailerDesiredComps_ReadsIdAmtPairs()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
// option_flags = DESIRED_COMPS (0x08); no SPELL_LISTS8 so legacy hotbar list (count=0).
writer.Write(0x08u);
writer.Write(0u); // options1
// Legacy hotbar list: count=0
writer.Write(0u);
// DESIRED_COMPS: u16 count=2, u16 padding, then 2 (id,amt) pairs of 8 bytes each.
writer.Write((ushort)2);
writer.Write((ushort)0);
writer.Write(0xAAu); writer.Write(50u);
writer.Write(0xBBu); writer.Write(75u);
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Equal(2, parsed!.Value.DesiredComps.Count);
Assert.Equal((0xAAu, 50u), parsed.Value.DesiredComps[0]);
Assert.Equal((0xBBu, 75u), parsed.Value.DesiredComps[1]);
}
-
Step 2: Run — expect FAIL.
-
Step 3: Implement. After the hotbar block in the trailer try-block, append:
if (optionFlags.HasFlag(CharacterOptionDataFlag.DesiredComps))
{
// holtburger events.rs:558-574 — u16 count + u16 padding (4-byte header).
if (payload.Length - pos < 4) throw new FormatException("truncated desired_comps header");
ushort count = ReadU16(payload, ref pos);
ReadU16(payload, ref pos); // padding/buckets — discarded
if (count > 10_000) throw new FormatException("unreasonable desired_comps count");
for (int i = 0; i < count; i++)
{
uint id = ReadU32(payload, ref pos);
uint amt = ReadU32(payload, ref pos);
desiredComps.Add((id, amt));
}
}
-
Step 4: Run — expect PASS. Run full suite green.
-
Step 5: Commit.
git add -u
git commit -m "feat(net): #13 read desired_comps list in PD trailer"
Task 6: Read SpellbookFilters (optional u32, defaults to 0)
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write the failing test.
[Fact]
public void TryParse_TrailerSpellbookFilters_ReadOptionalU32()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
writer.Write(0u); // option_flags = None
writer.Write(0u); // options1
// Legacy hotbar list: count=0
writer.Write(0u);
// spellbook_filters sentinel.
writer.Write(0xF00DBA42u);
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Equal(0xF00DBA42u, parsed!.Value.SpellbookFilters);
}
-
Step 2: Run — expect FAIL (defaults to 0).
-
Step 3: Implement. After the desired_comps block in the trailer try-block, append:
// holtburger events.rs:576-582 — spellbook_filters is optional; defaults
// to 0 if EOF.
if (payload.Length - pos >= 4)
spellbookFilters = ReadU32(payload, ref pos);
-
Step 4: Run — expect PASS. Run full suite green.
-
Step 5: Commit.
git add -u
git commit -m "feat(net): #13 read optional spellbook_filters u32"
Task 7: Read Options2 (gated on CHARACTER_OPTIONS2 bit)
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write the failing test.
[Fact]
public void TryParse_TrailerOptions2_GatedOnCharacterOptions2Bit()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
// option_flags = CHARACTER_OPTIONS2 (0x40)
writer.Write(0x40u);
writer.Write(0u); // options1
// Legacy hotbar list: count=0.
writer.Write(0u);
// spellbook_filters
writer.Write(0u);
// options2 sentinel
writer.Write(0xC0FFEE01u);
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Equal(0xC0FFEE01u, parsed!.Value.Options2);
}
-
Step 2: Run — expect FAIL.
-
Step 3: Implement. After the spellbook_filters block, append:
if (optionFlags.HasFlag(CharacterOptionDataFlag.CharacterOptions2))
options2 = ReadU32(payload, ref pos);
-
Step 4: Run — expect PASS. Run full suite green.
-
Step 5: Commit.
git add -u
git commit -m "feat(net): #13 read options2 gated on CHARACTER_OPTIONS2 flag"
Task 8: Strict Inventory + Equipped reader (no GAMEPLAY_OPTIONS path)
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write the failing test.
[Fact]
public void TryParse_TrailerInventoryEquippedStrict_NoGameplayOptionsBit()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
writer.Write(0u); // option_flags = None — no GAMEPLAY_OPTIONS
writer.Write(0u); // options1
writer.Write(0u); // legacy hotbar list count=0
writer.Write(0u); // spellbook_filters
// Inventory: 2 entries
writer.Write(2u);
writer.Write(0x500000A0u); writer.Write(0u); // NonContainer
writer.Write(0x500000A1u); writer.Write(1u); // Container
// Equipped: 1 entry
writer.Write(1u);
writer.Write(0x500000B0u); writer.Write(0x00000200u); writer.Write(1u); // ChestArmor, prio=1
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Equal(2, parsed!.Value.Inventory.Count);
Assert.Equal(0x500000A0u, parsed.Value.Inventory[0].Guid);
Assert.Equal(0u, parsed.Value.Inventory[0].ContainerType);
Assert.Equal(1u, parsed.Value.Inventory[1].ContainerType);
Assert.Single(parsed.Value.Equipped);
Assert.Equal(0x500000B0u, parsed.Value.Equipped[0].Guid);
Assert.Equal(0x00000200u, parsed.Value.Equipped[0].EquipLocation);
Assert.Equal(1u, parsed.Value.Equipped[0].Priority);
}
-
Step 2: Run — expect FAIL.
-
Step 3: Implement. Add a new helper method
TryUnpackInventoryStrictnear the bottom of the class, just above the primitive readers (~line 545):
/// <summary>Strict inventory + equipped block reader. Returns true if
/// the bytes from <paramref name="pos"/> parse cleanly per holtburger
/// events.rs:143-193 (<c>unpack_inventory_and_equipped_strict</c>).
/// Counts capped at 10,000; inventory ContainerType must be 0..2
/// (NonContainer / Container / Foci).</summary>
private static bool TryUnpackInventoryStrict(
ReadOnlySpan<byte> src, ref int pos,
List<InventoryEntry> inventory, List<EquippedEntry> equipped)
{
inventory.Clear();
equipped.Clear();
if (pos + 4 > src.Length) return false;
uint invCount = BinaryPrimitives.ReadUInt32LittleEndian(src.Slice(pos));
pos += 4;
if (invCount > 10_000) return false;
for (uint i = 0; i < invCount; i++)
{
if (pos + 8 > src.Length) return false;
uint guid = BinaryPrimitives.ReadUInt32LittleEndian(src.Slice(pos));
uint wtype = BinaryPrimitives.ReadUInt32LittleEndian(src.Slice(pos + 4));
pos += 8;
if (wtype > 2) return false;
inventory.Add(new InventoryEntry(guid, wtype));
}
if (pos + 4 > src.Length) return false;
uint eqCount = BinaryPrimitives.ReadUInt32LittleEndian(src.Slice(pos));
pos += 4;
if (eqCount > 10_000) return false;
for (uint i = 0; i < eqCount; i++)
{
if (pos + 12 > src.Length) return false;
uint guid = BinaryPrimitives.ReadUInt32LittleEndian(src.Slice(pos));
uint loc = BinaryPrimitives.ReadUInt32LittleEndian(src.Slice(pos + 4));
uint prio = BinaryPrimitives.ReadUInt32LittleEndian(src.Slice(pos + 8));
pos += 12;
equipped.Add(new EquippedEntry(guid, loc, prio));
}
return true;
}
After the options2 read in the trailer try-block, append:
if (!optionFlags.HasFlag(CharacterOptionDataFlag.GameplayOptions))
{
// Strict path: inventory + equipped follow directly.
TryUnpackInventoryStrict(payload, ref pos, inventory, equipped);
}
-
Step 4: Run — expect PASS. Run full suite green.
-
Step 5: Commit.
git add -u
git commit -m "feat(net): #13 strict inventory+equipped reader (no GAMEPLAY_OPTIONS)"
Task 9: Heuristic GAMEPLAY_OPTIONS path
Files:
-
Modify:
src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs -
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write the failing test.
[Fact]
public void TryParse_TrailerGameplayOptions_HeuristicLocatesInventoryStart()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
// option_flags = GAMEPLAY_OPTIONS (0x200)
writer.Write(0x200u);
writer.Write(0u); // options1
writer.Write(0u); // legacy hotbar count=0
writer.Write(0u); // spellbook_filters
// 16 bytes of opaque gameplay_options blob — values that *almost* look
// like an inventory header but fail validation (wtype > 2 or count too
// big), forcing the heuristic to walk past them.
writer.Write(0xDEADBEEFu); // looks like inv_count = 0xDEADBEEF (> 10_000) — rejected
writer.Write(0xCAFEBABEu);
writer.Write(0x12345678u);
writer.Write(0x87654321u);
// Real inventory: 1 entry, then equipped: 1 entry — must consume to EOF.
writer.Write(1u);
writer.Write(0x50000200u); writer.Write(0u);
writer.Write(1u);
writer.Write(0x50000300u); writer.Write(0x00000200u); writer.Write(1u);
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
Assert.Single(parsed!.Value.Inventory);
Assert.Equal(0x50000200u, parsed.Value.Inventory[0].Guid);
Assert.Single(parsed.Value.Equipped);
Assert.Equal(0x50000300u, parsed.Value.Equipped[0].Guid);
Assert.Equal(16, parsed.Value.GameplayOptions.Length);
}
-
Step 2: Run — expect FAIL.
-
Step 3: Implement. Add a new helper method just below
TryUnpackInventoryStrict:
/// <summary>4-byte-aligned forward scan from <paramref name="start"/>
/// looking for the first offset where <c>TryUnpackInventoryStrict</c>
/// consumes exactly to end-of-buffer. Mirrors holtburger
/// <c>find_inventory_start_after_gameplay_options</c> in events.rs:195-218.</summary>
private static bool TryHeuristicInventoryStart(
ReadOnlySpan<byte> src, int start,
out int invStart, out int end,
List<InventoryEntry> inventory, List<EquippedEntry> equipped)
{
invStart = end = 0;
inventory.Clear();
equipped.Clear();
if (start + 8 > src.Length) return false;
int candidate = start;
int misalign = candidate & 3;
if (misalign != 0) candidate += 4 - misalign;
int last = src.Length - 8;
while (candidate <= last)
{
int tmp = candidate;
var tmpInv = new List<InventoryEntry>();
var tmpEq = new List<EquippedEntry>();
if (TryUnpackInventoryStrict(src, ref tmp, tmpInv, tmpEq) && tmp == src.Length)
{
invStart = candidate;
end = tmp;
inventory.AddRange(tmpInv);
equipped.AddRange(tmpEq);
return true;
}
candidate += 4;
}
return false;
}
In the trailer try-block, replace the strict-only branch from Task 8 with the full conditional:
if (optionFlags.HasFlag(CharacterOptionDataFlag.GameplayOptions))
{
int gameplayStart = pos;
if (TryHeuristicInventoryStart(payload, gameplayStart, out int invStart, out int end,
inventory, equipped))
{
gameplayOptions = payload.Slice(gameplayStart, invStart - gameplayStart).ToArray();
pos = end;
}
}
else
{
TryUnpackInventoryStrict(payload, ref pos, inventory, equipped);
}
Note: payload.Slice(...) returns a ReadOnlySpan<byte> — we capture into byte[] then store as ReadOnlyMemory<byte> (the field type). The .ToArray() allocation is acceptable (one per PD = once per session).
-
Step 4: Run — expect PASS. Run full suite green.
-
Step 5: Commit.
git add -u
git commit -m "feat(net): #13 heuristic inventory locator after gameplay_options blob"
Task 10: Combined end-to-end fixture test
Files:
-
Modify:
tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs -
Step 1: Write a single test that exercises every section together — a real-shaped fixture.
[Fact]
public void TryParse_FullTrailer_AllSectionsPopulated()
{
var sb = new MemoryStream();
using var writer = new BinaryWriter(sb);
writer.Write(0u); // propertyFlags
writer.Write(0x52u); // weenieType
writer.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
writer.Write(1u); // has_health
writer.Write(0u); // empty attribute_flags
writer.Write(0u); // empty enchantment mask
// option_flags = SHORTCUT | DESIRED_COMPS | CHARACTER_OPTIONS2 | SPELL_LISTS8
// = 0x01 | 0x08 | 0x40 | 0x400 = 0x449
writer.Write(0x449u);
writer.Write(0xAA000001u); // options1
// Shortcuts: count=1
writer.Write(1u);
writer.Write(3u); writer.Write(0xCAFEFACEu); writer.Write((ushort)100); writer.Write((ushort)2);
// 8 hotbars, all empty for brevity.
for (int i = 0; i < 8; i++) writer.Write(0u);
// Desired comps: count=1
writer.Write((ushort)1); writer.Write((ushort)0);
writer.Write(0xC1u); writer.Write(99u);
// spellbook_filters
writer.Write(0xF11Du);
// options2
writer.Write(0xBB000002u);
// Inventory + equipped (no GAMEPLAY_OPTIONS, strict path)
writer.Write(1u);
writer.Write(0x50000400u); writer.Write(0u);
writer.Write(1u);
writer.Write(0x50000500u); writer.Write(0x00000200u); writer.Write(1u);
var parsed = PlayerDescriptionParser.TryParse(sb.ToArray());
Assert.NotNull(parsed);
var v = parsed!.Value;
Assert.Equal(0xAA000001u, v.Options1);
Assert.Equal(0xBB000002u, v.Options2);
Assert.Equal(0xF11Du, v.SpellbookFilters);
Assert.Single(v.Shortcuts);
Assert.Equal(0xCAFEFACEu, v.Shortcuts[0].ObjectGuid);
Assert.Equal(8, v.HotbarSpells.Count);
Assert.All(v.HotbarSpells, l => Assert.Empty(l));
Assert.Single(v.DesiredComps);
Assert.Equal((0xC1u, 99u), v.DesiredComps[0]);
Assert.Single(v.Inventory);
Assert.Equal(0x50000400u, v.Inventory[0].Guid);
Assert.Single(v.Equipped);
Assert.Equal(0x50000500u, v.Equipped[0].Guid);
}
-
Step 2: Run — expect PASS (no implementation change needed; this exercises the cumulative behavior).
-
Step 3: Run full suite green.
-
Step 4: Commit.
git add -u
git commit -m "test(net): #13 end-to-end PD trailer fixture covering every section"
Task 11: Wire Inventory into ItemRepository in GameEventWiring
Files:
-
Modify:
src/AcDream.Core.Net/GameEventWiring.cs -
Test: locate the test file referenced by the issue text —
tests/AcDream.Core.Net.Tests/GameEventWiringTests.csif it exists, otherwise add a new file. -
Step 1: Locate (or create) the wiring test file.
ls tests/AcDream.Core.Net.Tests/GameEventWiring*
If absent, create tests/AcDream.Core.Net.Tests/GameEventWiringInventoryTests.cs.
- Step 2: Write the failing test.
In tests/AcDream.Core.Net.Tests/GameEventWiringInventoryTests.cs:
using System;
using System.IO;
using AcDream.Core.Chat;
using AcDream.Core.Combat;
using AcDream.Core.Items;
using AcDream.Core.Net;
using AcDream.Core.Net.Messages;
using AcDream.Core.Spells;
namespace AcDream.Core.Net.Tests;
public sealed class GameEventWiringInventoryTests
{
[Fact]
public void PlayerDescription_RegistersInventoryEntries_InItemRepository()
{
var dispatcher = new GameEventDispatcher();
var items = new ItemRepository();
var combat = new CombatLog();
var spellbook = new Spellbook();
var chat = new ChatLog();
GameEventWiring.WireAll(dispatcher, items, combat, spellbook, chat);
// Build a minimal PlayerDescription with inventory: 2 entries.
var sb = new MemoryStream();
using var w = new BinaryWriter(sb);
w.Write(0u); // propertyFlags
w.Write(0x52u); // weenieType
w.Write(0x201u); // ATTRIBUTE | ENCHANTMENT
w.Write(1u); // has_health
w.Write(0u); // empty attribute_flags
w.Write(0u); // empty enchantment mask
w.Write(0u); // option_flags = None
w.Write(0u); // options1
w.Write(0u); // legacy hotbar count
w.Write(0u); // spellbook_filters
// Inventory: 2 entries, then 0 equipped.
w.Write(2u);
w.Write(0x50000A01u); w.Write(0u);
w.Write(0x50000A02u); w.Write(1u);
w.Write(0u);
// Construct an envelope-stripped GameEvent payload.
var evt = new GameEvent(GameEventType.PlayerDescription, sb.ToArray(), Sequence: 1);
Assert.Equal(0, items.ItemCount);
dispatcher.Dispatch(evt);
Assert.Equal(2, items.ItemCount);
Assert.NotNull(items.GetItem(0x50000A01u));
Assert.NotNull(items.GetItem(0x50000A02u));
}
}
Note for the executor: the constructor signature for
GameEventWiring.WireAllmay use named optional parameters (localPlayer,turbineChat, etc.). Inspect src/AcDream.Core.Net/GameEventWiring.cs:39-65 and pass only the non-optional positional args. The exactGameEventconstructor name + arg order is in src/AcDream.Core.Net/Messages/GameEvent.cs — adjust if the project usesPayload:instead of positional. IfSpellbookhas a different constructor (e.g. requires aWorld), use the existing test pattern fromGameEventWiringTestsif one exists, or passnull!-style defaults.
-
Step 3: Run — expect FAIL (current handler does not touch ItemRepository for trailer inventory).
-
Step 4: Implement. In
src/AcDream.Core.Net/GameEventWiring.cs, inside the existingdispatcher.Register(GameEventType.PlayerDescription, e => { ... })lambda (right before its closing});at line ~398), append:
// Issue #13 — register inventory entries with ItemRepository so panels
// (inventory, paperdoll, hotbars) light up after login. Equipped entries
// share the same ObjectId as inventory entries (an equipped item is
// also in inventory) — register both, but the equipped record carries
// the slot mask which we surface via MoveItem so paperdoll can render.
foreach (var inv in p.Value.Inventory)
{
if (items.GetItem(inv.Guid) is null)
{
items.AddOrUpdate(new ItemInstance
{
ObjectId = inv.Guid,
WeenieClassId = inv.ContainerType,
});
}
}
foreach (var eq in p.Value.Equipped)
{
if (items.GetItem(eq.Guid) is null)
{
items.AddOrUpdate(new ItemInstance
{
ObjectId = eq.Guid,
WeenieClassId = 0,
});
}
// Reflect the equip slot — paperdoll uses CurrentlyEquippedLocation.
items.MoveItem(
itemId: eq.Guid,
newContainerId: 0,
newSlot: -1,
newEquipLocation: (EquipMask)eq.EquipLocation);
}
If ItemInstance or EquipMask is not already imported in this file, add the using directives:
using AcDream.Core.Items;
-
Step 5: Run — expect PASS. Run full suite green.
-
Step 6: Commit.
git add -u
git commit -m "feat(net): #13 register PD trailer inventory+equipped in ItemRepository"
Task 12: Update issue tracker + close
Files:
-
Modify:
docs/ISSUES.md -
Step 1: Move #13 from OPEN to "Recently closed". In
docs/ISSUES.md, locate the## #13block (line ~1382) and theRecently closedsection. Move the block, update its status header to**Status:** DONE+ add a**Closed:** 2026-05-10+**Commit:** <hash>line. The fix-summary should note: full trailer walked; ItemRepository registration of inventory + equipped wired; tests added. -
Step 2: Run the full test suite one final time.
dotnet test
Expected: full green, no regressions.
- Step 3: Commit.
git add docs/ISSUES.md
git commit -m "docs: close ISSUES.md #13 — PD trailer parser shipped"
Self-Review Checklist (executed by plan author)
-
Spec coverage: Every section in the issue text is covered by a task — Options1 (Task 2), Shortcuts (Task 3), Hotbars (Task 4), DesiredComps (Task 5), SpellbookFilters (Task 6), Options2 (Task 7), strict inventory (Task 8), GameplayOptions blob + heuristic (Task 9), GameEventWiring routing (Task 11). PASS.
-
Placeholder scan: All test bodies + implementation snippets are complete C#. The one note in Task 11 about constructor signatures is annotated as a verification hint, not a placeholder. PASS.
-
Type consistency:
InventoryEntry(Guid, ContainerType)andEquippedEntry(Guid, EquipLocation, Priority)are introduced in Task 1 and used consistently in Tasks 8/9/11.Shortcut(Index, ObjectGuid, SpellId, Layer)matches in Tasks 1+3.CharacterOptionDataFlagfield names (Shortcut,DesiredComps,CharacterOptions2,GameplayOptions,SpellLists8) match the holtburger bit names. PASS. -
Edge cases addressed: Trailer try/catch isolates trailer corruption from upstream data (Task 2); count caps at 10,000 prevent attacker-controlled allocation (Tasks 3, 5, 8); 4-byte alignment in heuristic search (Task 9); legacy single-list hotbar fallback (Task 4 step 5). PASS.
Execution Handoff
Plan complete and saved to docs/superpowers/plans/2026-05-10-issue-13-pd-trailer.md. Two execution options:
- Subagent-Driven (recommended) — Dispatch a fresh subagent per task, review between tasks, fast iteration. Sonnet is correct per project subagent policy.
- Inline Execution — Execute tasks in this session using
superpowers:executing-plans, batch execution with checkpoints for review.
Which approach?