From 969e55350b8121855abfa43b850bab4d3b7d65da Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 18 Jun 2026 15:06:00 +0200 Subject: [PATCH] docs(D.5.4): client object/item data model design (brainstorm spec) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- ...2026-06-18-d54-object-item-model-design.md | 336 ++++++++++++++++++ 1 file changed, 336 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-18-d54-object-item-model-design.md diff --git a/docs/superpowers/specs/2026-06-18-d54-object-item-model-design.md b/docs/superpowers/specs/2026-06-18-d54-object-item-model-design.md new file mode 100644 index 00000000..ee5c7118 --- /dev/null +++ b/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`, `weenie_object_table : LongHash`, + matching `null_object_table` / `null_weenie_object_table` placeholders (for out-of-order + create), `visible_object_table`, and `object_inventory_table : LongHash` + (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>` keyed by containerGuid, kept + ordered by slot; updated on upsert / `MoveItem` / `Remove`; exposed via + `IReadOnlyList 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.