diff --git a/crates/benilla-app/assets/ui/ErrorsFrame.xml b/crates/benilla-app/assets/ui/ErrorsFrame.xml index 2f1ddae1..af5c4c0d 100644 --- a/crates/benilla-app/assets/ui/ErrorsFrame.xml +++ b/crates/benilla-app/assets/ui/ErrorsFrame.xml @@ -65,4 +65,39 @@ Fonts.xml:258), which is the face the hand-rolled version used too. --> + + + + + + + + + + + diff --git a/crates/benilla-app/assets/ui/UiPanels.xml b/crates/benilla-app/assets/ui/UiPanels.xml index 4d18b01a..0a2f6c22 100644 --- a/crates/benilla-app/assets/ui/UiPanels.xml +++ b/crates/benilla-app/assets/ui/UiPanels.xml @@ -1277,6 +1277,30 @@ -- isDisabled/SetDisabledTabState arm (no window has a disabled tab yet) and the SelectTab -- tab-tooltip hide (our tabs set no tooltip). First consumer: the merchant's -- Merchant/Buyback pair. + -- The kit's XML-facing entry point (ref l.2-4), and the one member of it this client never + -- had. Not an omission anyone decided: our own windows wire each tab's OnClick to their own + -- handler (the merchant's Merchant/Buyback pair, CharacterFrame's pages), so nothing here + -- ever reached for the generic one. An ADDON does the opposite — it puts the reference's + -- name straight in its and lets the kit do the work: + -- + -- Enchantrix EnchantrixBarker.xml:61 (Enchantrix_BarkerOptions_Frame) + -- Outfitter Outfitter.xml:7 (OutfitterFrame) + -- SimpleActionSets SimpleActionSets.xml:342 (this:GetParent()) + -- TheoryCraft TheoryCraft.xml:163 (TheoryCraft) + -- + -- Four addons, four separate files, so 1207's one-library caveat does not apply. A raw grep + -- says FIVE: QuestItemTemplates.xml:17 has the call commented out. Reading the line is what + -- tells those apart, and the survey's own count of 4 was the right one. + -- + -- Missing, every one of them raises on each click of the tab, so the addon's options window + -- stays on whatever page it opened with and cannot be navigated at all. `this` is the + -- clicked tab button, whose id is its slot in the row; the frame is passed in separately + -- because the tab's parent is not always the frame owning the row — SimpleActionSets is the + -- case that proves it, passing `this:GetParent()` explicitly. + function PanelTemplates_Tab_OnClick(frame) + PanelTemplates_SetTab(frame, this:GetID()) + end + function PanelTemplates_SetNumTabs(frame, numTabs) frame.numTabs = numTabs end diff --git a/crates/benilla-app/assets/ui/UnitFrames.xml b/crates/benilla-app/assets/ui/UnitFrames.xml index b1f3dd41..ee04fe8a 100644 --- a/crates/benilla-app/assets/ui/UnitFrames.xml +++ b/crates/benilla-app/assets/ui/UnitFrames.xml @@ -112,6 +112,41 @@ HAPPINESS = { r = 0.0, g = 1.0, b = 1.0 }, } + -- ── The raid-target mark atlas (ref TargetFrame.lua l.460-463 + l.475-484) ────────────── + -- `Interface\TargetingFrame\UI-RaidTargetingIcons` is one 256x256 sheet of 4x4 64px cells, + -- in mark order: star, circle, diamond, triangle, moon, square, cross, skull. The helper + -- turns a 1..8 mark index into that cell's texcoords on a CALLER-SUPPLIED texture — it owns + -- no frame and reads no unit, which is why an addon can hand it a texture of its own. + -- + -- Three corpus addons do, in two codebases (`Optional` and `oRA2` ship the same two files): + -- CustomNameplates options.lua:363 (its options preview) + -- Optional / oRA2 MainTank.lua:1216, PlayerTarget.lua:1472 + -- We answer `GetRaidTargetIndex(unit)` already (0434 §6); this is the other half — what an + -- addon DRAWS with that number — and without it those lines raise while redrawing a mark. + -- + -- TRANSCRIBED VERBATIM, INCLUDING THE REFERENCE'S OWN SLIP: the row is derived with + -- `/ RAID_TARGET_TEXTURE_ROWS` while the column uses `mod(.., RAID_TARGET_TEXTURE_COLUMNS)`. + -- Rows and columns are both 4, so the two agree for every index this sheet holds and the + -- output is right; "fixing" it to divide by COLUMNS computes the same coords today and would + -- quietly diverge from the client the day a caller changes a constant. These are globals in + -- the reference for exactly that reason — addons read them, and two of ours are addons that + -- draw their own marks. + RAID_TARGET_ICON_DIMENSION = 64 + RAID_TARGET_TEXTURE_DIMENSION = 256 + RAID_TARGET_TEXTURE_COLUMNS = 4 + RAID_TARGET_TEXTURE_ROWS = 4 + + function SetRaidTargetIconTexture(texture, raidTargetIconIndex) + raidTargetIconIndex = raidTargetIconIndex - 1 + local left, right, top, bottom + local coordIncrement = RAID_TARGET_ICON_DIMENSION / RAID_TARGET_TEXTURE_DIMENSION + left = mod(raidTargetIconIndex, RAID_TARGET_TEXTURE_COLUMNS) * coordIncrement + right = left + coordIncrement + top = floor(raidTargetIconIndex / RAID_TARGET_TEXTURE_ROWS) * coordIncrement + bottom = top + coordIncrement + texture:SetTexCoord(left, right, top, bottom) + end + -- The client's UnitReactionColor table (ref-TargetFrame.lua l.6-14), indexed by -- UnitReaction 1..8 (hated..exalted): 1-2 red, 3 orange, 4 neutral yellow, 5-7 green. The -- name-plate reaction tint (TargetFrame_CheckFaction) reads it — this is what makes a neutral @@ -1056,7 +1091,29 @@ of its own layer — the ref hides the number while resting by putting it in a LOWER layer than the badge. Shipped in OVERLAY by 1082, the number rode ON the badge (director report, 2026-08-07); decision 1093. --> - + diff --git a/crates/benilla-app/examples/addon_harness.rs b/crates/benilla-app/examples/addon_harness.rs index 21d05311..cf3d1bab 100644 --- a/crates/benilla-app/examples/addon_harness.rs +++ b/crates/benilla-app/examples/addon_harness.rs @@ -1,7 +1,7 @@ //! `addon_harness` — load a folder of addons, one per VM, and print what happened. //! //! ```text -//! cargo run -q -p benilla-app --example addon_harness -- [--verbose] [--why ] [--status ] [--diff ] +//! cargo run -q -p benilla-app --example addon_harness -- [--verbose] [--why ] [--deep [n]] [--status ] [--diff ] //! ``` //! //! The instrument decision 1188 phase 6 asks for: *"which addons work" is a number that can be @@ -17,6 +17,42 @@ use benilla_app::addon_harness; /// failure to its call site needed the frames below it, and the addon/file/line only appear there. const WHY_TRACEBACK_LINES: usize = 8; +/// Print a ranked demand table — and **say what was dropped**. +/// +/// Every one of these lists is a queue, and each was printed as a silent top-N. A silent cut reads +/// as "that is the whole list", and this arc has one expensive instance of exactly that: the +/// "a reference frame we do not build" class was swept, found clean, and recorded CLOSED — while +/// `ShapeshiftBarLeft` sat below the cut with two addons behind it, and stayed there until an +/// addon's first error named it (1219's class, first regions). The sweep was honest; the list it +/// swept was not complete and did not say so. +/// +/// So the tail is now stated: how many rows, and how many addon-mentions, were not shown. The +/// caller keeps its own `take` — this only refuses to hide the remainder. +/// +/// `--deep [n]` overrides every caller's `take` (see [`DEEP`]). 1242 made the cut VISIBLE and +/// rejected raising it — an unreadable report helps nobody. But "visible" only tells a sweep that +/// rows exist; it still cannot read them, and `--why` opens one name at a time, which is no way to +/// walk 1930 of them. So the cut stays where it is for the report, and a sweep asks for the rest. +fn ranked(rows: Vec<(String, usize)>, take: usize) { + let take = DEEP.get().copied().flatten().unwrap_or(take); + let total = rows.len(); + for (name, count) in rows.iter().take(take) { + println!(" {count:>4} {name}"); + } + if total > take { + let tail: usize = rows[take..].iter().map(|(_, c)| c).sum(); + println!( + " …{} more rows below the cut ({tail} addon-mentions) — TRUNCATED, not exhausted; \ + `--why ` opens any row.", + total - take + ); + } +} + +/// The `--deep` override, set once in `main`. `Some(n)` shows `n` rows of EVERY ranked list; +/// `--deep` with no number means all of them. Absent, each list keeps the `take` its caller chose. +static DEEP: std::sync::OnceLock> = std::sync::OnceLock::new(); + fn main() { let mut args = std::env::args().skip(1); let Some(root) = args.next() else { @@ -65,6 +101,13 @@ fn main() { .position(|a| a == "--diff") .and_then(|i| rest.get(i + 1)) .cloned(); + // `--deep [n]`: open the tails. A bare `--deep` (or one followed by the next flag) means all. + let deep = rest.iter().position(|a| a == "--deep").map(|i| { + rest.get(i + 1) + .and_then(|n| n.parse::().ok()) + .unwrap_or(usize::MAX) + }); + let _ = DEEP.set(deep); let root = std::path::PathBuf::from(root); let reports = addon_harness::survey(&root); @@ -322,9 +365,7 @@ fn main() { let templates = addon_harness::template_demand(&reports); if !templates.is_empty() { println!("\n most-wanted missing TEMPLATES (addons naming each in CreateFrame):"); - for (name, count) in templates.into_iter().take(12) { - println!(" {count:>4} {name}"); - } + ranked(templates, 12); } // The same question over the OTHER axis, and the one that actually moves the headline: a @@ -336,9 +377,7 @@ fn main() { let inherits = addon_harness::inherits_demand(&reports); if !inherits.is_empty() { println!("\n most-wanted missing TEMPLATES (addons naming each in an XML inherits=):"); - for (name, count) in inherits.into_iter().take(12) { - println!(" {count:>4} {name}"); - } + ranked(inherits, 12); } // Frames and tables, ranked separately: a missing function is a Rust verb to write, a missing @@ -347,9 +386,7 @@ fn main() { let tables = addon_harness::table_demand(&reports); if !tables.is_empty() { println!("\n most-wanted missing FRAMES/TABLES (addons indexing each):"); - for (name, count) in tables.into_iter().take(16) { - println!(" {count:>4} {name}"); - } + ranked(tables, 16); } // **Widget METHODS** — the third queue, and the one this report was blind to while the arc @@ -379,9 +416,7 @@ fn main() { " · a third-party library's own method on its own object is not ours to write." ); println!(" Open the call site before ranking it (decision 1240)."); - for (name, count) in methods.into_iter().take(16) { - println!(" {count:>4} {name}"); - } + ranked(methods, 16); } // **Per KIND** — the question the table above structurally cannot ask. It resolves a name @@ -397,9 +432,7 @@ fn main() { "\n most-wanted missing METHODS BY KIND (receiver typed from the call site; \"on X\" \ = the sibling that does answer it):" ); - for (name, count) in by_kind.into_iter().take(16) { - println!(" {count:>4} {name}"); - } + ranked(by_kind, 16); } // ...and the residue: a receiver nothing could type, on a name whose answer DEPENDS on the @@ -415,9 +448,7 @@ fn main() { is the AddMessage shape and one every kind but the two regions answers is not):", ambiguous.len() ); - for (name, count) in ambiguous.into_iter().take(12) { - println!(" {count:>4} {name}"); - } + ranked(ambiguous, 12); } // The other half of the same scan, and never merged into it: methods addons call **behind a @@ -428,15 +459,11 @@ fn main() { let optional = addon_harness::optional_method_demand(&reports); if !optional.is_empty() { println!("\n ...and methods they FEATURE-TEST and work around (not blockers):"); - for (name, count) in optional.into_iter().take(8) { - println!(" {count:>4} {name}"); - } + ranked(optional, 8); } println!("\n most-wanted missing globals (addons wanting each):"); - for (name, count) in addon_harness::demand(&reports).into_iter().take(30) { - println!(" {count:>4} {name}"); - } + ranked(addon_harness::demand(&reports), 30); if let Some(pattern) = &why { let hits = addon_harness::blocked_by(&reports, pattern); diff --git a/crates/benilla-app/src/addon_harness/mod.rs b/crates/benilla-app/src/addon_harness/mod.rs index 4353dd82..f442bb68 100644 --- a/crates/benilla-app/src/addon_harness/mod.rs +++ b/crates/benilla-app/src/addon_harness/mod.rs @@ -1323,6 +1323,43 @@ fn seat_a_session(script: &mut UiScript) { // resembling a session anyone has, and every row it lights up is one nobody can attribute. Two // tabs and four spells is a Tuesday. `Attack` is slot 1 because it is on every warrior's, and // three corpus addons look for exactly that name. + // EQUIPPED GEAR. Every `GetInventoryItem*("player", slot)` answered the empty shape, and a + // level-60 with no equipment at all is a state no character reaches — the same argument the + // spellbook and the backpack landed on. + // + // Three slots, not nineteen: head, chest and main hand. A fully-geared doll would be a state a + // real session presents, but it would also light up every path at once, and 1209's rule is + // that a row nobody can attribute is worth less than one nobody lit. Three is enough for an + // addon that WALKS the slots to reach real items, empty ones, and the ammo slot's absence. + { + let mut slots: benilla_ui::script::InventorySlots = Default::default(); + slots[1] = Some(equip_slot(12640, "Lionheart Helm")); + slots[5] = Some(equip_slot(11726, "Bloodmail Hauberk")); + slots[16] = Some(equip_slot(871, "Flurry Axe")); + script.set_inventory_slots(slots); + } + // THE BACKPACK. `GetContainerNumSlots(0)` answered 0 — and a character with no backpack has + // never existed: bag 0 is 16 slots from level 1, before a single bag is bought. Same argument + // as the spellbook above and the nil `UnitFactionGroup` below it, and the same expectation: + // seating a state every character is in should LOSE rows, because a path nothing walked is a + // path nothing tested. + // + // Backpack only, and deliberately: bags 1..4 are equipped bags, which a fresh character does + // NOT have, so seating them would manufacture a state rather than expose one. Two items in + // sixteen slots — the rest empty, which is itself a shape bag addons must handle. + { + let mut slots = std::collections::HashMap::new(); + slots.insert(1, bag_slot(6948, "Hearthstone", 1)); + slots.insert(5, bag_slot(2589, "Linen Cloth", 12)); + script.set_container( + 0, + Some(benilla_ui::script::ContainerState { + name: Some("Backpack".into()), + num_slots: 16, + slots, + }), + ); + } script.set_spellbook(benilla_ui::script::SpellBookState { tabs: vec![ benilla_ui::script::SpellTabView { @@ -1347,6 +1384,36 @@ fn seat_a_session(script: &mut UiScript) { }); } +/// One seated equipment slot — full durability, so no alert region lights up. +fn equip_slot(item_id: u32, name: &str) -> benilla_ui::script::InvSlotView { + benilla_ui::script::InvSlotView { + item_id, + icon: Some("Interface\\Icons\\INV_Misc_QuestionMark".into()), + count: 1, + quality: 2, + name: Some(name.to_string()), + // FULL durability on purpose. The setter recomputes the eleven alert statuses and fires + // UPDATE_INVENTORY_ALERTS, so a worn item would light DurabilityFrame's regions — a + // VISIBLE change, and this fixture's job is to present a plausible session, not to drive + // the alert law. Undamaged gear is the ordinary case anyway. + durability: Some((100, 100)), + link: Some(format!("|cff1eff00|Hitem:{item_id}:0:0:0|h[{name}]|h|r")), + ..Default::default() + } +} + +/// One seated backpack slot. +fn bag_slot(item_id: u32, name: &str, count: u32) -> benilla_ui::script::ContainerSlot { + benilla_ui::script::ContainerSlot { + texture: Some("Interface\\Icons\\INV_Misc_QuestionMark".into()), + count, + quality: Some(1), + item_id, + link: Some(format!("|cffffffff|Hitem:{item_id}:0:0:0|h[{name}]|h|r")), + ..Default::default() + } +} + /// One seated spellbook slot — the fields a book reader actually reads, defaulted otherwise. fn spell_slot(spell_id: u32, name: &str, rank: Option<&str>) -> benilla_ui::script::SpellSlotView { benilla_ui::script::SpellSlotView { @@ -3538,6 +3605,79 @@ mod dependency_tests { script } + /// **The session fixture is LIVE**, asserted through the same Lua surface an addon sees. + /// + /// A fixture that silently fails to seat reads EXACTLY like a fixture that exposed nothing — + /// both are "net +0" — and this arc has already been fooled once by a measurement that + /// returned an empty answer for a tooling reason (1242's note on `grep`). The backpack seat + /// produced no corpus delta at all; this is what makes that a finding rather than a + /// possibility. + /// + /// It is also a guard: delete a seat and this fails, instead of the columns quietly shifting + /// and the next A/B attributing the move to whatever landed beside it. + #[test] + fn the_seated_session_is_visible_from_lua() { + let s = seated_vm(); + + // The backpack — 16 slots from level 1, two of them filled. Exposed nothing, verifiably. + assert_eq!(s.eval::("return GetContainerNumSlots(0)").unwrap(), 16); + assert_eq!( + s.eval::("return GetBagName(0)").unwrap(), + "Backpack" + ); + assert_eq!( + s.eval::("local _, c = GetContainerItemInfo(0, 5) return c") + .unwrap(), + 12, + "the Linen Cloth stack" + ); + // Bags 1..4 are deliberately NOT seated: a fresh character has no equipped bags, and + // seating them would manufacture a state rather than expose one. + assert_eq!(s.eval::("return GetContainerNumSlots(1)").unwrap(), 0); + + // Equipped gear — head, chest, main hand, with the rest empty. Also exposed nothing. + assert_eq!( + s.eval::(r#"return GetInventoryItemTexture("player", 16)"#) + .unwrap(), + "Interface\\Icons\\INV_Misc_QuestionMark" + ); + assert!( + s.eval::(r#"return GetInventoryItemLink("player", 5)"#) + .unwrap() + .contains("Bloodmail Hauberk"), + "the chest slot's link" + ); + assert_eq!( + s.eval::(r#"return GetInventoryItemQuality("player", 1)"#) + .unwrap(), + 2 + ); + // An EMPTY slot still answers the absent shape — the case an addon walking 1..19 hits most. + assert!(s + .eval::(r#"return GetInventoryItemLink("player", 10) == nil"#) + .unwrap()); + // The slot ids come from the same table `GetInventorySlotInfo` serves, so a walk that + // resolves names to ids lands on the seated items rather than past them. + assert_eq!( + s.eval::(r#"return GetInventorySlotInfo("MainHandSlot")"#) + .unwrap(), + 16 + ); + + // The spellbook, as the positive control — this seat DID move the columns (+2). + assert_eq!( + s.eval::(r#"return GetSpellName(1, "spell")"#) + .unwrap(), + "Attack" + ); + assert_eq!( + s.eval::(r#"local _, r = GetSpellName(1, "spell") return r"#) + .unwrap(), + "", + "and its rank is the empty string, not nil" + ); + } + fn wanted(names: &[&str]) -> BTreeSet { names.iter().map(|n| (*n).to_string()).collect() } diff --git a/crates/benilla-app/src/ui_script/character_tests.rs b/crates/benilla-app/src/ui_script/character_tests.rs index 98366154..73725d70 100644 --- a/crates/benilla-app/src/ui_script/character_tests.rs +++ b/crates/benilla-app/src/ui_script/character_tests.rs @@ -1013,3 +1013,68 @@ fn a_keybind_page_switch_moves_the_tab_row_with_it() { assert!(shown(&mut s, "PaperDollFrame"), "…or change the page"); assert!(s.errors().is_empty(), "no handler errors: {:?}", s.errors()); } + +/// **The tab kit's XML-facing entry point, driven the way an addon drives it.** +/// +/// Four corpus addons (Enchantrix, Outfitter, SimpleActionSets, TheoryCraft) put +/// `PanelTemplates_Tab_OnClick()` straight into a tab's `` and let the kit do the +/// rest; this client had every other member of the kit and not that one, because our own windows +/// wire their tabs to their own handlers and never reached for the generic entry point. +/// +/// **It is built on a row of this test's own, and that is the finding, not a convenience.** The +/// reference's tab buttons carry `id="1".."4"` (ref `CharacterFrame.xml:79-133`) and ours carry +/// none: our row is id-based through `BenillaCharacterFrameTab_OnClick(id)`, which closes over the +/// number instead of reading it off the widget. So `CharacterFrameTab2:GetID()` is **0** here, and +/// driving OUR row through the generic entry point would select tab 0 — correct code meeting a +/// window that does not obey the contract it reads. An addon's own tabs do carry ids, which is why +/// the four callers above work; this builds a row that obeys the reference's contract and drives +/// that. +/// +/// The `this`/frame split is the whole reason the function exists: `this` is the clicked TAB and +/// the owning FRAME is a separate argument, which is the shape `SimpleActionSets.xml:342` relies on +/// when it passes `this:GetParent()`. +#[test] +fn an_addons_tab_click_selects_through_the_generic_entry_point() { + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + load_xml(&s, "Fonts.xml"); + load_xml(&s, "UiPanels.xml"); + load_xml(&s, "UIParent.xml"); + + // A conforming row: tabs named `Tab1..N` (what `PanelTemplates_UpdateTabs` getglobals) + // and each carrying its own id, exactly as an addon's XML declares them. + s.run( + r#" + TabKitFrame = CreateFrame("Frame", "TabKitFrame", UIParent) + PanelTemplates_SetNumTabs(TabKitFrame, 2) + for i = 1, 2 do + local t = CreateFrame("Button", "TabKitFrameTab" .. i, TabKitFrame, + "CharacterFrameTabButtonTemplate") + t:SetID(i) + end + PanelTemplates_SetTab(TabKitFrame, 1) + "#, + ) + .unwrap(); + let selected = |s: &mut UiScript| { + s.eval::("return PanelTemplates_GetSelectedTab(TabKitFrame)") + .unwrap() + }; + assert_eq!(selected(&mut s), 1); + + // The addon idiom, verbatim: `this` is the tab, the frame is the argument. + s.run("this = TabKitFrameTab2; PanelTemplates_Tab_OnClick(TabKitFrame); this = nil") + .unwrap(); + assert_eq!( + selected(&mut s), + 2, + "the kit takes the tab's OWN id from `this`, not from the frame it was handed" + ); + // The row repainted, not just the number — the same visible half the keybind test asserts. + assert!( + s.eval::("return TabKitFrameTab2MiddleDisabled:IsVisible()") + .unwrap(), + "the newly selected tab wears the Active art" + ); + assert!(s.errors().is_empty(), "no handler errors: {:?}", s.errors()); +} diff --git a/crates/benilla-app/src/ui_script/errors_tests.rs b/crates/benilla-app/src/ui_script/errors_tests.rs index 90ee6637..1b34a2eb 100644 --- a/crates/benilla-app/src/ui_script/errors_tests.rs +++ b/crates/benilla-app/src/ui_script/errors_tests.rs @@ -166,7 +166,18 @@ fn an_error_toast_draws_over_an_open_panel_window() { let above: Vec<&ExtractedQuad> = quads .iter() .filter(|q| q.z > toast) - .filter(|q| s.quad_owner_name(q.target).as_deref() != Some("UIErrorsFrame")) + // `RaidWarningFrame` is excluded on exactly the same grounds, and it is the reference's + // own arrangement rather than ours: it is HIGH + toplevel too (ref RaidWarning.xml:4), so + // it raises above the toast in z — but it sits BELOW UIErrorsFrame on screen (anchored to + // its BOTTOM at -10) and its declared `` carries no text, so nothing of it is + // ever painted over the toast. Excluding it keeps this test about a PANEL WINDOW drawing + // over the toast, which is what it is named for. + .filter(|q| { + !matches!( + s.quad_owner_name(q.target).as_deref(), + Some("UIErrorsFrame") | Some("RaidWarningFrame") + ) + }) .collect(); assert!( above.is_empty(), diff --git a/crates/benilla-app/src/ui_script/unit_frame_tests.rs b/crates/benilla-app/src/ui_script/unit_frame_tests.rs index 56b3a113..1a236f45 100644 --- a/crates/benilla-app/src/ui_script/unit_frame_tests.rs +++ b/crates/benilla-app/src/ui_script/unit_frame_tests.rs @@ -1702,3 +1702,44 @@ fn the_unit_frame_hover_hooks_carry_the_references_names() { s.errors() ); } + +/// **`SetRaidTargetIconTexture` lands each mark on its own cell of the 4×4 sheet.** +/// +/// The helper an addon uses to draw a raid mark it got from `GetRaidTargetIndex` — three corpus +/// addons in two codebases (CustomNameplates; Optional/oRA2's shared `MainTank.lua:1216` and +/// `PlayerTarget.lua:1472`). It owns no frame and reads no unit: the texture is the caller's. +/// +/// The coordinates are the point, so all eight are checked rather than one. Star is the top-left +/// cell and skull the bottom-right of the first two rows — which is also what pins the reference's +/// own `/ ROWS` row derivation as harmless here: with 4 rows and 4 columns the wrap lands where the +/// sheet's mark order says it should. +#[test] +fn the_raid_mark_helper_maps_each_index_to_its_cell() { + let s = UiScript::new().unwrap(); + load_unit_frames(&s); + s.run(r#"RTMark = UIParent:CreateTexture("RTMark", "OVERLAY")"#) + .unwrap(); + + // (index, left, right, top, bottom) — 0.25 per cell, four across then down a row. + let cells = [ + (1, 0.00, 0.25, 0.00, 0.25), // star + (2, 0.25, 0.50, 0.00, 0.25), // circle + (3, 0.50, 0.75, 0.00, 0.25), // diamond + (4, 0.75, 1.00, 0.00, 0.25), // triangle + (5, 0.00, 0.25, 0.25, 0.50), // moon — the wrap onto row 2 + (6, 0.25, 0.50, 0.25, 0.50), // square + (7, 0.50, 0.75, 0.25, 0.50), // cross + (8, 0.75, 1.00, 0.25, 0.50), // skull + ]; + for (i, l, r, t, b) in cells { + s.run(&format!("SetRaidTargetIconTexture(RTMark, {i})")) + .unwrap(); + let got: (f64, f64, f64, f64) = s.eval("return RTMark:GetTexCoord()").unwrap(); + assert_eq!( + got, + (l, r, t, b), + "mark {i} must sample the cell at ({l}, {r}, {t}, {b})" + ); + } + assert!(s.errors().is_empty(), "no errors: {:?}", s.errors()); +} diff --git a/crates/benilla-ui/src/script/object.rs b/crates/benilla-ui/src/script/object.rs index e0229793..2a39d97b 100644 --- a/crates/benilla-ui/src/script/object.rs +++ b/crates/benilla-ui/src/script/object.rs @@ -123,7 +123,10 @@ pub(super) fn point_from_str(s: &str) -> Option { } /// The inverse of [`point_from_str`] (the client's point-id table `0x811a38`, names back out). -fn point_name(p: Point) -> &'static str { +/// +/// `pub(super)` because the REGION cluster reports the same names from the same table — one +/// spelling of the point set, not two (`region/layout.rs`'s `GetPoint`). +pub(super) fn point_name(p: Point) -> &'static str { match p { Point::TopLeft => "TOPLEFT", Point::Top => "TOP", diff --git a/crates/benilla-ui/src/script/region/layout.rs b/crates/benilla-ui/src/script/region/layout.rs index 185aae67..d6305db6 100644 --- a/crates/benilla-ui/src/script/region/layout.rs +++ b/crates/benilla-ui/src/script/region/layout.rs @@ -4,8 +4,8 @@ use mlua::{Lua, Table, Value}; use crate::layout::{Anchor, Point}; -use crate::script::object::anchor_bits_eq; -use crate::script::Model; +use crate::script::object::{anchor_bits_eq, frame_wrapper, point_name}; +use crate::script::{Model, SCREEN}; /// Resolve `self` (a region wrapper) to its live [`RegionHandle`]. use super::{ @@ -151,5 +151,111 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { Ok(()) })?, )?; + + // ── The rest of the Region map `0xcf54b4` (wow-re `font-object-lua-surface.md`) ────────────── + // + // These four landed together because the MAP is the unit, not the name. `SetParent` shipped + // alone when one addon line named it, and its own getter stayed missing for months — which is + // how `TheoryCraft\TheoryCraftUI.lua:720` (`buttontext:GetParent()`, a FontString) died every + // session, and how the per-kind census came to read `115 GetParent (missing on Texture, + // FontString)`. The set is closed and byte-verified, so it is asserted as a set in + // `tests/reference_surface.rs` rather than grown a name at a time. + + // GetParent() → the OWNER frame's wrapper. A region always has one (`region_owner_id` falls + // back to the owner for every unresolved case), so unlike the frame twin this never answers nil. + m.set( + "GetParent", + lua.create_function(|lua, this: Table| { + let rh = region_handle_of(lua, &this)?; + let owner = { + let mut model = lua.app_data_mut::().expect("model"); + region_owner_id(&mut model, rh) + }; + frame_wrapper(lua, owner) + })?, + )?; + + // GetCenter() → the resolved rect's midpoint, or a nil PAIR before the first resolve — the same + // contract, and the same source, as the GetLeft/GetRight/GetTop/GetBottom readers above. + // + // **Deliberately unscaled, where the frame twin divides by GetEffectiveScale.** The region edge + // readers report raw resolved units; scaling only the centre would make `GetCenter()` disagree + // with `(GetLeft() + GetRight()) / 2` on any scaled subtree — a contradiction inside one method + // table is worse than a missing division, and regions have no scale of their own to divide by. + m.set( + "GetCenter", + lua.create_function(|lua, this: Table| { + let rh = region_handle_of(lua, &this)?; + let model = lua.app_data_ref::().expect("model"); + Ok(match model.region_resolved.get(&rh) { + Some(r) => ( + Value::Number(f64::from((r.left + r.right) * 0.5)), + Value::Number(f64::from((r.bottom + r.top) * 0.5)), + ), + None => (Value::Nil, Value::Nil), + }) + })?, + )?; + + // GetNumPoints() → how many anchors this region carries. Absent on our FRAMES too, which is the + // same drift one table up; this side is what the corpus named. + m.set( + "GetNumPoints", + lua.create_function(|lua, this: Table| { + let rh = region_handle_of(lua, &this)?; + let model = lua.app_data_ref::().expect("model"); + Ok(model + .region_data + .get(&rh) + .map_or(0, |d| d.anchors.len() as i64)) + })?, + )?; + + // GetPoint([n]) → point, relativeTo, relativePoint, xOfs, yOfs — the n-th (1-based, default + // first) anchor, mirroring the frame twin including its out-of-range answer (five nils). + // + // **The relativeTo dispatch is the part a frame-shaped copy gets wrong.** Region anchors live in + // ONE id space with frames and may target a sibling REGION — the real XML does it everywhere + // (`region.rs`'s `resolve_target`: frames first, then the region-name registry). So the id is + // matched against both tables and answered with the matching wrapper kind; handing back a frame + // wrapper for a region id would be a working-looking handle onto the wrong object. + m.set( + "GetPoint", + lua.create_function(|lua, (this, n): (Table, Option)| { + let rh = region_handle_of(lua, &this)?; + let anchor = { + let model = lua.app_data_ref::().expect("model"); + let idx = (n.unwrap_or(1).max(1) - 1) as usize; + model + .region_data + .get(&rh) + .and_then(|d| d.anchors.get(idx)) + .cloned() + }; + let Some(a) = anchor else { + return Ok((Value::Nil, Value::Nil, Value::Nil, Value::Nil, Value::Nil)); + }; + let rel = { + let is_frame = { + let model = lua.app_data_ref::().expect("model"); + model.id_to_frame.contains_key(&a.relative_to) + }; + if a.relative_to == SCREEN { + Value::Nil + } else if is_frame { + Value::Table(frame_wrapper(lua, a.relative_to)?) + } else { + Value::Table(super::region_wrapper(lua, a.relative_to)?) + } + }; + Ok(( + Value::String(lua.create_string(point_name(a.point))?), + rel, + Value::String(lua.create_string(point_name(a.relative_point))?), + Value::Number(f64::from(a.x_off)), + Value::Number(f64::from(a.y_off)), + )) + })?, + )?; Ok(()) } diff --git a/crates/benilla-ui/src/script/spellbook.rs b/crates/benilla-ui/src/script/spellbook.rs index 6352cd5a..fa96e2fc 100644 --- a/crates/benilla-ui/src/script/spellbook.rs +++ b/crates/benilla-ui/src/script/spellbook.rs @@ -426,22 +426,53 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // GetSpellName(id, bookType) -> name, rank (the Era tuple, `subSpellName` in the ref); pet or - // out-of-range -> a single nil. + // GetSpellName(id, bookType) -> name, rank. **Always TWO values, never one, and the rank of a + // rankless spell is the EMPTY STRING, never nil** (wow-re + // `scratch/getspellname-return-contract.md`, §5-derived from the bytes alone). + // + // Both returns go through the same push helper with **no rank-specific branch anywhere in the + // binding** — a ranked and a rankless spell run byte-identical code: `0x4b4063 call 0x6f3890` + // pushes `SpellRec.Name[locale]`, `0x4b4076` pushes `NameSubtext[locale]`, and the single + // DBC-resolved exit is `mov eax,0x2` at `0x4b407c`. `0x6f3890` decides on **pointer nullity + // alone** (`test edx,edx`), and the DBC pointer is never NULL: `SpellRec::Read 0x583750` fixes + // up every string column with an unconditional `add offset, stringBlockBase`, so an on-disk + // offset of 0 materializes as a pointer to the string block's own byte 0 — a NUL. `0x6f3840` + // then writes tag 4 (`LUA_TSTRING`) unconditionally; `len == 0` is not a special case. So the + // rankless rank is a real, interned, zero-length Lua string. + // + // This is not an edge case: **14,403 of the shipped Spell.dbc's 22,357 rows carry NameSubtext + // offset 0** — 64%, `Attack` (6603) among them. Our `Option::None` pushed nil there, + // and two 1.12 corpus addons walking the whole book die on it in two different idioms: + // `Roid-Macros/Generic.lua:35` uses the rank as a TABLE KEY (nil is illegal), and + // `CT_MasterMod/CT_Master.lua:16` passes it straight to `string.find` (nil raises). Both were + // invisible until the survey seated a spellbook. + // + // The `Option` is still the right shape for the MODEL — it records "this spell has no + // NameSubtext", which is a fact about the DBC row. What was wrong was rendering that absence + // as nil at the Lua boundary; the reference renders it as the empty string the pointer targets. g.set( "GetSpellName", lua.create_function(|lua, (id, book_type): (u32, String)| { + // An out-of-range index RAISES rather than answering nil. `0x4b3ec0` gates the index to + // `[0, 0x400)` (`0x4b3f0a`/`0x4b3f11`) before the binding proper runs, and the miss arm + // is `luaL_error("Invalid spell slot in GetSpellName")`, which longjmps and abandons + // the caller's statement (`super::binding_abi`). The in-binding bound check at + // `0x4b4018` is unreachable-as-taken. This file previously assumed the bound was + // "enforced by our slot lists being shorter than that anyway" — a short list answers + // nil, which is a different thing entirely. + if id >= 0x400 { + return Err(mlua::Error::runtime("Invalid spell slot in GetSpellName")); + } let model = lua.app_data_ref::().expect("model app_data"); let Some(slot) = book_slot(&model, id, &book_type) else { - return Ok(MultiValue::from_vec(vec![Value::Nil])); - }; - let rank = match &slot.rank { - Some(r) => Value::String(lua.create_string(r)?), - None => Value::Nil, + // An unfilled slot inside the range answers **two** nils (`0x4b4086` → `mov eax,0x2` + // at `0x4b4095`), not one — which `select('#', …)` and a two-name assignment can + // both tell apart. + return Ok(MultiValue::from_vec(vec![Value::Nil, Value::Nil])); }; Ok(MultiValue::from_vec(vec![ Value::String(lua.create_string(&slot.name)?), - rank, + Value::String(lua.create_string(slot.rank.as_deref().unwrap_or(""))?), ])) })?, )?; @@ -1279,4 +1310,87 @@ mod tests { (0.0, 0.0, 1) ); } + + /// **A rankless spell's rank is the empty string, not nil** — and the arity is always two. + /// + /// Both returns go through one push helper with no rank-specific branch (`0x4b4063`/`0x4b4076`, + /// single exit `mov eax,0x2` at `0x4b407c`), and the DBC pointer is never NULL, so an on-disk + /// NameSubtext offset of 0 becomes a pointer to the string block's byte 0 — a real zero-length + /// Lua string. 64% of the shipped Spell.dbc's rows are in that state, `Attack` among them. + /// + /// The two corpus idioms this was breaking are both asserted here, because "falsey either way" + /// is exactly the reasoning that made nil look acceptable: a nil is an ILLEGAL TABLE KEY + /// (Roid-Macros) and an ILLEGAL `string.find` argument (CT_MasterMod), while `""` is neither. + #[test] + fn a_rankless_spell_answers_an_empty_rank_and_still_two_values() { + let mut s = UiScript::new().unwrap(); + s.set_spellbook(SpellBookState { + tabs: Vec::new(), + slots: vec![ + SpellSlotView { + spell_id: 6603, + name: "Attack".into(), + rank: None, + ..Default::default() + }, + SpellSlotView { + spell_id: 78, + name: "Heroic Strike".into(), + rank: Some("Rank 1".into()), + ..Default::default() + }, + ], + }); + + assert_eq!( + s.eval::(r#"return select('#', GetSpellName(1, "spell"))"#) + .unwrap(), + 2 + ); + let (name, rank) = s + .eval::<(String, String)>(r#"return GetSpellName(1, "spell")"#) + .unwrap(); + assert_eq!((name.as_str(), rank.as_str()), ("Attack", "")); + assert!( + s.eval::(r#"local _, r = GetSpellName(1, "spell") return r ~= nil"#) + .unwrap(), + "the rankless rank must be a STRING, not nil" + ); + // Roid-Macros/Generic.lua:35 and CT_MasterMod/CT_Master.lua:16, in miniature. + s.run(r#"local _, r = GetSpellName(1, "spell") local t = {} t[r] = 1"#) + .expect("a rankless rank must be a legal table key"); + s.run(r#"local _, r = GetSpellName(1, "spell") string.find(r, "(%d+)")"#) + .expect("a rankless rank must be a legal string.find argument"); + // A ranked spell is unchanged. + assert_eq!( + s.eval::<(String, String)>(r#"return GetSpellName(2, "spell")"#) + .unwrap(), + ("Heroic Strike".to_string(), "Rank 1".to_string()) + ); + + // An unfilled slot INSIDE the range is two nils, not one — distinguishable by select('#'). + assert_eq!( + s.eval::(r#"return select('#', GetSpellName(9, "spell"))"#) + .unwrap(), + 2 + ); + assert!(s + .eval::(r#"local a, b = GetSpellName(9, "spell") return a == nil and b == nil"#) + .unwrap()); + + // Past the reference's [0, 0x400) gate it RAISES rather than answering nil. + let err = s + .run(r#"GetSpellName(1024, "spell")"#) + .expect_err("an out-of-range slot must raise"); + assert!( + format!("{err}").contains("Invalid spell slot in GetSpellName"), + "got {err}" + ); + // ...and 1023 is inside it, so it answers rather than raising. + assert_eq!( + s.eval::(r#"return select('#', GetSpellName(1023, "spell"))"#) + .unwrap(), + 2 + ); + } } diff --git a/crates/benilla-ui/src/script/tests/reference_surface.rs b/crates/benilla-ui/src/script/tests/reference_surface.rs index 73f05e5f..7d47090c 100644 --- a/crates/benilla-ui/src/script/tests/reference_surface.rs +++ b/crates/benilla-ui/src/script/tests/reference_surface.rs @@ -1119,3 +1119,154 @@ fn get_inventory_slot_info_folds_case_and_flags_only_the_ranged_slot() { "the reference does not interpolate the offending name: {err}" ); } + +/// **The whole Region method map, on both leaves that chain to it.** +/// +/// `SetParent` above landed as one name because one addon line named it. That is how this table has +/// always grown, and it is why `GetParent` — its own getter — was still absent while `SetParent` +/// worked. wow-re carves the map as a SET, not as names: FontString's lookup `0x79ee20` chains its +/// own map `0xcf5400` to the Region map `0xcf54b4`, whose 19 entries are +/// +/// ```text +/// GetObjectType IsObjectType GetName GetParent SetParent GetCenter GetLeft GetRight GetTop +/// GetBottom GetWidth SetWidth GetHeight SetHeight GetNumPoints GetPoint SetPoint SetAllPoints +/// ClearAllPoints +/// ``` +/// +/// (`system/ui/scratch/font-object-lua-surface.md` — the same note whose point is that a `` +/// object does NOT chain and so has none of these. Texture reaches the identical map through its +/// own leaf lookup, which is why both are asserted here.) +/// +/// So this asserts membership rather than behaviour: each name is *present and callable* on a +/// Texture and on a FontString. Behaviour belongs in the focused tests around it — what is pinned +/// here is that the set cannot quietly lose a member again, which is the failure `GetParent` was. +#[test] +fn every_region_map_method_is_callable_on_a_texture_and_a_fontstring() { + /// The map minus `GetObjectType`/`IsObjectType`, which are **dispatched, not forgotten**: both + /// are in `0xcf54b4`, but a faithful `IsObjectType` needs facts a reimplementation cannot + /// guess — whether it matches the leaf name only or walks a class chain, the exact strings in + /// that chain, whether it answers `1`/`nil` or `true`/`false`, and whether a bad argument + /// raises. Shipping a guess is how 1203/1205/1211 happened. They join this list with the + /// verified answer; until then their absence is a raise, which is loud and correct. + const REGION_MAP: &[&str] = &[ + "GetName", + "GetParent", + "SetParent", + "GetCenter", + "GetLeft", + "GetRight", + "GetTop", + "GetBottom", + "GetWidth", + "SetWidth", + "GetHeight", + "SetHeight", + "GetNumPoints", + "GetPoint", + "SetPoint", + "SetAllPoints", + "ClearAllPoints", + ]; + let mut s = crate::script::UiScript::new().unwrap(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + RMOwner = CreateFrame("Frame", "RMOwner") + RMOwner:SetPoint("BOTTOMLEFT", 0, 0) RMOwner:SetSize(100, 50) + RMTex = RMOwner:CreateTexture("RMTex", "ARTWORK") + RMStr = RMOwner:CreateFontString("RMStr", "ARTWORK") + "#, + ) + .unwrap(); + + let mut absent: Vec = Vec::new(); + for kind in ["RMTex", "RMStr"] { + for name in REGION_MAP { + if !s + .eval::(&format!("return type({kind}.{name}) == 'function'")) + .unwrap_or(false) + { + absent.push(format!("{kind}:{name}")); + } + } + } + assert!( + absent.is_empty(), + "the Region map 0xcf54b4 is missing from our regions: {absent:?}" + ); +} + +/// **The four Region-map readers, on the cases a frame-shaped copy gets wrong.** +/// +/// `TheoryCraftUI.lua:720` is the line that found them: `buttontext:GetParent():GetID()`, where +/// `buttontext` is a FontString — a working line on the real client, and `attempt to call method +/// 'GetParent' (a nil value)` here, every session. +#[test] +fn the_region_map_readers_answer_the_way_the_edges_do() { + let mut s = crate::script::UiScript::new().unwrap(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + RRFrame = CreateFrame("Frame", "RRFrame") + RRFrame:SetPoint("BOTTOMLEFT", 100, 200) RRFrame:SetSize(200, 100) + RRPlate = RRFrame:CreateTexture("RRPlate", "ARTWORK") + RRPlate:SetPoint("TOPLEFT", 10, -10) RRPlate:SetSize(50, 20) + -- the sibling-region anchor the real XML uses everywhere + RRLabel = RRFrame:CreateFontString("RRLabel", "OVERLAY") + RRLabel:SetPoint("LEFT", RRPlate, "RIGHT", 4, 0) + "#, + ) + .unwrap(); + s.resolve(); + + // GetParent is the OWNER frame — the identity TheoryCraft then calls :GetID() on. + assert!( + s.eval::("return RRPlate:GetParent() == RRFrame") + .unwrap(), + "a region's parent is the frame that created it" + ); + assert!( + s.eval::("return RRLabel:GetParent():GetName() == 'RRFrame'") + .unwrap(), + "…and it is a real frame handle, not a bare id" + ); + + // GetCenter agrees with the edge readers BY CONSTRUCTION — the invariant that forbids scaling + // one and not the others. + let (cx, cy): (f64, f64) = s.eval("return RRPlate:GetCenter()").unwrap(); + let (l, r, t, b): (f64, f64, f64, f64) = s + .eval("return RRPlate:GetLeft(), RRPlate:GetRight(), RRPlate:GetTop(), RRPlate:GetBottom()") + .unwrap(); + assert_eq!((cx, cy), ((l + r) * 0.5, (t + b) * 0.5)); + + // GetNumPoints counts what SetPoint wrote, and ClearAllPoints takes it back to 0. + assert_eq!(s.eval::("return RRPlate:GetNumPoints()").unwrap(), 1); + assert_eq!( + s.eval::("return RRLabel:GetNumPoints()").unwrap(), + 1, + "the sibling-anchored label carries its one point" + ); + + // GetPoint's relativeTo must come back as the SIBLING REGION, not a frame wrapper onto the same + // id — the one place regions genuinely differ from frames, since both share an id space. + let (p, rp, x, y): (String, String, f64, f64) = s + .eval("local p, _, rp, x, y = RRLabel:GetPoint(1) return p, rp, x, y") + .unwrap(); + assert_eq!((p.as_str(), rp.as_str(), x, y), ("LEFT", "RIGHT", 4.0, 0.0)); + assert!( + s.eval::("local _, rel = RRLabel:GetPoint(1) return rel == RRPlate") + .unwrap(), + "relativeTo is the sibling REGION handle itself" + ); + // Out of range is five nils, like the frame twin. + assert_eq!( + s.eval::("return select('#', RRPlate:GetPoint(7))") + .unwrap(), + 5, + "an out-of-range index still answers five values, all nil" + ); + assert!( + s.eval::("return RRPlate:GetPoint(7) == nil").unwrap(), + "…and the first of them is nil" + ); +}