fix(vendor): grand-gate findings — wire-truth container counts, the live split bar, arrival-gated use, prepend-order race
Some checks are pending
Headless portability / portable-headless (ubuntu-latest) (push) Waiting to run
Headless portability / portable-headless (windows-latest) (push) Waiting to run
Headless portability / linux-graphical (push) Waiting to run
Headless portability / linux-vulkan (push) Waiting to run
Some checks are pending
Headless portability / portable-headless (ubuntu-latest) (push) Waiting to run
Headless portability / portable-headless (windows-latest) (push) Waiting to run
Headless portability / linux-graphical (push) Waiting to run
Headless portability / linux-vulkan (push) Waiting to run
Four live findings, each with the paper-verification failure named: G1 the container-capacity guard counted containers by a local type/capacity heuristic that over-classifies ordinary items; retail buckets from the wire's ContainerProperties at insert. Now reads ClientObjectTable's existing ContainerTypeHint (AP-168 narrowed to the shop-stock half; a pre-check must never false-block). G2 the amount bar never showed live because ACE never sets StackSize on browse listings — DescStackSize is null for every real vendor item and the C4 paper test hand-set the field, bypassing the materializer. The materializer now falls back to the packed supply count (AP-169, ACE adaptation); the new test drives the REAL materializer. G3 an out-of-range Use now dispatches ON ARRIVAL (pickup's shape): ACE's HandleActionUseItem only opens the vendor when the Use finds the player in range — a click-time send is greeted and dropped (AP-170, ACE adaptation; retail's server walks the player, ACE does not). G4 bought items appended because ACE's placement echo (UIQueue) can beat the CreateObject (SmartboxQueue) — cross-queue, no ordering guarantee — and the early echo was silently dropped. ClientObjectTable now stashes unresolved placements and replays them at Ingest: buys land at the retail list head. No register row — this RESTORES parity. Clean-room complete solution: 11,521 passed / 4 skipped / 0 failed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
c68ad1e646
commit
68568a3a59
12 changed files with 1140 additions and 90 deletions
|
|
@ -457,17 +457,25 @@ public sealed class SelectionInteractionControllerTests
|
|||
|
||||
/// <summary>
|
||||
/// C1 (Slice 6b move-to-use, docs/research/2026-08-08-slice6b-vendor-
|
||||
/// completion-research.md Q2): an out-of-range Use kicks off the SAME
|
||||
/// local client-predicted MoveToObject approach Pickup's far-range
|
||||
/// branch already installs, giving the walk immediate visual feel. The
|
||||
/// wire send is never gated on arrival — retail's
|
||||
/// <c>ItemHolder::UseObject @ 0x00588A80</c> has no range check and
|
||||
/// sends unconditionally, so the dispatch and the approach both happen
|
||||
/// at click time, in that order. A later natural MoveTo completion must
|
||||
/// not re-dispatch (Use has no post-arrival token the way Pickup does).
|
||||
/// completion-research.md Q2, REVISED for G3 — grand-gate finding
|
||||
/// 2026-08-08, register AP-170): an out-of-range Use kicks off the SAME
|
||||
/// local client-predicted MoveToObject approach Pickup's close-range
|
||||
/// branch already arms on, giving the walk immediate visual feel. Unlike
|
||||
/// the original (send-immediately) design, the wire send is now GATED on
|
||||
/// natural arrival — live testing against the user's local ACE server
|
||||
/// showed retail's own "no range check, send immediately" assumption
|
||||
/// does not hold there: ACE's <c>Player.HandleActionUseItem</c> polls
|
||||
/// for the player to actually reach use range before calling
|
||||
/// <c>ActOnUse</c>, and a Use that arrives too early is silently lost
|
||||
/// (the vendor's cosmetic greeting fires — a distance-only reaction
|
||||
/// independent of Use — but <c>ApproachVendor</c> never comes). Nothing
|
||||
/// dispatches until <see cref="SelectionInteractionController.OnNaturalMoveToComplete"/>
|
||||
/// fires; a SECOND natural-completion call must not re-dispatch (the
|
||||
/// pending-use token is consumed on first resolve, exactly like Pickup's
|
||||
/// pending-pickup token).
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void FarUseApproachesThenDispatchesImmediatelyAndDoesNotRetryOnArrival()
|
||||
public void FarUseApproachesThenDispatchesOnNaturalArrival()
|
||||
{
|
||||
var h = new Harness();
|
||||
h.SetApproach(closeRange: false);
|
||||
|
|
@ -475,6 +483,11 @@ public sealed class SelectionInteractionControllerTests
|
|||
h.Controller.SendUse(Target);
|
||||
|
||||
PlayerInteractionMovementSinkAssertSingleApproach(h, Target);
|
||||
// Armed, not yet sent — the whole point of the fix.
|
||||
Assert.Empty(h.Transport.Uses);
|
||||
|
||||
h.Controller.OnNaturalMoveToComplete();
|
||||
|
||||
Assert.Equal(new[] { Target }, h.Transport.Uses);
|
||||
|
||||
h.Controller.OnNaturalMoveToComplete();
|
||||
|
|
@ -483,10 +496,10 @@ public sealed class SelectionInteractionControllerTests
|
|||
}
|
||||
|
||||
/// <summary>
|
||||
/// C1 cancellation coverage: a second far Use command (the player picked
|
||||
/// a new target, i.e. "moved on") supersedes the first local approach
|
||||
/// cleanly — no exception, no missing/duplicated dispatch, no leaked
|
||||
/// pending-pickup state (Use never arms one).
|
||||
/// C1/G3 cancellation coverage: a second far Use command (the player
|
||||
/// picked a new target, i.e. "moved on") supersedes the first local
|
||||
/// approach cleanly — the FIRST target's armed Use is cancelled (never
|
||||
/// sent), and only the SECOND dispatches, on ITS OWN natural arrival.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void NewFarUseCommandSupersedesThePreviousApproachCleanly()
|
||||
|
|
@ -509,23 +522,25 @@ public sealed class SelectionInteractionControllerTests
|
|||
Assert.Equal(2, h.Movement.Approaches.Count);
|
||||
Assert.Equal(Target, h.Movement.Approaches[0].Target.ServerGuid);
|
||||
Assert.Equal(otherTarget, h.Movement.Approaches[1].Target.ServerGuid);
|
||||
Assert.Equal(new[] { Target, otherTarget }, h.Transport.Uses);
|
||||
// Neither has sent yet — both are armed/superseded, not dispatched.
|
||||
Assert.Empty(h.Transport.Uses);
|
||||
|
||||
h.Controller.OnNaturalMoveToComplete();
|
||||
|
||||
Assert.Equal(new[] { Target, otherTarget }, h.Transport.Uses);
|
||||
// Only the surviving (second) approach's Use goes out.
|
||||
Assert.Equal(new[] { otherTarget }, h.Transport.Uses);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// C1 cancellation coverage: the underlying MoveTo controller cancelling
|
||||
/// out from under a far Use's local approach (player moved away with
|
||||
/// WASD, or any other source of <see cref="WeenieError"/>) must not
|
||||
/// retract or duplicate the Use, which already went out unconditionally
|
||||
/// at click time — Use holds no pending-pickup state for
|
||||
/// <c>OnMoveToCancelled</c> to touch.
|
||||
/// C1/G3 cancellation coverage: the underlying MoveTo controller
|
||||
/// cancelling out from under a far Use's local approach (player moved
|
||||
/// away with WASD, or any other source of <see cref="WeenieError"/>)
|
||||
/// must cancel the ARMED (not-yet-sent) Use — retail's own server-side
|
||||
/// poll would never have seen the player arrive either, so nothing
|
||||
/// should reach the wire.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void MovingAwayDuringAFarUseApproachDoesNotAffectTheAlreadyDispatchedUse()
|
||||
public void MovingAwayDuringAFarUseApproachCancelsTheArmedUse()
|
||||
{
|
||||
var h = new Harness();
|
||||
h.SetApproach(closeRange: false);
|
||||
|
|
@ -534,7 +549,7 @@ public sealed class SelectionInteractionControllerTests
|
|||
h.Controller.OnMoveToCancelled(WeenieError.ActionCancelled);
|
||||
h.Controller.OnNaturalMoveToComplete();
|
||||
|
||||
Assert.Equal(new[] { Target }, h.Transport.Uses);
|
||||
Assert.Empty(h.Transport.Uses);
|
||||
}
|
||||
|
||||
private static void PlayerInteractionMovementSinkAssertSingleApproach(
|
||||
|
|
|
|||
|
|
@ -673,6 +673,102 @@ public class SelectedObjectControllerTests
|
|||
// fails, and the failure is what gets fixed).
|
||||
// ══════════════════════════════════════════════════════════════════════
|
||||
|
||||
/// <summary>
|
||||
/// G2 (grand-gate finding): C4 passed by manually hand-setting
|
||||
/// <c>ClientObject.StackSize</c> directly, bypassing BOTH the real
|
||||
/// <see cref="VendorShopItemMaterializer"/> that populates it from a
|
||||
/// live <c>ApproachVendor</c> snapshot AND the real
|
||||
/// <c>ObjectUpdated</c>/<c>ObjectAdded</c> event wiring. This test drives
|
||||
/// the same scenario through the REAL materializer — <see cref="VendorState.Apply"/>
|
||||
/// fires <see cref="VendorState.Changed"/>, the REAL
|
||||
/// <see cref="VendorShopItemMaterializer"/> (subscribed exactly like
|
||||
/// production's <c>RuntimeInventoryState</c> constructor) ingests the
|
||||
/// shop item into the SAME <see cref="ClientObjectTable"/>, and only
|
||||
/// THEN is the item selected — closing the gap the C4 harness left open.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void G2_VendorStackSelection_ThroughRealMaterializer_ShowsSplitSlider()
|
||||
{
|
||||
const uint vendorGuid = 0x70000011u;
|
||||
const uint arrowsGuid = 0x60009011u;
|
||||
|
||||
ImportedLayout layout = FixtureLoader.LoadToolbar();
|
||||
var objects = new ClientObjectTable();
|
||||
var vendor = new VendorState();
|
||||
var selection = new SelectionState();
|
||||
var splitQuantity = new StackSplitQuantityState();
|
||||
|
||||
// REAL materializer, wired exactly like RuntimeInventoryState's
|
||||
// constructor (RuntimeInventoryState.cs:74):
|
||||
// VendorItems = new VendorShopItemMaterializer(Vendor, _entityObjects.Objects);
|
||||
using var materializer = new VendorShopItemMaterializer(vendor, objects);
|
||||
|
||||
SelectedObjectController controller = SelectedObjectController.Bind(
|
||||
layout,
|
||||
selection,
|
||||
subscribeHealthChanged: _ => { },
|
||||
unsubscribeHealthChanged: _ => { },
|
||||
subscribeItemManaChanged: _ => { },
|
||||
unsubscribeItemManaChanged: _ => { },
|
||||
isHealthTarget: _ => false,
|
||||
isOwnedByPlayer: _ => false,
|
||||
name: guid => objects.Get(guid)?.GetAppropriateName(),
|
||||
healthPercent: _ => 0f,
|
||||
hasHealth: _ => false,
|
||||
stackSize: guid => (uint)(objects.Get(guid)?.StackSize ?? 0),
|
||||
sendQueryHealth: _ => { },
|
||||
manaPercent: _ => 0f,
|
||||
sendQueryItemMana: _ => { },
|
||||
datFont: null,
|
||||
splitQuantity: splitQuantity,
|
||||
// REAL subscription this time (C4 used no-op lambdas here).
|
||||
subscribeObjectUpdated: h => objects.ObjectUpdated += h,
|
||||
unsubscribeObjectUpdated: h => objects.ObjectUpdated -= h,
|
||||
isVendorSplitExempt: guid =>
|
||||
vendor.VendorId != 0u
|
||||
&& objects.Get(guid) is { } vendorCandidate
|
||||
&& vendorCandidate.ContainerId == vendor.VendorId
|
||||
&& VendorSplitPolicy.IsSplitExempt(vendorCandidate.Type));
|
||||
|
||||
// G2 root cause: a REAL ACE vendor listing, byte-for-byte. ACE's
|
||||
// Vendor.LoadInventoryItem (Vendor.cs:144-172) builds the browse-list
|
||||
// WorldObject via WorldObjectFactory.CreateNewWorldObject and sets
|
||||
// ONLY wo.VendorShopCreateListStackSize = stackSize ?? -1 (the "how
|
||||
// many available" packed dword — our VendorShopItem.StackSize) —
|
||||
// it NEVER calls wo.SetStackSize(...), so the per-item
|
||||
// PublicWeenieDesc's own conditional StackSize field (our
|
||||
// DescStackSize — retail's pwd._stackSize, what
|
||||
// GameEventApproachVendor.cs:60's SerializeGameDataOnly walks) comes
|
||||
// back null on the real wire. StackSize=100 (packed "100 for sale"),
|
||||
// DescStackSize=null (ACE never sets it) is what a real vendor
|
||||
// listing looks like, NOT the DescStackSize=100 shape used above.
|
||||
vendor.Apply(
|
||||
vendorGuid,
|
||||
new VendorShopProfile(0u, 0u, 0u, false, 1.0f, 1.5f, 0u, 0u, ""),
|
||||
new[]
|
||||
{
|
||||
new VendorShopItem(
|
||||
arrowsGuid, StackSize: 100, WeenieClassId: 5u, Name: "Arrow",
|
||||
ItemType: (uint)ItemType.MissileWeapon, IconId: 200u, Value: 100,
|
||||
DescStackSize: null, PluralName: "Arrows"),
|
||||
});
|
||||
|
||||
// The item is now materialized (ClientObjectTable.Ingest ran inside
|
||||
// VendorState.Apply's Changed dispatch) BEFORE selection, exactly
|
||||
// like a real click on VendorUiController's item list.
|
||||
Assert.NotNull(objects.Get(arrowsGuid));
|
||||
Assert.Equal(100, objects.Get(arrowsGuid)!.StackSize);
|
||||
|
||||
selection.Select(arrowsGuid, SelectionChangeSource.Vendor);
|
||||
|
||||
var slider = Assert.IsType<UiScrollbar>(
|
||||
layout.FindElement(SelectedObjectController.StackSizeSliderId));
|
||||
Assert.True(slider.Visible);
|
||||
Assert.Equal(100u, splitQuantity.Maximum);
|
||||
|
||||
controller.Dispose();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void C4_VendorOwnedSplitExemptStackSelection_MatchesRetailsToolbarPresentation()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -1636,6 +1636,98 @@ public sealed class VendorUiControllerTests
|
|||
Assert.Equal(new[] { "You must empty some slots in your backpack first" }, h.SystemMessages);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// G1 (vendor gate finding): live testing showed "Buy All" false-blocking
|
||||
/// a container purchase while the player visibly had free container
|
||||
/// slots ("3 left"). Root cause: <c>CountPlayerContents</c> classified
|
||||
/// "is this occupied slot a container" from the item's own
|
||||
/// <see cref="ItemType.Container"/> bit OR nonzero
|
||||
/// <c>ItemsCapacity</c>/<c>ContainersCapacity</c> — a LOCAL heuristic —
|
||||
/// instead of retail's actual wire-carried classification
|
||||
/// (<c>ContainerProperties</c>, threaded onto
|
||||
/// <see cref="ClientObject.ContainerTypeHint"/> by every membership
|
||||
/// path). A non-Container-typed object with a stray nonzero capacity
|
||||
/// field (and a wire hint of <c>None</c>) was over-counted as an
|
||||
/// occupied CONTAINER slot, shrinking the free-slot count below the
|
||||
/// real one and false-blocking a purchase the player had room for.
|
||||
/// This test pins a player pack with one such object (armor, hint=None,
|
||||
/// but ItemsCapacity happens to read nonzero) plus 6 free container
|
||||
/// slots out of 7 — buying ONE more container must succeed.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void BuyAllButton_StrayCapacityFieldOnNonContainerItem_DoesNotFalseBlockWithFreeSlots()
|
||||
{
|
||||
var h = new Harness();
|
||||
// AddOrUpdate FIRST (creates the object carrying the stray capacity
|
||||
// field), InitializeInventoryManifest SECOND (updates the SAME
|
||||
// object's placement/hint in place — it does not touch Type/
|
||||
// ItemsCapacity, matching AddOrUpdate's own doc comment: "does NOT
|
||||
// update the container index").
|
||||
h.Objects.AddOrUpdate(new ClientObject
|
||||
{
|
||||
ObjectId = 0x60002001u,
|
||||
Type = ItemType.Armor,
|
||||
ItemsCapacity = 3,
|
||||
});
|
||||
h.Objects.InitializeInventoryManifest(Harness.PlayerGuid, new[]
|
||||
{
|
||||
// Wire truth (ContainerType=0/None): this is NOT a container.
|
||||
// Its Type is Armor (not Container) but it carries a stray
|
||||
// nonzero ItemsCapacity — the old heuristic misread that as
|
||||
// "occupies a container slot."
|
||||
new ContainerContentEntry(0x60002001u, 0u),
|
||||
});
|
||||
// Only 1 container slot total so a miscount of this ONE stray item
|
||||
// as a container (containersUsed 1 instead of 0) actually flips the
|
||||
// guard, instead of being absorbed by the harness's 7-slot default.
|
||||
h.Objects.Get(Harness.PlayerGuid)!.ContainersCapacity = 1;
|
||||
|
||||
h.State.Apply(VendorGuid, Profile(), new[]
|
||||
{
|
||||
new VendorShopItem(ArmorItemGuid, -1, 2u, "Birch Backpack", (uint)ItemType.Container, 200u, 100),
|
||||
});
|
||||
h.AddButton.OnClick!.Invoke();
|
||||
|
||||
h.BuyAllButton.OnClick!.Invoke();
|
||||
|
||||
// 1 capacity - 0 REAL containers used = 1 free -> buying 1 succeeds.
|
||||
Assert.Single(h.BuyAlls);
|
||||
Assert.Empty(h.SystemMessages);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// G1 companion: a REAL side-pack (wire hint ContainerType=1/Container,
|
||||
/// no ItemType.Container bit and no capacity fields populated — e.g. a
|
||||
/// container object seen only via a membership manifest, not its own
|
||||
/// full CreateObject) still correctly consumes a container slot. Proves
|
||||
/// the fix's hint-primary classification isn't just permissive by
|
||||
/// omission — it still catches a real container the OLD Type-bit-only
|
||||
/// fallback would have missed too.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void BuyAllButton_HintOnlyContainer_StillCountsAgainstContainerCapacity()
|
||||
{
|
||||
var h = new Harness();
|
||||
h.Objects.InitializeInventoryManifest(Harness.PlayerGuid, new[]
|
||||
{
|
||||
new ContainerContentEntry(0x60002010u, 1u), // Container, hint-only
|
||||
});
|
||||
|
||||
h.State.Apply(VendorGuid, Profile(), new[]
|
||||
{
|
||||
new VendorShopItem(ArmorItemGuid, -1, 2u, "Birch Backpack", (uint)ItemType.Container, 200u, 100),
|
||||
});
|
||||
h.AddButton.OnClick!.Invoke();
|
||||
h.Objects.Get(Harness.PlayerGuid)!.ContainersCapacity = 1;
|
||||
|
||||
h.BuyAllButton.OnClick!.Invoke();
|
||||
|
||||
// Capacity 1, 1 REAL container already used (via hint) -> 0 free,
|
||||
// buying 1 more must block.
|
||||
Assert.Empty(h.BuyAlls);
|
||||
Assert.Equal(new[] { "You must empty some slots in your backpack first" }, h.SystemMessages);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuyItemButton_BuysTheSelectedStagedItemAndRemovesItFromStagingOnSuccess()
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue