docs(D.5.4): client object/item data model design (brainstorm spec)

Two guid-keyed tables (retail shape), CreateObject = canonical merge-upsert
for the data table (ACCWeenieObject-equivalent holding ALL server objects),
container membership index, retire _liveEntityInfoByGuid + EnrichItem. Settles
the handoff crux against the named decomp: retail is TWO tables, not one, so
acdream's WorldEntity + item-table split is already faithful — fix ingestion,
don't unify.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

docs/superpowers/specs/2026-06-18-d54-object-item-model-design.md

@@ -0,0 +1,336 @@
+# D.5.4 — Client object/item data model (foundation) — design
+
+**Date:** 2026-06-18
+**Status:** design approved (brainstorm) → spec under review → writing-plans next
+**Phase:** D.5.4 — the data-model foundation under D.5 "Core panels" (D.2b retail-look track).
+Registered in the roadmap D.5 sub-phase ledger; blocks D.5.5+ (inventory / paperdoll /
+vendor / trade panels resolve items from this table).
+**Branch:** `claude/hopeful-maxwell-214a12` (D.5.1 + D.5.2 already landed here; this continues it).
+**User constraint:** *"architecturally solid, no quick fixes"* — do NOT band-aid `EnrichItem`
+to add new items; design the model properly.
+
+**Research evidence base (this spec cites; it does not re-derive):**
+- [`docs/research/2026-06-18-item-object-model-handoff.md`](../../research/2026-06-18-item-object-model-handoff.md) — the phase framing + the crux
+- [`docs/research/deepdives/r06-items-inventory.md`](../../research/deepdives/r06-items-inventory.md) — item/property/container model + `PublicWeenieDesc` wire layout (§4) + burden (§6) + 2-deep containers (§7)
+- [`docs/research/2026-06-16-ui-item-slot-icon-dragdrop-spine-deep-dive.md`](../../research/2026-06-16-ui-item-slot-icon-dragdrop-spine-deep-dive.md) — `ClientObjMaintSystem` / resolve-by-guid model
+- [`docs/research/2026-06-16-inventory-deep-dive.md`](../../research/2026-06-16-inventory-deep-dive.md) — inventory wire catalog + container learning
+- The named-retail decomp `acclient_2013_pseudo_c.txt` / `acclient.h` (the oracle for the two-table model)
+
+---
+
+## 1. Goal
+
+Replace acdream's **enrich-existing-only** item scaffold with retail's **canonical-create**
+object model. Today `ItemRepository.EnrichItem` (`ItemRepository.cs:162`) returns `false`
+and silently drops a `CreateObject` for any item that wasn't pre-seeded as a stub from
+`PlayerDescription` — so items acquired mid-session, ground items, vendor items, and pack
+items the login snapshot didn't enumerate never enter the model and render no icon (confirmed
+live on Coldeve, character Barris: 4 of 6 hotbar slots blank).
+
+After this phase: **`CreateObject (0xF745)` is the canonical create-or-update for every
+server object**, the data table holds the data side of *every* object (items and creatures
+alike), `PlayerDescription`/shortcuts are references, the container membership index is live,
+and all UI resolves objects by guid. The Coldeve blank-icon bug is fixed at the root, and the
+foundation D.5.5's panels sit on is in place.
+
+This is a **data-model + ingestion** phase. No new panels ship; the toolbar (D.5.1) is the
+only live consumer and must keep working (visually unchanged).
+
+## 2. The crux — settled (the three brainstorm decisions)
+
+The handoff's §2 framing ("retail unifies everything under one `ClientObjMaintSystem`") was a
+misread. The named decomp shows retail is a **two-table** design, and the brainstorm settled
+the architecture against that ground truth:
+
+1. **Two tables, fix ingestion** (not unify). Retail's `CObjectMaint` holds *two* hash tables
+   keyed by the same guid — `object_table` (`CPhysicsObj`, render/physics) and
+   `weenie_object_table` (`ACCWeenieObject`, data/UI) — cross-linked by pointer, created and
+   destroyed together. The UI *only* calls `GetWeenieObject(guid)`; physics *only* calls
+   `GetPhysicsObject(guid)`. acdream's existing `WorldEntity` + item-table split already
+   mirrors this. We keep them separate (joined by guid) and fix the *ingestion*, not the
+   structure. A merge would also violate Code Structure Rule 2 (`WorldEntity` carries
+   `MeshRef`/GfxObj dat handles and rendering-coupled AABB math; merging drags GL into
+   `AcDream.Core`).
+
+2. **Complete model + container index.** Capture the full item weenie-field set (currently
+   parsed-then-discarded) into the data object, maintain a live container-membership index
+   (`containerGuid → ordered items`), evict on `DeleteObject`, fix the `WeenieClassId` misuse,
+   and expose a formal resolve-by-guid surface. Defer only the panel UIs and panel-driven
+   flows to D.5.5.
+
+3. **All objects (true weenie table).** `CreateObject` upserts *every* object (creatures,
+   players, NPCs, items) into the data table, making it acdream's `ACCWeenieObject`-equivalent
+   and retiring the redundant `GameWindow._liveEntityInfoByGuid` (Name+ItemType duplicate) so
+   selection/target also resolves from the one table. End state = exactly retail's two tables.
+
+## 3. Retail anchors (the load-bearing facts)
+
+All from the named decomp (`acclient_2013_pseudo_c.txt` / `acclient.h`), verified during the
+D.5.4 code-map research:
+
+- **Two parallel tables, one manager.** `CObjectMaint` (`acclient.h:33078`) holds
+  `object_table : LongHash<CPhysicsObj>`, `weenie_object_table : LongHash<CWeenieObject>`,
+  matching `null_object_table` / `null_weenie_object_table` placeholders (for out-of-order
+  create), `visible_object_table`, and `object_inventory_table : LongHash<CObjectInventory>`
+  (the per-container contents lists). Both object tables keyed by the same `uint32` guid.
+- **`CreateObject` is create-OR-update (timestamp-driven upsert), not create-only.**
+  `SmartBox::HandleCreateObject` (decomp ~93740) first calls
+  `CObjectMaint::GetObjectA(guid, &phys, &weenie)` (the 3-arg overload, ~269768) to detect
+  whether *either* table already has the guid. Fresh → `ACCObjectMaint::CreateObject`
+  (~356155) which allocates the `ACCWeenieObject` (`ACCFactory::MakeCWeenieObject_Internal`,
+  0x150 bytes, ~354698), inserts it, fills `pwd` via `SetWeenieDesc`, cross-links, and inserts
+  the `CPhysicsObj`. Existing → the **update branch** patches in place via `SetWeenieDesc`
+  (data) + per-timestamp physics updates. **There is no full-object replace** — updates merge.
+- **The weenie object holds all item game-data** in `pwd` (`PublicWeenieDesc`, `acclient.h:37163+`):
+  `_iconID/_iconOverlayID/_iconUnderlayID`, `_effects`, `_type`, `_stackSize/_maxStackSize`,
+  `_value`, `_burden`, `_containerID/_wielderID/_location/_priority`,
+  `_itemsCapacity/_containersCapacity`, `_structure/_maxStructure`, `_workmanship`, …
+- **Every object is an `ACCWeenieObject`** — creatures/players included; the UI resolves a
+  selected creature's name/health from the same table via `GetWeenieObject`.
+- **`DeleteObject` frees both objects atomically** (`ACCObjectMaint::DeleteObject` ~355020 →
+  `CObjectMaint::DeleteObject(guid)` ~270149) — physics and weenie removed in one call.
+- **The wire layout** of the `PublicWeenieDesc` flag-gated tail is r06 §4; acdream's
+  `CreateObject.cs:558-806` already walks every field in exact ACE order (it skips the ones it
+  doesn't keep — capturing them is changing `pos += N` to read the value).
+
+## 4. Scope
+
+**In scope (D.5.4):**
+- Rename + broaden: `ItemRepository` → `ClientObjectTable`, `ItemInstance` → `ClientObject`,
+  events `Item*` → `Object*`. The data table holds the data side of **all** server objects.
+- `CreateObject.TryParse` captures the full item field set (see §6.1) — currently discarded.
+- **Upsert is a field-level merge** (create-if-absent, else patch wire-carried fields in
+  place, preserving the `PropertyBundle` and move-state). `EnrichItem` is deleted.
+- Ingestion wiring moves **off `GameWindow`** into `AcDream.Core.Net` (`ObjectTableWiring`):
+  `CreateObject`→upsert, `DeleteObject`→remove, the `0x02CE` UiEffects path→`UpdateIntProperty`.
+- Container membership index (`containerGuid → ordered item guids`), live on upsert + move +
+  remove, exposed via `GetContents(guid)`.
+- `WeenieClassId` captured from `CreateObject` (stop misusing `PlayerDescription`'s
+  `ContainerType` as the class id).
+- `PlayerDescription` becomes a membership manifest (records "this guid is mine / in
+  container / equipped at slot"); out-of-order with `CreateObject` is safe (whichever arrives
+  first creates the entry, the other merges).
+- Retire `GameWindow._liveEntityInfoByGuid`; migrate its consumers
+  (`IsLiveCreatureTarget`/`DescribeLiveEntity`/target-indicator) to `ClientObjectTable.Get`.
+- `ToolbarController` resolves via `ClientObjectTable.Get` and **filters its event handler by
+  guid** (only re-binds when a changed guid is one of its 18 shortcuts).
+- `DeleteObject` (0xF747) evicts from the table.
+- Conformance tests throughout (§8). Preserve the D.5.2 effects-contract tests.
+
+**Out of scope (D.5.5+, explicit non-goals):**
+- The panel UIs themselves (inventory / paperdoll / vendor / trade / spellbook).
+- `ViewContents (0x0196)` open/close flow + the still-unwired inbound move events
+  (`InventoryPutObjectIn3D 0x019A`, `CloseGroundContainer 0x0052`,
+  `InventoryServerSaveFailed 0x00A0`) and their builders (`DropItem`/`GetAndWieldItem`/
+  `NoLongerViewingContents`).
+- Drag-drop mutate wire (`AddShortcut`/`RemoveShortcut`, `PutItemInContainer` from UI, etc.).
+- `ShortCutManager` durable persistence (shortcuts stay in the current closure path).
+- The broader `PublicUpdateProperty*` family beyond the existing `UiEffects (0x02CE)` path
+  (live StackSize/Value/Structure updates) — captured at create time, but the per-property
+  live-update parsers are D.5.5/M2.
+- `null_object_table`-style pre-queuing of a child `CreateObject` that arrives before its
+  parent. (Our upsert already makes plain out-of-order PD↔CreateObject safe; the parent/child
+  parenting edge case is deferred — see §10 risks.)
+
+## 5. Architecture & components
+
+Two guid-keyed tables, joined by guid, both mutated on the render thread:
+
+| Table | acdream type | retail analogue | holds | layer |
+|---|---|---|---|---|
+| Render/physics | `WorldEntity` (+ `GpuWorldState`) | `object_table` / `CPhysicsObj` | mesh, position, AABB, cell | `AcDream.Core/World` + `AcDream.App` |
+| **Data/UI** | **`ClientObjectTable`** of **`ClientObject`** | `weenie_object_table` / `ACCWeenieObject` | icon, name, type, stack, value, container/equip, properties | `AcDream.Core/Items` (pure data) |
+
+**Components (file → responsibility → change):**
+
+1. **`ClientObject`** (`AcDream.Core/Items/ItemInstance.cs` → renamed file/type from
+   `ItemInstance`). Per-object data record. *Change:* add the §6.1 fields; make `WeenieClassId`
+   settable; keep `PropertyBundle`. Item-specific fields are simply unset for creatures
+   (faithful to retail's `ACCWeenieObject` for non-items).
+
+2. **`ClientObjectTable`** (`AcDream.Core/Items/ItemRepository.cs` → renamed). The guid-keyed
+   store + container index + event surface. *Change:*
+   - `AddOrUpdate` becomes a **field-level merge upsert** (§7.2), not a whole-object replace.
+   - Add the container index: `Dictionary<uint, List<uint>>` keyed by containerGuid, kept
+     ordered by slot; updated on upsert / `MoveItem` / `Remove`; exposed via
+     `IReadOnlyList<uint> GetContents(uint containerGuid)`.
+   - Events renamed `ObjectAdded/ObjectUpdated/ObjectRemoved/ObjectMoved`.
+   - `EnrichItem` deleted.
+   - Keep `ConcurrentDictionary` (plugin reads) + `GetItem`→`Get` resolve surface.
+
+3. **`ObjectTableWiring`** (new, `AcDream.Core.Net/ObjectTableWiring.cs`). Static
+   `Wire(WorldSession session, ClientObjectTable table)` subscribing the WorldSession
+   GameMessage-level events: `EntitySpawned`→`AddOrUpdate(merge)`, `EntityDeleted`→`Remove`,
+   `ObjectIntPropertyUpdated`→`UpdateIntProperty`. This is the seam that moves item ingestion
+   off `GameWindow` (Rule 1) while keeping `AcDream.Core` GL-free (Rule 2).
+
+4. **`CreateObject.cs`** (`AcDream.Core.Net/Messages`). *Change:* capture the §6.1 fields into
+   `Parsed` (extend the record); the wire-cursor walk already exists — replace the `pos += N`
+   skips with value reads. **Risk:** the `Parsed` positional ctor + `WorldSession.EntitySpawn`
+   mirror must both grow; cursor arithmetic must stay byte-identical (locked by tests).
+
+5. **`WorldSession.EntitySpawn`** (`AcDream.Core.Net/WorldSession.cs:47`). *Change:* add the
+   new fields so they reach the ingestion wiring.
+
+6. **`GameEventWiring.cs`** (`AcDream.Core.Net`). *Change:* `PlayerDescription` handler stops
+   creating "source of truth" stubs with `WeenieClassId = ContainerType`; instead it records
+   membership (a merge upsert that sets container/equip placement + marks the guid as the
+   player's). `WieldObject`/`InventoryPutObjInContainer` → `MoveItem` stays (already wired).
+
+7. **`GameWindow.cs`** (`AcDream.App`). *Change:* delete the `EnrichItem` call; construct
+   `ClientObjectTable` + call `ObjectTableWiring.Wire`; retire `_liveEntityInfoByGuid` and
+   point its consumers at `ClientObjectTable.Get`. Render-entity build is unchanged.
+
+8. **`ToolbarController.cs`** (`AcDream.App/UI/Layout`). *Change:* resolve via
+   `ClientObjectTable.Get`; event handler filters by guid (only re-bind affected shortcut
+   slots); subscribe to `ObjectRemoved` too (today it doesn't, leaving stale slots).
+
+9. **`IconComposer.cs`** — unchanged (takes fields, not the table).
+
+## 6. Data model
+
+### 6.1 `ClientObject` fields to add (capture from `CreateObject`)
+
+The `ClientObject` type **already declares** most of these fields (they exist on today's
+`ItemInstance`), but `CreateObject` **does not populate them** — it walks past them on the
+wire. This table is the wire-capture work: rows marked **new** also need a field added to the
+type; the rest just need the parser to read the value into the existing field instead of
+skipping it. The cursor walk already exists in `CreateObject.cs:558-806` (each field has a
+`pos += N` skip today). Wire bits per r06 §4 / `PublicWeenieDesc`:
+
+| Field | Wire bit | field state | Notes |
+|---|---|---|---|
+| `WeenieClassId` | fixed prefix PackedDword (`CreateObject.cs:538`) | **make settable** | discarded today; init-only on the type |
+| `Value` | `0x00000008` | exists | `pos += 4` today |
+| `StackSize` / `StackSizeMax` | `0x00001000` / `0x00002000` | exists | skipped today |
+| `Burden` | `0x00200000` | exists | skipped today |
+| `ContainerId` | `0x00004000` | exists | item's parent container guid (drives the index) |
+| `ValidLocations` | `0x00010000` | exists | EquipMask (paperdoll needs it) |
+| `CurrentWieldedLocation` | `0x00020000` | exists → `CurrentlyEquippedLocation` | EquipMask |
+| `ItemsCapacity` / `ContainersCapacity` | `0x00000002` / `0x00000004` | **new** | feed `Container` (u8 each) |
+| `WielderId` | `0x00008000` | **new** | equip placement |
+| `Priority` (ClothingPriority) | `0x00040000` | **new** | layer order |
+| `Structure` / `MaxStructure` | `0x00000400` / `0x00000800` | **new** | charges/uses |
+| `Workmanship` | `0x01000000` (f32) | **new** | salvage/tinker display |
+
+`ContainerType` (PD inventory entry, 0/1/2) moves to its own field on the entry/`Container`,
+no longer aliased onto `WeenieClassId`.
+
+### 6.2 Container index
+
+`ClientObjectTable` maintains the equivalent of retail's `object_inventory_table`:
+`containerGuid → ordered list of item guids` (ordered by `ContainerSlot`). It is derived data,
+rebuilt from each object's `ContainerId`/`ContainerSlot`:
+- **on upsert:** if the object has a non-zero `ContainerId`, (re)index it under that parent.
+- **on `MoveItem`:** remove from old container list, add to new (or to equip if `WielderId`).
+- **on `Remove`:** drop from its container list.
+- **expose** `GetContents(containerGuid)` → ordered item guids (inventory panel reads this).
+
+Equip placement (`WielderId` + `CurrentWieldedLocation`) is tracked the same way so paperdoll
+can ask "what's equipped in slot X" without scanning.
+
+## 7. Ingestion lifecycle
+
+### 7.1 The flow
+- **`CreateObject (0xF745)`** → `WorldSession` parses (full field set) → fires `EntitySpawned`
+  → **`ObjectTableWiring`** calls `ClientObjectTable.AddOrUpdate(merge)` for **every** object,
+  independent of whether it also becomes a `WorldEntity` (inventory items have no position).
+  `GameWindow` keeps its own `EntitySpawned` subscription for the render-entity build.
+- **`DeleteObject (0xF747)` / Pickup** → `EntityDeleted` → `ClientObjectTable.Remove(guid)`
+  (today this leaks until `Clear()`). Render teardown unchanged.
+- **`PlayerDescription (0x0013)`** → membership manifest: a merge upsert that marks each
+  inventory/equipped guid as the player's and records placement (container/equip slot). The
+  *data* (icon/name/type/…) arrives from `CreateObject`. Shortcuts stay on the existing path.
+- **`WieldObject 0x0023` / `InventoryPutObjInContainer 0x0022`** → `MoveItem` (already wired) →
+  re-parents in the container index.
+- **`PublicUpdatePropertyInt 0x02CE` (UiEffects)** → `UpdateIntProperty` (already wired,
+  preserved).
+
+### 7.2 Upsert = field-level merge (the key correctness rule)
+`AddOrUpdate` must NOT replace the whole object (today's `_items[id] = item` clobbers appraise
+`PropertyBundle` + move-state on a `CreateObject` re-send; retail's update branch patches via
+`SetWeenieDesc`). The merge rule:
+- **Absent** → insert the new object; fire `ObjectAdded`.
+- **Present** → patch only the wire-carried fields onto the existing object (Name, Type,
+  Icon*, Effects, Stack, Value, Burden, capacities, `WeenieClassId`, and placement
+  `ContainerId`/`CurrentWieldedLocation`/`WielderId` when the wire carries them); **preserve**
+  the `PropertyBundle` (appraise detail) and any state the wire didn't carry; fire
+  `ObjectUpdated`.
+- **Effects** keeps the D.5.2 contract: assign unconditionally from the parsed value (0 = "no
+  effect", a meaningful state) so re-composition reflects the current server state.
+
+### 7.3 Out-of-order safety
+Because upsert is create-or-merge, the PD↔CreateObject arrival order is irrelevant: whichever
+arrives first creates the entry; the other merges its fields in. No drops (the root fix for
+the Coldeve bug), no silent races.
+
+### 7.4 Threading
+Unchanged: the net channel drains on the render-thread `OnUpdate`; both tables mutate on the
+render thread; `ConcurrentDictionary` is retained only for safe plugin reads. Events fire
+synchronously on the render thread (matching today).
+
+## 8. Testing (conformance throughout)
+
+xUnit, hand-built byte fixtures (matching `CreateObjectTests` / `ItemRepositoryTests` style;
+no pcap, no Moq). New + changed tests:
+- **Full-field-capture parse:** each new weenie-header field reads correctly; cursor
+  arithmetic stays byte-identical (a packet with a mid-tail field set still reaches
+  IconOverlay/IconUnderlay). Extend `CreateObjectTests`.
+- **Upsert creates a brand-new object** (no PD stub) — the Coldeve bug; this test would have
+  failed before the fix and locks it.
+- **Upsert merge** preserves `PropertyBundle` (appraise) + move-state across a `CreateObject`
+  re-send; does not clobber.
+- **Out-of-order:** CreateObject-before-PD and PD-before-CreateObject converge to identical
+  state.
+- **Container index:** add/move/remove keeps `GetContents` correct and slot-ordered; 2-deep
+  container depth (r06 §7); equip placement queryable.
+- **`DeleteObject` eviction** removes from the table + the container index.
+- **`WeenieClassId`** is the real class id from CreateObject, not the PD ContainerType.
+- **`_liveEntityInfoByGuid` retirement regression:** selection/describe still resolve
+  name+type for a creature via `ClientObjectTable.Get`.
+- **Toolbar guid-filter:** an unrelated object's `ObjectAdded` does not re-bind a shortcut
+  slot; a shortcut's `ObjectUpdated` does.
+- **Preserve** the D.5.2 effects tests (`effects==0` clears; per-pixel tint) under the new
+  merge path.
+
+## 9. Divergence register
+
+- **Retire** the enrich-only stopgap rows (the `EnrichItem` drops-unseeded-items behavior is
+  gone). Delete those rows in the same commit that lands the fix.
+- **Add** a row for the global-event-with-guid-filter consumer model vs. retail's per-object
+  `NoticeRegistrar` observer dispatch (a deliberate simplification — consumers filter by guid
+  rather than registering per-object observers). Note it; don't hide it.
+- **Add** a row (or note under it) for the deferred `null_object_table`-style parent/child
+  pre-queue (out-of-order *parented* create) — see §10.
+
+## 10. Risks & open questions
+
+- **Cursor arithmetic regression** in `CreateObject.cs` is the highest-risk change: turning
+  skips into reads must not shift any offset. Mitigation: the field walk already exists and is
+  test-covered; add per-field value assertions and a "mixed flags reach IconOverlay" test.
+- **`AddOrUpdate` merge vs. replace** touches existing `AddOrUpdate` callers (PD seeding,
+  appraise `UpdateProperties`). Audit every caller; the merge must be a strict superset of
+  prior behavior for the toolbar path.
+- **Event volume:** upserting all objects fires `ObjectAdded` per creature spawn. The toolbar
+  guid-filter handles it; future panels must filter too (documented in the table's event
+  XML-doc).
+- **`_liveEntityInfoByGuid` retirement timing:** the ingestion wiring and `GameWindow`'s render
+  handler both subscribe to `EntitySpawned`; ensure the table is populated before any consumer
+  queries (consumers run on later user interaction, so this is safe, but assert it).
+- **Parented item ordering** (a child `CreateObject` arriving before its parent) — retail uses
+  `null_object_table` pre-queuing. Deferred; PD↔CreateObject ordering is handled, but document
+  the parent/child gap so D.5.5 picks it up if a panel needs it.
+- **Naming churn:** the rename touches `GameEventWiring`, `ToolbarController`, tests, and the
+  `IconComposer` call site. Mechanical but wide; do it as a focused rename commit so the diff
+  reads cleanly.
+
+## 11. Acceptance criteria
+
+- `dotnet build` + `dotnet test` green (the full suite, including the new conformance tests).
+- A `CreateObject` for an item with **no** prior PD stub registers it in the table and the
+  toolbar renders its icon (the Coldeve repro, exercised by a unit test; visual confirmation
+  on a live server is the user's gate).
+- The toolbar still renders correctly for pre-seeded items (no regression).
+- Selection/target still resolves creature name+type after `_liveEntityInfoByGuid` retirement.
+- Roadmap D.5 ledger updated (D.5.4 → shipped); divergence register rows added/retired;
+  memory digest updated if there's a durable lesson.