From d9c427cd6c6aa09a4f95349911a299a5cabe2441 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 18 Jun 2026 16:02:28 +0200 Subject: [PATCH] feat(D.5.4): ClientObjectTable.Ingest merge-upsert + RecordMembership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field-level merge (retail SetWeenieDesc): create-if-absent else patch present fields, preserve PropertyBundle. Effects unconditional (D.5.2 contract). RecordMembership = PD manifest. Locks the Coldeve no-prior-stub fix + out-of-order. Renames _items→_objects throughout; Reindex stub wired (Task 6 fills it). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/AcDream.Core/Items/ClientObjectTable.cs | 96 ++++++++++++++++--- .../Items/ClientObjectTableTests.cs | 87 +++++++++++++++++ 2 files changed, 171 insertions(+), 12 deletions(-) diff --git a/src/AcDream.Core/Items/ClientObjectTable.cs b/src/AcDream.Core/Items/ClientObjectTable.cs index 6e4e088e..98d4f062 100644 --- a/src/AcDream.Core/Items/ClientObjectTable.cs +++ b/src/AcDream.Core/Items/ClientObjectTable.cs @@ -40,7 +40,7 @@ namespace AcDream.Core.Items; /// public sealed class ClientObjectTable { - private readonly ConcurrentDictionary _items = new(); + private readonly ConcurrentDictionary _objects = new(); private readonly ConcurrentDictionary _containers = new(); /// Fires when an object is first added to the session. @@ -64,17 +64,17 @@ public sealed class ClientObjectTable /// . public const uint UiEffectsPropertyId = 18u; - public int ObjectCount => _items.Count; + public int ObjectCount => _objects.Count; public int ContainerCount => _containers.Count; - public IEnumerable Objects => _items.Values; + public IEnumerable Objects => _objects.Values; public IEnumerable Containers => _containers.Values; /// /// Look up an object by its server-assigned ObjectId. /// public ClientObject? Get(uint objectId) => - _items.TryGetValue(objectId, out var item) ? item : null; + _objects.TryGetValue(objectId, out var item) ? item : null; /// /// Look up a container by object id, creating a lightweight stub if @@ -93,8 +93,8 @@ public sealed class ClientObjectTable public void AddOrUpdate(ClientObject item) { ArgumentNullException.ThrowIfNull(item); - bool existed = _items.ContainsKey(item.ObjectId); - _items[item.ObjectId] = item; + bool existed = _objects.ContainsKey(item.ObjectId); + _objects[item.ObjectId] = item; if (!existed) ObjectAdded?.Invoke(item); else ObjectUpdated?.Invoke(item); } @@ -117,7 +117,7 @@ public sealed class ClientObjectTable public bool MoveItem(uint itemId, uint newContainerId, int newSlot = -1, EquipMask newEquipLocation = EquipMask.None) { - if (!_items.TryGetValue(itemId, out var item)) return false; + if (!_objects.TryGetValue(itemId, out var item)) return false; uint oldContainer = item.ContainerId; item.ContainerId = newContainerId; @@ -134,7 +134,7 @@ public sealed class ClientObjectTable /// public bool Remove(uint itemId) { - if (!_items.TryRemove(itemId, out var item)) return false; + if (!_objects.TryRemove(itemId, out var item)) return false; ObjectRemoved?.Invoke(item); return true; } @@ -157,7 +157,7 @@ public sealed class ClientObjectTable public bool EnrichItem(uint objectId, uint iconId, string name, ItemType type, uint iconOverlayId = 0, uint iconUnderlayId = 0, uint effects = 0) { - if (!_items.TryGetValue(objectId, out var item)) return false; + if (!_objects.TryGetValue(objectId, out var item)) return false; if (iconId != 0) item.IconId = iconId; if (!string.IsNullOrEmpty(name)) item.Name = name; if (type != default) item.Type = type; @@ -178,7 +178,7 @@ public sealed class ClientObjectTable /// public bool UpdateProperties(uint itemId, PropertyBundle incoming) { - if (!_items.TryGetValue(itemId, out var item)) return false; + if (!_objects.TryGetValue(itemId, out var item)) return false; foreach (var kv in incoming.Ints) item.Properties.Ints[kv.Key] = kv.Value; foreach (var kv in incoming.Int64s) item.Properties.Int64s[kv.Key] = kv.Value; foreach (var kv in incoming.Bools) item.Properties.Bools[kv.Key] = kv.Value; @@ -199,20 +199,92 @@ public sealed class ClientObjectTable /// public bool UpdateIntProperty(uint itemId, uint propertyId, int value) { - if (!_items.TryGetValue(itemId, out var item)) return false; + if (!_objects.TryGetValue(itemId, out var item)) return false; item.Properties.Ints[propertyId] = value; if (propertyId == UiEffectsPropertyId) item.Effects = (uint)value; ObjectUpdated?.Invoke(item); return true; } + /// + /// Canonical CreateObject ingestion: create-if-absent, else patch the + /// wire-carried fields in place (retail SetWeenieDesc). Preserves the + /// PropertyBundle (appraise) and any field the wire didn't carry. + /// Effects is assigned unconditionally (0 clears) — the D.5.2 icon contract. + /// + public ClientObject Ingest(WeenieData d) + { + bool existed = _objects.TryGetValue(d.Guid, out var obj); + if (!existed || obj is null) // keep: satisfies nullable flow analysis + { + obj = new ClientObject { ObjectId = d.Guid }; + _objects[d.Guid] = obj; + } + uint oldContainer = obj.ContainerId; + + if (!string.IsNullOrEmpty(d.Name)) obj.Name = d.Name!; + if (d.Type is { } t) obj.Type = t; + // WeenieClassId arrives on every CreateObject (fixed prefix) and is never + // legitimately 0 for a real weenie; the != 0 guard avoids clobbering a known + // class id with a spurious 0 (and leaves a PD stub's 0 until CreateObject fills it). + if (d.WeenieClassId != 0) obj.WeenieClassId = d.WeenieClassId; + if (d.IconId != 0) obj.IconId = d.IconId; + if (d.IconOverlayId != 0) obj.IconOverlayId = d.IconOverlayId; + if (d.IconUnderlayId != 0) obj.IconUnderlayId = d.IconUnderlayId; + obj.Effects = d.Effects; // D.5.2 contract + if (d.Value is { } v) obj.Value = v; + if (d.StackSize is { } s) obj.StackSize = s; + if (d.StackSizeMax is { } sm) obj.StackSizeMax = sm; + if (d.Burden is { } b) obj.Burden = b; + if (d.ContainerId is { } c) obj.ContainerId = c; + if (d.WielderId is { } w) obj.WielderId = w; + if (d.ValidLocations is { } vl) obj.ValidLocations = (EquipMask)vl; + if (d.CurrentWieldedLocation is { } cwl) obj.CurrentlyEquippedLocation = (EquipMask)cwl; + if (d.Priority is { } pr) obj.Priority = pr; + if (d.ItemsCapacity is { } ic) obj.ItemsCapacity = ic; + if (d.ContainersCapacity is { } cc) obj.ContainersCapacity = cc; + if (d.Structure is { } st) obj.Structure = st; + if (d.MaxStructure is { } ms) obj.MaxStructure = ms; + if (d.Workmanship is { } wm) obj.Workmanship = wm; + + Reindex(obj, oldContainer); + if (!existed) ObjectAdded?.Invoke(obj); else ObjectUpdated?.Invoke(obj); + return obj; + } + + /// + /// PlayerDescription manifest: record that this guid is the player's + /// (in inventory or equipped at ), creating an + /// empty entry if CreateObject hasn't arrived yet. Never touches + /// icon/name/type/effects — that data comes from CreateObject. + /// + public ClientObject RecordMembership(uint guid, uint containerId = 0, + EquipMask equip = EquipMask.None) + { + bool existed = _objects.TryGetValue(guid, out var obj); + if (!existed || obj is null) // keep: satisfies nullable flow analysis + { + obj = new ClientObject { ObjectId = guid }; + _objects[guid] = obj; + } + uint oldContainer = obj.ContainerId; + if (containerId != 0) obj.ContainerId = containerId; + if (equip != EquipMask.None) obj.CurrentlyEquippedLocation = equip; + Reindex(obj, oldContainer); + if (!existed) ObjectAdded?.Invoke(obj); else ObjectUpdated?.Invoke(obj); + return obj; + } + + // Filled in Task 6 (container index). No-op until then. + private void Reindex(ClientObject obj, uint oldContainerId) { } + /// /// Flush the table — typically called on logoff or teleport /// that drops the session's object state. /// public void Clear() { - _items.Clear(); + _objects.Clear(); _containers.Clear(); } } diff --git a/tests/AcDream.Core.Tests/Items/ClientObjectTableTests.cs b/tests/AcDream.Core.Tests/Items/ClientObjectTableTests.cs index a975d327..04309bc6 100644 --- a/tests/AcDream.Core.Tests/Items/ClientObjectTableTests.cs +++ b/tests/AcDream.Core.Tests/Items/ClientObjectTableTests.cs @@ -230,4 +230,91 @@ public sealed class ClientObjectTableTests Structure: null, MaxStructure: null, Workmanship: null); Assert.Equal(0x99u, d.ContainerId); } + + private static WeenieData FullWeenie(uint guid, uint icon = 0x06001234u, + string name = "Sword", ItemType type = ItemType.MeleeWeapon, uint effects = 0, + int? value = 100, int? stack = 1, uint? container = null, uint wcid = 0xABCDu) => + new WeenieData(guid, name, type, wcid, icon, 0, 0, effects, + value, stack, StackSizeMax: 1, Burden: 10, ContainerId: container, + WielderId: null, ValidLocations: null, CurrentWieldedLocation: null, + Priority: null, ItemsCapacity: null, ContainersCapacity: null, + Structure: null, MaxStructure: null, Workmanship: null); + + [Fact] + public void Ingest_NewItemWithNoPriorStub_Creates_AndFiresAdded() // the Coldeve bug + { + var table = new ClientObjectTable(); + ClientObject? added = null; + table.ObjectAdded += o => added = o; + var obj = table.Ingest(FullWeenie(0x500000B0u)); + Assert.NotNull(added); + Assert.Equal(0x06001234u, table.Get(0x500000B0u)!.IconId); + Assert.Equal(0xABCDu, obj.WeenieClassId); + } + + [Fact] + public void Ingest_Existing_PatchesInPlace_PreservesPropertyBundle() + { + var table = new ClientObjectTable(); + table.Ingest(FullWeenie(0x500000B1u)); + table.Get(0x500000B1u)!.Properties.Ints[999u] = 7; // simulate appraise + ClientObject? updated = null; + table.ObjectUpdated += o => updated = o; + table.Ingest(FullWeenie(0x500000B1u, name: "Renamed")); + Assert.NotNull(updated); + Assert.Equal("Renamed", table.Get(0x500000B1u)!.Name); + Assert.Equal(7, table.Get(0x500000B1u)!.Properties.Ints[999u]); // NOT clobbered + } + + [Fact] + public void Ingest_AbsentNullableField_DoesNotClobber() + { + var table = new ClientObjectTable(); + table.Ingest(FullWeenie(0x500000B2u, value: 100)); + var noValue = FullWeenie(0x500000B2u) with { Value = null }; + table.Ingest(noValue); + Assert.Equal(100, table.Get(0x500000B2u)!.Value); + } + + [Fact] + public void Ingest_Effects_AssignedUnconditionally_ClearsToZero() // D.5.2 contract + { + var table = new ClientObjectTable(); + table.Ingest(FullWeenie(0x500000B3u, effects: 0x1u)); + Assert.Equal(0x1u, table.Get(0x500000B3u)!.Effects); + table.Ingest(FullWeenie(0x500000B3u, effects: 0u)); + Assert.Equal(0u, table.Get(0x500000B3u)!.Effects); + } + + [Fact] + public void RecordMembership_CreatesEntry_AndSetsEquip() + { + var table = new ClientObjectTable(); + table.RecordMembership(0x500000B4u, equip: EquipMask.MeleeWeapon); + var o = table.Get(0x500000B4u); + Assert.NotNull(o); + Assert.Equal(EquipMask.MeleeWeapon, o!.CurrentlyEquippedLocation); + Assert.Equal(0u, o.IconId); // data not set — CreateObject fills it + } + + [Fact] + public void Ingest_AfterMembership_FillsData_NoDuplicate() // out-of-order: PD then CreateObject + { + var table = new ClientObjectTable(); + table.RecordMembership(0x500000B5u); + table.Ingest(FullWeenie(0x500000B5u)); + Assert.Equal(1, table.ObjectCount); + Assert.Equal(0x06001234u, table.Get(0x500000B5u)!.IconId); + } + + [Fact] + public void Membership_AfterIngest_NoDuplicate_PreservesData() // out-of-order: CreateObject then PD + { + var table = new ClientObjectTable(); + table.Ingest(FullWeenie(0x500000B6u)); // CreateObject first (ground/vendor item) + table.RecordMembership(0x500000B6u, equip: EquipMask.MeleeWeapon); // then PD manifest + Assert.Equal(1, table.ObjectCount); + Assert.Equal(0x06001234u, table.Get(0x500000B6u)!.IconId); // data NOT clobbered by membership + Assert.Equal(EquipMask.MeleeWeapon, table.Get(0x500000B6u)!.CurrentlyEquippedLocation); + } }