fix(D.2b): PickupEvent removes the 3D render, not the weenie — unwield lands in the pack
PickupEvent (0xF74A) and DeleteObject (0xF747) are semantically distinct: - 0xF747 = weenie DESTROYED → evict from ClientObjectTable (weenie_object_table) - 0xF74A = object LEFT THE 3D WORLD VIEW (moved into a container) → remove the 3D WorldEntity, but the weenie persists in ClientObjectTable Before this fix, both paths fired EntityDeleted identically, causing ObjectTableWiring to evict the weenie from ClientObjectTable. The follow-up InventoryPutObjInContainer (0x0022) then tried MoveItem on an unknown guid and no-op'd, so the unwielded item simply vanished. Fix: add `bool FromPickup` (default false) to DeleteObject.Parsed. WorldSession sets it true on the PickupEvent path and false on the DeleteObject path. ObjectTableWiring.Wire's EntityDeleted handler skips table.Remove when FromPickup is true, preserving the weenie for the container-move echo. GameWindow.OnLiveEntityDeleted (3D entity removal) is untouched — it fires for both pickups and destroys, as intended. Divergence register: AP-65 added (data ghosts for other-player pickups until teleport/relog clear; harmless — no UI queries ContainerId 0). Tests: +5 (DeleteObject FromPickup parser regression; wiring retain/evict semantics; Parsed default/explicit FromPickup). 343/343 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
058da60212
commit
db0cac03c4
6 changed files with 151 additions and 13 deletions
|
|
@ -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` |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -17,11 +17,23 @@ public static class DeleteObject
|
|||
{
|
||||
public const uint Opcode = 0xF747u;
|
||||
|
||||
public readonly record struct Parsed(uint Guid, ushort InstanceSequence);
|
||||
/// <param name="FromPickup">
|
||||
/// <see langword="true"/> when this delete was sourced from a
|
||||
/// <c>PickupEvent (0xF74A)</c> — the object left the 3-D world view but
|
||||
/// the weenie record still exists (it is moving into a container).
|
||||
/// <see langword="false"/> (default) when sourced from <c>DeleteObject
|
||||
/// (0xF747)</c> — the weenie was destroyed and must be evicted from
|
||||
/// <see cref="AcDream.Core.Items.ClientObjectTable"/>.
|
||||
/// Retail two-table model: PickupEvent removes from <c>object_table</c>;
|
||||
/// DeleteObject removes from <c>weenie_object_table</c>.
|
||||
/// </param>
|
||||
public readonly record struct Parsed(uint Guid, ushort InstanceSequence, bool FromPickup = false);
|
||||
|
||||
/// <summary>
|
||||
/// Parse a 0xF747 body. <paramref name="body"/> must start with the
|
||||
/// 4-byte opcode, matching every other parser in this namespace.
|
||||
/// The returned <see cref="Parsed.FromPickup"/> is always <see langword="false"/>
|
||||
/// — this parser handles true destroys only.
|
||||
/// </summary>
|
||||
public static Parsed? TryParse(ReadOnlySpan<byte> 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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -36,4 +36,42 @@ public sealed class DeleteObjectTests
|
|||
Assert.Equal(0x80000439u, parsed!.Value.Guid);
|
||||
Assert.Equal((ushort)0x1234, parsed.Value.InstanceSequence);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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).
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void TryParse_AlwaysReturnsFromPickupFalse()
|
||||
{
|
||||
Span<byte> 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.");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// FromPickup = true can be constructed via the record directly (as
|
||||
/// WorldSession does for PickupEvent 0xF74A). Default is false.
|
||||
/// </summary>
|
||||
[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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,10 +7,13 @@ namespace AcDream.Core.Net.Tests;
|
|||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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<DeleteObject.Parsed> 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);
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[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));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// DeleteObject path: FromPickup = false → weenie IS evicted from ClientObjectTable.
|
||||
/// Regression guard — true destroys (0xF747) must still clean up the table.
|
||||
/// </summary>
|
||||
[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));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue