From 9caacad153651e49ec441462ca2ca01613829375 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 11 Sep 2026 21:06:04 +0700 Subject: [PATCH] brightness saves, the glue box holds still, and Questie's notes hover - the Brightness slider in the options window changes the picture and keeps its value; it was writing to no setting at all, so every reopen read 50% again (2186) - the login, realm and character screens share one letterboxed frame, so clicking down the character roster no longer slides the backdrop and every panel over it; at 4:3 nothing changes (2187) - Questie's world-map notes show their tooltip on hover: the note hands the tooltip a frame where an anchor name goes, which benilla refused with a script error and the reference reads as the default anchor (2176) - with OmniCC installed, a warrior in a stance sees the countdown on action bar 1 again: raising that bar's buttons by script carried their cooldown sweep up over the count (2189) Co-Authored-By: Claude Fable 5.1 --- crates/benilla-app/assets/ui/OptionsFrame.xml | 15 +- crates/benilla-app/src/cvars.rs | 16 + crates/benilla-app/src/glue/mod.rs | 10 +- crates/benilla-app/src/portrait/framing.rs | 438 ++++++++++-------- crates/benilla-app/src/portrait/glue_booth.rs | 64 ++- crates/benilla-app/src/portrait/mod.rs | 9 +- .../src/ui_script/action_bar_tests.rs | 151 ++++-- .../src/ui_script/options_tests.rs | 76 +++ .../src/bin/benilla-extract/glueextent.rs | 4 +- crates/benilla-formats/src/lib.rs | 6 +- .../benilla-formats/src/models/art_extent.rs | 111 +++-- .../src/script/object/frame_state.rs | 8 +- crates/benilla-ui/src/script/tests/tooltip.rs | 57 +++ .../benilla-ui/src/script/tests/toplevel.rs | 26 ++ crates/benilla-ui/src/script/tooltip/verbs.rs | 20 +- 15 files changed, 698 insertions(+), 313 deletions(-) diff --git a/crates/benilla-app/assets/ui/OptionsFrame.xml b/crates/benilla-app/assets/ui/OptionsFrame.xml index c17a65e3..9619b109 100644 --- a/crates/benilla-app/assets/ui/OptionsFrame.xml +++ b/crates/benilla-app/assets/ui/OptionsFrame.xml @@ -980,6 +980,19 @@ -- The third kind is what Show Helm / Show Cloak are: the preference is a bit of the -- character's own server-side PLAYER_FLAGS, so there is nothing local to save and nothing -- to restore at load — the getter IS the state (decision 1472). + -- **Does this row have somewhere to put a value?** The three stores of 1134, asked in the + -- order `OptionsRow_Set` tries them. A row with none is display-only and must not be + -- written. + -- + -- It exists because the slider's OnValueChanged guarded on `row.cvar or row.uvar` — two of + -- the three — and so wrote nothing for a row whose store is an engine pair. The readout + -- still moved, which is the worst shape that bug can take: the control looks alive, the + -- setting never leaves the window, and nothing anywhere errors. Asking one predicate keeps + -- every caller in step with the setter by construction instead of by memory. + function OptionsRow_HasStore(row) + return row.uvar or row.setFunc or row.cvar + end + function OptionsRow_Get(row) if row.uvar then return getglobal(row.uvar) end if row.getFunc then @@ -1441,7 +1454,7 @@ end end getglobal(control:GetName() .. "Value"):SetText(OptionsRow_FormatValue(row, value)) - if (row.cvar or row.uvar) and not row.refreshing then + if OptionsRow_HasStore(row) and not row.refreshing then -- Two decimals cover every page's grid (the 5% volumes, uiScale's 0.01, whole -- yards); trailing zeros stripped so a value at the registered default -- compares equal to it as a STRING and stays out of the config file ("0.40" diff --git a/crates/benilla-app/src/cvars.rs b/crates/benilla-app/src/cvars.rs index 760f8e15..250aa5eb 100644 --- a/crates/benilla-app/src/cvars.rs +++ b/crates/benilla-app/src/cvars.rs @@ -2915,6 +2915,22 @@ mod tests { "weatherDensity {wrote}" ); } + // Brightness (2182): the CVar's own unit is the ramp exponent, NOT the slider's offset — + // `SetGamma` does the `1 - v` on the way in, so what arrives here is already `gamma`. + // Both ends of the stock slider land whole, and the consumer's clamp holds the values the + // reference accepts without one (`SetGamma(5)` writes -4 there). + for (wrote, want) in [("1.000000", 1.0), ("0.500000", 0.5), ("1.500000", 1.5)] { + assert!(apply_to_knobs("gamma", wrote, &mut knobs)); + assert_eq!(knobs.display_gamma.0, want, "gamma {wrote}"); + } + assert!(apply_to_knobs("gamma", "-4.000000", &mut knobs)); + assert_eq!( + knobs.display_gamma.0, + *crate::ui_gamma::GAMMA_RANGE.start(), + "a negative exponent clamps at the consumer, where it cannot blank the screen" + ); + assert!(apply_to_knobs("gamma", "99", &mut knobs)); + assert_eq!(knobs.display_gamma.0, *crate::ui_gamma::GAMMA_RANGE.end()); assert_eq!(knobs.clutter.frill_density(), 32.0); // And the pair is NAMED as a pair, in the registered spelling and the lowercased one, so // `$WOW_CLUTTER_DENSITY` cannot take one spelling of this knob for the session and leave diff --git a/crates/benilla-app/src/glue/mod.rs b/crates/benilla-app/src/glue/mod.rs index 07d576ee..1c221f99 100644 --- a/crates/benilla-app/src/glue/mod.rs +++ b/crates/benilla-app/src/glue/mod.rs @@ -251,15 +251,19 @@ pub(crate) fn glue_canvas() -> (GlueCanvas, FocusPolicy, Node) { /// only on change keeps it off Bevy's `Changed` path on the ~every frame nothing moves. pub(crate) fn fit_glue_canvas( window: Query<&Window, With>, - scene: Option>, mut canvases: Query<&mut Node, With>, ) { if canvases.is_empty() { return; } + // Off the **window**, not off `CreateScene` (decision 2187): the frame is one aspect for every + // scene now, so the canvas has no reason to wait on the booth — which is what used to move it + // when a stage swap cleared the box for a frame, and what made a screen spawned before its + // scene lay out against the window once and snap in. + let window = window.single().ok(); let (left, right) = crate::portrait::glue_canvas_bars( - window.single().ok(), - scene.and_then(|s| s.viewport_aspect()), + window, + window.and_then(|w| crate::portrait::glue_box_aspect(w.width() / w.height().max(1.0))), ); let (left, right) = (Val::Px(left), Val::Px(right)); for mut node in &mut canvases { diff --git a/crates/benilla-app/src/portrait/framing.rs b/crates/benilla-app/src/portrait/framing.rs index 5da0e4ed..37a6acf6 100644 --- a/crates/benilla-app/src/portrait/framing.rs +++ b/crates/benilla-app/src/portrait/framing.rs @@ -80,17 +80,14 @@ pub(super) use benilla_formats::{ArtExtent, GLUE_AUTHORED_ASPECT}; /// /// **…as far as the art goes** (decision 1619, B330). Every diorama is finite — a sky card of some /// authored width, a ground that stops — and 1587's hor+ had no ceiling: `UI_MainMenu` runs out -/// of backdrop before 16:9 and the frame past its slanted edges was the clear colour; `UI_Tauren` -/// runs out at ~2.24:1. **Not one of the seven scenes is drawn wider than about 3:2** -/// ([`benilla_formats::shipped_glue_art_extent`], measured per texel off the chain: MainMenu 1.54, -/// Human 1.47, Orc 1.57, Dwarf 1.43, NightElf 1.45, Scourge 1.57, Tauren 1.46 — 1587's "no void at -/// 21:9 on `UI_Human`" held only because the create screen's side panels stand exactly over it). -/// So `art` bounds both legs: the wide leg grows the width only to `art.half_w` and past it -/// **holds the width and closes the vertical** — the zoom the reference does everywhere, done -/// only where the art forces it — and the narrow leg does the same with `art.half_h`. The -/// authored box itself is never given up (`max(h0)` / `max(t0)`): a scene whose art does not fill -/// 4:3 is the art's problem, not a reason to zoom past the composition. `None` is 1587's -/// unbounded law. +/// of backdrop before 16:9 and the frame past its slanted edges was the clear colour. **Not one of +/// the seven scenes is drawn wider than about 3:2** ([`benilla_formats::SHIPPED_GLUE_SCENES`], +/// measured per texel off the chain: MainMenu 1.54, Human 1.47, Orc 1.57, Dwarf 1.42, NightElf +/// 1.31, Scourge 1.57, Tauren 1.46). So the wide leg grows the width only to a ceiling and past it +/// **holds the width and closes the vertical** — the zoom the reference does everywhere, done only +/// where the art forces it. The narrow leg does the same with `art.half_h`, and the authored box +/// itself is never given up (`max(t0)`): a scene whose art does not fill 4:3 is the art's problem, +/// not a reason to zoom past the composition. /// /// **…and never tighter than the reference at 16:9.** Holding the width zooms faster than the /// reference's diagonal law, so left alone it re-crops the character at 21:9 (B242, the report @@ -99,74 +96,82 @@ pub(super) use benilla_formats::{ArtExtent, GLUE_AUTHORED_ASPECT}; /// years, an empirical bound on "the character still fits", not a mechanism. (A floor of "what /// 16:9 needs to reach this scene's art" was tried and clipped the night elf male's hair and /// boots at 16:9 — 8% past the reference is past what fits.) Past the aspect where the floor's -/// opening cannot fill the art's width, the scene is **pillarboxed** — rendered at that aspect, -/// centred, black either side — which is what the modern client does past 16:9 and the only -/// honest answer once the art is narrower than the window: not void (B330), not a crop (B242), -/// a frame. +/// opening cannot fill the ceiling's width, the scene is **pillarboxed** — rendered at that +/// aspect, centred, black either side — which is what the modern client does past 16:9 and the +/// only honest answer once the art is narrower than the window: not void (B330), not a crop +/// (B242), a frame. /// -/// Where this lands: at 16:9 every scene but one zooms to within a few percent of the reference's -/// own `0.49·fov`, by construction rather than by a constant, and fills the window; the night -/// elves' stage — its sky card exactly 4:3 wide, so the box itself is its ceiling — is boxed at -/// 1.65:1 there, ~3.6% of the width each side, where the reference shows black corners past the -/// same card. At 21:9 the reference cropped -/// and 1587 showed void, and every scene now shows its 16:9 framing with bars (~9–13% of the -/// width, per scene). At 4:3 nothing changes: byte-identical. +/// **The ceiling is [`GLUE_BOX_ASPECT`]'s, not the scene's own** (decision 2187). 1619 read the +/// ceiling off each scene's measured art, so each of the seven boxed at its *own* aspect — 1.65:1 +/// in the night elf grove, 1.93:1 at the gate — and the box is what the chrome's canvas insets by +/// (2091). Clicking a night elf on the character screen therefore moved the whole screen: at 16:9 +/// the human, orc, scourge and tauren stages fill the window, the dwarves' boxes by a third of a +/// percent and the night elves' by 3.6% a side, so the backdrop and every panel over it jumped +/// inward and back as the selection moved down the roster. **One box for every scene** fixes that +/// at the cause: the frame is a property of the *window*, and nothing about it changes when the +/// scene does. The size is the narrowest box no scene has to give up its authored 4:3 composition +/// for ([`GLUE_BOX_ASPECT`]). +/// +/// Where this lands: at 4:3 nothing changes, byte-identical. Between 4:3 and 1.672:1 every scene +/// fills the window, zooming as the width outruns the box (within 1.4% of each other — the only +/// spread left is `tan`'s, across the scenes' 60°–86° fovs). At or past 1.672:1 every scene sits +/// on its own 16:9 floor — the *reference's* framing — inside a box of exactly that aspect: 3.0% +/// of the width a side at 16:9, 15% at 3440×1440. What it costs: the wide-art scenes no longer +/// fill a 21:9 panel the way 1619 let them (the gate went from 9.7% bars a side to 15%). That is +/// the price of one frame, and one frame is what was asked for. /// /// Scoped to the full-screen glue scene on purpose. A `` pane renders into a fixed /// rect that is *not* the window, so its crop is its own `W/H` and stays verbatim ([`diag_to_vert`], /// decisions 1089/1543). -pub(super) fn glue_scene_framing( - fov: f32, - window_aspect: f32, - art: Option, -) -> GlueFraming { +pub(super) fn glue_scene_framing(fov: f32, window_aspect: f32, art: Option) -> f32 { let authored = diag_to_vert(fov, GLUE_AUTHORED_ASPECT); - let full = |vert_fov: f32| GlueFraming { - vert_fov, - viewport_aspect: None, - }; if window_aspect <= 0.0 { - return full(authored); // a degenerate (mid-resize) window: the authored opening, finite + return authored; // a degenerate (mid-resize) window: the authored opening, finite } let t0 = (authored * 0.5).tan(); // the authored vertical half-extent let h0 = t0 * GLUE_AUTHORED_ASPECT; // …and horizontal if window_aspect >= GLUE_AUTHORED_ASPECT { - // hor+: the authored vertical opening, until the width reaches the art's edge; past it + // hor+: the authored vertical opening, until the width reaches the box's edge; past it // the width is held there and the vertical closes — down to the 16:9 floor, past which - // the window is wider than the framing can honestly fill: pillarbox. - let ceiling = art.map_or(f32::INFINITY, |a| a.half_w.max(h0)); - let half_w = (t0 * window_aspect).min(ceiling); + // the window is wider than the framing can honestly fill and [`glue_box_aspect`] boxes it. let floor = glue_zoom_floor(fov); - let half_h = half_w / window_aspect; - if half_h >= floor { - full(2.0 * half_h.atan()) - } else { - GlueFraming { - vert_fov: 2.0 * floor.atan(), - viewport_aspect: Some(half_w / floor), - } - } + let half_w = (t0 * window_aspect).min(GLUE_BOX_ASPECT * floor); + 2.0 * (half_w / window_aspect).max(floor).atan() } else { // The mirror: hold the authored horizontal half-extent and open upward, until the art's // top/bottom edge; past it the height is held and the sides crop — the stage's edges, // never the character, so no floor and no bars. let ceiling = art.map_or(f32::INFINITY, |a| a.half_h.max(t0)); - let half_h = (h0 / window_aspect).min(ceiling); - full(2.0 * half_h.atan()) + 2.0 * (h0 / window_aspect).min(ceiling).atan() } } -/// What [`glue_scene_framing`] decided for one window. -#[derive(Clone, Copy, Debug, PartialEq)] -pub(super) struct GlueFraming { - /// The booth camera's vertical opening (radians). - pub(super) vert_fov: f32, - /// `Some(aspect)`: render into a centred viewport of this aspect instead of the whole window - /// (pillarbox — the window is wider than the art can fill at the zoom floor). `None`: the - /// whole window. - pub(super) viewport_aspect: Option, +/// The **one box** every glue scene is framed in, for a window of this aspect: `Some(aspect)` when +/// the window is wider than [`GLUE_BOX_ASPECT`] (pillarbox — render into a centred box of that +/// aspect, black either side), `None` when the scene fills the window. +/// +/// A property of the **window alone** — no scene, no fov, no art (decision 2187). That is the whole +/// point: the booth camera's viewport and the chrome's canvas ([`crate::glue::GlueCanvas`]) both +/// come from here, so neither moves when the selected character's race changes the stage behind +/// them, and neither flickers in the frames a stage swap is in flight. +pub(crate) fn glue_box_aspect(window_aspect: f32) -> Option { + (window_aspect > GLUE_BOX_ASPECT).then_some(GLUE_BOX_ASPECT) } +/// **The glue frame: `1.672:1`** — the aspect every glue scene is pillarboxed to once the window +/// is wider than it (decision 2187). +/// +/// Derived, not chosen: it is `max` over the seven shipped scenes of `h0/floor` — each scene's +/// authored 4:3 **width** (`h0`) divided by the vertical half-extent the [`glue_zoom_floor`] holds +/// it at. Below that aspect a box would have to crop *inside* some scene's authored composition; +/// at it, the widest-fov scene (`UI_MainMenu`, 86°) shows exactly its authored 4:3 width at the +/// reference's own 16:9 opening, and the other six show a hair more than theirs. It is the +/// narrowest frame that costs no scene its composition, which is as close to "the smallest one" as +/// the seven allow. Pinned — with the two facts it rests on, that no scene's art is narrower than +/// the frame and that no scene's authored box is wider — by +/// [`tests::the_glue_frame_is_the_narrowest_box_no_scene_pays_for`]. +pub(super) const GLUE_BOX_ASPECT: f32 = 1.672_042_7; + /// The panel the reference was played on without complaint for fifteen years — the aspect whose /// reference framing is the zoom floor, and the one the modern client fills before it boxes. pub(super) const REFERENCE_PANEL: f32 = 16.0 / 9.0; @@ -179,8 +184,7 @@ pub(super) fn glue_zoom_floor(fov: f32) -> f32 { } /// **Where the pillarbox's box lands on the window**, in *physical* pixels: `(x, width)` of the -/// centred box, or `None` when the framing fills the window -/// ([`GlueFraming::viewport_aspect`] is `None`). +/// centred box, or `None` when the framing fills the window ([`glue_box_aspect`] is `None`). /// /// One formula, two readers, and that is the point of it being a function. The booth camera's /// `Viewport` ([`super::glue_booth::pillarbox_glue_scene`]) renders the scene into this rect; the @@ -815,37 +819,44 @@ mod tests { )); } - /// **B242 / decision 1587.** The full-screen glue scene holds the authored 4:3 view box on - /// every window shape, instead of spending height on width the way the reference's - /// `aspect = a_screen` does. - /// - /// Unbounded (no art): the authored `0.6·fov` survives at 4:3, at 16:9 and at the 3440×1440 - /// panel the report came from — where the un-pinned law gives `0.386·fov`, a 1.55× zoom, and - /// the character's head and feet leave the frame. + /// **B242 / decision 1587.** The full-screen glue scene holds the authored 4:3 view box + /// instead of spending height on width the way the reference's `aspect = a_screen` does — and + /// under 2187's one frame it is **never framed tighter than the reference's own 16:9 opening**, + /// on any window however wide. The 3440×1440 panel the report came from used to get + /// `0.386·fov`, a 1.55× zoom over the authored framing, with the character's head and feet out + /// of the frame; it gets `0.490·fov` — the 16:9 opening — inside the frame. #[test] - fn the_glue_scene_holds_its_authored_vertical_on_every_wide_window() { + fn the_glue_scene_is_never_framed_tighter_than_the_reference_at_16_9() { let close = |a: f32, b: f32| (a - b).abs() < 5e-4; let authored = diag_to_vert(1.0, GLUE_AUTHORED_ASPECT); assert!( close(authored, 0.6), "the authored 4:3 opening is the 0.6 legend" ); - for wide in [ - GLUE_AUTHORED_ASPECT, - 16.0 / 10.0, - 16.0 / 9.0, - 3440.0 / 1440.0, - 32.0 / 9.0, - ] { - let f = glue_scene_framing(1.0, wide, None); - assert!( - close(f.vert_fov, authored) && f.viewport_aspect.is_none(), - "a{wide} must keep the authored vertical opening, unboxed" - ); + // 4:3 itself: byte-identical to 1587, and unboxed. + assert!(close( + glue_scene_framing(1.0, GLUE_AUTHORED_ASPECT, None), + authored + )); + assert!(glue_box_aspect(GLUE_AUTHORED_ASPECT).is_none()); + // Between 4:3 and the frame the opening closes, monotonically, and the window still fills. + let mut last = authored; + for a in [1.4, 16.0 / 10.0, GLUE_BOX_ASPECT] { + let vert = glue_scene_framing(1.0, a, None); + assert!(vert <= last + 1e-6, "a{a}: {vert} > {last}"); + assert!(glue_box_aspect(a).is_none(), "a{a} fills the window"); + last = vert; } - // What we are NOT doing any more — the reference's own numbers, kept here so the size of - // the correction stays legible. - assert!(close(diag_to_vert(1.0, 16.0 / 9.0), 0.490_26)); + // At and past the frame: the reference's own 16:9 opening, exactly, for ever. + let reference_16_9 = diag_to_vert(1.0, REFERENCE_PANEL); + assert!(close(reference_16_9, 0.490_26)); + for wide in [REFERENCE_PANEL, 3440.0 / 1440.0, 32.0 / 9.0] { + let vert = glue_scene_framing(1.0, wide, None); + assert!(close(vert, reference_16_9), "a{wide}: {vert}"); + assert_eq!(glue_box_aspect(wide), Some(GLUE_BOX_ASPECT)); + } + // What we are NOT doing any more — the reference's own number on the reporter's panel, + // kept here so the size of the correction stays legible. assert!(close(diag_to_vert(1.0, 3440.0 / 1440.0), 0.386_14)); } @@ -860,7 +871,7 @@ mod tests { GLUE_AUTHORED_ASPECT, ); for narrow in [5.0 / 4.0, 1.0, 3.0 / 4.0] { - let got = half_width(glue_scene_framing(1.0, narrow, None).vert_fov, narrow); + let got = half_width(glue_scene_framing(1.0, narrow, None), narrow); assert!( (got - authored).abs() < 5e-4, "a{narrow}: authored half-width {authored} vs {got}" @@ -868,111 +879,152 @@ mod tests { } // …and it never *narrows* the view: a tall window sees more, never less. assert!( - glue_scene_framing(1.0, 1.0, None).vert_fov - > glue_scene_framing(1.0, GLUE_AUTHORED_ASPECT, None).vert_fov + glue_scene_framing(1.0, 1.0, None) + > glue_scene_framing(1.0, GLUE_AUTHORED_ASPECT, None) ); // A degenerate window (zero height, mid-resize) must not produce a NaN fov. - assert!(glue_scene_framing(1.0, 0.0, None).vert_fov.is_finite()); + assert!(glue_scene_framing(1.0, 0.0, None).is_finite()); } - /// The art ceiling (decision 1619): with a measured extent the wide leg widens exactly to the - /// art's edge, past it holds that width and closes the vertical, and once the vertical would - /// close below the reference's 16:9 opening the window is pillarboxed at the aspect that - /// opening fills the art at. At and below the art's aspect nothing differs from 1587's law. + /// **Decision 2187 — one frame, whatever is standing in it.** The wide leg widens to + /// [`GLUE_BOX_ASPECT`]'s edge, past it holds that width and closes the vertical, and once the + /// vertical would close below the reference's 16:9 opening the window is pillarboxed at + /// exactly that aspect — *the same* aspect for every scene, whatever its fov or its art, which + /// is what stops the backdrop and the chrome over it jumping as the roster selection moves. #[test] - fn a_wide_window_widens_to_the_arts_edge_then_zooms_then_pillarboxes() { + fn every_glue_scene_is_framed_in_the_same_box() { let close = |a: f32, b: f32| (a - b).abs() < 5e-4; - let authored = diag_to_vert(1.0, GLUE_AUTHORED_ASPECT); - let t0 = (authored * 0.5).tan(); - // Art that runs out of width at exactly 3:2 (the login gate's shape), tall enough never - // to bind. - let art = Some(ArtExtent { - half_w: t0 * 1.5, - half_h: 10.0, - }); - let half_width = - |f: GlueFraming, a: f32| (f.vert_fov * 0.5).tan() * f.viewport_aspect.unwrap_or(a); - // Up to 3:2 the vertical opening is the authored one — identical to the unbounded law. - for a in [GLUE_AUTHORED_ASPECT, 1.4, 1.5] { - let f = glue_scene_framing(1.0, a, art); - assert!( - close(f.vert_fov, authored) && f.viewport_aspect.is_none(), - "a{a}" - ); + let half_width = |vert: f32, a: f32| (vert * 0.5).tan() * a; + // The seven shipped fovs, 60°–86°, plus the unit fov the rest of this module reasons in. + let deg = std::f32::consts::PI / 180.0; + for fov in [1.0_f32, 60.0 * deg, 65.0 * deg, 80.0 * deg, 86.0 * deg] { + let authored = diag_to_vert(fov, GLUE_AUTHORED_ASPECT); + let t0 = (authored * 0.5).tan(); + let floor = glue_zoom_floor(fov); + assert!(close( + floor, + (diag_to_vert(fov, REFERENCE_PANEL) * 0.5).tan() + )); + // 4:3 is 1587's own answer, for every fov: the authored opening, filling the window. + assert!(close( + glue_scene_framing(fov, GLUE_AUTHORED_ASPECT, None), + authored + )); + // The zoom, between 4:3 and the frame: the width is the box's and the vertical pays. + for a in [1.5, 1.6, GLUE_BOX_ASPECT] { + let vert = glue_scene_framing(fov, a, None); + assert!( + close(half_width(vert, a), GLUE_BOX_ASPECT * floor), + "fov {fov} a{a}: half-width {}", + half_width(vert, a) + ); + assert!(vert < authored && vert >= 2.0 * floor.atan() - 1e-6); + } + // Past it: the floor holds the opening and the box is the frame — one number, no + // scene of its own in it. + for a in [REFERENCE_PANEL, 3440.0 / 1440.0, 32.0 / 9.0] { + let vert = glue_scene_framing(fov, a, None); + assert!( + close((vert * 0.5).tan(), floor), + "fov {fov} a{a} off the floor" + ); + assert_eq!(glue_box_aspect(a), Some(GLUE_BOX_ASPECT)); + assert!( + close(half_width(vert, GLUE_BOX_ASPECT), GLUE_BOX_ASPECT * floor), + "fov {fov} a{a}: the box ends where the width does" + ); + } + // The art no longer reaches the wide leg at all: a scene measured narrow and a scene + // measured wide are framed identically. (`half_h` still binds the NARROW leg.) + for art in [ + Some(ArtExtent { + half_w: t0 * 1.31, + half_h: 10.0, + }), + Some(ArtExtent { + half_w: t0 * 4.0, + half_h: 10.0, + }), + ] { + for a in [1.5, REFERENCE_PANEL, 3440.0 / 1440.0] { + assert_eq!( + glue_scene_framing(fov, a, art), + glue_scene_framing(fov, a, None), + "fov {fov} a{a}: the wide leg read the art" + ); + } + } } - // Past it the width is pinned at the art's edge and the vertical closes: the zoom. At - // 16:9 that lands beside the reference's own diagonal-law number (0.490) — a 3:2 edge - // gives 2·atan(1.5·t0/(16/9)) — within a few percent, and no constant says so. - let f = glue_scene_framing(1.0, 16.0 / 9.0, art); - assert!(f.viewport_aspect.is_none(), "16:9 fills the window"); - assert!(close(half_width(f, 16.0 / 9.0), t0 * 1.5)); + } + + /// **[`GLUE_BOX_ASPECT`] is derived, and the two facts it rests on hold** (decision 2187): + /// it is the narrowest box that costs no shipped scene its authored 4:3 composition, and at + /// that width no scene's art runs out inside the frame — except the night elves', whose sky + /// card ends a hair inside their own authored box and always did (1619's table doc). + #[test] + fn the_glue_frame_is_the_narrowest_box_no_scene_pays_for() { + use benilla_formats::{authored_half_height, SHIPPED_GLUE_SCENES}; + let mut widest = 0.0_f32; + for scene in SHIPPED_GLUE_SCENES { + let t0 = authored_half_height(scene.fov); + let h0 = t0 * GLUE_AUTHORED_ASPECT; + let floor = glue_zoom_floor(scene.fov); + // No scene gives up its authored width: the frame reaches at least h0. + assert!( + GLUE_BOX_ASPECT * floor >= h0 - 1e-4, + "UI_{}: the frame stops at {} inside the authored {h0}", + scene.token, + GLUE_BOX_ASPECT * floor + ); + // …and no scene's art stops inside the frame. The night elf grove is the exception + // 1619 measured and named — its 4:3 sky card is narrower than its own authored box, + // so the frame runs 1.3% past it into the black the bars are made of anyway, where + // 1619's own box ran 1.6% past the card for the same reason. + if scene.token == "NightElf" { + assert!(scene.art.half_w < h0, "the night elf exception closed?"); + assert!( + GLUE_BOX_ASPECT * floor <= h0 * 1.015, + "UI_NightElf: the frame reaches {} past the authored {h0}", + GLUE_BOX_ASPECT * floor + ); + } else { + assert!( + GLUE_BOX_ASPECT * floor <= scene.art.half_w, + "UI_{}: the frame reaches {} past the art's {}", + scene.token, + GLUE_BOX_ASPECT * floor, + scene.art.half_w + ); + } + widest = widest.max(h0 / floor); + } + // …and it is the NARROWEST such box: the widest-fov scene sits exactly on it. assert!( - f.vert_fov < authored && (f.vert_fov - 0.490_26).abs() < 0.03, - "{}", - f.vert_fov + (GLUE_BOX_ASPECT - widest).abs() < 5e-4, + "the frame is {GLUE_BOX_ASPECT}, the narrowest box no scene pays for is {widest}" ); - assert!(f.vert_fov >= 2.0 * glue_zoom_floor(1.0).atan() - 1e-6); - // Wider still, the zoom would pass the reference's 16:9 opening: the floor holds it - // there and the scene is boxed at the aspect the floor fills the art at (1.5·t0 / floor), - // the same box on any wider window. - let floor = glue_zoom_floor(1.0); - assert!(close(floor, (diag_to_vert(1.0, 16.0 / 9.0) * 0.5).tan())); - for a in [3440.0 / 1440.0, 32.0 / 9.0] { - let f = glue_scene_framing(1.0, a, art); - assert!( - close((f.vert_fov * 0.5).tan(), floor), - "a{a} sits on the floor" - ); - let boxed = f.viewport_aspect.expect("pillarboxed"); - assert!(close(boxed, t0 * 1.5 / floor), "a{a}: box {boxed}"); - assert!(boxed < a, "the box is narrower than the window"); - assert!( - close(half_width(f, a), t0 * 1.5), - "…and still ends at the art" - ); - } - // Wide art (a scene drawn out to 21:9) never binds: 1587's answer, unchanged. - let wide_art = Some(ArtExtent { - half_w: t0 * 4.0, - half_h: 10.0, - }); - let f = glue_scene_framing(1.0, 3440.0 / 1440.0, wide_art); - assert!(close(f.vert_fov, authored) && f.viewport_aspect.is_none()); - // Art narrower than 16:9 can reach at the reference's opening (the night elves' sky, a - // 4:3 card — 1.31, inside the authored box, so the box's own width is the ceiling): the - // floor holds and 16:9 itself is boxed, at the aspect the floor fills the box at — the - // same box on any wider window. - let narrow_art = Some(ArtExtent { - half_w: t0 * 1.31, - half_h: 10.0, - }); - let h0 = t0 * GLUE_AUTHORED_ASPECT; - for a in [16.0 / 9.0, 3440.0 / 1440.0] { - let f = glue_scene_framing(1.0, a, narrow_art); - assert!( - close((f.vert_fov * 0.5).tan(), floor), - "a{a} sits on the floor" - ); - let boxed = f.viewport_aspect.expect("boxed"); - assert!((boxed - h0 / floor).abs() < 2e-3, "a{a}: box {boxed}"); - } - // …and 16:10 is not: the box's width still fills it above the floor. - assert!(glue_scene_framing(1.0, 1.6, narrow_art) - .viewport_aspect - .is_none()); } /// **B377 / decision 2091 — the chrome's canvas IS the camera's box.** /// - /// At the reporter's own 3440×1440 the login gate boxes at 1.93:1 (1619 §3): `330 | 2779 | - /// 331` physical pixels, the bars he measured off the screenshot (332/331). The canvas insets - /// by exactly those two numbers — not by a symmetric half, which would hang a pixel of chrome - /// over the right bar — and it converts to Bevy UI's *logical* px on the way, which is the - /// half that a 2× display makes visible. + /// The arithmetic is pinned against pixels a reporter counted: at his own 3440×1440 the login + /// gate boxed at 1.93:1 under 1619's per-scene ceiling — `330 | 2779 | 331` physical pixels, + /// the bars he measured off the screenshot (332/331). The canvas insets by exactly those two + /// numbers — not by a symmetric half, which would hang a pixel of chrome over the right bar — + /// and it converts to Bevy UI's *logical* px on the way, which is the half that a 2× display + /// makes visible. (2187 framed every scene in one box instead, so that panel now boxes at + /// 1.672:1 — `516 | 2408 | 516`, asserted below. The odd-leftover case is what 1.93 is kept + /// for.) #[test] fn the_chrome_canvas_is_the_cameras_own_box() { use bevy::window::WindowResolution; + // The frame the same panel gets today, off the one constant, with no scene in the call. + assert_eq!( + glue_box_physical(3440, 1440, glue_box_aspect(3440.0 / 1440.0)), + Some((516, 2408)) + ); + let (x, box_w) = glue_box_physical(3440, 1440, Some(1.93)).expect("21:9 boxes the gate"); assert_eq!((x, box_w), (330, 2779)); assert_eq!( @@ -1016,17 +1068,9 @@ mod tests { /// the frame the chrome lays out in are the same rect at every window the law boxes. #[test] fn the_box_the_camera_renders_is_the_box_the_chrome_lays_out_in() { - let t0 = (diag_to_vert(1.0, GLUE_AUTHORED_ASPECT) * 0.5).tan(); - // The login gate's shape: art that runs out of width at 1.54·t0 (MainMenu, 1619 §2). - let art = Some(ArtExtent { - half_w: t0 * 1.54, - half_h: 10.0, - }); for (w, h) in [(3440, 1440), (2560, 1080), (5120, 1440), (3439, 1440)] { - let f = glue_scene_framing(1.0, w as f32 / h as f32, art); - let boxed = f - .viewport_aspect - .expect("every 21:9-and-wider window is boxed"); + let boxed = + glue_box_aspect(w as f32 / h as f32).expect("every window past the frame is boxed"); let (x, box_w) = glue_box_physical(w, h, Some(boxed)).expect("boxed"); let (left, right) = glue_canvas_bars( Some(&Window { @@ -1049,8 +1093,12 @@ mod tests { } } - /// The authored box is never given up: art that does not even reach it (a `0.0` extent, or - /// one inside 4:3) leaves the 4:3 framing exactly where 1587 put it — on both legs. + /// The authored box is never given up — on either leg, and at the frame's own edge. + /// + /// The wide leg: [`GLUE_BOX_ASPECT`] is defined so its width reaches at least the authored + /// `h0` at every shipped fov (its own pin test), so a scene whose art stops short of 4:3 is + /// still framed on its whole composition, never zoomed inside it. The narrow leg reads the + /// art, and clamps the same way: an extent inside the box leaves 1587's framing alone. #[test] fn art_inside_the_authored_box_never_zooms_past_the_composition() { let close = |a: f32, b: f32| (a - b).abs() < 5e-4; @@ -1060,36 +1108,34 @@ mod tests { half_h: 0.0, }); assert!(close( - glue_scene_framing(1.0, GLUE_AUTHORED_ASPECT, none).vert_fov, + glue_scene_framing(1.0, GLUE_AUTHORED_ASPECT, none), authored )); - assert!(close(glue_scene_framing(1.0, 1.0, none).vert_fov, authored)); - // At 16:9 with no art past the box, the box's own width is held — and holding 4:3's - // width in a 16:9 window closes the vertical past the reference's 16:9 opening, so the - // floor takes it: the authored box at that opening, boxed. - let f = glue_scene_framing(1.0, 16.0 / 9.0, none); + assert!(close(glue_scene_framing(1.0, 1.0, none), authored)); + // At the frame's own aspect the vertical has closed to the floor and the width is the + // frame's — which is still at or outside the authored 4:3 width. let t0 = (authored * 0.5).tan(); let floor = glue_zoom_floor(1.0); - assert!(close((f.vert_fov * 0.5).tan(), floor)); - let boxed = f.viewport_aspect.expect("boxed"); + let v = glue_scene_framing(1.0, GLUE_BOX_ASPECT, none); + assert!(close((v * 0.5).tan(), floor)); assert!( - (boxed - t0 * GLUE_AUTHORED_ASPECT / floor).abs() < 2e-3, - "box {boxed}" + (v * 0.5).tan() * GLUE_BOX_ASPECT >= t0 * GLUE_AUTHORED_ASPECT - 1e-4, + "the frame stopped inside the authored width" ); - // 16:10 still fills: 4:3's width there is above the floor. - assert!(glue_scene_framing(1.0, 1.6, none).viewport_aspect.is_none()); + // 16:10 still fills the window: it is narrower than the frame. + assert!(glue_box_aspect(1.6).is_none()); // …and a narrow window with a bounded height holds that height, cropping the sides. let short = Some(ArtExtent { half_w: 10.0, half_h: t0 * 1.1, }); - let v = glue_scene_framing(1.0, 0.75, short).vert_fov; + let v = glue_scene_framing(1.0, 0.75, short); assert!( close((v * 0.5).tan(), t0 * 1.1), "height held at the art's edge" ); assert!( - glue_scene_framing(1.0, 0.75, None).vert_fov > v, + glue_scene_framing(1.0, 0.75, None) > v, "…below the unbounded law's" ); } diff --git a/crates/benilla-app/src/portrait/glue_booth.rs b/crates/benilla-app/src/portrait/glue_booth.rs index 84c73775..d397283d 100644 --- a/crates/benilla-app/src/portrait/glue_booth.rs +++ b/crates/benilla-app/src/portrait/glue_booth.rs @@ -26,8 +26,8 @@ use benilla_assets::m2_url; use benilla_assets::materials::WowModelMaterial; use super::framing::{ - attachment_point, diag_to_vert, glue_box_physical, glue_scene_framing, ArtExtent, - GLUE_AUTHORED_ASPECT, PORTRAIT_ASPECT, + attachment_point, diag_to_vert, glue_box_aspect, glue_box_physical, glue_scene_framing, + ArtExtent, GLUE_AUTHORED_ASPECT, PORTRAIT_ASPECT, }; use super::{ aim, body_frame, new_target_image, spawn_booth_effects, spawn_booth_model, Booth, @@ -198,11 +198,19 @@ pub(crate) struct CreateScene { cam: Option, /// How far the scene's art paints around that camera — the shipped scene's measured extent /// ([`benilla_formats::shipped_glue_art_extent`], decision 1619) — the ceiling on the framing - /// law's widening ([`glue_scene_framing`]). `None` with no scene. + /// law's opening UPWARD on a narrow window ([`glue_scene_framing`]). `None` with no scene. + /// The wide leg stopped reading it under 2187: one frame for every scene, so its width is + /// [`super::framing::GLUE_BOX_ASPECT`]'s, not this stage's. It is still what says the frame + /// is safe there (the spawn log below, and the test that pins the constant). art: Option, - /// `Some(aspect)` while the framing law pillarboxes the scene (the window is wider than the - /// art can fill at the zoom floor): the booth camera renders into a centred viewport of this - /// aspect, black either side ([`pillarbox_glue_scene`]). `None`: the whole window. + /// `Some(aspect)` while the frame pillarboxes the scene — the window is wider than + /// [`super::framing::GLUE_BOX_ASPECT`]: the booth camera renders into a centred viewport of + /// this aspect, black either side ([`pillarbox_glue_scene`]). `None`: the whole window. + /// + /// A property of the **window**, not of the scene (decision 2187) — set from the window while + /// any glue screen is up, cleared only when the booth is torn down. It is what the chrome's + /// canvas insets by (2091), and 1619's per-scene box is why that canvas used to jump as the + /// roster selection moved between races. viewport_aspect: Option, /// The character's stage spot — scene attachment 0 (Bevy model space), `ZERO` with no scene. /// (Verified for select too: the body seats on attachment **0**, `0x473039` — attachment 1 is @@ -259,15 +267,6 @@ pub(crate) struct CreateScene { pending: Option, } -impl CreateScene { - /// The framing law's pillarbox for the window it last ran on: `Some(aspect)` while the scene - /// is boxed, `None` while it fills the window (decision 1619). Read by the glue screens' - /// chrome canvas, which lays out inside that box rather than over its bars (decision 2091). - pub(crate) fn viewport_aspect(&self) -> Option { - self.viewport_aspect - } -} - /// A requested stage swap, waiting for its half of the pair. /// /// **The two halves of a glue booth swap independently, and that is the whole defect.** Clicking a @@ -899,6 +898,13 @@ pub(super) fn sync_glue_scene( // The booth target follows the window while the screen is up — the scene is a fullscreen // render, so the bake wants window-native resolution (the fallback character-only render // shares it; its projection is aspect-aware). + // + // …and so does the **frame** (decision 2187). One box for every scene means the box is a + // property of the window alone, so it is set here — from the window, before a token is even + // resolved — and not down in the camera block with the scene's own fov. Two consequences, and + // both are the point: the chrome's canvas ([`crate::glue::GlueCanvas`]) cannot move when the + // selected character's race changes the stage, and it cannot flicker out to the full window + // for the frames a stage swap is in flight. if let Ok(w) = window.single() { resize_target( &mut images, @@ -906,6 +912,10 @@ pub(super) fn sync_glue_scene( w.physical_width().max(1), w.physical_height().max(1), ); + let box_aspect = glue_box_aspect(w.width() / w.height().max(1.0)); + if scene.viewport_aspect != box_aspect { + scene.viewport_aspect = box_aspect; + } } let token = scene_token(which); @@ -970,7 +980,9 @@ pub(super) fn sync_glue_scene( scene.spawned = false; scene.cam = None; scene.art = None; - scene.viewport_aspect = None; + // `viewport_aspect` is deliberately NOT cleared: the frame is the window's, not this + // stage's (2187), and clearing it here is what made the chrome jump out to the full + // window for the frames between two stages. scene.char_spot = Vec3::ZERO; clear_pet(&mut commands, &mut scene); commands.entity(scene.root).despawn_related::(); @@ -1144,14 +1156,23 @@ pub(super) fn sync_glue_scene( scene.art = benilla_formats::shipped_glue_art_extent(token); if let (Some(art), Some(cam)) = (scene.art, model.camera0.as_ref()) { let t0 = benilla_formats::authored_half_height(cam.fov); + // Where the frame's edge lands on THIS stage's art (2187): the constant is derived + // from the seven shipped scenes, so on the shipped chain this reads "inside" for six + // of them and a hair past the night elves' 4:3 sky card. A patched or replaced scene + // that paints narrower would say so here. + let frame = super::framing::GLUE_BOX_ASPECT * super::framing::glue_zoom_floor(cam.fov); info!( "create scene: UI_{token} art extent — half_w {:.4} (widens to {:.2}:1), half_h {:.4} \ - (opens to 1:{:.2}); boxes past {:.2}:1", + (opens to 1:{:.2}); the frame ends at {frame:.4} — {}", art.half_w, art.half_w / t0, art.half_h, art.half_h / (t0 * GLUE_AUTHORED_ASPECT), - art.half_w.max(t0 * GLUE_AUTHORED_ASPECT) / super::framing::glue_zoom_floor(cam.fov), + if frame <= art.half_w.max(t0 * GLUE_AUTHORED_ASPECT) { + "inside the art" + } else { + "PAST the art (void at the frame's edges)" + }, ); } scene.char_spot = stage; @@ -1207,14 +1228,11 @@ pub(super) fn sync_glue_scene( // a 1.55× zoom at 21:9 that crops head and feet (B242). [`glue_scene_framing`] pins the // authored 4:3 view box instead; its doc carries the law. Far kept generous: fog is not // rendered yet, so the authored far (27.8 on Orc) would slice unfogged geometry. - let framing = glue_scene_framing(cam.fov, aspect, scene.art); - if scene.viewport_aspect != framing.viewport_aspect { - scene.viewport_aspect = framing.viewport_aspect; - } + let vert_fov = glue_scene_framing(cam.fov, aspect, scene.art); let rig = ( Transform::from_translation(cam.eye).looking_at(cam.target, up), Projection::from(PerspectiveProjection { - fov: framing.vert_fov, + fov: vert_fov, near: cam.near, far: cam.far.max(1000.0), ..default() diff --git a/crates/benilla-app/src/portrait/mod.rs b/crates/benilla-app/src/portrait/mod.rs index 22bd823f..1edf2e3a 100644 --- a/crates/benilla-app/src/portrait/mod.rs +++ b/crates/benilla-app/src/portrait/mod.rs @@ -79,8 +79,8 @@ use benilla_assets::materials::WowModelMaterial; mod framing; pub(crate) use framing::{ - attachment_point, glue_canvas_bars, head_anchor, pane_projection, PortraitAnchors, - WowPortraitProjection, + attachment_point, glue_box_aspect, glue_canvas_bars, head_anchor, pane_projection, + PortraitAnchors, WowPortraitProjection, }; use framing::{body_frame, frame, PORTRAIT_FOV}; mod booth; @@ -96,9 +96,8 @@ mod dressup; pub(crate) use dressup::{DressUpBake, DressUpLook, DressUpPreview}; mod glue_booth; pub(crate) use glue_booth::{ - CreateLook, CreateScene, GhostKit, GlueLook, GluePetBake, GluePreview, GluePreviewBake, - GlueScene, PetLook, PreviewBillboard, PreviewEffects, PreviewPart, PreviewRider, SelectLook, - GLUE_SLOT, + CreateLook, GhostKit, GlueLook, GluePetBake, GluePreview, GluePreviewBake, GlueScene, PetLook, + PreviewBillboard, PreviewEffects, PreviewPart, PreviewRider, SelectLook, GLUE_SLOT, }; mod light; pub(crate) use light::{material_variant, VariantLane}; diff --git a/crates/benilla-app/src/ui_script/action_bar_tests.rs b/crates/benilla-app/src/ui_script/action_bar_tests.rs index 17aa74dd..565644a6 100644 --- a/crates/benilla-app/src/ui_script/action_bar_tests.rs +++ b/crates/benilla-app/src/ui_script/action_bar_tests.rs @@ -469,41 +469,8 @@ fn a_cooldown_count_addons_hook_leaves_the_sweep_running() { load_action_bar(&s); // The addon loads after FrameXML (`!` sorts it first among addons, all of which run after the // interface), so the global it captures is the stock one. - s.run( - r#" - local original = CooldownFrame_SetTimer - seen = 0 - CooldownFrame_SetTimer = function(cd, start, duration, enable) - seen = seen + 1 - original(cd, start, duration, enable) - if start > 0 and duration > 3 and enable > 0 then - local count = cd.textFrame - if not count then - local icon = getglobal(cd:GetParent():GetName() .. "Icon") - if icon then - count = CreateFrame("Frame", nil, cd:GetParent()) - count:SetAllPoints(cd:GetParent()) - count:SetFrameLevel(count:GetFrameLevel() + 1) - count.text = count:CreateFontString(nil, "OVERLAY") - count.text:SetFontObject(GameFontNormal) - count.text:SetPoint("CENTER", count, "CENTER", 0, 1) - count.icon = icon - count:SetScript("OnUpdate", function() end) - cd.textFrame = count - end - end - if count then - count.start = start - count.duration = duration - count:Show() - end - elseif cd.textFrame then - cd.textFrame:Hide() - end - end - "#, - ) - .expect("the addon's hook installs"); + s.run(COOLDOWN_COUNT_HOOK) + .expect("the addon's hook installs"); s.set_action( 1, @@ -563,6 +530,120 @@ fn a_cooldown_count_addons_hook_leaves_the_sweep_running() { assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } +/// `!OmniCC` 6.8.30's hook, as shape: it captures `CooldownFrame_SetTimer` in an upvalue, replaces +/// the global with a function that calls through, and hangs its countdown off the BUTTON — a +/// `CreateFrame` parented there lands at `button + 1`, then its own `+ 1` puts it at `button + 2`, +/// one over where it trusts the cooldown to sit. The handle rides a field of the cooldown widget +/// itself (`cd.textFrame`). +const COOLDOWN_COUNT_HOOK: &str = r#" + local original = CooldownFrame_SetTimer + seen = 0 + CooldownFrame_SetTimer = function(cd, start, duration, enable) + seen = seen + 1 + original(cd, start, duration, enable) + if start > 0 and duration > 3 and enable > 0 then + local count = cd.textFrame + if not count then + local icon = getglobal(cd:GetParent():GetName() .. "Icon") + if icon then + count = CreateFrame("Frame", nil, cd:GetParent()) + count:SetAllPoints(cd:GetParent()) + count:SetFrameLevel(count:GetFrameLevel() + 1) + count.text = count:CreateFontString(nil, "OVERLAY") + count.text:SetFontObject(GameFontNormal) + count.text:SetPoint("CENTER", count, "CENTER", 0, 1) + count.icon = icon + count:SetScript("OnUpdate", function() end) + cd.textFrame = count + end + end + if count then + count.start = start + count.duration = duration + count:Show() + end + elseif cd.textFrame then + cd.textFrame:Hide() + end + end +"#; + +/// **On the bonus bar the countdown draws over the sweep, as it does on every other bar** +/// (decision 2189). It came in as "the cooldown counter on action bar 1 is hidden behind the pie"; +/// a warrior in a stance — a druid in a form, a rogue in stealth — sees the bonus bar there. +/// +/// Stock `BonusActionButtonTemplate`'s `OnLoad` raises the button `+2` and then its cooldown `+2` +/// **by hand**, which is only one level of separation because a script level change carries no +/// children (`0x774560` → `set_frame_level(…, propagate=0)`). Our binding carried them, so the +/// cooldown came out at `button + 3` — over the count text the hook hangs at `button + 2`. +#[test] +fn a_cooldown_count_draws_over_the_bonus_bars_sweep() { + use benilla_ui::script::ActionState; + + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + load_action_bar(&s); + s.run(COOLDOWN_COUNT_HOOK) + .expect("the addon's hook installs"); + let level = |s: &UiScript, frame: &str| { + s.eval::(&format!("return {frame}:GetFrameLevel()")) + .unwrap() + }; + assert_eq!( + level(&s, "BonusActionButton1Cooldown"), + level(&s, "BonusActionButton1") + 1, + "the template's two hand raises leave the sweep ONE level over its button" + ); + + // Bonus page 1 (a warrior's Battle Stance): button 1 is action 73. + s.set_action( + 73, + Some(ActionSlot { + texture: Some("Interface\\Icons\\Ability_Racial_BloodRage".into()), + kind: 0x00, + action: 2687, + count: 0, + consumable: false, + }), + ); + s.fire_event("PLAYER_ENTERING_WORLD", vec![]); + s.run("BonusActionBarFrame:Show()").unwrap(); + s.tick(10.0); + s.set_action_state( + 73, + Some(ActionState { + usable: true, + cooldown: Some((6_000, 60_000, true)), + ..Default::default() + }), + ); + s.fire_event("ACTIONBAR_UPDATE_COOLDOWN", vec![]); + super::test_ui::cooldown_facts(&mut s); + // The addon writes its digits from its OnUpdate; the transcription's is a no-op. + s.run(r#"BonusActionButton1Cooldown.textFrame.text:SetText("27")"#) + .expect("the hook hung its countdown off the bonus button"); + s.tick(0.0); + s.resolve(); + + let quads = s.extract(); + let sweep = quads + .iter() + .position(|q| { + matches!(q.content, QuadContent::ModelPane { .. }) + && s.quad_owner_name(q.target).as_deref() == Some("BonusActionButton1Cooldown") + }) + .expect("the bonus button's sweep is on the paint list"); + let count = quads + .iter() + .position(|q| matches!(&q.content, QuadContent::Text { text: Some(t), .. } if t == "27")) + .expect("the countdown is on the paint list"); + assert!( + sweep < count, + "the countdown (index {count}) must paint over the bonus button's sweep (index {sweep})" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + /// An action button is a TWO-button button (decision 0908; director's report B200: "I can't right /// click food on my bar to eat it or right click spells"). The ref's `ActionButton_OnLoad` /// registers `("LeftButtonUp", "RightButtonUp")` (ActionButton.lua:109) and its OnClick body reads diff --git a/crates/benilla-app/src/ui_script/options_tests.rs b/crates/benilla-app/src/ui_script/options_tests.rs index fb05ac7c..afa2b1c9 100644 --- a/crates/benilla-app/src/ui_script/options_tests.rs +++ b/crates/benilla-app/src/ui_script/options_tests.rs @@ -1328,6 +1328,82 @@ fn the_vertical_sync_row_reads_and_writes_the_present_mode_cvar() { /// Environment Detail is the reference's own slider (1649): 0..2 step 1 over `WorldDetail`, with /// 0992's Low/Medium/High names kept in the readout seat — a groove whose readout says "1" tells a /// player nothing. Dragging writes the CVar; a value from outside the range shows the nearest stop +/// **The Brightness slider actually writes** (decision 2182, and the bug that shipped with it). +/// +/// This row is the kit's first slider whose store is an engine PAIR rather than a CVar or a +/// saved-variable global — 1.12's own arrangement for its slider 6 — and +/// `OptionsSlider_OnValueChanged` guarded its write on `row.cvar or row.uvar`, two of the kit's +/// three stores. So dragging it moved the READOUT and wrote nothing: the control looked alive, the +/// picture never changed, and reopening the window re-read an untouched CVar and snapped back to +/// the centre. No error anywhere, on any path. +/// +/// The shipped test missed it by calling `SetGamma` directly and checking the row's *wiring* — the +/// bounds, the numeric flag, the readout at the default. Everything it asserted was true. What it +/// never did was move the slider, which is the only thing a player does. So this one drags. +#[test] +fn the_brightness_slider_writes_through_its_engine_pair_and_survives_a_reopen() { + const ROW: &str = "BenillaOptionsFrameContainerBodyGraphicsRowBrightness"; + let s = audio_harness(); + let mut s = harness_on(s); + s.run("ShowUIPanel(BenillaOptionsFrame)").unwrap(); + s.run("BenillaOptionsFrameCategoryListRowGraphics:Click()") + .unwrap(); + assert_eq!( + s.eval::(&format!("return {ROW}Label:GetText()")) + .unwrap(), + "Brightness" + ); + // The registered `gamma = "1.0"` is `GetGamma() == 0`, the centre of the stock slider's + // travel — and selecting the page must not write it back. + assert_eq!( + s.eval::(&format!("return {ROW}ControlValue:GetText()")) + .unwrap(), + "50%" + ); + assert!( + s.take_cvar_changes().is_empty(), + "reading the pair on select must not write it back" + ); + + // **A drag to each end WRITES.** The value is the reference's own `1 - slider`, six decimals, + // because `SetGamma` owns the formatting — this is the assertion the shipped test lacked. + for (slider, cvar, readout) in [ + (0.5, "0.500000", "100%"), + (-0.5, "1.500000", "0%"), + (0.2, "0.800000", "70%"), + ] { + s.run(&format!("{ROW}ControlSlider:SetValue({slider})")) + .unwrap(); + assert_eq!( + s.take_cvar_changes(), + vec![("gamma".to_string(), cvar.to_string())], + "dragging to {slider} must reach the store" + ); + assert_eq!( + s.eval::(&format!("return {ROW}ControlValue:GetText()")) + .unwrap(), + readout + ); + } + + // **And it comes back.** Leaving the page and returning re-reads the pair rather than the + // control, which is the half the player sees as "it didn't save": the row was left at 0.2. + s.run("BenillaOptionsFrameCategoryListRowAudio:Click()") + .unwrap(); + s.run("BenillaOptionsFrameCategoryListRowGraphics:Click()") + .unwrap(); + assert_eq!( + s.eval::(&format!("return {ROW}ControlValue:GetText()")) + .unwrap(), + "70%", + "the reopened page reads the store, not the registered default" + ); + assert!( + s.take_cvar_changes().is_empty(), + "…and the refresh itself writes nothing" + ); +} + /// and writes nothing back (0959's out-of-range law). #[test] fn the_world_detail_slider_writes_the_cvar_and_the_readout_names_its_stop() { diff --git a/crates/benilla-formats/src/bin/benilla-extract/glueextent.rs b/crates/benilla-formats/src/bin/benilla-extract/glueextent.rs index 23513d35..15070f2a 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/glueextent.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/glueextent.rs @@ -23,7 +23,7 @@ const SCENES: [&str; 7] = [ pub fn glueextent(chain: &mut Chain, batches: bool) -> Result<()> { println!( - "{:<9} {:>6} {:>7} {:>7} | {:>7} {:>7} | {:>9} {:>9} | {:>7} {:>7}", + "{:<9} {:>10} {:>7} {:>7} | {:>7} {:>7} | {:>9} {:>9} | {:>7} {:>7}", "scene", "fov", "t0", "h0", "half_w", "half_h", "wide@", "narrow@", "opaque", "+all" ); for token in SCENES { @@ -65,7 +65,7 @@ pub fn glueextent(chain: &mut Chain, batches: bool) -> Result<()> { / t0; let with_all = glue_art_extent(&subs, &cam, |_| Some(Coverage::Full)).half_w / t0; println!( - "{token:<9} {:>6.3} {:>7.4} {:>7.4} | {:>7.4} {:>7.4} | {:>9.3} {:>9.3} | {:>7.3} {:>7.3}", + "{token:<9} {:>10.7} {:>7.4} {:>7.4} | {:>7.4} {:>7.4} | {:>9.3} {:>9.3} | {:>7.3} {:>7.3}", cam.fov, t0, h0, diff --git a/crates/benilla-formats/src/lib.rs b/crates/benilla-formats/src/lib.rs index b5a4a3b3..2d334618 100644 --- a/crates/benilla-formats/src/lib.rs +++ b/crates/benilla-formats/src/lib.rs @@ -254,11 +254,11 @@ pub use models::{ CoverageReader, EmitterBoneLink, EventMarker, FogPolicy, FootprintTris, GlobalSeqBone, GlobalSeqChannel, GroundQuad, KeyAnim, M2AnimSummary, M2Attachment, M2Bounds, M2CameraTracks, M2Light, M2PaneCamera, M2PortraitCamera, ModelAnimation, ModelBlend, ParentArm, ParentBasis, - PlayableAnim, RenderSubmesh, RgbAnim, ScalarAnim, SeqLoops, Skeleton, SkeletonBone, - StringAnchors, UvAnim, UvRotAnim, WmoBatchClass, WmoDoodad, WmoDoodadSet, WmoFog, + PlayableAnim, RenderSubmesh, RgbAnim, ScalarAnim, SeqLoops, ShippedGlueScene, Skeleton, + SkeletonBone, StringAnchors, UvAnim, UvRotAnim, WmoBatchClass, WmoDoodad, WmoDoodadSet, WmoFog, WmoGroupHeader, WmoGroupInfo, WmoLight, WmoPortalInfo, WmoPortalRef, WmoPortals, WmoRoot, ALPHA_KEY_REF, DEGENERATE_RING_FOOTPRINT, GLUE_AUTHORED_ASPECT, NO_GROUP_LIQUID, - OWNER_RUNG_BUCKETS, + OWNER_RUNG_BUCKETS, SHIPPED_GLUE_SCENES, }; mod terrain; pub use terrain::{ diff --git a/crates/benilla-formats/src/models/art_extent.rs b/crates/benilla-formats/src/models/art_extent.rs index 2c27c03a..27e0b202 100644 --- a/crates/benilla-formats/src/models/art_extent.rs +++ b/crates/benilla-formats/src/models/art_extent.rs @@ -79,8 +79,9 @@ pub const GLUE_AUTHORED_ASPECT: f32 = 4.0 / 3.0; /// (wow-re `models.md` — `ALPHAREF` 224, `GEQUAL`). The one threshold this module paints by. pub const ALPHA_KEY_REF: u8 = 224; -/// The **shipped** scenes' measured extents — `benilla-extract glueextent` on the 1.12.1 (5875) -/// chain, transcribed. Keyed by the scene token (`UI_.m2`). The test re-measures them. +/// The **shipped** scenes' measured extents and authored fovs — `benilla-extract glueextent` on +/// the 1.12.1 (5875) chain, transcribed. Keyed by the scene token (`UI_.m2`). The test +/// re-measures them. /// /// Read as aspects (`half_w / t0`, the window aspect past which the art runs out of width under /// the authored vertical): MainMenu 1.54 · Human 1.47 · Orc 1.57 · Dwarf 1.42 · NightElf 1.31 · @@ -94,62 +95,92 @@ pub const ALPHA_KEY_REF: u8 = 224; /// what shows through is the page behind the scene, near-black, read as sky. The framing law never /// opens past the authored box on an axis the art does not reach, so the number costs nothing /// there. -const SHIPPED: [(&str, ArtExtent); 7] = [ - ( - "MainMenu", - ArtExtent { +/// One degree in radians — the unit the `UI_*` cameras were dialled in. Every one of the seven +/// authored fovs is a whole number of degrees, to the last bit of the `f32` the M2 carries +/// (`benilla-extract glueextent`: 86°, 80°, 65°, 60°), so the table says so rather than +/// transcribing decimals. +const DEG: f32 = std::f32::consts::PI / 180.0; + +/// One shipped glue scene, as the framing law needs it: the scene token, its authored camera-0 +/// **fov** ([`DEG`] — 86° on the gate, 80° on the human street, 65° on the four orc-family +/// stages, 60° in the night elf grove), and the measured +/// [`ArtExtent`]. The fov rides the table because the framing constant is derived from it +/// ([`benilla`]'s `GLUE_BOX_ASPECT`) and both halves are re-measured by the same chain test. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct ShippedGlueScene { + pub token: &'static str, + pub fov: f32, + pub art: ArtExtent, +} + +/// The seven shipped scenes: the login gate plus the six race stages (Gnome shares Dwarf's, +/// Troll shares Orc's). See [`ShippedGlueScene`]. +pub const SHIPPED_GLUE_SCENES: [ShippedGlueScene; 7] = [ + ShippedGlueScene { + token: "MainMenu", + fov: 86.0 * DEG, + art: ArtExtent { half_w: 0.7431, half_h: 0.5067, }, - ), - ( - "Human", - ArtExtent { + }, + ShippedGlueScene { + token: "Human", + fov: 80.0 * DEG, + art: ArtExtent { half_w: 0.6548, half_h: 0.4866, }, - ), - ( - "Orc", - ArtExtent { + }, + ShippedGlueScene { + token: "Orc", + fov: 65.0 * DEG, + art: ArtExtent { half_w: 0.5573, half_h: 0.3954, }, - ), - ( - "Dwarf", - ArtExtent { + }, + ShippedGlueScene { + token: "Dwarf", + fov: 65.0 * DEG, + art: ArtExtent { half_w: 0.5040, half_h: 0.4272, }, - ), - ( - "NightElf", - ArtExtent { + }, + ShippedGlueScene { + token: "NightElf", + fov: 60.0 * DEG, + art: ArtExtent { half_w: 0.4262, half_h: 0.1221, }, - ), - ( - "Scourge", - ArtExtent { + }, + ShippedGlueScene { + token: "Scourge", + fov: 65.0 * DEG, + art: ArtExtent { half_w: 0.5542, half_h: 0.4776, }, - ), - ( - "Tauren", - ArtExtent { + }, + ShippedGlueScene { + token: "Tauren", + fov: 65.0 * DEG, + art: ArtExtent { half_w: 0.5177, half_h: 0.4066, }, - ), + }, ]; /// The shipped scene's measured [`ArtExtent`] by token, `None` for a token that is not one of /// the seven `UI_*` dioramas (the consumer then keeps 1587's unbounded law). pub fn shipped_glue_art_extent(token: &str) -> Option { - SHIPPED.iter().find(|(t, _)| *t == token).map(|(_, e)| *e) + SHIPPED_GLUE_SCENES + .iter() + .find(|s| s.token == token) + .map(|s| s.art) } /// The authored **vertical** half-extent (tan units) of a glue camera's `fov` at 4:3 — the number @@ -821,14 +852,14 @@ mod tests { } /// Measure one shipped scene off the chain, the way the tool does. - fn measure(chain: &mut Chain, token: &str) -> (ArtExtent, f32) { + fn measure(chain: &mut Chain, token: &str) -> (ArtExtent, f32, f32) { let name = format!("Interface\\Glues\\Models\\UI_{token}\\UI_{token}.m2"); let bytes = chain.read_file(&name).expect("read scene"); let subs = crate::parse_m2_render_submeshes(&bytes, "", &[]).expect("parse"); let cam = crate::parse_m2_camera(&bytes, 0).expect("camera 0"); let mut reader = CoverageReader::new(chain); let ext = glue_art_extent(&subs, &cam, |s| reader.coverage(s).expect("texture")); - (ext, authored_half_height(cam.fov)) + (ext, cam.fov, authored_half_height(cam.fov)) } /// The transcription is the measurement: every shipped scene re-measures to the table's @@ -838,13 +869,21 @@ mod tests { fn the_shipped_table_matches_the_measurement() { let data = crate::wow_data_or_skip!(); let mut chain = crate::open_chain(&data).expect("open chain"); - for (token, shipped) in SHIPPED { - let (ext, t0) = measure(&mut chain, token); + for scene in SHIPPED_GLUE_SCENES { + let token = scene.token; + let shipped = scene.art; + let (ext, fov, t0) = measure(&mut chain, token); assert!( (ext.half_w - shipped.half_w).abs() < 2e-3 && (ext.half_h - shipped.half_h).abs() < 2e-3, "UI_{token}: measured {ext:?}, table {shipped:?}" ); + // The fov rides the table because the glue framing constant is derived from it. + assert!( + (fov - scene.fov).abs() < 1e-6, + "UI_{token}: camera 0 fov {fov}, table {}", + scene.fov + ); let runs_out_at = ext.half_w / t0; assert!( (GLUE_AUTHORED_ASPECT - 0.03..16.0 / 9.0).contains(&runs_out_at), diff --git a/crates/benilla-ui/src/script/object/frame_state.rs b/crates/benilla-ui/src/script/object/frame_state.rs index dce61f17..39930b2d 100644 --- a/crates/benilla-ui/src/script/object/frame_state.rs +++ b/crates/benilla-ui/src/script/object/frame_state.rs @@ -511,10 +511,16 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { lua.create_function(|lua, (this, level): (Table, i64)| { let h = frame_handle_of(lua, &this)?; let lvl = level.clamp(0, i64::from(u16::MAX)) as u16; + // **A script level change carries no children** — the binding `0x774560` calls + // `set_frame_level 0x76a4f0` with `propagate=0` (wow-re `ui/ui.md`, default levels). + // Only the toplevel raise shifts a subtree. Stock `BonusActionButtonTemplate` is written + // for this: it raises the button +2 and then its cooldown +2 by hand, landing the sweep + // one level over the button — carrying the children made it three, over the + // cooldown-count text an addon hangs at button + 2 (decision 2189). lua.app_data_mut::() .expect("model") .arena - .set_frame_level(h, lvl, true); + .set_frame_level(h, lvl, false); Ok(()) })?, )?; diff --git a/crates/benilla-ui/src/script/tests/tooltip.rs b/crates/benilla-ui/src/script/tests/tooltip.rs index 904fe48b..01ce05c4 100644 --- a/crates/benilla-ui/src/script/tests/tooltip.rs +++ b/crates/benilla-ui/src/script/tests/tooltip.rs @@ -670,6 +670,63 @@ fn set_owner_defaults_to_anchor_left_and_only_preserve_keeps_the_placement() { ); } +/// **A NON-STRING anchor argument must not raise** — and for a year it did, because the binding +/// took arg 3 as `Option` and let mlua's converter be the gate. +/// +/// The reference's gate is `lua_isstring 0x6f3510`, whose entire body is `lua_type` then +/// `cmp eax,4 / cmp eax,3`: LUA_TSTRING or LUA_TNUMBER pass and are read by coercion, and every +/// other tag takes `0x53121b je 0x53133a` over the whole compare chain to the join, leaving the +/// zeroed local — mode 0, `ANCHOR_LEFT`, silently, with no `luaL_error` anywhere in +/// `[0x531221, 0x53133a)` (wow-re `system/ui/scratch/tooltip-cursor-anchor-law.md` §0.2, the same +/// section decision 2176 already quotes in this file's doc comments). So a boolean, a table, a +/// function and absent are *indistinguishable* at this position. +/// +/// The symptom that found it: `Questie` hovers a world-map note with +/// `Tooltip:SetOwner(this, this)` (`QuestieNotes.lua` `Questie_Tooltip_OnEnter`) — a frame where +/// the anchor goes — and got `bad argument #3: error converting Lua table to String` where the +/// reference draws the tooltip. +#[test] +fn a_non_string_anchor_is_silently_mode_zero_and_never_raises() { + let s = script(); + s.run( + r#" + Plate = CreateFrame("GameTooltip", "Plate") + Owner = CreateFrame("Frame", "Owner") + "#, + ) + .unwrap(); + + // A TABLE — Questie's own call shape, a frame passed where the anchor string goes. + s.run("Plate:SetOwner(Owner, Owner)") + .expect("a table anchor takes the isstring gate's jump, it does not raise"); + assert_eq!( + s.eval::("return Plate:GetAnchorType()").unwrap(), + "ANCHOR_LEFT" + ); + + // A BOOLEAN and an explicit nil are the same jump. + for arg in ["true", "false", "nil"] { + s.run(&format!( + r#"Plate:SetOwner(Owner, "ANCHOR_RIGHT") Plate:SetOwner(Owner, {arg})"# + )) + .unwrap_or_else(|e| panic!("SetOwner(Owner, {arg}) raised: {e}")); + assert_eq!( + s.eval::("return Plate:GetAnchorType()").unwrap(), + "ANCHOR_LEFT", + "{arg} is indistinguishable from absent at this position" + ); + } + + // A NUMBER is the one non-string tag the gate DOES admit: it is stringified and run through + // the chain, which no number can match — the same mode 0, by the other route. + s.run(r#"Plate:SetOwner(Owner, "ANCHOR_RIGHT") Plate:SetOwner(Owner, 5)"#) + .unwrap(); + assert_eq!( + s.eval::("return Plate:GetAnchorType()").unwrap(), + "ANCHOR_LEFT" + ); +} + /// **`ANCHOR_CURSOR` is a real ninth mode**, not a string to warn about (2142's open thread 2, and /// nine corpus files ask for it): mode 6 clears at `SetOwner` time like mode 7, and then the /// per-frame update `0x530b20` pins the plate's **BOTTOM** to the screen's **BOTTOMLEFT** at the diff --git a/crates/benilla-ui/src/script/tests/toplevel.rs b/crates/benilla-ui/src/script/tests/toplevel.rs index a37d9f1b..5176fd1a 100644 --- a/crates/benilla-ui/src/script/tests/toplevel.rs +++ b/crates/benilla-ui/src/script/tests/toplevel.rs @@ -801,3 +801,29 @@ fn a_raise_with_a_hidden_child_keeps_the_windows_own_siblings_level() { ); assert!(s.errors().is_empty(), "{:?}", s.errors()); } + +/// **A script level change carries no children** (decision 2189): the Lua binding `0x774560` +/// calls `set_frame_level 0x76a4f0` with `propagate=0` — only the raise shifts a subtree. Stock +/// FrameXML is written against it (`BonusActionButtonTemplate` raises the button and then its +/// cooldown by hand), and carrying the children put the bonus bar's sweep over an addon's +/// cooldown count. +#[test] +fn a_script_level_change_leaves_the_children_where_they_were() { + let mut s = script(); + s.run( + r#" + Parent = CreateFrame("Frame", "Parent") + Child = CreateFrame("Frame", "Child", Parent) + Grandchild = CreateFrame("Frame", "Grandchild", Child) + "#, + ) + .unwrap(); + let (child, grandchild) = (level(&mut s, "Child"), level(&mut s, "Grandchild")); + s.run("Parent:SetFrameLevel(7)").unwrap(); + assert_eq!(level(&mut s, "Parent"), 7); + assert_eq!( + (level(&mut s, "Child"), level(&mut s, "Grandchild")), + (child, grandchild), + "the children keep their absolute levels" + ); +} diff --git a/crates/benilla-ui/src/script/tooltip/verbs.rs b/crates/benilla-ui/src/script/tooltip/verbs.rs index 70f77d63..cf26dbd5 100644 --- a/crates/benilla-ui/src/script/tooltip/verbs.rs +++ b/crates/benilla-ui/src/script/tooltip/verbs.rs @@ -6,6 +6,7 @@ use mlua::{Lua, Table, Value, Variadic}; use crate::layout::{Anchor, Point}; +use crate::script::binding_abi::optional_string; use crate::script::object::{frame_handle_of, frame_wrapper}; use crate::script::region::region_handle_of; use crate::script::Model; @@ -52,16 +53,19 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { m.set( "SetOwner", lua.create_function( - |lua, - (this, owner, anchor, x, y): ( - Table, - Table, - Option, - Option, - Option, - )| { + |lua, (this, owner, anchor, x, y): (Table, Table, Value, Option, Option)| { let h = frame_handle_of(lua, &this)?; let owner_h = frame_handle_of(lua, &owner)?; + // The gate is `lua_isstring 0x6f3510`, whose whole body is `lua_type` then + // `cmp eax,4 / cmp eax,3` — LUA_TSTRING or LUA_TNUMBER and nothing else — so the + // anchor argument is read the reference's way, by COERCION, and a boolean, table, + // function or userdata is indistinguishable from absent. Taking it as + // `Option` made mlua's converter the gate instead, and mlua raises on a + // table: `Questie`'s `Tooltip:SetOwner(this, this)` (QuestieNotes.lua, the map-note + // hover) died with "bad argument #3: error converting Lua table to String" on a + // call the reference completes silently at mode 0. This function's doc comment has + // stated the law since 2176 — only the signature disagreed. + let anchor = optional_string(lua, &anchor); { let mut model = lua.app_data_mut::().expect("model app_data"); clear_content(&mut model, h);