docs: close ISSUES.md #13 — PD trailer parser shipped

Issue #13 closed in `078919c`. Full trailer (Options1 / Shortcuts /
HotbarSpells / DesiredComps / SpellbookFilters / Options2 /
GameplayOptions blob / Inventory / Equipped) now walked by
PlayerDescriptionParser. ItemRepository wired up to receive parsed
Inventory + Equipped at login. 20 PD parser tests + 1 wiring
acceptance test. 282/282 Net.Tests pass.

Plan archived at
docs/superpowers/plans/2026-05-10-issue-13-pd-trailer.md.
This commit is contained in:
Erik 2026-05-10 09:47:51 +02:00
parent 078919cc18
commit 95aaa6c92e

View file

@ -1377,29 +1377,6 @@ one live creature case no longer use the single-cylinder fallback.
--- ---
---
## #13 — PlayerDescription trailer past enchantments (options / shortcuts / hotbars / desired_comps / spellbook_filters / options2 / gameplay_options / inventory / equipped)
**Status:** OPEN
**Severity:** LOW (no current user-visible bug; future panels will need the data)
**Filed:** 2026-04-25
**Component:** net / player-state
**Description:** `PlayerDescriptionParser` walks through enchantments (Phase H, 2026-04-25). The trailer beyond that — Options1 / Shortcuts / HotbarSpells (8 lists) / DesiredComps / SpellbookFilters / Options2 / GameplayOptions blob / Inventory / Equipped — is not yet parsed. Required for future Spellbook UI panel, hotbar UI, inventory UI, character options panel.
**Root cause / status:** Holtburger `events.rs:462-625` has the full layout. The trickiest piece is `gameplay_options` — a variable-length opaque blob; holtburger uses a heuristic forward search (`find_inventory_start_after_gameplay_options`) for plausibly-aligned inventory-count + GUID pairs to find the inventory start. Other sections are well-formed.
**Files:**
- `src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs` — extend `Parsed` record + walker.
- `tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs` — add fixtures per section.
- `src/AcDream.Core.Net/GameEventWiring.cs` — route `parsed.Inventory` + `Equipped` to ItemRepository.
**Research:** holtburger `events.rs:462-625`; `references/actestclient/TestClient/messages.xml`.
**Acceptance:** All sections of a real-world PlayerDescription parse to completion (no truncation). New tests cover synthetic fixtures per section. `ItemRepository.Count` after login > 0.
---
--- ---
@ -1700,6 +1677,62 @@ Unverified. The likely culprits, ranked by suspected probability:
# Recently closed # Recently closed
## #13 — [DONE 2026-05-10 · d3b58c9..078919c] PlayerDescription trailer past enchantments
**Closed:** 2026-05-10
**Commits:** `d3b58c9` (scaffold) → `6587034` (rename nit) → `becbde6` (OptionFlags+Options1) → `9a0dfe0` (TrailerTruncated + diag) → `f7a5eea` (Shortcuts) → `8cbb991` (HotbarSpells) → `75e8e26` (DesiredComps) → `b17dc3b` (SpellbookFilters) → `98eebef` (Options2) → `d9a5e40` (strict Inventory+Equipped) → `91693ea` (heuristic GAMEPLAY_OPTIONS walker) → `58095d8` (combined fixture test) → `078919c` (ItemRepository wiring)
**Component:** net / player-state
**Plan:** [`docs/superpowers/plans/2026-05-10-issue-13-pd-trailer.md`](../docs/superpowers/plans/2026-05-10-issue-13-pd-trailer.md)
**Resolution.** `PlayerDescriptionParser` now walks every trailer
section through Inventory + Equipped, ported faithfully from holtburger
`events.rs:503-625` + `shortcuts.rs:13-34`. The trickiest piece —
`gameplay_options` — uses a 4-byte-aligned forward heuristic
(`TryHeuristicInventoryStart`) that probes candidate offsets with a
strict `(inventory + equipped consume to EOF)` test, mirroring
holtburger's `find_inventory_start_after_gameplay_options`.
The trailer walk is wrapped in its own inner try/catch (separate from
the outer parse-wide catch) so a malformed trailer cannot destroy the
already-extracted attribute / skill / spell / enchantment data. A new
`Parsed.TrailerTruncated` flag lets callers distinguish a clean parse
from a graceful-degradation parse (set true if the inner catch fires;
log under `ACDREAM_DUMP_VITALS=1`).
`GameEventWiring`'s `PlayerDescription` handler now registers each
inventory entry with `ItemRepository.AddOrUpdate(...)` and applies
`MoveItem(...)` for equipped entries so paperdoll picks up
`CurrentlyEquippedLocation` at login. The acceptance criterion
"`ItemRepository.Count` after login > 0" is now exercised by
`PlayerDescription_RegistersInventoryEntries_InItemRepository` in
`GameEventWiringTests`.
12 tasks, 13 commits, +9 PD parser tests + 1 wiring test (20 PD tests
total, 282 Net.Tests pass). Code-review nits during the run produced
two refactor commits: `Shortcut → ShortcutEntry` rename to avoid a
homograph with the `CharacterOptionDataFlag.Shortcut` flag bit
(`6587034`); `TrailerTruncated` flag + diagnostic logging
(`9a0dfe0`).
Forward-looking notes (low priority, no follow-up issues filed):
- `WeenieClassId = inv.ContainerType` for inventory entries is a
placeholder; `CreateObject` overwrites it with the real weenie class
later in the login sequence.
- The 10,000 count cap throws `FormatException` on validation failure,
which the inner catch treats the same as truncation. If a future
diagnostic UI needs to distinguish "EOF mid-section" from "garbage
count rejected", split `TrailerTruncated` into two flags. For now
the `ACDREAM_DUMP_VITALS=1` log message gives the developer enough
signal.
Files: `src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs`,
`src/AcDream.Core.Net/GameEventWiring.cs`,
`tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs`,
`tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs`.
---
## #51 — [DONE 2026-05-09 · da56063 + N.5b SHIP] WB's terrain-split formula diverges from retail's `FSplitNESW` ## #51 — [DONE 2026-05-09 · da56063 + N.5b SHIP] WB's terrain-split formula diverges from retail's `FSplitNESW`
**Closed:** 2026-05-09 **Closed:** 2026-05-09