diff --git a/crates/benilla-ui/src/script/modelframe.rs b/crates/benilla-ui/src/script/modelframe.rs index f812b517..18a3be1f 100644 --- a/crates/benilla-ui/src/script/modelframe.rs +++ b/crates/benilla-ui/src/script/modelframe.rs @@ -76,9 +76,14 @@ impl Model { /// A size was authored on `h` (`SetWidth`, `SetHeight`, or XML `` through them): the /// implicit rect yields to it for good, as the reference's override does. + /// + /// A `GameTooltip`'s auto-size yields to it too, but only until the plate's content next + /// changes or it is `Show`n again ([`crate::widget::TooltipState::authored_size`]). pub(crate) fn note_authored_size(&mut self, h: FrameHandle) { - if let Some(KindState::Model(m)) = self.arena.frame_mut(h).map(|f| &mut f.kind_state) { - m.implicit_size = false; + match self.arena.frame_mut(h).map(|f| &mut f.kind_state) { + Some(KindState::Model(m)) => m.implicit_size = false, + Some(KindState::Tooltip(t)) => t.authored_size = true, + _ => {} } } diff --git a/crates/benilla-ui/src/script/tests/tooltip.rs b/crates/benilla-ui/src/script/tests/tooltip.rs index 14bda3b7..c5ed37e3 100644 --- a/crates/benilla-ui/src/script/tests/tooltip.rs +++ b/crates/benilla-ui/src/script/tests/tooltip.rs @@ -312,6 +312,75 @@ fn unmeasured_lines_hold_declared_size() { .unwrap(); } +/// A size Lua writes once the plate is filled holds: the reference re-sizes a tooltip only from +/// its own content verbs and from `Show` (`0x530640`, six call sites — `0x52ad59`, `0x52b029`, +/// `0x52e5ae`, `0x52f869`, `0x530615`, `0x530aad` — all inside the tooltip code and none in the +/// frame pump), so nothing runs again to take it back. ATSW2's recipe tooltip is the case: it +/// `Show()`s and then sizes the plate from its own line count and column width +/// (`AdvancedTradeSkillWindow2.lua:5913-5920`). benilla auto-sizes in a pre-pass of every +/// `resolve`, which used to overwrite that on the very next frame — the plate drew one line tall +/// and its right column landed on top of its left one. +#[test] +fn a_size_authored_after_the_content_holds_until_the_content_changes() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + local a = CreateFrame("Button", "A9"); a:SetPoint("TOPLEFT", 100, -100); a:SetWidth(10); a:SetHeight(10) + local tt = CreateFrame("GameTooltip", "TT9") + tt:SetOwner(a, "ANCHOR_RIGHT") + tt:AddLine("one") + tt:Show() + tt:SetHeight(170) + tt:SetWidth(350) + "#, + ) + .unwrap(); + measure_all(&mut s, &[("one", 20.0, 12.0)]); + s.run( + r#" + assert(TT9:GetWidth() == 350, "authored width holds, got " .. TT9:GetWidth()) + assert(TT9:GetHeight() == 170, "authored height holds, got " .. TT9:GetHeight()) + "#, + ) + .unwrap(); + // Fresh content takes the plate back, both dimensions: one 20x12 line ⇒ 40 x 32. + s.run(r#"TT9:AddLine("two")"#).unwrap(); + measure_all(&mut s, &[("one", 20.0, 12.0), ("two", 20.0, 12.0)]); + s.run( + r#" + assert(TT9:GetWidth() == 40, "auto width is back, got " .. TT9:GetWidth()) + assert(TT9:GetHeight() == 46, "auto height is back, got " .. TT9:GetHeight()) + "#, + ) + .unwrap(); +} + +/// The other half of that rule: a size written BEFORE `Show` is overwritten, because `Show` is +/// itself one of the resize's call sites (`0x530aad`). That is what makes pfUI's +/// `GameTooltip:SetWidth(180)` ahead of its `Show` (`pfUI/modules/addons.lua:245`) and pfQuest's +/// `SetWidth(100)` (`pfQuest/config.lua:634`) no-ops on the reference too — freezing the shared +/// `GameTooltip` at their width for every hover after would be the regression. +#[test] +fn a_size_authored_before_show_is_overwritten_by_the_auto_size() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + local a = CreateFrame("Button", "A10"); a:SetPoint("TOPLEFT", 100, -100); a:SetWidth(10); a:SetHeight(10) + local tt = CreateFrame("GameTooltip", "TT10") + tt:SetOwner(a, "ANCHOR_RIGHT") + tt:AddLine("one") + tt:SetWidth(180) + tt:Show() + "#, + ) + .unwrap(); + measure_all(&mut s, &[("one", 20.0, 12.0)]); + s.run(r#"assert(TT10:GetWidth() == 40, "auto width wins, got " .. TT10:GetWidth())"#) + .unwrap(); +} + #[test] fn minimum_width_floors_autosize() { let mut s = script(); diff --git a/crates/benilla-ui/src/script/tooltip/mod.rs b/crates/benilla-ui/src/script/tooltip/mod.rs index 172de3f6..8d2325c3 100644 --- a/crates/benilla-ui/src/script/tooltip/mod.rs +++ b/crates/benilla-ui/src/script/tooltip/mod.rs @@ -271,6 +271,8 @@ pub(super) fn clear_content(model: &mut Model, h: FrameHandle) { if let Ok(t) = tip_mut(model, h) { t.num_lines = 0; t.min_width = 0.0; + // Fresh content: the auto-size takes the plate back from any Lua-authored size. + t.authored_size = false; t.unit_token = None; // The set-time guid goes with the token it was recorded beside: a cleared tooltip shows no // unit, so `GetUnitGUID` must stop answering for the one it used to show. @@ -503,6 +505,7 @@ pub(super) fn append_line( } if let Ok(t) = tip_mut(&mut model, h) { t.num_lines = n; + t.authored_size = false; } cancel_fade(&mut model, h); Ok(()) @@ -594,17 +597,22 @@ pub(super) fn layout_tooltips(model: &mut Model) { if cursor_mode { cursor_anchor(model, h); } - let (num, lefts, rights, min_w, pad_w) = match model.arena.frame(h).map(|f| &f.kind_state) { - Some(KindState::Tooltip(t)) => ( - t.num_lines, - t.left_lines.clone(), - t.right_lines.clone(), - t.min_width, - t.padding, - ), - _ => continue, - }; - if num == 0 { + let (num, lefts, rights, min_w, pad_w, authored) = + match model.arena.frame(h).map(|f| &f.kind_state) { + Some(KindState::Tooltip(t)) => ( + t.num_lines, + t.left_lines.clone(), + t.right_lines.clone(), + t.min_width, + t.padding, + t.authored_size, + ), + _ => continue, + }; + // Lua sized this plate itself after its last content verb: leave it alone, size and right + // column both, until the content changes or it is `Show`n again. The reference's resize + // (`0x530640`) simply does not run again in between ([`TooltipState::authored_size`]). + if num == 0 || authored { continue; } let n = num.min(lefts.len()).min(rights.len()); diff --git a/crates/benilla-ui/src/script/tooltip/verbs.rs b/crates/benilla-ui/src/script/tooltip/verbs.rs index b1628390..0b25dc34 100644 --- a/crates/benilla-ui/src/script/tooltip/verbs.rs +++ b/crates/benilla-ui/src/script/tooltip/verbs.rs @@ -397,7 +397,11 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { m.set( "SetMinimumWidth", lua.create_function(|lua, (this, w): (Table, f32)| { - with_tip(lua, &this, |t| t.min_width = w.max(0.0)) + with_tip(lua, &this, |t| { + t.min_width = w.max(0.0); + // An auto-size input: the plate goes back to auto-sizing, as it does on content. + t.authored_size = false; + }) })?, )?; // SetPadding(w): extra auto-size width that survives a content clear (ItemRefTooltip's 16 @@ -405,7 +409,10 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { m.set( "SetPadding", lua.create_function(|lua, (this, w): (Table, f32)| { - with_tip(lua, &this, |t| t.padding = w.max(0.0)) + with_tip(lua, &this, |t| { + t.padding = w.max(0.0); + t.authored_size = false; + }) })?, )?; // FadeOut(): alpha ramps to 0 over `TOOLTIP_FADE_SECS`, then the full hide; new content or @@ -433,7 +440,14 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { let mut model = lua.app_data_mut::().expect("model app_data"); full_alpha(&mut model, h); tip_mut(&mut model, h) - .map(|t| t.owner.is_some() && t.num_lines > 0) + .map(|t| { + // Show re-sizes from the content (`0x530aad` calls `0x530640`), so a size + // authored before it is overwritten — which is what makes pfUI's + // `GameTooltip:SetWidth(180)` ahead of its `Show` a no-op on the reference + // too (`pfUI/modules/addons.lua:245`). One written after it stands. + t.authored_size = false; + t.owner.is_some() && t.num_lines > 0 + }) .unwrap_or(false) }; match live { diff --git a/crates/benilla-ui/src/widget/kinds/mod.rs b/crates/benilla-ui/src/widget/kinds/mod.rs index acbb3eeb..1695dbc5 100644 --- a/crates/benilla-ui/src/widget/kinds/mod.rs +++ b/crates/benilla-ui/src/widget/kinds/mod.rs @@ -208,6 +208,17 @@ pub struct TooltipState { pub equipped_header_armed: bool, /// `SetPadding`: extra width past the content (`ItemRef.xml:40` sets 16 for the close button). pub padding: f32, + /// Lua sized this plate itself (`SetWidth`/`SetHeight`) since its content last changed, so + /// the auto-size yields to it until the content does change. The reference re-sizes a tooltip + /// only from its own content verbs and from `Show` — `0x530640` has six call sites + /// (`0x52ad59`, `0x52b029`, `0x52e5ae`, `0x52f869`, `0x530615`, `0x530aad`), all inside the + /// tooltip code and none in the frame pump — so a size written after that call stands. + /// benilla auto-sizes in a pre-pass of every `resolve` instead (the measures live there), and + /// without this flag that pre-pass overwrote the explicit size on the very next frame. + /// ATSW2's recipe tooltip is the case: it `Show()`s, then sizes the plate from its own line + /// count and column width (`AdvancedTradeSkillWindow2.lua:5913-5920`), and drew as a + /// one-line box with its right column on top of its left one. + pub authored_size: bool, /// Line 1 came from XML (`ShoppingTooltipTemplate`), so later lines clone the previous fonts. pub xml_declared_lines: bool, /// What this tooltip is currently showing, for ClassicAPI's `GetItem`/`GetSpell`/`Has*` diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 81cca4dd..2de82be6 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -4551,3 +4551,103 @@ vignetted by `ATSWBackgroundShadow`) — the one piece of this window version 1 for. **Hidden, not blanked**, because the addon re-sets the texture on every open and every profession switch; `Hide` survives both, and a `hooksecurefunc` on `ATSW_UpdateBackground` is belt and braces. Nothing anchors to either texture, so nothing moved. + +## A tooltip the addon sized itself was re-sized out from under it every frame + +grom, with a screenshot: *"the tooltip box when using atsw2 is not drawing correctly for some +reason."* In the shot the plate's border wraps the recipe's name and stops; the rest of the +tooltip's text hangs outside it over the window, and one line is two strings drawn on top of each +other — `Required reagents:` with `Bags Bank Alts` sitting on it. + +Both are one defect, and it is ours. + +### The rule the reference keeps + +A `GameTooltip` auto-sizes from its lines in `0x530640`, which measures each line (`0x772890`) and +writes the plate. Scanning `.text` for `call` relocations to it finds **six** call sites — +`0x52ad59`, `0x52b029`, `0x52e5ae`, `0x52f869`, `0x530615` and `0x530aad` — all inside the tooltip's +own code, and the last of those is `Show` (`0x530a80`). **None is in the frame pump.** So the +reference re-sizes a tooltip when its content changes and when it is shown, and at no other time: a +`SetWidth`/`SetHeight` written after that call stands until the next one. + +benilla auto-sizes in a pre-pass of every `resolve` instead (`script::tooltip::layout_tooltips`), +because that is where the text measures are. That is a fine place to *compute* it and the wrong +rule for *when*: the pre-pass overwrote the explicit size on the very next frame. Ten lines of Lua +prove it on their own, no addon involved: + +``` +tip1 SetWidth/SetHeight after Show immediately 350.0 x 170.0 | a few frames later 23.8 x 33.3 +tip2 SetWidth/SetHeight before Show immediately 350.0 x 170.0 | a few frames later 23.8 x 33.3 +``` + +ATSW2 builds its recipe tooltip by hand — its own line pool, its own `AddDoubleLine`, a +`ClearLines` that shadows ours — and sizes the plate itself at the end: +`Tooltip:Show()`, then `SetHeight(NumLines * 18 + 12 - …)`, then `SetWidth(MaxLineWidth)`, which is +floored at 350 (`AdvancedTradeSkillWindow2.lua:5913-5920`). Losing that width is both symptoms at +once: the backdrop collapses to the one blank line the addon pushed through our `AddLine`, and +every right column, anchored `RIGHT, -10` to the plate, lands on top of its left column. + +### The fix + +`TooltipState::authored_size`: set by `SetWidth`/`SetHeight` through the existing +`note_authored_size`, cleared by any content change (`clear_content`, `append_line`), by +`SetMinimumWidth`/`SetPadding`, and by `Show`. `layout_tooltips` skips a plate that carries it — +size and right-column flush both, since the reference's resize is all-or-nothing and simply does +not run again. + +Clearing it on `Show` is not a detail. pfUI (`modules/addons.lua:245`) and pfQuest +(`config.lua:634`) both call `GameTooltip:SetWidth(…)` *before* `Show`, where the reference's resize +overwrites it — those calls are no-ops on a real client, and honouring them would freeze the shared +`GameTooltip` at 180 or 100 px wide for every hover afterwards. Two tests pin both halves, and +`unmeasured_lines_hold_declared_size` already covered the third case (a declared size holding until +the measures land). + +Measured after, driving ATSW2's own `OnEnter`: plate **350.0 x 81.7**, `Required reagents:` at +x=566 and `Bags Bank Alts` at x=798 — 232 px apart, where before they shared a pixel column. + +### The right-hand pane was black text on a dark panel + +*"can we make the font on the right like yellow like the title or white so it's readable?"* + +Measured rather than guessed — `GetTextColor()` on each string of that pane: + +| string | colour | | +|---|---|---| +| `ATSWRecipeName` | `0/0/0` | the item's name, 17 pt, no outline | +| `ATSWTool1Text` | `0/0/0` | the tools line | +| `ATSWCostText`, `ATSWCraftDescription` | `0/0/0` | | +| `ATSWToolsLabel`, `ATSWReagentsLabel` | `1/0.82/0` | already right | +| `ATSWReagent1Title` | `1/1/1` | | + +They are **black** by the addon's own design — `ATSWFont` and `CostFont` declare +`` and the tool template's `` does too — because the addon +paints this pane over its own light parchment. That parchment came off last round at his request, +so the black now sits on a dark pfUI backdrop. The skin repairs what the skin broke: the name goes +yellow like the title, the rest white. + +The tools line needed more than a colour write, because it has three writers: `ATSW_ShowRecipe` +paints it `SetTextColor(R, 0, 0)` where `R` is 1 only when the tool is **missing**, the template's +`OnEnter` paints it white, and its `OnLeave` paints `(this.R, 0, 0)` back. Rather than chase all +three, the string's own `SetTextColor` is wrapped to send black to white and leave every other +colour alone — so a tool you lack is still red, which is the only thing that colour carries. + +### A border for the task list + +*"would also love to see some sort of border around the queued task box because its a little hard +to distinguish right now."* + +`ATSWTaskScrollFrame` is the box's geometry (L=625 R=981 T=375 B=260 against a window of L=279 +R=1047 T=722 B=210) but it is **not** the rows' parent and it is not shown: version 2 scrolls the +list itself and parents every `ATSWTask%d` to `ATSWFrame`, exactly as it does with the recipe rows +and `ATSWListScrollFrame`. A backdrop on a hidden frame draws nothing, so the panel is its own +frame anchored to that rect, held one frame level below the rows so it sits behind them. + +### Driving ATSW2 in a capture needs version 1 out of the way + +Both addons are installed and **both use the `ATSW` prefix** — `ATSWFrame`, `ATSWRecipe1`, +`ATSW_RECIPES_DISPLAYED`. A capture loads every folder under `AddOns/` regardless of what the +player has enabled, so version 1 took the globals and `getglobal("ATSWRecipe1")` answered nil while +a perfectly good trade-skill window was on screen — version 1's. `BENILLA_HOME=` with an +`AddOns/` of symlinks to every folder but `AdvancedTradeSkillWindow`, and a `saved` symlink beside +it for `WOW_CAPTURE_SAVED`, gives a deterministic version-2 run without touching the player's +install.