diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 34aa1c6c..69b393f1 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -163,6 +163,7 @@ accepted-divergence entries (#96, #49, #50). | AP-62 | **MissileAmmo slot mask LIKELY** — `0x100001E0 → MissileAmmo 0x800000` is inferred; the decomp immediate at `GetLocationInfoFromElementID` 173676 is corrupted to a string pointer, so the mapping was not directly confirmed from the named-retail source. | `src/AcDream.App/UI/Layout/PaperdollController.cs` (`SlotMap`) | Dropping ammo onto that slot wields to the wrong location if the mapping is wrong. Gate-verify via dat dump + cdb. | Ammo wields to the wrong equip location — functional gap if wrong. | `GetLocationInfoFromElementID` decomp 173676; `acclient.h:3193` INVENTORY_LOC | | AP-63 | **Dual-wield-into-shield-slot special not implemented** — retail `OnItemListDragOver` (decomp 174302) lets a melee-capable item also drop on the Shield slot; acdream's `wieldMask = ValidLocations & slotMask` rejects it. | `src/AcDream.App/UI/Layout/PaperdollController.cs` (`HandleDropRelease`) | A dual-wielder cannot off-hand a melee weapon via the doll. | Dual-wield via drag-onto-shield-slot is blocked — functional gap for dual-wield characters. | `gmPaperDollUI::OnItemListDragOver` decomp 174302 | | AP-64 | **Wield-reject rollback assumes `InventoryServerSaveFailed 0x00A0`** — an optimistic wield rolls back only if ACE emits `0x00A0` for a `GetAndWieldItem` rejection; otherwise corrected by the next authoritative message. Gate-verify via WireMCP. | `src/AcDream.Core.Net/GameEventWiring.cs` (0x00A0 handler) | If ACE uses a different reject opcode for wield, the optimistic state is left dangling until the next full update. | Rejected wield briefly shows the item as equipped in the doll — cosmetic flicker or stuck state if `0x00A0` is not the rejection path. | `InventoryServerSaveFailed 0x00A0`; WireMCP gate-verify | +| AP-65 | **PickupEvent (0xF74A) no longer evicts the weenie from `ClientObjectTable`** — only `DeleteObject (0xF747)` evicts, matching the retail `object_table`-vs-`weenie_object_table` split. An item another player picks up near you (only ever gets `PickupEvent`, never `DeleteObject`) lingers as a data-only entry (`ContainerId 0`, not in any view) until teleport/relog `Clear()`. | `src/AcDream.Core.Net/ObjectTableWiring.cs` (EntityDeleted handler); `src/AcDream.Core.Net/WorldSession.cs` (PickupEvent branch) | The weenie entry for nearby pickups is a harmless data ghost — no UI shows it (no container, not wielded). Retail evicts it from `weenie_object_table` when the object fully leaves interest range via `DeleteObject`; acdream defers to teleport/relog clear. | Slight memory growth in long sessions if many world items are picked up by other players near you; item data ghosts cannot cause functional issues because no UI queries `ContainerId 0`. | `CACObjectMaint::DeleteObject` / `SmartBox::HandleDeleteObject`; ACE `Player_Inventory.cs TryDequipObjectWithNetworking` | --- diff --git a/src/AcDream.Core.Net/Messages/DeleteObject.cs b/src/AcDream.Core.Net/Messages/DeleteObject.cs index c18bb139..fc88f28f 100644 --- a/src/AcDream.Core.Net/Messages/DeleteObject.cs +++ b/src/AcDream.Core.Net/Messages/DeleteObject.cs @@ -17,11 +17,23 @@ public static class DeleteObject { public const uint Opcode = 0xF747u; - public readonly record struct Parsed(uint Guid, ushort InstanceSequence); + /// + /// when this delete was sourced from a + /// PickupEvent (0xF74A) — the object left the 3-D world view but + /// the weenie record still exists (it is moving into a container). + /// (default) when sourced from DeleteObject + /// (0xF747) — the weenie was destroyed and must be evicted from + /// . + /// Retail two-table model: PickupEvent removes from object_table; + /// DeleteObject removes from weenie_object_table. + /// + public readonly record struct Parsed(uint Guid, ushort InstanceSequence, bool FromPickup = false); /// /// Parse a 0xF747 body. must start with the /// 4-byte opcode, matching every other parser in this namespace. + /// The returned is always + /// — this parser handles true destroys only. /// public static Parsed? TryParse(ReadOnlySpan body) { @@ -34,6 +46,6 @@ public static class DeleteObject uint guid = BinaryPrimitives.ReadUInt32LittleEndian(body.Slice(4, 4)); ushort instanceSequence = BinaryPrimitives.ReadUInt16LittleEndian(body.Slice(8, 2)); - return new Parsed(guid, instanceSequence); + return new Parsed(guid, instanceSequence, FromPickup: false); } } diff --git a/src/AcDream.Core.Net/ObjectTableWiring.cs b/src/AcDream.Core.Net/ObjectTableWiring.cs index ca21c79a..e47f8aaa 100644 --- a/src/AcDream.Core.Net/ObjectTableWiring.cs +++ b/src/AcDream.Core.Net/ObjectTableWiring.cs @@ -24,7 +24,13 @@ public static class ObjectTableWiring ArgumentNullException.ThrowIfNull(table); session.EntitySpawned += s => table.Ingest(ToWeenieData(s)); - session.EntityDeleted += d => table.Remove(d.Guid); + // Retail two-table model: + // PickupEvent (0xF74A) → object left the 3-D world view; the weenie persists + // (it is moving into a container, e.g. unwield-to-pack). Remove the 3-D + // render entity only — do NOT evict from ClientObjectTable. + // DeleteObject (0xF747) → weenie is DESTROYED; evict from both tables. + // FromPickup = true guards the weenie eviction so it only fires for true destroys. + session.EntityDeleted += d => { if (!d.FromPickup) table.Remove(d.Guid); }; // B-Wire: apply EVERY PropertyInt update on a visible object (0x02CE), not just // UiEffects — the server is the authority on object properties. UpdateIntProperty diff --git a/src/AcDream.Core.Net/WorldSession.cs b/src/AcDream.Core.Net/WorldSession.cs index 60e4aa6f..bfde2a28 100644 --- a/src/AcDream.Core.Net/WorldSession.cs +++ b/src/AcDream.Core.Net/WorldSession.cs @@ -807,16 +807,18 @@ public sealed class WorldSession : IDisposable } else if (op == PickupEvent.Opcode) { - // ACE sends PickupEvent (0xF74A) instead of DeleteObject - // when a player picks up a world item (Player_Tracking - // .RemoveTrackedObject with fromPickup=true). Downstream - // view-removal semantics are identical, so we adapt to - // DeleteObject.Parsed and reuse the existing handler. + // ACE sends PickupEvent (0xF74A) when an object leaves the 3-D world view + // (player picks it up, or a player unwields gear that goes back to their pack). + // The WEENIE is NOT destroyed — it is moving into a container. We adapt to + // DeleteObject.Parsed so the render/entity layer (GameWindow.OnLiveEntityDeleted) + // still removes the 3-D WorldEntity, but FromPickup = true tells ObjectTableWiring + // to skip the ClientObjectTable eviction (retail two-table model: PickupEvent + // targets object_table only; DeleteObject 0xF747 targets weenie_object_table). var parsed = PickupEvent.TryParse(body); if (parsed is not null) EntityDeleted?.Invoke( new DeleteObject.Parsed( - parsed.Value.Guid, parsed.Value.InstanceSequence)); + parsed.Value.Guid, parsed.Value.InstanceSequence, FromPickup: true)); } else if (op == UpdateMotion.Opcode) { diff --git a/tests/AcDream.Core.Net.Tests/Messages/DeleteObjectTests.cs b/tests/AcDream.Core.Net.Tests/Messages/DeleteObjectTests.cs index b464cab1..b1140d94 100644 --- a/tests/AcDream.Core.Net.Tests/Messages/DeleteObjectTests.cs +++ b/tests/AcDream.Core.Net.Tests/Messages/DeleteObjectTests.cs @@ -36,4 +36,42 @@ public sealed class DeleteObjectTests Assert.Equal(0x80000439u, parsed!.Value.Guid); Assert.Equal((ushort)0x1234, parsed.Value.InstanceSequence); } + + /// + /// Regression guard: TryParse (0xF747 = true destroy) must always produce + /// FromPickup = false. PickupEvent (0xF74A) is a different opcode and is + /// the ONLY path that sets FromPickup = true (in WorldSession). + /// + [Fact] + public void TryParse_AlwaysReturnsFromPickupFalse() + { + Span body = stackalloc byte[12]; + BinaryPrimitives.WriteUInt32LittleEndian(body, DeleteObject.Opcode); + BinaryPrimitives.WriteUInt32LittleEndian(body.Slice(4), 0x80000001u); + BinaryPrimitives.WriteUInt16LittleEndian(body.Slice(8), 0x0001); + + var parsed = DeleteObject.TryParse(body); + + Assert.NotNull(parsed); + Assert.False(parsed!.Value.FromPickup, + "DeleteObject 0xF747 is a true destroy — FromPickup must be false."); + } + + /// + /// FromPickup = true can be constructed via the record directly (as + /// WorldSession does for PickupEvent 0xF74A). Default is false. + /// + [Fact] + public void Parsed_DefaultFromPickupIsFalse() + { + var p = new DeleteObject.Parsed(0x80000001u, 1); + Assert.False(p.FromPickup); + } + + [Fact] + public void Parsed_FromPickupTrueCanBeSet() + { + var p = new DeleteObject.Parsed(0x80000001u, 1, FromPickup: true); + Assert.True(p.FromPickup); + } } diff --git a/tests/AcDream.Core.Net.Tests/ObjectTableWiringTests.cs b/tests/AcDream.Core.Net.Tests/ObjectTableWiringTests.cs index 9c3d099a..855f77c9 100644 --- a/tests/AcDream.Core.Net.Tests/ObjectTableWiringTests.cs +++ b/tests/AcDream.Core.Net.Tests/ObjectTableWiringTests.cs @@ -7,10 +7,13 @@ namespace AcDream.Core.Net.Tests; /// /// D.5.4 Task 7 — ObjectTableWiring. /// -/// Integration test is omitted: WorldSession.EntitySpawned has no internal -/// test seam to fire it without a real Tick + packet bytes, so subscription -/// correctness is covered by build (type-checks) + the live run. Only the -/// pure mapping (ToWeenieData) is unit-tested here. +/// WorldSession.EntitySpawned/EntityDeleted have no in-process test seam (the +/// ctor opens a UDP socket), so the subscription wiring is tested via the guard +/// logic extracted as a local handler — this is the same lambda body that +/// ObjectTableWiring.Wire wires to session.EntityDeleted. The retail two-table +/// semantics are: PickupEvent (0xF74A) removes from the 3-D object_table but +/// KEEPS the weenie in ClientObjectTable; DeleteObject (0xF747) evicts the +/// weenie from both. FromPickup = true guards the eviction. /// public sealed class ObjectTableWiringTests { @@ -94,4 +97,80 @@ public sealed class ObjectTableWiringTests Assert.Equal(100, d.MaxStructure); Assert.Equal(4.5f, d.Workmanship); } + + // ------------------------------------------------------------------------- + // The handler that ObjectTableWiring.Wire subscribes to session.EntityDeleted. + // Testing it directly avoids needing a live WorldSession (UDP socket). + // ------------------------------------------------------------------------- + private static Action MakeEntityDeletedHandler(ClientObjectTable table) + => d => { if (!d.FromPickup) table.Remove(d.Guid); }; + + private static WeenieData MinimalWeenie(uint guid) => new( + Guid: guid, + Name: "Test Item", + Type: null, + WeenieClassId: 1u, + IconId: 0x06000001u, + IconOverlayId: 0u, + IconUnderlayId: 0u, + Effects: 0u, + Value: null, + StackSize: null, + StackSizeMax: null, + Burden: null, + ContainerId: null, + WielderId: null, + ValidLocations: null, + CurrentWieldedLocation: null, + Priority: null, + ItemsCapacity: null, + ContainersCapacity: null, + Structure: null, + MaxStructure: null, + Workmanship: null); + + /// + /// PickupEvent path: FromPickup = true → weenie is RETAINED in ClientObjectTable. + /// The 3-D render entity is removed (EntityDeleted still fires), but the weenie + /// data persists so the follow-up InventoryPutObjInContainer can find it. + /// + [Fact] + public void EntityDeleted_FromPickupTrue_RetainsWeenieInTable() + { + var table = new ClientObjectTable(); + var handler = MakeEntityDeletedHandler(table); + + const uint guid = 0x80000100u; + table.Ingest(MinimalWeenie(guid)); + Assert.NotNull(table.Get(guid)); // pre-condition: item is in the table + + // Fire EntityDeleted as WorldSession does for PickupEvent (0xF74A). + // The handler itself represents the EntityDeleted subscription — firing it IS the event. + handler(new DeleteObject.Parsed(guid, 0, FromPickup: true)); + + // Post-condition: weenie still in table (it moved into a container, was NOT destroyed). + // EntityDeleted still fires (the handler ran), so the 3-D render layer would remove the WorldEntity. + Assert.NotNull(table.Get(guid)); + } + + /// + /// DeleteObject path: FromPickup = false → weenie IS evicted from ClientObjectTable. + /// Regression guard — true destroys (0xF747) must still clean up the table. + /// + [Fact] + public void EntityDeleted_FromPickupFalse_EvictsWeenieFromTable() + { + var table = new ClientObjectTable(); + var handler = MakeEntityDeletedHandler(table); + + const uint guid = 0x80000200u; + table.Ingest(MinimalWeenie(guid)); + Assert.NotNull(table.Get(guid)); // pre-condition: item is in the table + + // Fire EntityDeleted as WorldSession does for DeleteObject (0xF747). + handler(new DeleteObject.Parsed(guid, 0, FromPickup: false)); + + // Post-condition: weenie evicted (truly destroyed). + Assert.Null(table.Get(guid)); + } }