compat: FontString:GetStringHeight, which AceGUI calls unguarded

1.12 registers no `GetStringHeight` (`script/button.rs`: `0x782390` reads the label's height
instead) and ClassicAPI backports it, so `compat/classic_api/frame.rs` is its home. That file had
it on its own not-built list under "needs per-line layout facts the host measurer is not asked for
today" — true of `GetNumLines` and `GetWrappedWidth`, and not of this one: the whole-string height
is the other half of the number `GetStringWidth` already reads. `measure::current_measure` now owns
the staleness rule for both, so the two cannot disagree about when a measure has gone stale, and a
changed string still reads 0 rather than the previous string's height.

It is not cosmetic. AceGUI-3.0 calls it **unguarded** — `AceGUIWidget-Label.lua:54` is
`height = label:GetStringHeight()` and `AceGUIWidget-CheckBox.lua:90` is
`self:SetHeight(28 + self.desc:GetStringHeight())` — so every Ace options panel on this install
drew its labels and checkbox descriptions at the library's own "avoid zero-height labels" fallback
of one pixel: a window with all of its prose invisible. `addon_harness --probe AI_VoiceOver` builds
that panel's full 32-widget tree with zero errors and every `Label` at `624x1`, which is what
"can't see the actual options in VoiceOver" was.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
2026-10-16 18:45:25 -06:00
co-authored by Claude Opus 5
parent 7fef604615
commit 75f04b4f28
6 changed files with 141 additions and 17 deletions
@@ -27,11 +27,13 @@
//! built, because stripped of secure semantics they are a per-frame key/value store and that
//! much is honest; the drivers that *watch* attributes and re-evaluate macro conditionals are
//! the actual subsystem and are not.
//! - The **text-measurement** getters (`GetNumLines`, `GetStringHeight`, `GetUnboundedStringWidth`,
//! - The **per-line** text-measurement getters (`GetNumLines`, `GetUnboundedStringWidth`,
//! `GetWrappedWidth`, `GetLineHeight`, `IsTruncated`, `SetMaxLines`, `GetMaxLines`): these need
//! per-line layout facts the host measurer is not asked for today (`script/measure.rs` answers a
//! whole-string width/height through one `TextMeasure` hook), so each would need a new host
//! question, not a read of something already known.
//! question, not a read of something already known. **`GetStringHeight` was on this list and
//! should not have been**: the whole-string height is the other half of the number
//! `GetStringWidth` already reads, so it is built below.
//! - `IsMouseOver`, `GetMouseFoci`, `IsDragging`, `SetResizeBounds`, `RegisterUnitEvent`,
//! `SetColorTexture`, `SetDesaturation`/`GetDesaturation`, `SetRotation`, and the EditBox
//! cursor/highlight/history getters: each plausible, none traced this pass.
@@ -157,6 +159,32 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
}
delegate(lua, REG_FONTSTRING_METHODS, FONTSTRING_DELEGATES)?;
// `GetStringHeight()`: the laid-out height of the current text — wrapped to the declared
// width, so a three-line paragraph answers three lines tall, where the font's own height
// would answer one. Measured inline exactly as the stock `GetStringWidth` is
// (`script/measure::current_measure`, which both read), and 0 before the host has answered
// for this text, never the previous string's height.
//
// 1.12 registers no such method at all (`script/button.rs`: `0x782390` reads the label's
// height instead), and that absence is load-bearing for AceGUI-3.0, which calls it
// **unguarded**: `AceGUIWidget-Label.lua:54` is `height = label:GetStringHeight()` and
// `AceGUIWidget-CheckBox.lua:90,210` is `self:SetHeight(28 + self.desc:GetStringHeight())`.
// Without it every Ace options panel on this install draws its labels and checkbox
// descriptions at the library's own "avoid zero-height labels" fallback of **1 pixel** — the
// whole of an options panel's prose, invisible. AI_VoiceOver's is the one that was reported.
{
let fs: Table = lua.named_registry_value(REG_FONTSTRING_METHODS)?;
fs.set(
"GetStringHeight",
lua.create_function(|lua, this: Table| {
let rh = crate::script::region::region_handle_of(lua, &this)?;
Ok(crate::script::measure::current_measure(lua, rh)
.map(|m| m.h)
.unwrap_or(0.0))
})?,
)?;
}
let m: Table = lua.named_registry_value(REG_FRAME_METHODS)?;
m.set(
+22
View File
@@ -18,6 +18,28 @@ pub trait TextMeasure {
fn measure(&mut self, req: &MeasureRequest) -> (f32, f32, f32);
}
/// This FontString's measure as it stands *right now*, measuring first when a host engine is
/// installed ([`ensure_measured`]); `None` when nothing has been measured under the key the
/// region's current text, font and width make — which every metric read answers as 0, never as
/// the previous string's number.
///
/// One owner for that staleness rule, because two readers want the same measure for different
/// halves of it: the stock `GetStringWidth` takes `natural_w` (`region/text.rs`) and ClassicAPI's
/// `GetStringHeight` takes `h` (`compat/classic_api/frame.rs`).
pub(super) fn current_measure(lua: &Lua, rh: RegionHandle) -> Option<MeasuredText> {
ensure_measured(lua, rh);
let model = lua.app_data_ref::<Model>().expect("model app_data");
let d = model.region_data.get(&rh)?;
// The measure is keyed by the drawn size, so the owner's scale is part of the key.
let scale = model
.arena
.region(rh)
.and_then(|r| model.arena.frame(r.owner))
.map(|f| f.effective_scale)
.unwrap_or(1.0);
d.measured.filter(|m| m.key == d.measure_key(scale))
}
/// Measure region `rh` now if a measurer is installed and its stored measure is stale; a current
/// one is left alone, so a per-frame poll does not re-measure.
pub(super) fn ensure_measured(lua: &Lua, rh: RegionHandle) {
+2 -14
View File
@@ -59,22 +59,10 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> {
// `GetStringHeight`; `GetHeight` (`0x7a2030`) serves.
fn natural_w(lua: &Lua, this: &Table) -> mlua::Result<f32> {
let rh = region_handle_of(lua, this)?;
// Measure now when a host font engine is installed, as the reference measures inline, so
// Measured now when a host font engine is installed, as the reference measures inline, so
// `SetText` then `GetStringWidth` works in one tick; without one it is 0 until the host
// answers.
crate::script::measure::ensure_measured(lua, rh);
let model = lua.app_data_ref::<Model>().expect("model");
let Some(d) = model.region_data.get(&rh) else {
return Ok(0.0);
};
let scale = model
.arena
.region(rh)
.and_then(|r| model.arena.frame(r.owner))
.map(|f| f.effective_scale)
.unwrap_or(1.0);
Ok(d.measured
.filter(|m| m.key == d.measure_key(scale))
Ok(crate::script::measure::current_measure(lua, rh)
.map(|m| m.natural_w)
.unwrap_or(0.0))
}
@@ -514,3 +514,83 @@ fn a_settled_region_is_refound_after_each_measure_input_write() {
assert_eq!(reqs[0].text, "Go");
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
/// `GetStringHeight` is the laid-out, **wrapped** height of the current text: the other half of
/// the one measure `GetStringWidth` reads, so it costs no new host question. 1.12 registers no
/// such FontString method (`script/button.rs`: `0x782390` reads the label's height instead) and
/// ClassicAPI backports it, which is why it lives in `compat/classic_api/frame.rs`.
///
/// It is not cosmetic. AceGUI-3.0 calls it **unguarded** — `AceGUIWidget-Label.lua:54` is
/// `height = label:GetStringHeight()` and `AceGUIWidget-CheckBox.lua:90` is
/// `self:SetHeight(28 + self.desc:GetStringHeight())` — so with the method missing every Ace
/// options panel fell back to the library's own "avoid zero-height labels" value of 1 pixel and
/// the prose of the panel was invisible.
#[test]
fn get_string_height_is_the_wrapped_height_the_measure_already_carries() {
let mut s = script();
s.set_screen_size(800.0, 600.0);
s.run(
r#"
local w = CreateFrame("Frame", "Panel")
w:SetPoint("TOPLEFT", 0, 0); w:SetWidth(400); w:SetHeight(300)
local d = w:CreateFontString("Desc", "ARTWORK")
d:SetText("an option description long enough to wrap")
d:SetWidth(270)
d:SetPoint("TOPLEFT", 0, 0)
"#,
)
.unwrap();
s.resolve();
// Before the host answers, 0 — never a guess and never another string's number.
assert_eq!(s.eval::<f64>("return Desc:GetStringHeight()").unwrap(), 0.0);
let reqs = s.fontstrings_needing_measure();
assert_eq!(reqs.len(), 1, "the height-less FontString wants measuring");
let r = &reqs[0];
// Three wrapped lines of 16, as the host would answer at this width.
s.set_measured_text_unwrapped(&[(r.id, 250.0, 48.0, r.key)]);
assert_eq!(
s.eval::<f64>("return Desc:GetStringHeight()").unwrap(),
48.0,
"the wrapped height, not the font's single line"
);
// And the pair really is one measure read two ways: the width arm is the *unwrapped* number.
assert_eq!(
s.eval::<f64>("return Desc:GetStringWidth()").unwrap(),
250.0
);
// A changed string reads 0 until its own measure lands, exactly as the width arm does: both
// go through `measure::current_measure`, which checks the key.
s.run(r#"Desc:SetText("something else entirely, also long enough to wrap")"#)
.unwrap();
assert_eq!(
s.eval::<f64>("return Desc:GetStringHeight()").unwrap(),
0.0,
"a stale measure is not an answer"
);
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
/// The AceGUI line that was failing, as written: a nil method raises in 1.12's Lua, so the
/// library's own zero-height fallback below it never ran — the label kept whatever height it
/// had. This is the regression that pins the method stays *callable*, not merely present.
#[test]
fn the_acegui_label_line_runs_against_a_fontstring() {
let mut s = script();
s.set_screen_size(800.0, 600.0);
s.run(
r#"
local f = CreateFrame("Frame", "AceLabelish")
f:SetWidth(400); f:SetHeight(20); f:SetPoint("TOPLEFT", 0, 0)
label = f:CreateFontString(nil, "ARTWORK")
label:SetText("a description")
label:SetPoint("TOPLEFT")
label:SetWidth(400)
-- AceGUIWidget-Label.lua:54-63, verbatim in shape.
local height = label:GetStringHeight()
if not height or height == 0 then height = 1 end
f:SetHeight(height)
"#,
)
.unwrap();
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
@@ -279,6 +279,12 @@ const ALLOWED_BEYOND: &[(&str, &str, &str)] = &[
"FontString",
"ClassicAPI: SetText(format(...)), composed from the stock SetText",
),
(
"GetStringHeight",
"FontString",
"ClassicAPI: the laid-out wrapped height, the other half of GetStringWidth's measure — \
AceGUI-3.0's Label and CheckBox call it unguarded",
),
(
"SetShown",
"Button CheckButton ColorSelect DressUpModel EditBox FontString Frame GameTooltip LootButton MessageFrame Minimap Model MovieFrame PlayerModel ScrollFrame ScrollingMessageFrame SimpleHTML Slider StatusBar TabardModel Texture WorldFrame",
+1 -1
View File
@@ -140,7 +140,7 @@ in this file on the strength of the namreeb research. pepo's v2 has its own
| Combat | partial | `compat/classic_api/misc.rs`. `InCombatLockdown()` → always `false`, and that is the honest answer rather than a stub: the lockdown exists on modern clients to refuse secure-frame edits mid-combat and 1.12 has no secure-template system at all, so nothing here is ever locked down. `StartAttack`/`StopAttack` deliberately not added here — they are 1.12's *own* verbs that benilla hasn't built yet, so registering them from `compat/` would put a stock gap in the wrong layer and hide it from the reference-surface accounting |
| Class, Totem, Tracking, Instance | todo, blocked | `FillLocalizedClassList`: the nine localized class names are DBC-resolved app-side (`benilla-app`'s `ui_unit::class_names`) and `benilla-ui` deliberately holds no DBC — the player's own class reaches it through `PlayerRecord`, but not all nine, so this needs a feed like `SelfMovement`'s, not a binding. `Totem` (4 functions): no totem state exists in the model at all; 1.12's own totem bar is unbuilt, so there is nothing to wrap. `Tracking` (3): `Model::tracking` is the one *active* tracking aura (behind the stock `GetTrackingTexture`), not the enumerable list of known tracking spells these index into. `GetInstanceInfo`: `script/instance.rs` feeds only the instance *type*; the modern tuple also wants the instance name, difficulty index and name, and max players — none tracked, and the name would have to be guessed from zone text |
| Hooks (`hooksecurefunc`) | done | `compat/classic_api/hooks.rs`, both documented forms (over a global, over a table field). Pure Lua machinery, no game state. The contract it keeps, each covered by its own test: the original runs first with the caller's arguments; **the original's return values pass through untouched** and the hook's are discarded; a hook that errors has its error recorded (`Model::record_script_error`) and the original's returns still come back, while the *original's* own error still propagates; and hooking twice **chains** in install order rather than replacing. A name holding no function is a silent no-op — the reference raises, but an addon written against a client that has the verb it meant to hook is the common case here and taking its UI down over a missing one would be worse. The original and hook live in the Lua registry, since this crate holds no persistent `Function` handle |
| Frame (widget methods) | partial | `compat/classic_api/frame.rs`. These are **methods, not globals**, written straight into the stock class method tables (`__benilla_frame_methods` and the two region leaves) — `object.rs`'s `frame_meta_for` *links* those tables rather than copying them ("a later write to a table shows through"), so a late install from `compat/` reaches every widget already made and every one made later. Built: `SetShown`, `GetSize`/`SetSize`, `GetRect`, `SetFormattedText` (FontString), `HookScript`, `IsEventRegistered`, `GetEffectiveAlpha`, and `SetAttribute`/`SetAttributeNoHandler`/`GetAttribute`/`ClearAttribute`. The geometry/visibility/text ones are composed from the stock verbs as a Lua chunk, so they reuse the stock resolution exactly instead of duplicating it in Rust — note the chunks return a function rather than reading `...`, since this VM is 1.12's Lua **5.0** where `...` is a parameter-list token only, never an expression. Skipped, each with the reason in that file's header: the **mask** and **atlas** families (benilla's UI quad carries one texture and a UV rect — no mask slot, no atlas registry; needs a render-path feature first), the **secure-template** family (`RegisterStateDriver`, `RegisterUnitWatch`, `SecureCmdOptionParse`, `SecureButton_*`, … — modern restricted execution, which 1.12 has no equivalent of; `SetAttribute`/`GetAttribute` ARE built because stripped of secure semantics they are just a per-frame key/value store, and that much is honest, but the drivers that *watch* attributes are the actual subsystem and are not), the **text-measurement** getters (`GetNumLines`, `GetStringHeight`, `GetWrappedWidth`, `IsTruncated`, … — need per-line layout facts the host measurer is not asked for today; `measure.rs` answers one whole-string width/height), and `IsMouseOver`/`GetMouseFoci`/`IsDragging`/`SetResizeBounds`/`RegisterUnitEvent`/`SetColorTexture`/desaturation/rotation/the EditBox cursor-and-highlight getters (plausible, untraced this pass). All 12 new methods are registered in `widget_surface.rs`'s `ALLOWED_BEYOND` with their reasons |
| Frame (widget methods) | partial | `compat/classic_api/frame.rs`. These are **methods, not globals**, written straight into the stock class method tables (`__benilla_frame_methods` and the two region leaves) — `object.rs`'s `frame_meta_for` *links* those tables rather than copying them ("a later write to a table shows through"), so a late install from `compat/` reaches every widget already made and every one made later. Built: `SetShown`, `GetSize`/`SetSize`, `GetRect`, `SetFormattedText` (FontString), `HookScript`, `IsEventRegistered`, `GetEffectiveAlpha`, and `SetAttribute`/`SetAttributeNoHandler`/`GetAttribute`/`ClearAttribute`. The geometry/visibility/text ones are composed from the stock verbs as a Lua chunk, so they reuse the stock resolution exactly instead of duplicating it in Rust — note the chunks return a function rather than reading `...`, since this VM is 1.12's Lua **5.0** where `...` is a parameter-list token only, never an expression. Skipped, each with the reason in that file's header: the **mask** and **atlas** families (benilla's UI quad carries one texture and a UV rect — no mask slot, no atlas registry; needs a render-path feature first), the **secure-template** family (`RegisterStateDriver`, `RegisterUnitWatch`, `SecureCmdOptionParse`, `SecureButton_*`, … — modern restricted execution, which 1.12 has no equivalent of; `SetAttribute`/`GetAttribute` ARE built because stripped of secure semantics they are just a per-frame key/value store, and that much is honest, but the drivers that *watch* attributes are the actual subsystem and are not), the **per-line** text-measurement getters (`GetNumLines`, `GetWrappedWidth`, `IsTruncated`, … — need per-line layout facts the host measurer is not asked for today; `measure.rs` answers one whole-string width/height). **`GetStringHeight` was on that skipped list and has been built**, because the blanket reason did not apply to it: the whole-string height is the other half of the number `GetStringWidth` already reads, so `measure::current_measure` now owns the staleness rule for both and the method is a read, not a new host question. It had to be built — AceGUI-3.0 calls it **unguarded** (`AceGUIWidget-Label.lua:54` `height = label:GetStringHeight()`; `AceGUIWidget-CheckBox.lua:90,210` `self:SetHeight(28 + self.desc:GetStringHeight())`), so on this install every Ace options panel drew its labels and checkbox descriptions at the library's own "avoid zero-height labels" fallback of **1 pixel**: an options window with its prose invisible, which is what "can't see the actual options in VoiceOver" was. Confirmed by probe before and after — `addon_harness --probe AI_VoiceOver` builds the panel's full 32-widget tree with 0 errors and every `Label` at `624x1`, and `IsMouseOver`/`GetMouseFoci`/`IsDragging`/`SetResizeBounds`/`RegisterUnitEvent`/`SetColorTexture`/desaturation/rotation/the EditBox cursor-and-highlight getters (plausible, untraced this pass). All 12 new methods are registered in `widget_surface.rs`'s `ALLOWED_BEYOND` with their reasons |
| Container (`C_Container`) | done | `compat/classic_api/container.rs`; almost entirely translation over the stock 1.12 bag verbs' own data (`Model.containers`), not new game logic. `GetContainerItemID/Info`, `HasContainerItem`, `GetContainerItemDurability`, `GetContainerNumFreeSlots`/`GetContainerFreeSlots`, `CalculateTotalNumberOfFreeBagSlots`, `PlayerHasHearthstone`/`UseHearthstone` (scans bags 0-4 for item id 6948), `SwapItems`/`MoveItem`/`AutoStoreItem` (queue the same `ContainerMove`/`BagAutoStore` the stock pickup/place path does), `SortBags`/`SortBankBags` (no-ops — 1.12 has no sort algorithm to run) and the three sort-preference toggle pairs (stored, nothing downstream reads them) are real. Deliberately nil/false, not guessed: `GetContainerItemCharges` (same charge-slot-identity blocker as SuperWoW's), `GetContainerItemRepairCost` (server-only in vanilla), `GetContainerItemQuestInfo`/`GetContainerItemEquipmentSetInfo` (no such tracking exists, and no such feature exists in 1.12 at all for the latter), `IsContainerItemOpenable` (no lockbox flag tracked) |
| CreatureInfo (`C_CreatureInfo`) | partial | `compat/classic_api/creature_info.rs`, the 3 of 9 anything installed actually calls. `GetRaceInfo` and `GetFactionInfo` read a **new app→VM feed of the player's own `ChrRaces.dbc`** (`benilla-formats`' `chr_races.rs` → `benilla-app`'s `chr_races.rs` → `UiScript::set_race_info`), built because this crate holds no DBC and a frozen race table gets a modified realm's names wrong: **OctoWoW's row 10 displays "High Elf" where Blizzard's says "Blood Elf"**, while both keep `clientFileString "BloodElf"` for the art paths — the two columns genuinely disagree, so neither can be derived from the other, and a test pins that. The side on `GetFactionInfo` comes from `ui_unit.rs`'s `race_faction_group`, the same answer `UnitFactionGroup("player")` gives, so the two cannot disagree about a race; a non-playable row carries no side and reads nil rather than a guess. **`GetRaceInfo` answering nil past the last race is load-bearing** — pfUI walks `i = 1, 2, 3…` until nil, so an always-non-nil answer hangs the client at load; its own test says so. `GetCreatureID` decodes the template entry out of bits 24-47 of the 16-hex-digit GUID token the rest of `compat/` passes around, accepting **only** the creature high word so a player or pet GUID answers nil instead of a plausible-looking wrong number (the decode is inlined rather than calling `benilla_protocol::guid::entry` — this crate depends on no protocol or engine crate, and one shift is not worth breaking that boundary for). Not built: `GetCreatureInfoByID`, `RequestLoadCreatureByID`, `GetCreatureFamilyInfo`/`IDs`, `GetCreatureTypeInfo`/`IDs` (each needs a creature/family/type table this VM is not fed, and nothing installed asks) and `GetClassInfo` (the class table exists app-side as `ChrClasses` but has no VM feed yet — the same shape of work this row's race feed just did, if an addon ever wants it). With no install the race table stays empty and every reader answers nil, so an addon's race walk finds none and still loads |
| Item (`C_Item`) | partial | `compat/classic_api/item.rs`, 37 entries. Modern id-first and `ItemLocation`-keyed readers over the same state the stock item verbs read: `Model::item_templates` (the one `ItemTemplateView` cache every item tooltip renders through — a miss records the same `CMSG_ITEM_QUERY` ask the stock `GetItemInfo` does, so a modern caller warms the one shared cache, not a parallel one), `Model::containers` and `Model::inventory_slots`. **`ItemLocation` is accepted as the plain table addons build** (`{bagID=, slotIndex=}` / `{equipmentSlotIndex=}`): modern WoW's mixin is pure Lua sugar over exactly those two fields and every entry point only reads them, so the table *is* the contract; the mixin constructors themselves belong with `Mixin`/`CreateFromMixins` in the `Lua` namespace and are not faked here. Built: the existence/identity trio, `GetItemData`/`GetItemDataByID` (the upstream's kitchen-sink table, including the `isConjured`/`isOpenable`/`isLootable`/`isWrapper` flag bools off the template's raw `flags`), `GetItemInfoInstant`, the by-id and by-location getter pairs (name, quality, max stack, sell price, set id, inventory type, icon), `GetItemLink`, `GetStackCount`, `GetItemCount` (bags + worn, with the `includeBank` and charge-multiplying `includeUses` flags), `GetItemSpell` (the on-use trigger only — triggers 0/5, not an equip or chance-on-hit effect), `IsUsableItem`/`IsConsumableItem`/`IsEquippableItem`/`IsEquippedItem`/`IsItemOpenable`/`IsBound`, the `IsItemDataCached` pair (which deliberately record **no** ask — polling is not asking), the two inventory-slot name getters, and `UseItemByName`/`EquipItemByName` over the stock container-use queue. Not built, each with the reason in that file's header: the **guid** family (`GetItemGUID`, `GetItemLocation`, `IsItemGUIDInInventory`, …) — nothing in the model keys items by instance guid, and the modern `"Item-Server-…"` string has no source here; the **item-level** family (`GetCurrentItemLevel`, `GetDetailedItemLevelInfo`, `GetAverageItemLevel`) — item level is not in `SMSG_ITEM_QUERY_SINGLE_RESPONSE` and 1.12 exposes no such concept to the client; `GetItemStats`/`GetItemStatDelta` — the modern shape is keyed by `ITEM_MOD_*` *string* names resolved app-side, and returning the raw numeric keys under the modern name would silently mismatch; `GetItemUniqueness` (wants a limit-category pair 1.12 has no counterpart for); the `LockItem`/`UnlockItem` family (`ContainerSlot::locked` is a pending-operation flag the app owns, not a lock Lua may set); and `UseAtCursor`/`UseAtUnit`/`PickupItem`/`RequestLoadItemData*` (each needs an app-drained queue that does not exist yet) |