From 998ab3f2f092fa2d0bfbe20f69172bdd6900ae95 Mon Sep 17 00:00:00 2001 From: grom Date: Thu, 29 Oct 2026 10:57:44 -0600 Subject: [PATCH] Two one-cause clusters: the pet bar never loaded, the auction window never built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **The pet bar, nine tests.** No script errors, and the shelf simply never drew. PetActionBarFrame had no scripts attached at all: the kit seats a bare stand-in under that name from the managed-positions pass — which it must, since the pass does PetActionBarFrame:IsShown() (UIParent.lua:2065) — and the real frame could never take the name back, a publish to _G being non-overwriting (0x701bd0). So PetActionBar_OnLoad never ran, showgrid stayed nil instead of 0, and ShowPetActionBar's `showgrid == 0` gate refused forever. The kit was loading BonusActionBarFrame.xml, whose ShapeshiftBar_OnLoad calls that pass, *before* PetActionBarFrame.xml. The realm's FrameXML.toc has them at rows 83 and 84, the other way round. Loading them in toc order fixes eight; the ninth asserted stock 1.12's unguarded `Attack ()`, and the realm now wraps that append in `if ( key ~= "" )`, so the client was right. Same shape and the same seat list that cost 137 tests through ChatFrame1. **The auction window, fourteen tests.** The whole Blizzard_AuctionUI load raised: the realm's rewritten CalculateAuctionDeposit is `floor(price * (duration/120) * (1 + (maxStack - stackSize) * 0.05) * 0.025)`, and GetAuctionSellItemInfo answered the reference's six values — 1.12 returns exactly six — so maxStack was nil and the arithmetic raised out of AuctionFrameAuctions_OnLoad. The window never built. It answers eight now: maxStack from the template's stack size, and the link. Then the realm's own CalculateAuctionDeposit replaces the engine binding, as it is meant to, which is why that test's numbers move — its fixture also needed Linen Cloth's real stack of 20, since a missing one reads 1 and makes the factor shrink below 1 for a two-item stack. Also recorded, because it changes what every earlier count in this session meant: `cargo test --workspace` stops at the first failing binary, so the "96 failures" I had been tracking was benilla-app's lib target alone. With --no-fail-fast the real total was 257; these two clusters take it to 243. benilla-ui is fully green (2061), and 80 of the rest are the known benilla-formats data goldens. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- .../src/ui_script/pet_bar_tests.rs | 30 ++++++-- crates/benilla-app/tests/auction_frame.rs | 18 ++++- crates/benilla-ui/src/script/auction.rs | 68 ++++++++++++++++--- crates/benilla-ui/src/script/editbox/tests.rs | 14 ++-- 4 files changed, 108 insertions(+), 22 deletions(-) diff --git a/crates/benilla-app/src/ui_script/pet_bar_tests.rs b/crates/benilla-app/src/ui_script/pet_bar_tests.rs index 22319ead..c581dc1c 100644 --- a/crates/benilla-app/src/ui_script/pet_bar_tests.rs +++ b/crates/benilla-app/src/ui_script/pet_bar_tests.rs @@ -21,7 +21,6 @@ pub(super) fn load_pet_bar(s: &UiScript) { r"Interface\FrameXML\MoneyFrame.xml", "Interface\\FrameXML\\GameTooltip.xml", "Interface\\FrameXML\\ActionBarFrame.xml", - "Interface\\FrameXML\\BonusActionBarFrame.xml", r"Interface\FrameXML\UIPanelTemplates.lua", r"Interface\FrameXML\UIPanelTemplates.xml", r"Interface\FrameXML\OptionsFrameTemplates.xml", @@ -33,7 +32,19 @@ pub(super) fn load_pet_bar(s: &UiScript) { "KeyBindingsPage.xml", "OptionsFrame.xml", "Interface\\FrameXML\\MultiActionBars.xml", + // **Toc order, and it is load-bearing.** `PetActionBarFrame.xml` is row 83 of the realm's + // FrameXML.toc and `BonusActionBarFrame.xml` is row 84. Loaded the other way round, the + // managed-positions pass runs first — `ShapeshiftBar_OnLoad` calls it + // (`BonusActionBarFrame.lua:109` → `:157`) — and the kit seats a bare stand-in under the + // name `PetActionBarFrame`, because that pass does `PetActionBarFrame:IsShown()` + // (`UIParent.lua:2065`) and a nil would raise. A frame's publish to `_G` is + // non-overwriting (`0x701bd0`), so the real frame could never take the name back: it + // loaded with no scripts attached, `PetActionBar_OnLoad` never ran, `showgrid` stayed nil + // instead of 0, and `ShowPetActionBar`'s `showgrid == 0` gate refused forever. Nine tests, + // no script errors, and the shelf simply never drew — the same shape, and the same seat + // list, that cost 137 tests through `ChatFrame1`. "Interface\\FrameXML\\PetActionBarFrame.xml", + "Interface\\FrameXML\\BonusActionBarFrame.xml", "Interface\\FrameXML\\UIMenu.xml", "Interface\\FrameXML\\ChatFrame.xml", "Interface\\FrameXML\\FloatingChatFrame.xml", @@ -638,10 +649,17 @@ fn token_tooltips_name_their_binding_and_pet_spells_do_not() { assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } -/// The append is unguarded and `GetBindingText` answers "" for a nil key (UIParent.lua:1819-1821), -/// so an unbound token shows empty parentheses. Stock binds CTRL-1..CTRL-0, so this unbinds one. +/// An unbound token row shows **no parentheses**, because the realm guards the append. +/// +/// Stock 1.12 concatenated unconditionally and `GetBindingText` answers `""` for a nil key +/// (`UIParent.lua:1819-1821`), so an unbound row there read `Attack ()`. The realm's +/// `PetActionButton_SetHotkeys` neighbour now wraps it — `if ( key ~= "" ) then ... AppendText` +/// (`PetActionBarFrame.lua:290`) — so the empty pair never gets written. This test asserted the +/// older shape and was red for that reason alone; the client was right. +/// +/// Stock binds CTRL-1..CTRL-0, so this unbinds one. #[test] -fn an_unbound_token_row_renders_the_references_empty_parentheses() { +fn an_unbound_token_row_renders_no_parentheses_at_all() { benilla_formats::wow_data_or_skip!(); let mut s = hovered_pet_bar(); s.run(r#"SetBinding("CTRL-1")"#).unwrap(); @@ -656,8 +674,8 @@ fn an_unbound_token_row_renders_the_references_empty_parentheses() { s.resolve(); assert_eq!( tooltip_line1(&s), - "Attack|cffffd200 ()|r", - "unbound: the parentheses stay, empty — the ref's own unguarded concatenation" + "Attack", + "unbound: the realm's `if ( key ~= \"\" )` guard means no pair is appended at all" ); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } diff --git a/crates/benilla-app/tests/auction_frame.rs b/crates/benilla-app/tests/auction_frame.rs index 7198b73c..ad34bb7b 100644 --- a/crates/benilla-app/tests/auction_frame.rs +++ b/crates/benilla-app/tests/auction_frame.rs @@ -736,6 +736,11 @@ fn the_create_gate_and_the_deposit() { benilla_ui::script::ItemTemplateView { name: "Linen Cloth".into(), sell_price: 25, + // Linen Cloth's real stack. It matters now: the realm's own + // `CalculateAuctionDeposit` scales by `(maxStack - stackSize)` + // (`Blizzard_AuctionUI.lua:1123`), and a template with no stack size reads 1, which + // makes the factor *shrink* below 1 for a two-item stack. + stackable: 20, ..Default::default() }, ); @@ -785,11 +790,20 @@ fn the_create_gate_and_the_deposit() { .unwrap(), "an item, a 1g start price and no buyout is a valid auction" ); - // 5% of the stack's vendor value (2 x 25c = 50c) floors to 2c, per two-hour unit: 24h is 24c. + // **The realm's arithmetic, not ours.** `Blizzard_AuctionUI.lua:1118` defines its own + // `CalculateAuctionDeposit` and so replaces the engine binding the moment that file loads: + // floor(price * (duration/120) * (1 + (maxStack - stackSize) * 0.05) * 0.025) + // with price the stack's vendor value (2 x 25c = 50c), maxStack 20 and stackSize 2. + // 24h: 50 * 12 * 1.9 * 0.025 = 28.5 -> 28. 2h: 50 * 1 * 1.9 * 0.025 = 2.375 -> 2. + // + // It used to read 24 and 2, which was the engine's own `deposit_for`, because the realm's + // file **never loaded**: its `AuctionFrameAuctions_OnLoad` raised on a nil `maxStack` — + // `GetAuctionSellItemInfo` answered the reference's six values and that formula wants eight. + // The raise took the whole Blizzard_AuctionUI load with it, and 14 of this file's tests. assert_eq!( s.eval::("return CalculateAuctionDeposit(1440)") .unwrap(), - 24 + 28 ); assert_eq!( s.eval::("return CalculateAuctionDeposit(120)") diff --git a/crates/benilla-ui/src/script/auction.rs b/crates/benilla-ui/src/script/auction.rs index 5c466cdb..e19853fe 100644 --- a/crates/benilla-ui/src/script/auction.rs +++ b/crates/benilla-ui/src/script/auction.rs @@ -248,9 +248,16 @@ fn row_at(model: &Model, kind: &str, index: usize) -> Option { .cloned() } -/// The sell slot's `(name, texture, count, quality, canUse, stackValue)`, `stackValue` being the -/// vendor price times the stack: the base of the suggested opening price and the deposit. -fn sell_item_info(model: &Model) -> Option<(String, Option, u32, i64, bool, u32)> { +/// The sell slot's `(name, texture, count, quality, canUse, stackValue, maxStack, link)`. +/// +/// `stackValue` is the vendor price times the stack: the base of the suggested opening price and +/// the deposit. The last two are **beyond 1.12** — the reference returns exactly six +/// (`reference/1.12-shapes.tsv`) — and they are here because the realm's own +/// `Blizzard_AuctionUI.lua` reads them: its rewritten `CalculateAuctionDeposit` is +/// `floor(price * (duration/120) * (1 + (maxStack - stackSize) * 0.05) * 0.025)` (l.1123), so a +/// nil `maxStack` is arithmetic on nil. That raise came out of `AuctionFrameAuctions_OnLoad`, so +/// it took the **whole** Blizzard_AuctionUI load with it — the window never built at all. +fn sell_item_info(model: &Model) -> Option { let it = model.auction_sell_item.as_ref()?; // The real stack size, not the split-carry field: the deposit is per stack. let count = cursor::held_count(model, it); @@ -259,14 +266,33 @@ fn sell_item_info(model: &Model) -> Option<(String, Option, u32, i64, bo let quality = it.quality.map_or(-1, i64::from); let sell_price = template.map_or(0, |v| v.sell_price); let can_use = super::item_stats::item_usable_by_id(model, it.item_id); - Some(( + // `GetItemInfo`'s `itemStackCount`, which is what a per-stack deposit scales against; 1 when + // the template has not landed, so the realm's formula degenerates to the un-stacked price + // rather than to nil. + let max_stack = template.map_or(1, |v| i64::from(v.stackable.max(1))); + Some(SellItemInfo { name, - it.texture.clone(), + texture: it.texture.clone(), count, quality, can_use, - sell_price.saturating_mul(count), - )) + stack_value: sell_price.saturating_mul(count), + max_stack, + link: it.link.clone(), + }) +} + +/// [`sell_item_info`]'s answer, which is `GetAuctionSellItemInfo`'s return run in order. +struct SellItemInfo { + name: String, + texture: Option, + count: u32, + quality: i64, + can_use: bool, + /// The vendor price times the stack. + stack_value: u32, + max_stack: i64, + link: Option, } /// The reference client's deposit, `floor(rate × stackValue / 100) × floor(minutes / 120)`: the @@ -682,12 +708,14 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { lua.create_function(|lua, minutes: u32| { let model = lua.app_data_ref::().expect("model app_data"); let rate = model.auction.as_ref().map_or(0, |a| a.deposit_percent); - let stack_value = sell_item_info(&model).map_or(0, |(_, _, _, _, _, v)| v); + let stack_value = sell_item_info(&model).map_or(0, |i| i.stack_value); Ok(i64::from(deposit_for(rate, stack_value, minutes))) })?, )?; - // GetAuctionSellItemInfo() → name, texture, count, quality, canUse, price: six values always. + // GetAuctionSellItemInfo() → name, texture, count, quality, canUse, price, maxStack, link. + // The reference returns the first six and always six; the last two are the realm's, and its + // own auction window does not load without them — see [`sell_item_info`]. // An empty slot answers count 1 and quality -1: the stock Lua reads `count > 1` unguarded. g.set( "GetAuctionSellItemInfo", @@ -696,7 +724,17 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { let model = lua.app_data_ref::().expect("model app_data"); sell_item_info(&model) }; - let Some((name, texture, count, quality, can_use, price)) = info else { + let Some(SellItemInfo { + name, + texture, + count, + quality, + can_use, + stack_value: price, + max_stack, + link, + }) = info + else { return Ok(MultiValue::from_vec(vec![ Value::Nil, Value::Nil, @@ -704,6 +742,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Value::Integer(-1), Value::Nil, Value::Integer(0), + // A 1 rather than a nil for the same reason the count is: the realm's deposit + // formula does arithmetic on it with no guard, and an empty slot is a state + // its radio buttons pass through on load. + Value::Integer(1), + Value::Nil, ])); }; Ok(MultiValue::from_vec(vec![ @@ -720,6 +763,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Value::Integer(quality), flag(can_use), Value::Integer(i64::from(price)), + Value::Integer(max_stack), + match &link { + Some(l) => Value::String(lua.create_string(l)?), + None => Value::Nil, + }, ])) })?, )?; diff --git a/crates/benilla-ui/src/script/editbox/tests.rs b/crates/benilla-ui/src/script/editbox/tests.rs index 8aedec31..8081bdee 100644 --- a/crates/benilla-ui/src/script/editbox/tests.rs +++ b/crates/benilla-ui/src/script/editbox/tests.rs @@ -1379,15 +1379,21 @@ fn an_editbox_is_born_with_the_ctors_five_regions_ahead_of_its_authored_ones() { "typed", "region 1 is the ctor's embedded text FontString" ); - // 2..5, the selection quads and the caret, carry no art: benilla paints both host-side. + // 2..5, the selection quads and the caret, carry no *file*: benilla paints both host-side. + // They answer `GetTexture` with `"Solid Texture"` all the same, because the reference builds + // them as colour-set textures and that is what it answers for one (the string is in + // `WoW.exe`). Not cosmetic: the idiom for finding a frame's art is to walk `GetRegions` and + // filter on `GetTexture`, and a scanner reads **nil** as "no name to match, take this one" — + // pfUI's `GetNoNameObject` does exactly that, which is how it came to skin a money box's + // caret as its gold coin and collapse the trade window's money row. assert!( s.eval::( "local a,b,c,d,e = Box:GetRegions() \ - return b:GetTexture() == nil and c:GetTexture() == nil \ - and d:GetTexture() == nil and e:GetTexture() == nil" + return b:GetTexture() == 'Solid Texture' and c:GetTexture() == 'Solid Texture' \ + and d:GetTexture() == 'Solid Texture' and e:GetTexture() == 'Solid Texture'" ) .unwrap(), - "the selection trio and the caret are blank quads" + "the selection trio and the caret are solid quads, and say so" ); }