erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(ui): Slice 5.4 review corrections — the dropdown renders from its authored popup, retail cost semantics, auto-select, icon overlays
Some checks are pending
Headless portability / portable-headless (ubuntu-latest) (push) Waiting to run
Details
Headless portability / portable-headless (windows-latest) (push) Waiting to run
Details
Headless portability / linux-graphical (push) Waiting to run
Details
Headless portability / linux-vulkan (push) Waiting to run
Details

Browse source 
All nine review findings closed at root (one sub-item consciously
deferred):

F1 the category dropdown now draws: sprites/fonts wired and the popup
geometry read from the vendor menu's own authored popup LayoutDesc
0x21000043 (root 0x1000034F — correcting the review's 0x1000014F
transcription) per UIElement_Menu::MakePopup (pc:120705); chat's menu
is untouched and its tests prove it. The new test drives selection
through the REAL open/hit path the review flagged as bypassed.
F2+F3 the selected-item cost display ports VendorItemsUI::UpdateItemsUI
verbatim: quantity via the 0xDC41CB0 split-size mask (whole-stack for
ammo, per-unit for groceries/components; mask lives at the toolbar
SEEDING site pc:198784), plural names with retail's
fall-back-to-singular (pc:409056 — correcting the review's "name+s"
guess), full cost sentences with comma grouping and the player's coin
total, and Buy/Add buttons that disable without a selection.
F4 category switches auto-select the first filtered item (pc:201180).
F5 icon underlay/overlay/effects + plural name forwarded from the
already-parsed wire fields through VendorShopItem to the icon
composer. F6 a DIFFERENT vendor opens on its own first category;
same-vendor refresh preserves per the clamp. F7 scroll resets on
rebuild and authored empty slots fill; the right-click examine route
is consciously DEFERRED (shop items are not in ClientObjectTable and
the appraisal panel hard-requires it — documented, not faked).
F8 VendorState.Apply's fanout gets the same per-listener isolation as
Close/Reset. F9 AP-110/AP-161 wording corrected ("quantity-correct
pricing") and AP-161 rewritten to exactly the remaining conscious
gaps.

Clean-room complete solution with the #348 cursor fix in the same
tree: 11,334 passed / 4 skipped / 0 failed.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-07 18:26:17 +02:00
parent 9d3df5f627
commit e602f84be2
6 changed files with 696 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

309
tests/AcDream.App.Tests/UI/Layout/VendorUiControllerTests.cs
View file

@ -3,6 +3,7 @@ using System.Linq;
using AcDream.App.UI;
using AcDream.App.UI.Layout;
using AcDream.Core.Items;
using AcDream.Core.Properties;
namespace AcDream.App.Tests.UI.Layout;
@ -44,15 +45,26 @@ public sealed class VendorUiControllerTests
layout,
new VendorState(),
window,
static (_, _, _, _, _) => 0u);
static (_, _, _, _, _) => 0u,
new ClientObjectTable(),
static () => 0u,
datFont: null,
debugFont: null,
static _ => (0u, 0, 0));
Assert.NotNull(controller);
}
private sealed class Harness
{
// F2/F3: a deterministic non-zero player coin total so the cost-text
// "(you have ...)" tail is assertable.
public const int DefaultPlayerCoinValue = 1500;
private const uint PlayerGuid = 0x50000001u;
public readonly VendorState State = new();
public readonly UiRoot Screen = new() { Width = 800f, Height = 600f };
public readonly ClientObjectTable Objects = new();
public readonly UiItemList ItemList = new();
public readonly UiScrollbar ItemScrollbar = new();
public readonly UiMenu TypeMenu = new();
@ -65,15 +77,27 @@ public sealed class VendorUiControllerTests
public readonly UiElement BuyingPage = new TestElement();
public readonly UiElement SellingPage = new TestElement();
public readonly UiButton CloseButton;
public readonly UiButton BuyButton;
public readonly UiButton AddButton;
public readonly RetailWindowHandle Window;
public readonly VendorUiController Controller;
public Harness()
{
var bundle = new PropertyBundle();
bundle.Ints[(uint)PropertyInt.CoinValue] = DefaultPlayerCoinValue;
Objects.UpsertProperties(PlayerGuid, bundle);
var root = new TestElement { Width = 800f, Height = 110f };
CloseButton = new UiButton(
new ElementInfo { Id = VendorUiController.CloseId, Type = 1 },
static _ => (0u, 0, 0));
BuyButton = new UiButton(
new ElementInfo { Id = VendorUiController.BuyButtonId, Type = 1 },
static _ => (0u, 0, 0));
AddButton = new UiButton(
new ElementInfo { Id = VendorUiController.AddButtonId, Type = 1 },
static _ => (0u, 0, 0));
root.AddChild(CloseButton);
root.AddChild(ItemsTab);
@ -87,6 +111,8 @@ public sealed class VendorUiControllerTests
ItemsPage.AddChild(TypeMenu);
ItemsPage.AddChild(ItemNameText);
ItemsPage.AddChild(ItemCostText);
ItemsPage.AddChild(BuyButton);
ItemsPage.AddChild(AddButton);
var layout = new ImportedLayout(root, new Dictionary<uint, UiElement>
{
@ -102,6 +128,8 @@ public sealed class VendorUiControllerTests
[VendorUiController.TypeFilterMenuId] = TypeMenu,
[VendorUiController.ItemNameTextId] = ItemNameText,
[VendorUiController.ItemCostTextId] = ItemCostText,
[VendorUiController.BuyButtonId] = BuyButton,
[VendorUiController.AddButtonId] = AddButton,
});
Window = RetailWindowFrame.Mount(
@ -121,14 +149,22 @@ public sealed class VendorUiControllerTests
layout,
State,
Window,
static (_, iconId, _, _, _) => iconId != 0u ? iconId + 1000u : 0u)!;
// F5: sum every argument so a test can prove underlay/overlay/
// effects were actually forwarded (iconId-only would silently
// regress to dropping the last three).
static (_, iconId, underlay, overlay, effects) => iconId + underlay + overlay + effects,
Objects,
static () => PlayerGuid,
datFont: null,
debugFont: null,
static _ => (0u, 0, 0))!;
Screen.WindowManager.AttachController(WindowNames.Vendor, Controller);
}
}
private static VendorShopProfile Profile(
float sellRate = 1.5f, uint altCurrency = 0u, string altName = "") =>
new(0u, 0u, 0u, false, 1.0f, sellRate, altCurrency, 0u, altName);
float sellRate = 1.5f, uint altCurrency = 0u, string altName = "", uint altAmount = 0u) =>
new(0u, 0u, 0u, false, 1.0f, sellRate, altCurrency, altAmount, altName);
private static string GetText(UiText text)
=> string.Concat(text.LinesProvider().Select(line => line.Text));
@ -202,13 +238,18 @@ public sealed class VendorUiControllerTests
}
[Fact]
public void SelectingItem_ShowsRetailCorrectPerUnitPrice_ForStackedItem()
public void SelectingItem_NotInSplitExemptMask_ShowsWholeStackPriceAndFallbackPluralName()
{
// F2/F3 (Slice 5.4 review): MissileWeapon (0x100) does NOT intersect
// the 0xDC41CB0 split-exempt mask, so retail prices/names the WHOLE
// stack, not one unit — the review flagged the OLD test (asserting a
// per-unit price + bare "Arrows") as wrong.
var h = new Harness();
h.State.Apply(VendorGuid, Profile(sellRate: 2.0f), new[]
{
// Value=1000 is the STACK-TOTAL wire value for a 100-unit stack;
// per-unit = 1000/100 = 10. SellPrice = ceil(2.0*10*1 - 0.1) = 20
// per-unit = 1000/100 = 10. quantity = DescStackSize = 100 (not
// exempt). SellPrice = ceil(2.0*10*100 - 0.1) = 2000
// (VendorPricing.SellPrice, ShopSystem::SellPrice pc:702107).
new VendorShopItem(
StackedItemGuid, -1, 3u, "Arrows", (uint)ItemType.MissileWeapon, 300u, 1000,
@ -217,17 +258,66 @@ public sealed class VendorUiControllerTests
h.ItemList.GetItem(0)!.Clicked?.Invoke();
Assert.Equal("Arrows", GetText(h.ItemNameText));
Assert.Equal("20", GetText(h.ItemCostText));
// No authored PluralName -> ACCWeenieObject::GetObjectName falls back
// to the singular name UNCHANGED (not an invented "Arrows" + "s").
Assert.Equal("100 Arrows", GetText(h.ItemNameText));
Assert.Equal(
$"cost {2000:N0} (you have {Harness.DefaultPlayerCoinValue:N0})",
GetText(h.ItemCostText));
}
[Fact]
public void SelectingItem_WithAlternateCurrency_AppendsCurrencyName()
public void SelectingItem_InSplitExemptMask_ShowsPerUnitPriceAndSingularName()
{
// F2/F3: Food (0x20) DOES intersect the split-exempt mask, so
// quantity is forced to 1 even though the item has a larger
// authored stack — the mask-member case the review asked for.
var h = new Harness();
h.State.Apply(VendorGuid, Profile(sellRate: 2.0f), new[]
{
// perUnit = 500/50 = 10. quantity forced to 1 (Food is masked).
// SellPrice = ceil(2.0*10*1 - 0.1) = 20.
new VendorShopItem(
FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 500,
DescStackSize: 50),
});
h.ItemList.GetItem(0)!.Clicked?.Invoke();
Assert.Equal("Bread", GetText(h.ItemNameText));
Assert.Equal(
$"costs {20:N0} (you have {Harness.DefaultPlayerCoinValue:N0})",
GetText(h.ItemCostText));
}
[Fact]
public void SelectingItem_WithAuthoredPluralName_UsesItInsteadOfSingular()
{
var h = new Harness();
h.State.Apply(VendorGuid, Profile(), new[]
{
new VendorShopItem(
StackedItemGuid, -1, 3u, "Iron Key", (uint)ItemType.Key, 50u, 100,
DescStackSize: 10, PluralName: "Iron Keys"),
});
h.ItemList.GetItem(0)!.Clicked?.Invoke();
Assert.Equal("10 Iron Keys", GetText(h.ItemNameText));
}
[Fact]
public void SelectingItem_WithAlternateCurrency_ShowsFullRetailCostSentence()
{
// F2/F3: the OLD implementation just appended the currency name after
// a bare number ("50 Trade Notes"). Retail's actual alt-currency
// sentence is a full two-clause sentence with a "you have" tail
// (acclient_2013_pseudo_c.txt:991314), using RAW (non-comma-grouped)
// integers — unlike the primary-currency branch.
var h = new Harness();
h.State.Apply(
VendorGuid,
Profile(sellRate: 1.0f, altCurrency: 0x12345678u, altName: "Trade Notes"),
Profile(sellRate: 1.0f, altCurrency: 0x12345678u, altName: "Trade Notes", altAmount: 12345u),
new[]
{
new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 50),
@ -235,7 +325,52 @@ public sealed class VendorUiControllerTests
h.ItemList.GetItem(0)!.Clicked?.Invoke();
Assert.Equal("50 Trade Notes", GetText(h.ItemCostText));
// Armor (not in the split-exempt mask) with DescStackSize null ->
// PerUnitValue returns Value unchanged (50); quantity = 1 (no
// authored stack) -> SellPrice(50, Armor, 1.0, 1) = ceil(50-0.1) = 50.
Assert.Equal(
"This item costs 50 Trade Notes. You have 12345 Trade Notes.",
GetText(h.ItemCostText));
}
[Fact]
public void NoSelection_DisablesBuyAndAddButtons_SelectionEnablesThem()
{
var h = new Harness();
Assert.False(h.BuyButton.Enabled);
Assert.False(h.AddButton.Enabled);
h.State.Apply(VendorGuid, Profile(), new[]
{
new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500),
});
// F4 auto-selects the sole item on open, so the buttons are already
// enabled — SetState(1), pc:202784-202789.
Assert.True(h.BuyButton.Enabled);
Assert.True(h.AddButton.Enabled);
// Closing clears the selection -> SetState(0xd), pc:202572-202577.
h.State.Close();
Assert.False(h.BuyButton.Enabled);
Assert.False(h.AddButton.Enabled);
}
[Fact]
public void ShopItem_WithIconUnderlayOverlayEffects_ForwardsThemToResolveIcon()
{
// F5: PublicWeenieDescBody already carries these three; the review
// flagged VendorUiController hardcoding 0u/0u/0u instead of
// forwarding item.IconUnderlayId/IconOverlayId/Effects.
var h = new Harness();
var item = new VendorShopItem(
ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, IconId: 200u, Value: 500,
IconUnderlayId: 10u, IconOverlayId: 100u, Effects: 1000u);
h.State.Apply(VendorGuid, Profile(), new[] { item });
// Harness's resolveIcon sums every argument (see its comment) — this
// regresses to 200u alone if the three are dropped again.
Assert.Equal(200u + 10u + 100u + 1000u, h.ItemList.GetItem(0)!.IconTexture);
}
[Fact]
@ -254,6 +389,158 @@ public sealed class VendorUiControllerTests
Assert.Equal(FoodItemGuid, h.ItemList.GetItem(0)!.ItemId);
}
[Fact]
public void SelectingDifferentCategory_AutoSelectsFirstFilteredItem()
{
// F4: VendorItemsUI::UpdateItemsList's tail (pc:201180-201190) —
// after a category switch the first item that passed the filter
// becomes the display selection, not a cleared blank.
var h = new Harness();
var armor = new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500);
var food = new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5);
h.State.Apply(VendorGuid, Profile(), new[] { armor, food });
object? foodPayload = h.TypeMenu.Items.First(i => i.Label == "Food").Payload;
h.TypeMenu.OnSelect!.Invoke(foodPayload);
Assert.Equal(FoodItemGuid, h.ItemList.GetItem(0)!.ItemId);
Assert.True(h.ItemList.GetItem(0)!.Selected);
Assert.NotEqual(string.Empty, GetText(h.ItemNameText));
Assert.NotEqual(string.Empty, GetText(h.ItemCostText));
Assert.True(h.BuyButton.Enabled);
}
[Fact]
public void SelectingDifferentCategory_ResetsListScrollToStart()
{
// F7a: VendorItemsUI::UpdateItemsList's tail (pc:201186-201190) —
// ScrollToShow(m_shopList, 0) fires unconditionally on every rebuild.
var h = new Harness();
var armor = new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500);
var food = new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5);
h.State.Apply(VendorGuid, Profile(), new[] { armor, food });
object? foodPayload = h.TypeMenu.Items.First(i => i.Label == "Food").Payload;
h.TypeMenu.OnSelect!.Invoke(foodPayload);
Assert.Equal(0, h.ItemList.Scroll.ScrollY);
}
[Fact]
public void ItemList_FillsVisibleEmptySlots()
{
// F7b: mirrors ExternalContainerController.ConfigureList's empty-slot
// fill so the authored 22-slot strip doesn't show blank space.
var h = new Harness();
Assert.True(h.ItemList.FillVisibleEmptySlots);
Assert.NotNull(h.ItemList.EmptySlotFactory);
}
[Fact]
public void Opened_WithDifferentVendor_ResetsToFirstPresentCategory_NotThePreviousVendors()
{
// F6: gmVendorUI::OpenVendor flushes sub-UIs when sameVendor==0
// (pc:203664-203667) — a different vendor must not inherit whatever
// category index the LAST vendor happened to have selected.
const uint OtherVendorGuid = 0x70000020u;
const uint OtherArmorGuid = 0x60000201u;
const uint OtherFoodGuid = 0x60000202u;
var h = new Harness();
h.State.Apply(VendorGuid, Profile(), new[]
{
new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500),
new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5),
});
object? foodPayload = h.TypeMenu.Items.First(i => i.Label == "Food").Payload;
h.TypeMenu.OnSelect!.Invoke(foodPayload);
Assert.Equal("Food", h.TypeMenu.Items.Single(i => Equals(i.Payload, h.TypeMenu.Selected)).Label);
// A DIFFERENT vendor, which ALSO has >=2 categories (Armor then
// Food, table order) so a stale numeric index (1) would silently
// "validate" without the fix and land on Food again by coincidence
// rather than by the vendor's own contents.
h.State.Apply(OtherVendorGuid, Profile(), new[]
{
new VendorShopItem(OtherArmorGuid, -1, 4u, "Buckler", (uint)ItemType.Armor, 900u, 40),
new VendorShopItem(OtherFoodGuid, -1, 5u, "Ale", (uint)ItemType.Food, 100u, 3),
});
Assert.Equal("Armor", h.TypeMenu.Items.Single(i => Equals(i.Payload, h.TypeMenu.Selected)).Label);
Assert.Equal(OtherArmorGuid, h.ItemList.GetItem(0)!.ItemId);
}
[Fact]
public void Refreshed_SameVendor_PreservesCategorySelection()
{
// F6 counterpart: gmVendorUI::OpenVendor's sameVendor==1 path does
// NOT flush sub-UIs, so a same-vendor refresh (Slice 6 post-buy/sell)
// must keep the player's chosen category.
var h = new Harness();
var items = new[]
{
new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500),
new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5),
};
h.State.Apply(VendorGuid, Profile(), items);
object? foodPayload = h.TypeMenu.Items.First(i => i.Label == "Food").Payload;
h.TypeMenu.OnSelect!.Invoke(foodPayload);
// SAME vendor id re-approaches -> VendorStateTransitionKind.Refreshed.
h.State.Apply(VendorGuid, Profile(), items);
Assert.Equal("Food", h.TypeMenu.Items.Single(i => Equals(i.Payload, h.TypeMenu.Selected)).Label);
}
[Fact]
public void CategoryMenu_OpensAndSelectsThroughRealHitPath_UsingAuthoredPopupGeometry()
{
// F1: the popup never rendered because SpriteResolve/fonts/sprites
// were never wired, and the geometry was chat's own authored values
// instead of the vendor dropdown's. This drives the SAME UiEvent
// hit-test path UiMenuTests uses (not a direct OnSelect call), and
// reads the geometry back off the live widget instead of hardcoding
// pixel offsets tied to a specific constant set.
var h = new Harness();
h.State.Apply(VendorGuid, Profile(), new[]
{
new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500),
new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5),
});
// Wiring: without these UiMenu.OnDrawOverlay early-returns (nothing
// renders) and the button face never draws either.
Assert.NotNull(h.TypeMenu.SpriteResolve);
Assert.NotEqual(0u, h.TypeMenu.NormalSprite);
Assert.NotEqual(0u, h.TypeMenu.ItemNormalSprite);
Assert.NotEqual(0u, h.TypeMenu.ItemHighlightSprite);
// Geometry from the authored template (LayoutDesc 0x21000043's row
// element 0x10000352 and ListBox 0x10000350), NOT chat's 7/17/191.
Assert.Equal(6, h.TypeMenu.RowsPerColumn);
Assert.Equal(18f, h.TypeMenu.RowHeight);
Assert.Equal(100f, h.TypeMenu.ColumnWidth);
// Open via the real widget event path.
Assert.True(h.TypeMenu.OnEvent(new UiEvent(0, h.TypeMenu, UiEventType.MouseDown, 0, 10, 5)));
// "Food" is the 2nd present category in table order (Armor, then
// Food) -> row index 1, column 0. Derive the click point from the
// widget's own live geometry (mirrors UiMenu.OnEvent's own math)
// rather than a hardcoded pixel constant.
const int border = 5; // RetailChromeSprites.Border (UiMenu's private bevel thickness)
float outerH = h.TypeMenu.RowsPerColumn * h.TypeMenu.RowHeight + 2 * border;
const int targetRow = 1;
float iy = targetRow * h.TypeMenu.RowHeight + h.TypeMenu.RowHeight / 2f;
float ly = iy - outerH + border;
Assert.True(h.TypeMenu.OnEvent(new UiEvent(0, h.TypeMenu, UiEventType.MouseDown, 0, 10, (int)ly)));
Assert.Equal("Food", h.TypeMenu.Items.Single(i => Equals(i.Payload, h.TypeMenu.Selected)).Label);
Assert.Equal(FoodItemGuid, h.ItemList.GetItem(0)!.ItemId);
}
[Fact]
public void CloseButton_RoutesThroughVendorStateClose_NotADirectFieldWrite()
{