From 5432a8bfbd6aa26ed82aec2e29d3b5ac496d7f05 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 11 Sep 2026 20:52:20 +0700 Subject: [PATCH] a slider snaps to its step, a pushed button releases, and warnings show - a slider with a value step snaps to it: a stepped drag fires once per row instead of once per pixel, a scrollbar cannot rest between two rows, and one wheel notch on the trainer list reaches the bottom (2133) - setting a step on a live slider moves its value onto the new lattice and runs its handler, which an addon on the real client can count on (2143) - a button pushed from script releases on the next mouse-up, so Tablet-2.0's rows stop staying pressed forever; a release off the button's rect un-presses it, and a locked state holds (2134) - the mail body and the GM ticket box scroll as you type past their bottom, and the Use UI Scale checkbox reads a registered setting (2141) - an engine warning, a call that was accepted and did nothing, reaches the /errors window in grey instead of one terminal line; a text row trimmed mid-character no longer kills the window's repaint (2135, 2138) Co-Authored-By: Claude Fable 5.1 --- .../benilla-app/assets/ui/ScriptLogFrame.xml | 40 ++- crates/benilla-app/examples/addon_harness.rs | 30 ++ crates/benilla-app/src/addon_harness/mod.rs | 50 +++ crates/benilla-app/src/cvars.rs | 18 +- crates/benilla-app/src/ui_script/addons.rs | 5 + .../src/ui_script/chat_options_tests.rs | 8 +- .../src/ui_script/color_picker_tests.rs | 8 +- .../src/ui_script/faux_scroll_tests.rs | 32 +- .../src/ui_script/options_tests.rs | 9 +- .../src/ui_script/trainer_tests.rs | 17 +- .../src/ui_script/world_entry_tests.rs | 66 +++- crates/benilla-ui/src/loader/tests.rs | 15 +- crates/benilla-ui/src/script/addon.rs | 12 +- crates/benilla-ui/src/script/binding_abi.rs | 36 +- crates/benilla-ui/src/script/button.rs | 173 +++++----- crates/benilla-ui/src/script/cursor/drag.rs | 7 + crates/benilla-ui/src/script/cvars.rs | 2 +- crates/benilla-ui/src/script/diagnostics.rs | 94 +++++ crates/benilla-ui/src/script/editbox.rs | 87 ++++- .../benilla-ui/src/script/editbox/methods.rs | 6 +- crates/benilla-ui/src/script/editbox/tests.rs | 84 +++++ crates/benilla-ui/src/script/event.rs | 18 +- crates/benilla-ui/src/script/extract.rs | 43 +-- crates/benilla-ui/src/script/layout.rs | 12 +- crates/benilla-ui/src/script/macros.rs | 6 +- crates/benilla-ui/src/script/mod.rs | 15 +- crates/benilla-ui/src/script/model.rs | 29 ++ crates/benilla-ui/src/script/object.rs | 6 +- .../src/script/object/events_regions.rs | 18 +- .../src/script/object/layout_methods.rs | 2 +- crates/benilla-ui/src/script/pointer.rs | 59 +++- crates/benilla-ui/src/script/region.rs | 2 +- crates/benilla-ui/src/script/region/text.rs | 8 +- crates/benilla-ui/src/script/saved.rs | 2 +- .../benilla-ui/src/script/simplehtml/mod.rs | 7 +- crates/benilla-ui/src/script/slider.rs | 6 +- crates/benilla-ui/src/script/tests/button.rs | 194 +++++++++++ crates/benilla-ui/src/script/tests/input.rs | 6 +- crates/benilla-ui/src/script/tests/slider.rs | 211 ++++++++++++ crates/benilla-ui/src/script/tick.rs | 5 + crates/benilla-ui/src/script/tooltip/verbs.rs | 2 +- crates/benilla-ui/src/widget/kinds/editbox.rs | 6 + crates/benilla-ui/src/widget/kinds/mod.rs | 326 ++++++++++++++---- crates/benilla-ui/src/widget/mod.rs | 8 +- 44 files changed, 1525 insertions(+), 265 deletions(-) diff --git a/crates/benilla-app/assets/ui/ScriptLogFrame.xml b/crates/benilla-app/assets/ui/ScriptLogFrame.xml index c0fa01c0..21407722 100644 --- a/crates/benilla-app/assets/ui/ScriptLogFrame.xml +++ b/crates/benilla-app/assets/ui/ScriptLogFrame.xml @@ -57,21 +57,38 @@ -- The first line of a message, trimmed to a row's width. A traceback's later lines are -- indented continuations; on one row they read as noise. + -- `strsub` is BYTE-indexed (Lua 5.0 strings are byte strings, and benilla keeps them that + -- way — decision 1193), so a cut that lands inside a multi-byte character leaves half of + -- one. Every message here is UTF-8 and ours are full of em dashes, so this was not + -- theoretical: the log window's own repaint raised on the first warning it was ever asked + -- to show (2135/2138). Back the cut up over any continuation byte (`10xxxxxx`, i.e. + -- 0x80..0xBF) so it lands on a character boundary. function BenillaScriptLog_RowText(message) local cut = strfind(message, "\n", 1, 1) local head = message if cut then head = strsub(message, 1, cut - 1) end if strlen(head) > BENILLA_SCRIPTLOG_ROW_CHARS then - head = strsub(head, 1, BENILLA_SCRIPTLOG_ROW_CHARS - 3) .. "..." + local last = BENILLA_SCRIPTLOG_ROW_CHARS - 3 + while last > 0 do + local b = strbyte(head, last + 1) + if not b or b < 128 or b >= 192 then break end + last = last - 1 + end + head = strsub(head, 1, last) .. "..." end return head end - -- `load` rows are red and `error` rows amber, and the split is by CONSEQUENCE: a `load` row - -- means an addon is not running at all, which is the thing the player can act on. Colours - -- come from the shipped tables so this window matches the rest of the UI. + -- The three kinds, coloured by CONSEQUENCE — how much of the addon is broken: + -- `load` red an addon is not running at all (the thing a player can act on) + -- `error` amber code ran and raised; the addon is otherwise up + -- `warn` grey a call was accepted and quietly did not do what it said (2135) + -- Grey rather than a fourth bright colour because a warning is the least urgent of the + -- three and there are more of them: colour has to keep ranking the list, not just label + -- it. Colours come from the shipped tables so this window matches the rest of the UI. function BenillaScriptLog_RowColor(kind) if kind == "load" then return 1.0, 0.3, 0.3 end + if kind == "warn" then return 0.65, 0.65, 0.65 end return 1.0, 0.82, 0.0 end @@ -81,15 +98,18 @@ -- The summary line carries the one number a bug report needs and the window cannot -- otherwise show: how many were DROPPED. `total` counts distinct failures ever -- recorded; `shown` is what survived the cap. + -- "problems", not "errors": the log carries three kinds now and two of them are not + -- errors (2135). The window is still `/errors` and still opens on the same key — + -- what changed is that a count of 7 stopped meaning "7 things raised". local summary if total == 0 then - summary = "No errors this session." + summary = "No problems this session." elseif total > shown then - summary = format("%d errors (%d older not kept)", total, total - shown) + summary = format("%d problems (%d older not kept)", total, total - shown) elseif total == 1 then - summary = "1 error" + summary = "1 problem" else - summary = format("%d errors", total) + summary = format("%d problems", total) end getglobal("BenillaScriptLogSummary"):SetText(summary) @@ -152,7 +172,7 @@ end end if text == "" then - text = "Select an error above to read it in full." + text = "Select a row above to read it in full." end getglobal("BenillaScriptLogDetailText"):SetText(text) end @@ -212,7 +232,7 @@ if getglobal("BenillaScriptLogFrame"):IsVisible() then BenillaScriptLog_Update() end - DEFAULT_CHAT_FRAME:AddMessage("Script error log cleared.", 1.0, 1.0, 0.0) + DEFAULT_CHAT_FRAME:AddMessage("Script log cleared.", 1.0, 1.0, 0.0) end ]]> diff --git a/crates/benilla-app/examples/addon_harness.rs b/crates/benilla-app/examples/addon_harness.rs index f49a8a86..0b037a4a 100644 --- a/crates/benilla-app/examples/addon_harness.rs +++ b/crates/benilla-app/examples/addon_harness.rs @@ -565,6 +565,36 @@ fn main() { } } + // **What was WARNED about, ranked** — the channel that reached nobody until 2135. It is + // printed after the error rankings and before the distribution on purpose: these are not + // blockers (nothing raised, the addon is running), so they must not outrank a row somebody is + // stuck on — but they are the only column that can see an addon quietly getting the wrong + // thing, which is the class `render` was added for and reaches later. + // + // Ranked by how many ADDONS hit each row, not by total occurrences: a warning one addon fires + // in an OnUpdate would otherwise bury one that fifty addons hit once. + let mut warned: std::collections::BTreeMap = Default::default(); + for r in &reports { + let mut seen: std::collections::BTreeSet = Default::default(); + for w in &r.warnings { + seen.insert(addon_harness::normalise(w)); + } + for w in seen { + *warned.entry(w).or_default() += 1; + } + } + if !warned.is_empty() { + let mut rows: Vec<(String, usize)> = warned.into_iter().collect(); + rows.sort_by(|a, b| b.1.cmp(&a.1).then(a.0.cmp(&b.0))); + let addons = reports.iter().filter(|r| !r.warnings.is_empty()).count(); + println!( + "\n what was WARNED about ({addons} addons raised at least one, by addon count):" + ); + for (w, count) in rows.into_iter().take(12) { + println!(" {count:>4} {w}"); + } + } + // The distribution, because a mean would hide the shape. let mut buckets = [0usize; 5]; for r in &reports { diff --git a/crates/benilla-app/src/addon_harness/mod.rs b/crates/benilla-app/src/addon_harness/mod.rs index 18626ad6..69f3bf88 100644 --- a/crates/benilla-app/src/addon_harness/mod.rs +++ b/crates/benilla-app/src/addon_harness/mod.rs @@ -295,6 +295,28 @@ pub struct AddonReport { /// different question, and redefining an existing number would make every past run /// incomparable. pub probe_errors: Vec, + /// **Warnings raised while this addon loaded and ran** — the engine's own non-fatal channel + /// (decision 2135), which nothing in this survey could see until it had one. + /// + /// A warning is the class of failure that *does not announce itself*: an `inherits=` argument + /// that was not a template name and got dropped, a `SetPoint` whose `relativeTo` did not + /// resolve and silently re-anchored, a `SetCVar` on a name nothing registered, a saved + /// variable that would not serialise, a loader note about a template of the wrong kind. + /// Nothing raises, so `errors`, `session_errors` and `probe_errors` are all blind to it, and + /// the addon scores a clean pass while doing the wrong thing. That is exactly the shape + /// `render` was added for — and this reaches the cases `render` cannot, because a warning + /// usually fires long before anything would have been drawn. + /// + /// A NEW column, never folded into another (1213's rule, again). + /// + /// **Read off the retained diagnostic log by `seq`, not off the per-frame drain**, so it + /// carries the host's loader warnings (which are recorded with their `` prefix and never + /// touch the drain) as well as the engine's. One consequence of the log's dedupe is worth + /// stating: a warning whose exact text the FrameXML underneath already produced collapses onto + /// that older row and does not appear here. The corpus's warnings name the addon's own frames + /// and files, so this is rare rather than theoretical — but it means the column, like every + /// other one here, is a floor. + pub warnings: Vec, /// **What it actually PUT ON SCREEN** — see [`render`], whose header is the design. /// /// The column every other one here was blind to. `loaded`, `session_errors` and `probe_errors` @@ -549,6 +571,11 @@ fn survey_one( // Taken here, after the dependency chain, so a library's frames are not charged to its // consumer — the same rule `load_dependencies` applies to errors. let baseline = RenderBaseline::of(&script); + // THE WARNING MARK, taken at the same seam and for the same reason: everything the FrameXML + // underneath and this addon's dependency chain warned about is already behind us, so what + // follows is this addon's (decision 2135). `seq` is monotonic and never reused, which is what + // makes a high-water mark a valid cut of a log that also evicts and dedupes. + let warn_mark = script.diagnostics().last().map_or(0, |d| d.seq); let (errors, absent) = load_addon_files(&script, root, name, &toc); // The registry has to agree with the VM about what has loaded: the live walk stamps each @@ -566,6 +593,22 @@ fn survey_one( let missing_inherits = missing_inherits(&script, root, name, &toc); let session_errors = drive_session_start(&mut script, name, &dep_order, installed); let probe_errors = drive_ui_probe(&mut script); + // Everything filed since the mark, warnings only — the errors have their own columns and are + // retained under their own kinds. + let warnings: Vec = script + .diagnostics() + .into_iter() + .filter(|d| { + d.seq > warn_mark && d.kind == benilla_ui::script::diagnostics::DiagnosticKind::Warning + }) + .map(|d| { + if d.count > 1 { + format!("{} (x{})", d.message, d.count) + } else { + d.message + } + }) + .collect(); // AFTER the UI probe and BEFORE the method oracle, and both halves of that are load-bearing. // After, because the probe leaves the addon fully driven — and this pass re-OPENS what the // probe's second toggle closed, which is why it is a separate probe rather than a read at the @@ -619,6 +662,7 @@ fn survey_one( ambiguous_methods, session_errors, probe_errors, + warnings, render, used, } @@ -1868,6 +1912,12 @@ fn load_addon_files( match benilla_ui::framexml::parse(&benilla_ui::source::decode(&bytes)) { Ok(doc) => { let report = benilla_ui::loader::load_in(script, &doc, &path, &provider); + // Retained with the file prefix so the survey's `warnings` column can see them + // (2135); they were dropped with the report before that, which is why an + // unresolved `inherits=` scored as a clean load. + for w in report.warnings { + script.report_warning(&format!("{file}: {w}")); + } errors.extend(report.errors.into_iter().map(|e| format!("{file}: {e}"))); } Err(e) => errors.push(format!("{file}: {e}")), diff --git a/crates/benilla-app/src/cvars.rs b/crates/benilla-app/src/cvars.rs index 176634fb..11587a44 100644 --- a/crates/benilla-app/src/cvars.rs +++ b/crates/benilla-app/src/cvars.rs @@ -306,9 +306,23 @@ pub(crate) const REGISTERED: &[Registered] = &[ (`0x48fce4`), and the OFF leg `0x492f70` computes clamp(768/height, 0.9, 1.0) instead — \ 0.9 at 854 px tall and up, which is every window we ship against. It is 1.0 at 768 and \ below, where our flat 0.9 does diverge; `ui_script::DEFAULT_UI_SCALE` carries that. \ - `useUiScale` itself has no row here: nothing reads it, and registering it would only \ - offer a switch whose ON path we do not implement", + See `useUiScale` below, whose row this one used to say did not exist", ), + // **`useUiScale` (`0x8430c0`, default `"0"`)** — the switch the row above gates on. + // + // Its absence used to be argued for here as *"nothing reads it, and registering it would only + // offer a switch whose ON path we do not implement"*, and the first half of that has been + // false since the interface went stock: `ContainerFrame.lua:483` and `UIDropDownMenu.lua:525` + // both branch on `GetCVar("useUiScale") == "1"`, and `OptionsFrame.lua:13` gives it a + // checkbox. Nobody noticed because the only thing that said so was a host warning with + // nowhere to go (decision 2135, which is how this was found). + // + // Registering it changes no behaviour today — `nil ~= "1"` and `"0" ~= "1"` take the same + // branch — and makes the read the reference's read rather than an accident. The ON path + // lands where the reference's does, because our `uiScale` default *is* the reference's OFF-leg + // result: at `useUiScale = 1` both clients scale the bag frames and the dropdown list by + // `GetCVar("uiscale")`, and both read 0.9 there on every window we ship against. + same("useUiScale", "0"), same("farclip", "350"), // The Controls-page trio (0961). `deselectOnClick`/`mouseInvertPitch` are 1.12's own // Interface Options CVars (UIOptionsFrame.lua indices 45/1); their defaults are the diff --git a/crates/benilla-app/src/ui_script/addons.rs b/crates/benilla-app/src/ui_script/addons.rs index 25a0152f..03d0783e 100644 --- a/crates/benilla-app/src/ui_script/addons.rs +++ b/crates/benilla-app/src/ui_script/addons.rs @@ -279,6 +279,11 @@ impl Addon { let report = benilla_ui::loader::load_in(script, &doc, &path, &provider); for w in &report.warnings { warn!("ui_script({}/{file}): {w}", self.name); + // …and retained where a player can read it (2135). The `warn!` above is the + // terminal's copy and it is gone the moment the line scrolls; this is the one + // that survives to `/errors`, and the prefix is why it is recorded here rather + // than inside the loader — only this caller knows which file the warning is from. + script.report_warning(&format!("{}/{file}: {w}", self.name)); } for e in &report.errors { error!("ui_script({}/{file}): {e}", self.name); diff --git a/crates/benilla-app/src/ui_script/chat_options_tests.rs b/crates/benilla-app/src/ui_script/chat_options_tests.rs index c0eefa58..5bcf0d1c 100644 --- a/crates/benilla-app/src/ui_script/chat_options_tests.rs +++ b/crates/benilla-app/src/ui_script/chat_options_tests.rs @@ -242,8 +242,14 @@ fn the_background_row_opens_the_picker_and_its_opacity_slider_drives_the_window( let stored: f64 = s .eval("local _,_,_,_,_,a = GetChatWindowInfo(1) return a") .unwrap(); + // 204/255, and the last byte of that is 2133's doing. The slider carries + // `valueStep="0.01"`, so `SetValue` snaps `1 - 0.8` onto the lattice and rebuilds it as + // `n·step`; the reconstruction lands a hair BELOW the literal, `1 - value` a hair above 0.8, + // and the alpha store's floor quantizer then keeps 204 where it used to shed one to 203. + // The player asked for 0.8 and the store holds 0.8 — the client's own arithmetic, improved + // by accident. assert!( - (stored - 203.0 / 255.0).abs() < 1e-9, + (stored - 204.0 / 255.0).abs() < 1e-9, "the drag reached the engine store, quantized to its byte: {stored}" ); s.mouse_move(1500.0, 850.0); diff --git a/crates/benilla-app/src/ui_script/color_picker_tests.rs b/crates/benilla-app/src/ui_script/color_picker_tests.rs index 18bb3e89..df528e32 100644 --- a/crates/benilla-app/src/ui_script/color_picker_tests.rs +++ b/crates/benilla-app/src/ui_script/color_picker_tests.rs @@ -302,7 +302,13 @@ fn cancel_restores_the_previous_colour_through_cancel_func() { .eval("local t = applied[table.getn(applied)] return t.r, t.a") .unwrap(); assert_eq!(mid_r, 1.0, "the live preview really did go red"); - assert!((mid_a - 0.95).abs() < 1e-9); + // 1e-6, like the accept test's alpha above and for one more reason on top of the f32 store: + // `OpacitySliderFrame` carries `valueStep="0.01"`, so `SetValue` rebuilds the value as + // `n·step + min` (2133) and the reconstruction costs a few more ulps than the literal. + assert!( + (mid_a - 0.95).abs() < 1e-6, + "alpha is 1 - the slider's 0.05, got {mid_a}" + ); // …then backs out. s.run("ColorPickerCancelButton:Click()").unwrap(); diff --git a/crates/benilla-app/src/ui_script/faux_scroll_tests.rs b/crates/benilla-app/src/ui_script/faux_scroll_tests.rs index 44fb69a0..9fbe934c 100644 --- a/crates/benilla-app/src/ui_script/faux_scroll_tests.rs +++ b/crates/benilla-app/src/ui_script/faux_scroll_tests.rs @@ -304,20 +304,38 @@ fn dragging_the_bar_steps_the_offset_by_rows_and_repaints() { "the drag repainted the owner's list" ); - // A sub-row nudge rounds to the NEAREST row (`floor(v/itemHeight + 0.5)`) and lands on the - // same one, so the offset does not move. + // A sub-row nudge on the BAR reaches nothing at all: `FauxScrollFrame_Update` set the bar's + // step to one row, and `SetValue` quantises onto `min + n·step` before its change compare + // (2133, `0x789930`), so 51px resolves to the 48 the bar already holds — no + // `OnValueChanged`, no `SetVerticalScroll`, no repaint. This is the snap a 1.12 list + // scrollbar has and ours did not: the bar cannot come to rest between two rows. let settled = s.eval::("return TestRepaints").unwrap(); s.run("TestScrollScrollBar:SetValue(51)").unwrap(); + assert_eq!( + s.eval::("return TestScrollScrollBar:GetValue()") + .unwrap(), + 48.0, + "51px is not a lattice point; the bar stays on row 3's" + ); + assert_eq!( + s.eval::("return TestRepaints").unwrap(), + settled, + "and a value that did not move fires nothing" + ); + + // The unconditional repaint is still there — it lives one level up, on the path a wheel or a + // `SetVerticalScroll` takes. `FauxScrollFrame_OnVerticalScroll` ends in a bare + // `updateFunction();` (UIPanelTemplates.lua:228-232) with no compare against the previous + // offset, so a sub-row scroll through the FRAME repaints even though the bar does not move. + // Our deleted kit repainted only when the row actually changed; the reference does not, and + // that difference is the migration's, not a regression to chase (1860). + s.run("TestScroll:SetVerticalScroll(51)").unwrap(); assert_eq!( s.eval::("return FauxScrollFrame_GetOffset(TestScroll)") .unwrap(), 3, - "51px is still row 3 once rounded" + "51px is still row 3 once rounded (`floor(v/itemHeight + 0.5)`)" ); - // …but it STILL repaints. `FauxScrollFrame_OnVerticalScroll` ends in a bare - // `updateFunction();` (UIPanelTemplates.lua:228-233) — unconditional, with no compare against - // the previous offset. Our deleted kit repainted only when the row actually changed; the - // reference does not, and that difference is the migration's, not a regression to chase (1860). assert!( s.eval::("return TestRepaints").unwrap() > settled, "the reference repaints on every scroll, changed row or not" diff --git a/crates/benilla-app/src/ui_script/options_tests.rs b/crates/benilla-app/src/ui_script/options_tests.rs index 72fc6220..faca6229 100644 --- a/crates/benilla-app/src/ui_script/options_tests.rs +++ b/crates/benilla-app/src/ui_script/options_tests.rs @@ -1822,12 +1822,19 @@ fn defaults_resets_the_graphics_page_to_registered_defaults() { 2, "only the default writes queue — never the dead pending: {changes:?}" ); + // **357, not 350, and that is the reference's ladder rather than a rounding slip.** The row + // is the reference's own farclip slider — `minValue = OPTIONS_FARCLIP_MIN (177)`, + // `maxValue = OPTIONS_FARCLIP_MAX (777)`, `valueStep = (max − min)/10` — so its ten stops are + // 177, 237, 297, **357**, …, 777, and `SetValue` snaps onto them (2133). The registered + // default is 350 (1804: the reference's own `CVar::Register` value), which is not a stop, so + // seeding the slider from it lands on 357. 1.12 does exactly this and nobody can see it, + // because its option sliders print no number; ours prints one, so the ladder shows. assert_eq!( s.eval::( "return OptionsFrameContainerBodyGraphicsRowFarclipControlValue:GetText()" ) .unwrap(), - "350" + "357" ); assert_eq!( s.eval::( diff --git a/crates/benilla-app/src/ui_script/trainer_tests.rs b/crates/benilla-app/src/ui_script/trainer_tests.rs index 6eb17d64..b23cfe9c 100644 --- a/crates/benilla-app/src/ui_script/trainer_tests.rs +++ b/crates/benilla-app/src/ui_script/trainer_tests.rs @@ -675,8 +675,21 @@ fn wheel_scroll_is_silent_but_the_arrows_click() { "the wheel scroll is silent" ); - // The down arrow (enabled at the top), though — clicking it plays the ref's arrow click. - s.run("ClassTrainerListScrollFrameScrollBarScrollDownButton:Click()") + // That one notch reached the BOTTOM, and the arrow to click afterwards is therefore the UP + // one. `ScrollFrameTemplate_OnMouseWheel` moves half the bar's height — 76px on this window's + // 152-tall bar — and `SetValue` snaps that onto the row lattice (step 16), which rounds 76 up + // to the range's own 80 (2133). `FauxScrollFrame_Update` then greys the DOWN arrow on its + // `GetValue() - scrollFrameHeight == 0` test, so clicking it would be clicking a disabled + // button. Pinned rather than worked around: this snap is the reference's. + assert_eq!( + s.eval::("return ClassTrainerListScrollFrameScrollBar:GetValue()") + .unwrap(), + 80.0, + "one wheel notch = 76px, snapped to the 5-row bottom of an 80px range" + ); + + // The up arrow, now the enabled one — clicking it plays the ref's arrow click. + s.run("ClassTrainerListScrollFrameScrollBarScrollUpButton:Click()") .unwrap(); assert!( s.take_sounds().contains(&click), diff --git a/crates/benilla-app/src/ui_script/world_entry_tests.rs b/crates/benilla-app/src/ui_script/world_entry_tests.rs index 1aee597c..bb35799c 100644 --- a/crates/benilla-app/src/ui_script/world_entry_tests.rs +++ b/crates/benilla-app/src/ui_script/world_entry_tests.rs @@ -880,20 +880,27 @@ fn an_addon_that_fails_to_load_without_raising_is_readable_in_the_error_log() { script .eval::<()>("BenillaScriptLog_Update() return nil") .expect("the window repaints over a real log without raising"); - let row_label: String = script - .eval::>("return BenillaScriptLogRow1Label:GetText()") - .expect("eval") - .unwrap_or_default(); + // **A ROW shows it — not necessarily row 1.** The log is ordered by first occurrence and it + // carries warnings now (2135), which a world entry produces before any addon is reached, so + // asserting an index would be asserting how many warnings the stock UI happens to raise. + // What this pins is the row TEXT, which is the window's own trim path over a real message. + let row_labels: Vec = (1..=13) + .filter_map(|i| { + script + .eval::>(&format!("return BenillaScriptLogRow{i}Label:GetText()")) + .expect("eval") + }) + .collect(); assert!( - row_label.contains("AaMissing"), - "row 1 shows the failure, trimmed to the row's width: {row_label:?}" + row_labels.iter().any(|l| l.contains("AaMissing")), + "a row shows the failure, trimmed to the row's width: {row_labels:?}" ); let summary: String = script .eval::>("return BenillaScriptLogSummary:GetText()") .expect("eval") .unwrap_or_default(); assert!( - summary.contains("error"), + summary.contains("problem"), "the summary line counted them: {summary:?}" ); @@ -1159,3 +1166,48 @@ fn a_placed_window_comes_back_after_a_reload() { drop(world); let _ = std::fs::remove_dir_all(&tmp); } + +/// **A clean world entry raises the KNOWN warnings and no others** — the tripwire that keeps +/// decision 2135's channel worth having. +/// +/// The stock FrameXML is loaded here with no addons at all, so every row is a gap of *ours*. +/// Before 2135 these reached a terminal and nothing else, and four of them had been sitting in +/// every session for as long as the interface has been stock: two `OnCursorChanged` refusals (the +/// mail body and the GM ticket box, which therefore did not scroll as you typed — 2141), one +/// `OnInputLanguageChanged`, and an unregistered `useUiScale` that `ContainerFrame.lua` and +/// `UIDropDownMenu.lua` both read. +/// +/// The allowlist is deliberately a **list of names, not a count**: a new silent gap in a stock +/// file reddens this instead of scrolling past, and closing one means deleting a line here. +#[test] +fn a_clean_world_entry_raises_only_the_warnings_we_have_named() { + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let (_tmp, _c, _h) = hermetic_probe("clean-entry-warnings"); + let mut world = booted_world(); + world.init_resource::(); + log_in_as(&mut world, "Onehunter", 1); + let script = world + .get_non_send_resource::() + .expect("VM"); + + // **`OnInputLanguageChanged` stays out permanently**, and this is where that is said out loud. + // It is a real 1.12 slot (`FloatingChatFrame.xml` wires it to the IME language indicator) with + // zero corpus call sites, and benilla has no IME — so nothing here could ever fire it, and + // `SCRIPT_KINDS`' rule is that a name we cannot fire stays out. The refusal is the honest + // answer; the row is the price of saying it out loud. + const KNOWN: [&str; 1] = ["OnInputLanguageChanged"]; + + let unexpected: Vec = script + .diagnostics() + .into_iter() + .filter(|d| d.kind == benilla_ui::script::diagnostics::DiagnosticKind::Warning) + .map(|d| d.message) + .filter(|m| !KNOWN.iter().any(|k| m.contains(k))) + .collect(); + assert!( + unexpected.is_empty(), + "a stock world entry warned about something new — fix it or name it here: {unexpected:#?}" + ); +} diff --git a/crates/benilla-ui/src/loader/tests.rs b/crates/benilla-ui/src/loader/tests.rs index 03685727..d4eb9885 100644 --- a/crates/benilla-ui/src/loader/tests.rs +++ b/crates/benilla-ui/src/loader/tests.rs @@ -590,15 +590,18 @@ mod loader_tests { } /// Unsupported handler names are warn-once gaps, not hard errors, and don't stop the frame - /// from building. The example is `OnCursorChanged` — caret geometry is host-side here, so its - /// four float args would all be zero, which is the silent-drop this warn exists to avoid. - /// (It used to be `OnKeyDown`: that one is *fired* since decision 1319 and now belongs to - /// `script::tests::keyboard`.) + /// from building. The example is `OnAttributeChanged` — 2.0's secure-frame system, which no + /// 1.12 resolver has a slot for, so it is the one name on that list that is out permanently. + /// + /// It has been three names now, and each move is the rule working: `OnKeyDown` left when 1319 + /// built the delivery walk, `OnCursorChanged` when 2141 built the caret flush's fire. A name + /// is accepted only once something fires it, so this test's subject is whatever is still + /// waiting. #[test] fn unsupported_script_name_is_a_warning() { let s = UiScript::new().unwrap(); let doc = parse( - r#"x = 1"#, + r#"x = 1"#, ); let report = load(&s, &doc, &no_files); assert_eq!(report.frames, 1); @@ -606,7 +609,7 @@ mod loader_tests { assert!(report .warnings .iter() - .any(|w| w.contains("OnCursorChanged"))); + .any(|w| w.contains("OnAttributeChanged"))); } /// An unknown frame type is an error that drops that subtree but not the rest of the load. diff --git a/crates/benilla-ui/src/script/addon.rs b/crates/benilla-ui/src/script/addon.rs index aaf1ce35..b82b0b25 100644 --- a/crates/benilla-ui/src/script/addon.rs +++ b/crates/benilla-ui/src/script/addon.rs @@ -634,6 +634,13 @@ fn run_files(lua: &Lua, name: &str, read: &Reader, files: &[String]) { } }; let report = crate::loader::load_into(lua, &doc, &path, &provider); + // The warnings were dropped on the floor here until 2135 — an unresolved `inherits=`, a + // template of the wrong kind, an attribute nobody reads: the addon loads "clean", paints + // nothing, and the one thing that knew why was a `LoadReport` that went out of scope. + // They carry the `/` prefix because a bare loader warning names no document. + for w in report.warnings { + crate::script::diagnostics::record_warning(lua, &format!("{name}/{file}: {w}")); + } for e in report.errors { log_error(lua, &format!("{name}/{file}: {e}")); } @@ -659,10 +666,11 @@ fn load_miss(lua: &Lua, name: &str, file: &str) { fn load_failure(lua: &Lua, msg: &str) { let msg = format!("LoadAddOn: {msg}"); crate::script::diagnostics::record_load_failure(lua, &msg); + // `warn_host_only`, not `record_warning`: `record_load_failure` above has already retained + // this exact sentence as a `Load` row, which is the truer kind (the addon is not running). lua.app_data_mut::() .expect("model") - .warnings - .push(msg); + .warn_host_only(msg); } /// `root/rel`, refusing to escape `root` — the AddOns-root sandbox (1186), lexical and applied diff --git a/crates/benilla-ui/src/script/binding_abi.rs b/crates/benilla-ui/src/script/binding_abi.rs index dbdd542b..124e7b1a 100644 --- a/crates/benilla-ui/src/script/binding_abi.rs +++ b/crates/benilla-ui/src/script/binding_abi.rs @@ -90,7 +90,11 @@ pub(crate) fn coerced_number(lua: &Lua, v: Option) -> f64 { /// finds nothing, and answers `nil`. Everything else (nil, boolean, table, function) raises. pub(crate) fn string_arg(lua: &Lua, v: Value, usage: &'static str) -> mlua::Result { match lua.coerce_string(v)? { - Some(s) => Ok(s.to_str()?.to_owned()), + // Lossy, per decision 1193: a Lua 5.0 string is BYTES and the reference reads it as + // bytes, so an argument that is not valid UTF-8 costs a glyph here — it does not cost + // the call. `to_str()` used to raise, which turned one stray byte in one cp1252 addon's + // literal into a dead handler (2138). + Some(s) => Ok(s.to_string_lossy()), None => Err(mlua::Error::RuntimeError(usage.into())), } } @@ -118,15 +122,43 @@ pub(crate) fn string_arg(lua: &Lua, v: Value, usage: &'static str) -> mlua::Resu /// argument, and only there) test the tag themselves before calling this. pub(crate) fn optional_string(lua: &Lua, v: &Value) -> Option { match v { + // Lossy, per decision 1193 and for the same reason as [`string_arg`] — with a sharper + // symptom, because this one's failure was SILENT: `to_str().ok()` folded a string that + // was not valid UTF-8 into `None`, which every caller here reads as *the argument was + // not there*. `AddMessage` printed nothing, `CreateFrame` produced an unnamed frame, and + // nothing anywhere said why (2138). Value::String(_) | Value::Number(_) | Value::Integer(_) => lua .coerce_string(v.clone()) .ok() .flatten() - .and_then(|s| s.to_str().ok().map(|t| t.to_owned())), + .map(|s| s.to_string_lossy()), _ => None, } } +/// **A free-text argument — the player-visible kind — as the reference takes it** (decision 2138). +/// +/// The text sinks (`FontString`/`Button`/`EditBox`/`SimpleHTML` `SetText`, `SetFormattedText`, +/// `EditBox:Insert`) took their argument as mlua's `Option`, whose `FromLua` demands valid +/// UTF-8 and **raises** otherwise. Decision 1193 settled that a UI source file is bytes and that +/// "the right place to turn bytes into text is the boundary that actually requires text… A stray +/// byte should cost one glyph, not the file" — and then these boundaries cost the whole call. +/// `strsub` is byte-indexed (1193 keeps it that way deliberately), so *any* addon slicing +/// non-ASCII text hands a sink a broken sequence, and a cp1252 literal is a broken sequence to +/// begin with. +/// +/// This is `Option`'s conversion with exactly one arm changed: a **string** becomes its +/// lossy text. Every other type behaves as before — absent and `nil` are `None`, a number +/// coerces, a table or a function raises mlua's own conversion error — so the raise/absent split +/// each binding already had (§ [`optional_string`]'s table) is untouched. +pub(crate) fn text_arg(lua: &Lua, v: Option) -> mlua::Result> { + match v { + None | Some(Value::Nil) => Ok(None), + Some(Value::String(s)) => Ok(Some(s.to_string_lossy())), + Some(other) => mlua::FromLua::from_lua(other, lua).map(Some), + } +} + /// `GetBoolOrDefault` (`0x6f1c10`) — the reference's **boolean argument** coercion, which is *not* /// Lua truthiness and gets three arms backwards from the obvious reading. Byte-VERIFIED: wow-re /// `system/ui/scratch/action-bar-toggles.md` §2.1 re-derives the whole jump table at `0x6f1ce8` diff --git a/crates/benilla-ui/src/script/button.rs b/crates/benilla-ui/src/script/button.rs index 83081c75..b4cb86a9 100644 --- a/crates/benilla-ui/src/script/button.rs +++ b/crates/benilla-ui/src/script/button.rs @@ -231,11 +231,13 @@ pub(crate) fn set_label_font_justify_h_lua( } /// Run `f` over a frame's Button state under one short write borrow. -/// Run `f` over a frame's Button state under one short write borrow, then **settle the state -/// machine** — `f` is every Lua write that can move an input (`Enable`/`Disable`, -/// `SetButtonState`, `RegisterForClicks`), and the client's own writers call `SetState` on the -/// spot rather than leaving a paint to notice. Settling a read is a no-op (the transition guard is -/// `new == state`), so this stays on the one path instead of splitting into read/write halves. +/// +/// **It no longer settles a state machine afterwards, because there is no longer one to settle** +/// (decision 2134). The button's state is a latch the engine's own edges write; a Lua write moves +/// it only when the write IS an edge (`Enable`/`Disable`, `SetButtonState`), and those call the +/// edge themselves. `RegisterForClicks` and the texture setters no longer disturb the state at +/// all — which is the reference's behaviour: re-registering a held button's clicks does not +/// un-press it. fn with_button( lua: &Lua, this: &Table, @@ -243,34 +245,25 @@ fn with_button( ) -> mlua::Result { let h = frame_handle_of(lua, this)?; let mut model = lua.app_data_mut::().expect("model app_data"); - let (hovered, held) = press_inputs(&model, h); let frame = model .arena .frame_mut(h) .ok_or_else(|| mlua::Error::runtime("stale frame handle"))?; match &mut frame.kind_state { - KindState::Button(bs) => { - let out = f(bs); - bs.settle(hovered, held); - Ok(out) - } + KindState::Button(bs) => Ok(f(bs)), _ => Err(mlua::Error::runtime("not a Button")), } } -/// The two interaction inputs the model owns rather than the button: the cursor is over it, and a -/// registered press is holding it. -fn press_inputs(model: &Model, h: FrameHandle) -> (bool, bool) { - (model.mouseover == Some(h), press_held(model, h)) -} - /// `Enable()` / `Disable()` — and the second thing they do, which is not the state texture. /// /// `Disable 0x77ffd0` ends `0x78009a call [vtbl+0x90](0)`, reaching the shared helper /// `0x779160`; the constructor reaches the same helper with `1` (`0x778766`), which is what puts /// a fresh button in NORMAL. That helper does **two** things: /// -/// - `[vtbl+0x9c](state)` = `SetState 0x779790` — [`ButtonState::settle`]'s half; and +/// - `[vtbl+0x9c](state, 0)` = `SetButtonState 0x779790` — [`ButtonState::set_enabled`]'s half, +/// whose two early-outs (nothing happens if the button is already in the state being asked for) +/// are the helper's own; and /// - `0x7791bb push 4; call 0x76a730` — the per-layer enable for layer **4 = HIGHLIGHT**, written /// into the very `[frame+0x198]` array that `Enable/DisableDrawLayer` writes and that /// `0x76b3a0` reads back at draw time. @@ -281,23 +274,49 @@ fn press_inputs(model: &Model, h: FrameHandle) -> (bool, bool) { /// the client keeps it, instead of a second rule that agreed with it on the common case and /// disagreed on `` art the button did not put there itself. /// -/// The reference has one array and one writer, so an addon's `DisableDrawLayer("HIGHLIGHT")` is -/// undone by the next `Enable()`, exactly as here. +/// **The restore is NOT symmetric, and this comment used to claim it was** — *"one array and one +/// writer, so an addon's `DisableDrawLayer("HIGHLIGHT")` is undone by the next `Enable()`"*. Both +/// halves are false (wow-re `scratch/button-disabled-state-texture-law.md` §7, sharpened +/// 2026-09-09 off `0x779160` read contiguously for exactly this question). The indexed form +/// `[reg + 4*reg + 0x198]` has **three** sites image-wide — `0x76a717` (=1), `0x76a737` (=0) and +/// `DrawLayer 0x76b3a6`'s read — so there are **two** writers; and the enable arm calls +/// *neither* helper. It reaches the ON one only one hop away and only when the button is the +/// frame under the cursor: +/// +/// ```text +/// 779183 mov ecx,[esi+0xa0] ; the CSimpleTop singleton +/// 779189 cmp esi,[ecx+0x7c] ; am I the hover target? +/// 77918c jne 0x7791ce ; no -> return, layer untouched +/// 779192 call [edx+0x4c] ; = 0x779490 -> 0x76b6a0 -> 0x76b6c9 push 4; call 0x76a710 +/// ``` +/// +/// — i.e. `Enable()` on a hovered button restores the layer **by re-running the frame's Lua +/// ``**, and on a non-hovered one restores nothing (the next hover-enter does it). Both +/// restore paths are gated on the LockHighlight flag `[frame+0xf8]`, and the disable arm's +/// `0x7791bb call 0x76a730(4)` is the one site in the image that is *not* — so +/// `LockHighlight()` → `Disable()` → `Enable()` leaves the layer off in the reference, and only +/// `UnlockHighlight()`+`LockHighlight()` or an explicit `EnableDrawLayer` brings it back. +/// +/// **Ours is the symmetric bit-flip**, which draws the same in the ordinary case — a highlight is +/// only visible while hovered, and every hover-enter sets it — and diverges in two script-visible +/// ways: we do not fire the `` the reference synthesises when you `Enable()` a hovered +/// button, and we restore a locked-then-disabled highlight the reference leaves dark. Named here +/// rather than fixed, because the faithful shape needs the layer restore moved onto the hover +/// path where the reference keeps it. fn set_enabled(lua: &Lua, this: &Table, on: bool) -> mlua::Result<()> { let h = frame_handle_of(lua, this)?; let mut model = lua.app_data_mut::().expect("model app_data"); - let (hovered, held) = press_inputs(&model, h); let frame = model .arena .frame_mut(h) .ok_or_else(|| mlua::Error::runtime("stale frame handle"))?; + // The state half is [`ButtonState::set_enabled`], whose two early-outs are the client's: + // `Enable()` on a button that is not DISABLED does nothing at all, and neither does + // `Disable()` on one that already is. match &mut frame.kind_state { - KindState::Button(bs) => { - bs.enabled = on; - bs.settle(hovered, held); - } + KindState::Button(bs) => bs.set_enabled(on), _ => return Err(mlua::Error::runtime("not a Button")), - } + }; let bit = 1u8 << DrawLayer::Highlight.index(); if on { frame.disabled_layers &= !bit; @@ -307,18 +326,19 @@ fn set_enabled(lua: &Lua, this: &Table, on: bool) -> mlua::Result<()> { Ok(()) } -/// Re-latch a button's state texture after something OUTSIDE the widget moved an input — the -/// mouse crossing its boundary, a press landing or lifting, a capture dropped when the pointer -/// left the window, or the hover cleared because the frame was hidden under the cursor. +/// Run one of the engine's own state edges over frame `h`, if it is a live Button. +/// +/// The reference's edge set is exactly seven `SetButtonState` call sites and **six of them are +/// engine-side** (wow-re `scratch/button-state-edge-set.md`, VERIFIED): Enable, Disable, the hide +/// notify, mouse-down, mouse-up and the drag-threshold crossing. There is **no enter or leave +/// edge** — which is why the pointer path no longer calls anything here when the cursor crosses a +/// button's boundary (decision 2134). /// -/// These are the client's own `SetState` call sites (`0x7791ed` enter, `0x7793f0` leave, -/// `0x7792ad` down, `0x7793c2` up); ours reach the same transition through the derived inputs. /// Harmless on a non-Button handle, and on a stale one. -pub(super) fn settle(model: &mut Model, h: FrameHandle) { - let (hovered, held) = press_inputs(model, h); +pub(super) fn edge(model: &mut Model, h: FrameHandle, f: impl FnOnce(&mut ButtonState)) { if let Some(frame) = model.arena.frame_mut(h) { if let KindState::Button(bs) = &mut frame.kind_state { - bs.settle(hovered, held); + f(bs); } } } @@ -558,7 +578,8 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { // SetText/GetText target the ButtonText fontstring (RF-28: the `text` attr routes there). m.set( "SetText", - lua.create_function(|lua, (this, text): (Table, Option)| { + lua.create_function(|lua, (this, text): (Table, Option)| { + let text = super::binding_abi::text_arg(lua, text)?; // `CSimpleButton::SetText 0x778dc0` opens `if (!text) return;` (`0x778dcc`, wow-re // `button-label-build-and-anchor-order.md`) — a nil never reaches the label at all, so // it neither clears the text nor lazily creates the FontString. That guard is the @@ -991,7 +1012,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { m.set( "IsEnabled", lua.create_function(|lua, this: Table| { - with_button(lua, &this, |bs| i64::from(bs.enabled)) + with_button(lua, &this, |bs| i64::from(bs.enabled())) })?, )?; @@ -1012,49 +1033,53 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // SetButtonState("PUSHED"/"NORMAL") / GetButtonState — the scripted press state - // (`0x780270`/`0x780180`; ref `ActionButtonDown/Up`, ActionButton.lua:15-28: DOWN pushes - // only from NORMAL, UP fires only from PUSHED — the state doubles as the keybind debounce). - // Case-insensitive like the API's other string enums; an unknown state is a runtime error. - // DISABLED is Enable/Disable's to set, not this method's (the 1.12 FrameXML never passes it). + // **`SetButtonState(state[, lock]) 0x780270` — TWO arguments**, and the second is not + // cosmetic (wow-re `scratch/binding-shape-arity-law.md` §3, VERIFIED). It reads three stack + // indices and returns none; the state maps case-insensitively over exactly + // `{"DISABLED", "NORMAL", "PUSHED"}` (`0x780390`, `SStrCmpI`) and an unrecognised string + // raises `Usage: %s:SetButtonState("state", lock)`; the flag is `GetBoolOrDefault` with + // **default 0** (`0x78032c push 0`), whose number arm truncates through `_ftol`, so the + // literal `1` that `MainMenuBarMicroButtons.lua` passes arms it. + // + // **The lock is the whole of decision 2134.** While it is set the button ignores the mouse + // press/release transitions; while it is clear, the next release un-pushes a scripted push — + // which is what Tablet-2.0's rows depend on, since the library pushes them and never calls + // `SetButtonState("NORMAL")` anywhere. + // + // DISABLED really is accepted here — the mapper takes it — even though the 1.12 FrameXML + // never passes it. m.set( "SetButtonState", - lua.create_function(|lua, (this, state): (Table, String)| { - let pushed = if state.eq_ignore_ascii_case("PUSHED") { - true + lua.create_function(|lua, (this, state, lock): (Table, String, MultiValue)| { + let new = if state.eq_ignore_ascii_case("PUSHED") { + ButtonVisualState::Pushed } else if state.eq_ignore_ascii_case("NORMAL") { - false + ButtonVisualState::Normal + } else if state.eq_ignore_ascii_case("DISABLED") { + ButtonVisualState::Disabled } else { return Err(mlua::Error::runtime(format!( - "SetButtonState: unknown state '{state}'" + "Usage: SetButtonState(\"state\", lock) — unknown state '{state}'" ))); }; - with_button(lua, &this, |bs| bs.pushed_state = pushed) + let first = lock.into_iter().next(); + let locked = super::binding_abi::bool_or_default(first.as_ref(), false); + with_button(lua, &this, |bs| bs.set_button_state(new, locked)) })?, )?; m.set( "GetButtonState", lua.create_function(|lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - // The LIVE mouse-held press counts too (hovered + a registered button down on this - // frame — the same predicate the extract pass renders the PushedTexture by), closing - // the documented INTERIM where a mouse-held button answered "NORMAL": the chat scroll - // buttons' hold-repeat (ref MessageFrameScrollButton_OnUpdate) polls exactly this - // mid-press (decision 0288 P3). It reads the same in the reference for the same - // reason — `0x7792ad` writes ONE state variable and `0x780180` reads it back, so a - // right-held button answers "PUSHED" as surely as a left-held one. - let held = { - let model = lua.app_data_ref::().expect("model app_data"); - model.mouseover == Some(h) && press_held(&model, h) - }; - with_button(lua, &this, move |bs| { - if !bs.enabled { - "DISABLED" - } else if bs.pushed_state || held { - "PUSHED" - } else { - "NORMAL" - } + // ONE variable, read back (`0x780180` reads `[obj+0x328]`). This used to OR in the + // live mouse-held press, because the state was derived and the press lived outside + // the widget; now the press *wrote* the state at its edge, so a mouse-held button + // answers "PUSHED" for the same reason the reference does — and cannot disagree with + // the art the way a re-derived answer could (decision 2134). The chat scroll buttons' + // hold-repeat (ref `MessageFrameScrollButton_OnUpdate`) still reads what it needs. + with_button(lua, &this, |bs| match bs.button_state() { + ButtonVisualState::Disabled => "DISABLED", + ButtonVisualState::Pushed => "PUSHED", + ButtonVisualState::Normal => "NORMAL", }) })?, )?; @@ -1176,18 +1201,6 @@ pub(super) fn wants_press_visual(model: &Model, h: FrameHandle, button: &str) -> } } -/// Is a press currently holding this button down — the `held` half of the PushedTexture rule. -/// -/// Any captured mouse button counts, gated by [`wants_press_visual`]. Callers pair it with -/// `hovered`, which is `0x779256`'s hit test at press time plus `0x7793f0`'s restore-to-NORMAL -/// when the pointer leaves mid-press. -pub(super) fn press_held(model: &Model, h: FrameHandle) -> bool { - model - .mouse_down_on - .iter() - .any(|(button, &pressed)| pressed == h && wants_press_visual(model, h, button)) -} - /// The click behavior shared by the input path and `Click()`: gated on `enabled`; a CheckButton /// **toggles before OnClick fires** (the documented widget contract — a handler reading /// `self:GetChecked()` sees the new state); then `OnClick(self, button, down)` — `down` is `true` @@ -1236,7 +1249,7 @@ pub(super) fn click_button(lua: &Lua, id: u32, button: &str, down: bool, scripte match &mut frame.kind_state { // A Button/CheckButton click is gated on its enabled flag… KindState::Button(bs) => { - if bs.enabled { + if bs.enabled() { if is_check { bs.checked = !bs.checked; } diff --git a/crates/benilla-ui/src/script/cursor/drag.rs b/crates/benilla-ui/src/script/cursor/drag.rs index 22404018..ffee63e2 100644 --- a/crates/benilla-ui/src/script/cursor/drag.rs +++ b/crates/benilla-ui/src/script/cursor/drag.rs @@ -72,6 +72,13 @@ pub(crate) fn maybe_start_drag(model: &mut Model, pos: (f32, f32)) -> Option<(u3 (g.source, g.button.clone()) }; model.drag.as_mut().expect("checked Some above").started = true; + // **The button's drag-start edge** — `CSimpleButton` overrides `+0x74` with `0x7793f0`, which + // un-presses (`0x779410`, guarded on `locked == 0 && state != DISABLED`) and then forwards to + // the base notify that fires ``. So a button you drag off releases its pushed art + // at the THRESHOLD crossing, which is here — not when the cursor leaves its rect, and not for + // a frame that never registered for drag (wow-re `scratch/button-state-edge-set.md`; + // decision 2134). + super::super::button::edge(model, source, crate::widget::ButtonState::on_drag_start); let id = model .arena .frame(source) diff --git a/crates/benilla-ui/src/script/cvars.rs b/crates/benilla-ui/src/script/cvars.rs index 9eb54fca..be0e8a89 100644 --- a/crates/benilla-ui/src/script/cvars.rs +++ b/crates/benilla-ui/src/script/cvars.rs @@ -223,7 +223,7 @@ pub(super) fn write_cvar( fn warn_unknown(model: &mut Model, name: &str) { let key = name.to_ascii_lowercase(); if model.cvars_warned.insert(key) { - model.warnings.push(format!( + model.record_warning(format!( "unknown CVar '{name}' (not host-registered) — ignored" )); } diff --git a/crates/benilla-ui/src/script/diagnostics.rs b/crates/benilla-ui/src/script/diagnostics.rs index 6826199e..1ea3ee2d 100644 --- a/crates/benilla-ui/src/script/diagnostics.rs +++ b/crates/benilla-ui/src/script/diagnostics.rs @@ -88,6 +88,18 @@ pub enum DiagnosticKind { /// dependency that isn't there) land here — from the player's side they are one thing: *the /// addon isn't running*. Load, + /// **The call was accepted and did not do what it said.** Nothing raised and nothing failed to + /// load: an `inherits=` argument that was not a template name and got dropped, a `SetPoint` + /// whose `relativeTo` did not resolve and re-anchored to the owner, a `SetCVar` on a name + /// nothing registered, a saved variable that could not be serialised, a `CreateMacro` that + /// was refused. The addon runs on, believing it got what it asked for. + /// + /// **This kind exists because the channel was a dead end** (decision 2135). These messages had + /// one consumer — a `warn!` line in the host's terminal, drained and discarded every frame — + /// so they reached neither the player (who cannot read a terminal, which is the whole + /// argument of 1495 above) nor the addon survey (whose columns read `errors`). A warning is + /// exactly the class an instrument is *for*: it is the failure that does not announce itself. + Warning, } impl DiagnosticKind { @@ -96,6 +108,7 @@ impl DiagnosticKind { match self { Self::Error => "error", Self::Load => "load", + Self::Warning => "warn", } } } @@ -212,6 +225,17 @@ impl super::UiScript { pub fn report_load_failure(&self, msg: &str) { record_load_failure(&self.lua, msg); } + + /// Retain one non-fatal warning the HOST caught and has already logged — a loader warning off + /// an addon's XML, carrying the `/` prefix only the caller knows (decision 2135). + /// + /// The retention half alone, exactly like [`Self::report_script_error`]: the caller writes its + /// own `warn!` line, and putting the message on [`Model::warnings`] as well would double it at + /// the app's next per-frame drain. For a warning the ENGINE raises, the door is + /// [`Model::record_warning`], which owns both halves. + pub fn report_warning(&self, msg: &str) { + record_warning(&self.lua, msg); + } } /// **THE rule for "an addon file did not load"** — one implementation, both loaders (decision @@ -237,6 +261,20 @@ pub(crate) fn record_load_failure(lua: &Lua, msg: &str) { .record(DiagnosticKind::Load, msg); } +/// **THE rule for "something was accepted and did not do what it said"** — the retention half of +/// the warning channel, for a caller that holds `&Lua` (decision 2135). +/// +/// Its `&UiScript` face is [`super::UiScript::report_warning`] and its engine-internal sibling is +/// [`Model::record_warning`]; the difference is only which of the two channels the caller still +/// owes. A host caller has already written its own log line and wants retention only; an engine +/// caller wants both. +pub(crate) fn record_warning(lua: &Lua, msg: &str) { + lua.app_data_mut::() + .expect("model app_data set") + .diagnostics + .record(DiagnosticKind::Warning, msg); +} + /// Register the error-log reads the `BenillaScriptLogFrame` polls. /// /// **`Benilla`-prefixed because they are ours**, not 1.12's — the prefix rule as @@ -304,6 +342,62 @@ mod tests { assert_eq!(log.total(), 1); } + #[test] + fn a_warning_reaches_the_retained_log_and_the_host_drain() { + // 2135's whole subject. Before it, a warning existed for exactly as long as one terminal + // line: `take_warnings` drained it and nothing kept a copy, so neither the player's + // `/errors` window nor the addon survey could see one. + let mut s = crate::script::UiScript::new().expect("vm"); + s.run(r#"SetCVar("thisCVarDoesNotExist", "1")"#).unwrap(); + + let host = s.take_warnings(); + assert!( + host.iter().any(|w| w.contains("thisCVarDoesNotExist")), + "the host's per-frame drain still gets it: {host:?}" + ); + let kept = s.diagnostics(); + let row = kept + .iter() + .find(|d| d.message.contains("thisCVarDoesNotExist")) + .unwrap_or_else(|| panic!("…and it is RETAINED: {kept:#?}")); + assert_eq!(row.kind, DiagnosticKind::Warning, "under its own kind"); + + // The drain is a drain; the log is a memory. A second frame does not lose the row. + assert!(s.take_warnings().is_empty()); + assert!(s + .diagnostics() + .iter() + .any(|d| d.message.contains("thisCVarDoesNotExist"))); + } + + #[test] + fn a_text_sink_takes_bytes_rather_than_raising_on_them() { + // Decision 2138, and 1193's rule finally applied at the boundary 1193 named. `strsub` is + // byte-indexed by design, so a slice through a multi-byte character is a string an addon + // really does hand to `SetText` — and mlua's `String` conversion raised on it, killing the + // handler. A stray byte costs a glyph. + let s = crate::script::UiScript::new().expect("vm"); + s.run( + r#" + f = CreateFrame("Frame", "BytesFrame") + t = f:CreateFontString("BytesText") + -- "a—b" is 5 bytes; cutting at 2 leaves the em dash's lead byte alone. + local sliced = strsub("a\226\128\148b", 1, 2) + t:SetText(sliced) + e = CreateFrame("EditBox", "BytesBox") + e:SetText(sliced) + "#, + ) + .expect("a sliced multi-byte string must not raise"); + assert!(s.errors().is_empty(), "{:?}", s.errors()); + let got: Option = s.eval("return BytesText:GetText()").unwrap(); + assert_eq!( + got.as_deref(), + Some("a\u{fffd}"), + "the broken byte became one replacement glyph, and the call completed" + ); + } + #[test] fn kind_is_part_of_identity() { let mut log = DiagnosticLog::default(); diff --git a/crates/benilla-ui/src/script/editbox.rs b/crates/benilla-ui/src/script/editbox.rs index 30e42c78..0bebc20c 100644 --- a/crates/benilla-ui/src/script/editbox.rs +++ b/crates/benilla-ui/src/script/editbox.rs @@ -46,7 +46,7 @@ //! Windows/Linux; decision 0301) and reuse the byte-verified word classes + selection-first //! delete law. -use mlua::{Lua, Table}; +use mlua::{Lua, Table, Value}; use super::object::frame_handle_of; use super::types::{EditAction, EditUnit}; @@ -611,6 +611,91 @@ fn fire_script(lua: &Lua, id: u32, name: &str) { } } +/// **The caret flush's fire — `OnCursorChanged(x, y, w, h)`** (`0x77da80`). +/// +/// The reference recomputes the caret's anchor inside `0x77da80` and, having done so, fires this +/// with four floats *"each value scaled by `f`, so all four are in FrameXML UI units"* (wow-re +/// `system/ui/ui.md`, the RF-0085 caret law, VERIFIED): +/// +/// * `x` — the caret's advance from its line's start, the same number the paint anchors by +/// ([`EditBoxState::caret_row_x`]). +/// * `y` — `−lineIndex · (snap(H·scale) + snap(spacing·scale))/scale`, i.e. minus the row index +/// times the row pitch. **Negative-downward**, which is what `ScrollingEdit_OnCursorChanged`'s +/// `cursorOffset = y` then `-this.cursorOffset` reads as a positive distance down the page. +/// * `w` — **`≡ 4.0`**, a constant in the reference (the caret texture's own width, read through +/// its layout vtable `+0x1c`). +/// * `h` — the font line height, read through `+0x20`. +/// +/// Ours takes the row pitch [`EditBoxState::cell_h`] for both the `y` factor and `h`. The two are +/// the same number whenever the font declares no extra `spacing`, which is every box in the +/// shipped 1.12 UI; a box that *does* set spacing would report an `h` a little tall. Named rather +/// than guessed at — the alternative is carrying a second measured number through the advance +/// answer for a case nothing in the corpus produces. +/// +/// **The edge is a CHANGE, not a frame.** `0x77da80` has exactly one caller, `0x77d475`, gated on +/// dirty bit 2, so the handler runs when the caret has actually moved — which is what makes +/// `ScrollingEdit_OnUpdate`'s `if (this.cursorOffset)` idiom terminate. [`EditBoxState::cursor_fired`] +/// is that bit's memory here. +/// +/// **Scoped to the focused box**, because the caret geometry rides the advance table and only the +/// focused box asks the host for one ([`super::editbox::seam`]). The reference flushes any dirty +/// box; the difference is invisible for the customer this exists for — you are typing in the box +/// you are looking at — and an unfocused box's caret is never drawn either way. +pub(in crate::script) fn drain_cursor_changed(lua: &Lua) { + let fire = { + let mut model = lua.app_data_mut::().expect("model app_data"); + let Some(h) = model.focused_editbox else { + return; + }; + if !model.arena.frame(h).is_some_and(|f| f.effective_visible) { + return; + } + let id = model.frame_id(h); + let Some(KindState::EditBox(eb)) = model.arena.frame_mut(h).map(|f| &mut f.kind_state) + else { + return; + }; + // No answered advance table yet ⇒ no caret geometry to report. The next tick after the + // host answers carries it, exactly as the reference's next flush would. + let display = eb.display(); + if eb.advances.len() != display.len() + 1 { + return; + } + let cursor_d = eb.text_to_display(eb.cursor).min(display.len()); + let (row, x) = if eb.multi_line { + eb.caret_row_x(cursor_d) + } else { + (0, eb.advances[cursor_d]) + }; + if eb.cursor_fired == Some((row, x)) { + return; + } + eb.cursor_fired = Some((row, x)); + let pitch = eb.cell_h; + (id, x, -(row as f32) * pitch, pitch) + }; + let (id, x, y, h) = fire; + if let Err(e) = event::fire_widget_handler( + lua, + id, + "OnCursorChanged", + vec![ + Value::Number(f64::from(x)), + Value::Number(f64::from(y)), + Value::Number(f64::from(CARET_WIDTH)), + Value::Number(f64::from(h)), + ], + ) { + lua.app_data_mut::() + .expect("model app_data") + .record_script_error(e.to_string()); + } +} + +/// The caret's width as `OnCursorChanged` reports it — **a constant in the reference**, not a +/// measurement (wow-re: `w ≡ 4.0`). +const CARET_WIDTH: f32 = 4.0; + // The Lua method surface (SetText/GetText/HighlightText/SetFocus/…, consulted before the shared // frame table only for EditBox frames) — a child module over this file's focus/editing // primitives, like `interact`/`seam`. diff --git a/crates/benilla-ui/src/script/editbox/methods.rs b/crates/benilla-ui/src/script/editbox/methods.rs index 54443ae4..f8d8c88f 100644 --- a/crates/benilla-ui/src/script/editbox/methods.rs +++ b/crates/benilla-ui/src/script/editbox/methods.rs @@ -30,7 +30,8 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { m.set( "SetText", - lua.create_function(|lua, (this, s): (Table, Option)| { + lua.create_function(|lua, (this, s): (Table, Option)| { + let s = crate::script::binding_abi::text_arg(lua, s)?; let h = frame_handle_of(lua, &this)?; // Programmatic SetText KEEPS a history browse in progress: the chat live parse // rewrites the box on every recalled slash line ("/s hi" → Say + "hi"), and ending @@ -100,7 +101,8 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { // `Option` coercion follows it. m.set( "Insert", - lua.create_function(|lua, (this, s): (Table, Option)| { + lua.create_function(|lua, (this, s): (Table, Option)| { + let s = crate::script::binding_abi::text_arg(lua, s)?; let h = frame_handle_of(lua, &this)?; if let Some(s) = s { insert(lua, h, &s, true); diff --git a/crates/benilla-ui/src/script/editbox/tests.rs b/crates/benilla-ui/src/script/editbox/tests.rs index 1a894f59..61f7953d 100644 --- a/crates/benilla-ui/src/script/editbox/tests.rs +++ b/crates/benilla-ui/src/script/editbox/tests.rs @@ -1585,3 +1585,87 @@ fn set_max_bytes_caps_the_buffer_in_bytes_with_minus_one_unlimited() { assert!(s.run("E:SetMaxBytes()").is_err(), "too few raises"); assert!(s.run("E:SetMaxBytes(1, 2)").is_err(), "too many raises"); } + +/// **`OnCursorChanged(x, y, w, h)` fires when the caret moves** — the edge the shipped +/// `MailFrame.xml` and `HelpFrame.xml` wire `ScrollingEdit_OnCursorChanged` to, and the reason a +/// multiline box follows its caret as you type past the bottom (decision 2141). +/// +/// The four args are the reference's (wow-re's RF-0085 caret law, VERIFIED): `x` the caret's +/// advance along its line, `y` **negative-downward** by row (which is what +/// `ScrollingEdit_OnCursorChanged`'s `cursorOffset = y` then `-this.cursorOffset` reads back as a +/// positive distance), `w` the constant `4.0`, `h` the line height. +#[test] +fn the_caret_flush_fires_on_cursor_changed_with_the_references_four_args() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + fires = {} + E = CreateFrame("EditBox", "E") + E:SetWidth(200); E:SetHeight(64) + E:SetPoint("BOTTOMLEFT", 100, 50) + E:SetMultiLine(true) + E:SetScript("OnCursorChanged", function() + table.insert(fires, { x = arg1, y = arg2, w = arg3, h = arg4 }) + end) + E:SetFocus() + "#, + ) + .unwrap(); + s.resolve(); + // Three bytes on row 0, then a wrap onto row 1 — the host answers the rows and the pitch. + for ch in "abcdef".chars() { + s.char_input(&ch.to_string()); + } + s.resolve(); + if let Some(req) = s.editbox_advances_request() { + // A plain monotonic 7 px/byte table (the rig's own), wrapped into two rows at byte 3 — + // `caret_row_x` subtracts the row start's cumulative advance, which is the whole point. + let cum: Vec = (0..=req.text.len()).map(|i| i as f32 * 7.0).collect(); + s.set_editbox_advances(req.id, req.key, cum, vec![0, 3], 12.0); + } + s.tick(0.016); + assert!(s.errors().is_empty(), "{:?}", s.errors()); + + let n: i64 = s.eval("return table.getn(fires)").unwrap(); + assert!(n >= 1, "the flush fired at least once"); + let (x, y, w, h): (f64, f64, f64, f64) = s + .eval("local f = fires[table.getn(fires)] return f.x, f.y, f.w, f.h") + .unwrap(); + // The caret sits after 6 bytes: row 1 (rows start at 0 and 3), 3 bytes along it. + assert_eq!( + x, 21.0, + "x is the advance from the ROW's start, not the text's" + ); + assert_eq!( + y, -12.0, + "y is minus the row index times the pitch — downward is negative" + ); + assert_eq!(w, 4.0, "w is the reference's constant, not a measurement"); + assert_eq!(h, 12.0, "h is the line height"); + + // **The edge is a CHANGE.** A tick that moves nothing fires nothing — which is what makes + // `ScrollingEdit_OnUpdate`'s `if (this.cursorOffset)` loop terminate instead of re-scrolling + // every frame. + let before: i64 = s.eval("return table.getn(fires)").unwrap(); + s.tick(0.016); + s.tick(0.016); + assert_eq!( + s.eval::("return table.getn(fires)").unwrap(), + before, + "two quiet ticks fire nothing" + ); + + // …and a caret move fires again. + s.editbox_action(EditAction::Move { + unit: EditUnit::Char, + back: true, + extend: false, + }); + s.tick(0.016); + assert!( + s.eval::("return table.getn(fires)").unwrap() > before, + "moving the caret one char fires the flush" + ); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} diff --git a/crates/benilla-ui/src/script/event.rs b/crates/benilla-ui/src/script/event.rs index 5870809e..83bf1767 100644 --- a/crates/benilla-ui/src/script/event.rs +++ b/crates/benilla-ui/src/script/event.rs @@ -22,7 +22,7 @@ use mlua::{Function, Lua, MultiValue, Table, Value}; use super::{Model, ScriptValue, REG_SCRIPTS}; use crate::script::object::frame_wrapper; -use crate::widget::FrameHandle; +use crate::widget::{ButtonState, FrameHandle}; /// Fire `event` at every frame registered for it (the engine-internal twin of /// `UiScript::fire_event`, for engine code holding only the Lua context — the compare drive's @@ -169,10 +169,6 @@ pub(super) fn fire_visibility_changes(lua: &Lua, changed: Vec) { .filter(|&m| items.iter().any(|&(h, _, vis)| h == m && !vis)); if let Some(m) = hidden_hover { model.mouseover = None; - // The hover the hide just dropped is an input to the button state machine, so the - // frame under it re-latches here as it would on a mouse-out (`0x7793f0`) — a button - // hidden mid-press must not come back up still wearing its pushed art. - super::button::settle(&mut model, m); if model.drag.as_ref().is_some_and(|d| d.source == m) { model.drag = None; } @@ -191,6 +187,18 @@ pub(super) fn fire_visibility_changes(lua: &Lua, changed: Vec) { if visible { let mut model = lua.app_data_mut::().expect("model"); super::object::toplevel::raise_on_show(&mut model, h); + } else { + // **The button's HIDE edge** — `CSimpleButton` overrides the hide notify (`+0x34`, + // `0x7791e0`) to un-press itself before tail-jumping the base, so a button hidden + // while held does not come back up wearing its pushed art (wow-re + // `scratch/button-state-edge-set.md`; the guard is `state != DISABLED && locked == 0`). + // + // It hangs off the VISIBILITY transition, not off the hover, which is what it is in + // the reference — a button hidden nowhere near the cursor un-presses too, and one + // hidden under the cursor no longer needs the hover-drop above to notice + // (decision 2134). + let mut model = lua.app_data_mut::().expect("model"); + super::button::edge(&mut model, h, ButtonState::on_hide); } let name = if visible { "OnShow" } else { "OnHide" }; if let Err(e) = fire(lua, id, name, None, Vec::new()) { diff --git a/crates/benilla-ui/src/script/extract.rs b/crates/benilla-ui/src/script/extract.rs index efa01eef..6923130a 100644 --- a/crates/benilla-ui/src/script/extract.rs +++ b/crates/benilla-ui/src/script/extract.rs @@ -245,7 +245,7 @@ impl UiScript { let hovered = owner.is_some() && model.mouseover == owner; // The PRESS is not read here. Which state texture shows is latched on the // transition (`ButtonState::set_state`), so the press reaches the paint - // through `button::settle` at the moment the mouse moves it — not by + // through the press EDGE at the moment the button goes down — not by // being re-derived every frame. `hovered` survives because the Highlight // is not a state texture and carries no latch. if !bs.region_visible(rh, hovered) { @@ -286,26 +286,27 @@ impl UiScript { // shows its label. Every state-colour caller in our own UI ships the // matching font object, so the two readings agree on all of them. let highlighted = hovered || bs.locked_highlight; - let (name, color, justify) = - if !bs.enabled && bs.disabled_font.is_some() { - ( - bs.disabled_font.as_ref(), - bs.disabled_color, - bs.disabled_justify_h, - ) - } else if bs.enabled && highlighted && bs.highlight_font.is_some() { - ( - bs.highlight_font.as_ref(), - bs.highlight_color, - bs.highlight_justify_h, - ) - } else { - ( - bs.normal_font.as_ref(), - bs.normal_color, - bs.normal_justify_h, - ) - }; + let (name, color, justify) = if !bs.enabled() + && bs.disabled_font.is_some() + { + ( + bs.disabled_font.as_ref(), + bs.disabled_color, + bs.disabled_justify_h, + ) + } else if bs.enabled() && highlighted && bs.highlight_font.is_some() { + ( + bs.highlight_font.as_ref(), + bs.highlight_color, + bs.highlight_justify_h, + ) + } else { + ( + bs.normal_font.as_ref(), + bs.normal_color, + bs.normal_justify_h, + ) + }; state_font = name.and_then(|n| model.font_object(n)); button_font = bs.font.as_ref(); state_color = color; diff --git a/crates/benilla-ui/src/script/layout.rs b/crates/benilla-ui/src/script/layout.rs index 788701df..b63cc8b0 100644 --- a/crates/benilla-ui/src/script/layout.rs +++ b/crates/benilla-ui/src/script/layout.rs @@ -827,6 +827,7 @@ impl UiScript { frame_to_id, screen, warnings, + diagnostics, solver, layout_scope: scope, layout_fingerprint, @@ -1355,10 +1356,17 @@ impl UiScript { return; } if round + 1 == round_cap { - warnings.push(format!( + // `Model::record_warning`'s two halves, spelled out because this pass holds the + // model destructured and cannot call it: the host drain, and the retained log + // (2135). `WOW_LAYOUT_VERIFY`'s falsifier re-run truncates `warnings` and does + // NOT roll the log back — the log dedupes, so a verify build reads `×2` on a row + // that happened once, and that build is not a production path. + let msg = format!( "layout: anchor graph did not converge in {round_cap} rounds — \ an anchor cycle? (rects left at their last pass)" - )); + ); + diagnostics.record(super::diagnostics::DiagnosticKind::Warning, &msg); + warnings.push(msg); } } // The cycle bail (the loop ran out of rounds and warned above): the rects it leaves are diff --git a/crates/benilla-ui/src/script/macros.rs b/crates/benilla-ui/src/script/macros.rs index 3de6bd0c..7ef1cbd2 100644 --- a/crates/benilla-ui/src/script/macros.rs +++ b/crates/benilla-ui/src/script/macros.rs @@ -217,14 +217,12 @@ fn create_macro( ) -> Option { let name = clamp_chars(name.trim(), MAX_MACRO_NAME); if name.is_empty() { - model - .warnings - .push("CreateMacro() failed, no name specified".into()); + model.record_warning("CreateMacro() failed, no name specified"); return None; } let list = model.macros.list_mut(per_character); if list.len() >= MAX_MACROS { - model.warnings.push(format!( + model.record_warning(format!( "CreateMacro() failed, already have {MAX_MACROS} macros" )); return None; diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 0c18a00e..59d4515a 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -451,7 +451,7 @@ pub const SCREEN: crate::layout::Handle = 0; /// corpus call sites across 91 addons); it removes working behaviour from the 23 sites that remain, /// so it is still a change to make deliberately rather than as a side effect of widening this list /// — but the FrameXML half of "with FrameXML fixed first" is most of the way there now. -const SCRIPT_KINDS: [&str; 37] = [ +const SCRIPT_KINDS: [&str; 38] = [ "OnLoad", "OnEvent", "OnUpdate", @@ -475,6 +475,11 @@ const SCRIPT_KINDS: [&str; 37] = [ "OnTabPressed", "OnTextChanged", "OnTextSet", + // The caret flush's own (`0x77da80`), fired by the tick's `drain_cursor_changed` when the + // caret has moved — the edge `ScrollingEdit_OnCursorChanged` + `ScrollingEdit_OnUpdate` scroll + // a multiline box by. It earns its row here the way this list's rule requires: together with + // the code that fires it (decisions 2135/2141). + "OnCursorChanged", "OnEditFocusGained", "OnEditFocusLost", "OnVerticalScroll", @@ -1255,11 +1260,11 @@ impl UiScript { let mut model = self.model_mut(); let held: Vec = model.mouse_down_on.values().copied().collect(); model.mouse_down_on.clear(); - // Every button that capture was holding down goes back to NORMAL — the release the OS - // never fed us (`0x7793c2`'s transition), without which a button walked off the window - // edge mid-press keeps its pushed art for the rest of the session. + // Every button that capture was holding down goes back to NORMAL — the release edge the + // OS never fed us (`0x7793de`), without which a button walked off the window edge + // mid-press keeps its pushed art for the rest of the session. for h in held { - button::settle(&mut model, h); + button::edge(&mut model, h, crate::widget::ButtonState::on_mouse_up); } // …and its one-slot twin `root+0x80`, which the mouse-down raise reads: a capture left // behind would aim the next press's raise at whatever the pointer was last holding. diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index fb1b6729..872a6d6c 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -1738,6 +1738,35 @@ impl Model { self.errors.push(msg); } + /// **Record one non-fatal warning — the channel's one door** (decision 2135). Both halves: + /// the host's per-frame `warnings` drain (a `warn!` line in the terminal, for whoever is + /// running the client), and the retained diagnostic log, which is the only copy a player or an + /// instrument can read after the frame that produced it. + /// + /// Before this there was only the first half, and it was a dead end: `take_warnings` empties + /// every frame, so a warning existed for the length of one terminal line and then nowhere at + /// all. The messages it carries are the failures that never announce themselves — a dropped + /// `inherits=`, an unresolved anchor, an unregistered CVar — which is precisely the class the + /// diagnostic log was built for. + /// + /// Its sibling is [`Self::warn_host_only`], for the one message already retained under a + /// truer kind. + pub(crate) fn record_warning(&mut self, msg: impl Into) { + let msg = msg.into(); + self.diagnostics + .record(super::diagnostics::DiagnosticKind::Warning, &msg); + self.warnings.push(msg); + } + + /// The host channel **alone** — for a message that is already retained under a kind that says + /// more than `Warning` does. Exactly one caller: `LoadAddOn`'s failure line, which + /// `record_load_failure` has already filed as a [`Load`](super::diagnostics::DiagnosticKind) + /// row ("the addon isn't running", which is the fact worth keeping) and which still wants its + /// terminal line. Retaining it twice would put the same sentence in the log under two kinds. + pub(crate) fn warn_host_only(&mut self, msg: String) { + self.warnings.push(msg); + } + pub(crate) fn unit(&self, token: &str) -> Option<&UnitState> { if token.bytes().any(|b| b.is_ascii_uppercase()) { self.units_by_lower.get(&token.to_ascii_lowercase()) diff --git a/crates/benilla-ui/src/script/object.rs b/crates/benilla-ui/src/script/object.rs index 6cd61aae..d46cba38 100644 --- a/crates/benilla-ui/src/script/object.rs +++ b/crates/benilla-ui/src/script/object.rs @@ -652,7 +652,7 @@ pub(super) fn create_frame( if template.is_none() { if let Some(v) = inherits.as_ref().filter(|v| !v.is_nil()) { let mut model = lua.app_data_mut::().expect("model app_data"); - model.warnings.push(format!( + model.record_warning(format!( "CreateFrame: the 4th argument (inherits) must be a template-name string, got {}; \ ignored for '{}'", v.type_name(), @@ -727,7 +727,9 @@ pub(super) fn create_frame( let messages = crate::loader::apply_template(lua, &wrapper, &kind, &template); if !messages.is_empty() { let mut model = lua.app_data_mut::().expect("model app_data"); - model.warnings.extend(messages); + for m in messages { + model.record_warning(m); + } } } Ok(wrapper) diff --git a/crates/benilla-ui/src/script/object/events_regions.rs b/crates/benilla-ui/src/script/object/events_regions.rs index 83d612ef..fb8bcf1b 100644 --- a/crates/benilla-ui/src/script/object/events_regions.rs +++ b/crates/benilla-ui/src/script/object/events_regions.rs @@ -309,14 +309,14 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { /// the mechanism: the kind buckets, the strata 8→0 order, the key-name table, and the /// existence-not-handling consumption gate). They were this list's standing exception; the rule /// above let them in the moment something fired them. -/// * **`OnCursorChanged`** (4 sites over 3 addons — all of them the Era `ScrollingEdit_OnCursorChanged` -/// auto-scroll idiom) — **raising**, by this list's own rule: nothing fires it. It is the -/// EditBox's own slot (RF-28 `+0x428`), fired by the reference's caret flush `0x77da80` with -/// **four float caret-POSITION args**. The geometry itself is no longer the obstacle — the paint -/// seam computes `caret_row`/`caret_x` off the host-answered advance table -/// ([`crate::script::editbox::seam`]) — but that is a per-extract recompute with no event edge, -/// so there is no counterpart to the flush to fire from. Accepting the name before building one -/// would mean a scroll box that silently never follows the caret. +/// * **`OnCursorChanged`** (4 sites over 3 addons, plus two in the SHIPPED FrameXML — all of them +/// the `ScrollingEdit_OnCursorChanged` auto-scroll idiom) — **accepted since decision 2141**, +/// which built the edge this entry said was missing. The EditBox's own slot (RF-28 `+0x428`), +/// fired by the caret flush `0x77da80` with four float caret-position args in UI units; +/// `editbox::drain_cursor_changed` is our counterpart, on the tick, gated on the caret having +/// actually moved. The stock `MailFrame.xml` and `HelpFrame.xml` declare it, so until 2141 the +/// mail body and the GM ticket box did not scroll as you typed past their bottom — a bug nobody +/// could see, because the load-time refusal only reached a terminal (decision 2135). /// * **`OnAttributeChanged`** (1 site, `Roid-Macros`) — **raising, permanently.** It is 2.0's secure /// frame/attribute system; there is no such slot in any 1.12 resolver. That addon is asking for a /// later client and should hear so. @@ -431,7 +431,7 @@ fn apply_region_inherits(lua: &Lua, wrapper: &Table, from: &str) -> mlua::Result }; if is_template { let mut model = lua.app_data_mut::().expect("model"); - model.warnings.push(format!( + model.record_warning(format!( "CreateTexture/CreateFontString: '{from}' is a registered TEMPLATE, not a font object; the region is created but the template's content is not applied (no corpus caller does this)" )); return Ok(()); diff --git a/crates/benilla-ui/src/script/object/layout_methods.rs b/crates/benilla-ui/src/script/object/layout_methods.rs index 34053a57..2d95d4e7 100644 --- a/crates/benilla-ui/src/script/object/layout_methods.rs +++ b/crates/benilla-ui/src/script/object/layout_methods.rs @@ -439,7 +439,7 @@ fn set_point(lua: &Lua, this: &Table, point: &str, rest: [Value; 4]) -> mlua::Re .frame(h) .and_then(|f| f.name.clone()) .unwrap_or_else(|| "".into()); - model.warnings.push(format!( + model.record_warning(format!( "SetPoint({who}): relativeTo '{}' does not resolve — anchored to the parent", nt.name )); diff --git a/crates/benilla-ui/src/script/pointer.rs b/crates/benilla-ui/src/script/pointer.rs index ade5df13..8d2ff41a 100644 --- a/crates/benilla-ui/src/script/pointer.rs +++ b/crates/benilla-ui/src/script/pointer.rs @@ -48,7 +48,7 @@ use mlua::Value; use crate::layout::Rect; use crate::order; -use crate::widget::FrameHandle; +use crate::widget::{ButtonState, FrameHandle}; use super::clip::{effective_clip, scroll_clip_sources}; use super::{button, cursor, editbox, event, Model, UiScript}; @@ -316,14 +316,17 @@ impl UiScript { }); let old_handle = model.mouseover; model.mouseover = new_handle; - // The client's `0x7793f0` (leave) and `0x7791ed` (enter) both call `SetState`: - // a press held over a button and then walked off it drops back to NORMAL, and - // walking back on picks PUSHED up again. Ours latches the same transition from - // the moved hover — both sides of the boundary, since each one's `hovered` - // changed. - for h in [old_handle, new_handle].into_iter().flatten() { - button::settle(&mut model, h); - } + // **Crossing a button's boundary moves NO button state**, and the comment that + // used to sit here claimed the opposite off two mis-attributed addresses: + // `0x7793f0` is the drag-start override, not a leave, and `0x7791e0` is the hide + // override, not an enter. The real enter/leave notifies `0x779490`/`0x7794e0` + // read `[+0x328]` only as a DISABLED guard and never write it (wow-re + // `scratch/button-state-edge-set.md`, a §5 census of every store to the field). + // + // So a press held over a button and walked off it stays PUSHED in the reference — + // it is the *drag threshold* (below) that un-presses one, and only for a frame + // that registered for drag. Decision 2134. + let _ = old_handle; (old_id, drag_start, true, slider_change, color_change) } }; @@ -546,11 +549,25 @@ impl UiScript { Some(h) => model.mouse_down_on.insert(button.to_string(), h), None => model.mouse_down_on.remove(button), }; - // `0x7792ad`: the press SetState(PUSHED), unconditional past the registration - // and hit gates. The displaced entry is settled too — a second press of the same - // button elsewhere releases whatever it was holding. - for h in [displaced, hit_handle].into_iter().flatten() { - button::settle(&mut model, h); + // `0x7792b1`: the press edge, `SetButtonState(PUSHED, 0)`. + // + // It is unconditional *past* `0x779210`'s own two gates, and the first of them is + // the registration mask `[this+0x330]` tested as `m | m << 8` at `0x77924b` — + // "registered for this mouse button EITHER WAY, up or down". That is + // [`button::wants_press_visual`], and it is why a right-click lights up an action + // slot (it registers `RightButtonUp`) and does nothing at all to a plain + // `LeftButtonUp` button. Past it the edge is gated only on the button's own + // `locked == 0 && state != DISABLED`. + if let Some(h) = hit_handle.filter(|&h| button::wants_press_visual(&model, h, button)) + { + button::edge(&mut model, h, ButtonState::on_mouse_down); + } + // A press arriving while this same mouse button was already holding another + // frame is a release we never got (the OS ate it — a focus loss, a modal). Give + // the displaced frame that release rather than leaving it pushed forever; the + // reference never reaches this because its capture guarantees the release. + if let Some(h) = displaced { + button::edge(&mut model, h, ButtonState::on_mouse_up); } // `0x7663e6` writes the resolved target into `root+0x80` — capture-else-hover, so // this is an `or`, not an assignment: an existing capture is not displaced by a @@ -588,10 +605,6 @@ impl UiScript { (click, None, false, jump, None, abandoned, color_jump) } else { let pressed = model.mouse_down_on.remove(button); - // `0x7793c2`: the release SetState(NORMAL) on the frame the press captured. - if let Some(h) = pressed { - button::settle(&mut model, h); - } // `root+0x80` is cleared at `0x7664bb` **only when the post-event button mask is // zero** — a chorded release keeps the capture for the button still held. With the // per-button map already drained above, "mask is zero" is "the map is empty". @@ -609,6 +622,16 @@ impl UiScript { ); let release = cursor::take_drag(&mut model, button); let started = release.as_ref().is_some_and(|r| r.started); + // **The release edge** — `0x7793de`'s `SetButtonState(NORMAL, 0)`, on the frame + // the PRESS captured and regardless of where the cursor now is (`0x7792d0` runs + // no hit test of its own). It is guarded on the button's own `locked == 0` and on + // the state actually being PUSHED, and it is **skipped when the base finalized a + // drag** — `0x7792df`'s `0x76c040` test, which is this `started` (the button + // already un-pressed at the drag-start edge below, and un-pressing again would + // fire nothing but would be a second transition the reference does not make). + if let (Some(h), false) = (pressed, started) { + button::edge(&mut model, h, ButtonState::on_mouse_up); + } // The world drop: press AND release both over THE WORLD, no drag started — // a completed left click on the game world (never a drag release, which just // keeps carrying; decision 0218's byte-verified trigger). What actually drops diff --git a/crates/benilla-ui/src/script/region.rs b/crates/benilla-ui/src/script/region.rs index bf186e7b..d4d5a71c 100644 --- a/crates/benilla-ui/src/script/region.rs +++ b/crates/benilla-ui/src/script/region.rs @@ -776,7 +776,7 @@ pub(super) fn resolve_target( .and_then(|&h| model.arena.frame(h)) .and_then(|f| f.name.clone()) .unwrap_or_else(|| "".into()); - model.warnings.push(format!( + model.record_warning(format!( "SetPoint(region of {who}): relativeTo '{}' does not resolve — anchored to the owner", nt.name )); diff --git a/crates/benilla-ui/src/script/region/text.rs b/crates/benilla-ui/src/script/region/text.rs index e5ffdace..291f58bd 100644 --- a/crates/benilla-ui/src/script/region/text.rs +++ b/crates/benilla-ui/src/script/region/text.rs @@ -27,7 +27,10 @@ use super::region_handle_of; pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { m.set( "SetText", - lua.create_function(|lua, (this, text): (Table, Option)| { + lua.create_function(|lua, (this, text): (Table, Option)| { + // `text_arg`, not `Option`: a Lua string is bytes and a sliced one need not be + // valid UTF-8 (decision 2138 — this raise took the whole handler down). + let text = crate::script::binding_abi::text_arg(lua, text)?; let rh = region_handle_of(lua, &this)?; let mut model = lua.app_data_mut::().expect("model"); let data = model.region_data.entry(rh).or_default(); @@ -50,7 +53,8 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { .globals() .get::("string")? .get::("format")?; - let text: String = format.call(args)?; + // The formatted result is bytes too — `%s` of a sliced string is a sliced string. + let text = format.call::(args)?.to_string_lossy(); let rh = region_handle_of(lua, &this)?; let mut model = lua.app_data_mut::().expect("model"); let data = model.region_data.entry(rh).or_default(); diff --git a/crates/benilla-ui/src/script/saved.rs b/crates/benilla-ui/src/script/saved.rs index 15efd204..fb551242 100644 --- a/crates/benilla-ui/src/script/saved.rs +++ b/crates/benilla-ui/src/script/saved.rs @@ -83,7 +83,7 @@ impl super::UiScript { } } if !unwritable.is_empty() { - self.model_mut().warnings.push(format!( + self.model_mut().record_warning(format!( "saved variables: not serializable, skipped: {}", unwritable.join(", ") )); diff --git a/crates/benilla-ui/src/script/simplehtml/mod.rs b/crates/benilla-ui/src/script/simplehtml/mod.rs index a968269f..0a9f683b 100644 --- a/crates/benilla-ui/src/script/simplehtml/mod.rs +++ b/crates/benilla-ui/src/script/simplehtml/mod.rs @@ -244,7 +244,9 @@ pub(crate) fn set_text(model: &mut Model, fh: FrameHandle, raw: &str) -> bool { let parsed = parse::parse_markup(raw, &frame_name, &hyperlink_format); // The reference pushes these through the frame's error sink to the console. Ours are host // warnings: a malformed page is the player's content being wrong, not a script raising. - model.warnings.extend(parsed.errors.iter().cloned()); + for e in &parsed.errors { + model.record_warning(e.clone()); + } build(model, fh, &parsed.blocks); parsed.used_markup @@ -836,7 +838,8 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { "SetText", lua.create_function(|lua, (this, text): (Table, Value)| { let raw = match &text { - Value::String(s) => s.to_str()?.to_string(), + // Bytes, lossily — never a raise (2138). + Value::String(s) => s.to_string_lossy(), Value::Number(_) | Value::Integer(_) => super::object::as_f32(&text).to_string(), _ => String::new(), }; diff --git a/crates/benilla-ui/src/script/slider.rs b/crates/benilla-ui/src/script/slider.rs index c8ba91bd..99fcbb59 100644 --- a/crates/benilla-ui/src/script/slider.rs +++ b/crates/benilla-ui/src/script/slider.rs @@ -136,8 +136,12 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { )?; m.set( "SetValueStep", + // Not a field write (2143): past the step store it re-pushes the range through + // `SetMinMaxValues`, which re-clamps and re-quantises the held value onto the new lattice + // and fires `OnValueChanged` if it moved — so this binding fires like the other two. lua.create_function(|lua, (this, step): (Table, f32)| { - with_slider(lua, &this, |s| s.step = step) + let changed = with_slider(lua, &this, |s| s.set_value_step(step))?; + fire_value_changed(lua, &this, changed) })?, )?; m.set( diff --git a/crates/benilla-ui/src/script/tests/button.rs b/crates/benilla-ui/src/script/tests/button.rs index c3b91c80..d2fba471 100644 --- a/crates/benilla-ui/src/script/tests/button.rs +++ b/crates/benilla-ui/src/script/tests/button.rs @@ -1244,3 +1244,197 @@ fn a_state_texture_slot_takes_an_object_and_a_nil() { "the slot must read empty after nil" ); } + +/// **The unlocked scripted push is released by the next mouse release — Tablet-2.0's rows** +/// (decision 2134). +/// +/// `SetButtonState(state[, lock])` writes `[+0x32c]` unconditionally, and the mouse-up edge +/// `0x7793de` un-presses whenever `locked == 0`. Tablet-2.0 (`Tablet-2.0.lua:1645`) pushes a row it +/// finds `clicked` and calls `SetButtonState("NORMAL")` *nowhere in the library* — it relies on +/// exactly this. Ours kept a scripted push until Lua cleared it, so a Questie/FuBar/oRA2 row stayed +/// depressed for the rest of the session. +#[test] +fn an_unlocked_scripted_push_is_released_by_the_next_mouse_release() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + row = CreateFrame("Button", "TabletRow") + row:SetPoint("BOTTOMLEFT", 0, 0); row:SetSize(100, 100) + row:SetNormalTexture("Interface\\RowN.blp") + row:SetPushedTexture("Interface\\RowP.blp") + row:SetButtonState("PUSHED") -- Tablet-2.0's call, verbatim: no lock argument + "#, + ) + .unwrap(); + s.resolve(); // a press only reaches a frame with a resolved rect + assert_eq!( + s.eval::("return TabletRow:GetButtonState()") + .unwrap(), + "PUSHED" + ); + + // A press and release over the row: the release edge finds it PUSHED and unlocked. + s.mouse_move(50.0, 50.0); + s.mouse_button(50.0, 50.0, "LeftButton", true); + s.mouse_button(50.0, 50.0, "LeftButton", false); + assert_eq!( + s.eval::("return TabletRow:GetButtonState()") + .unwrap(), + "NORMAL", + "the release un-pushes an unlocked scripted push — the row does not stay depressed" + ); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} + +/// **`lock` pins the state against the mouse — the micro buttons** (`0x780270`'s third index, +/// `GetBoolOrDefault` with default 0; wow-re `binding-shape-arity-law.md` §3). +/// +/// `MainMenuBarMicroButtons.lua` calls `SetButtonState("PUSHED", 1)` when its panel opens, and the +/// button must stay depressed through every press and release until the panel closes. The same +/// flag pins a NORMAL button *against* being pushed, which is the half a "sticky push" model +/// cannot express at all. +#[test] +fn a_locked_state_ignores_the_mouse_and_enable_disable_clears_the_lock() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + micro = CreateFrame("Button", "MicroButton") + micro:SetPoint("BOTTOMLEFT", 0, 0); micro:SetSize(100, 100) + micro:SetButtonState("PUSHED", 1) -- MainMenuBarMicroButtons.lua, verbatim + pin = CreateFrame("Button", "PinnedNormal") + pin:SetPoint("BOTTOMLEFT", 200, 0); pin:SetSize(100, 100) + pin:SetButtonState("NORMAL", 1) + "#, + ) + .unwrap(); + s.resolve(); + + s.mouse_move(50.0, 50.0); + s.mouse_button(50.0, 50.0, "LeftButton", true); + s.mouse_button(50.0, 50.0, "LeftButton", false); + assert_eq!( + s.eval::("return MicroButton:GetButtonState()") + .unwrap(), + "PUSHED", + "a locked push survives a whole click" + ); + + s.mouse_move(250.0, 50.0); + s.mouse_button(250.0, 50.0, "LeftButton", true); + assert_eq!( + s.eval::("return PinnedNormal:GetButtonState()") + .unwrap(), + "NORMAL", + "…and a locked NORMAL cannot be pushed by the mouse at all" + ); + s.mouse_button(250.0, 50.0, "LeftButton", false); + + // `0x779160` passes `locked = 0` on both arms, so Enable/Disable UNLOCK. The panel closing + // with a plain `SetButtonState("NORMAL")` (flag defaulting to 0) does the same. + s.run("MicroButton:Disable() MicroButton:Enable()").unwrap(); + assert_eq!( + s.eval::("return MicroButton:GetButtonState()") + .unwrap(), + "NORMAL", + "Disable() wrote DISABLED, Enable() wrote NORMAL — and both cleared the lock" + ); + s.mouse_move(50.0, 50.0); + s.mouse_button(50.0, 50.0, "LeftButton", true); + assert_eq!( + s.eval::("return MicroButton:GetButtonState()") + .unwrap(), + "PUSHED", + "so the mouse reaches it again" + ); + s.mouse_button(50.0, 50.0, "LeftButton", false); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} + +/// **Walking off a held button does NOT un-press it, and walking back on does not re-press it.** +/// +/// The correction decision 2134 is built on. Our old model derived the press as +/// `(held && hovered) || pushed_state` and re-evaluated it whenever the hover moved; the +/// reference's enter and leave notifies (`0x779490`/`0x7794e0`) read `[+0x328]` only as a DISABLED +/// guard and **never write it** — proved by a §5 census of every store to the field image-wide. +/// The only thing that un-presses a held button before its release is the drag-threshold crossing, +/// and only for a frame that registered for drag. +#[test] +fn the_hover_is_not_an_input_to_the_press_state() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + b = CreateFrame("Button", "HeldButton") + b:SetPoint("BOTTOMLEFT", 0, 0); b:SetSize(100, 100) + b:SetNormalTexture("Interface\\HeldN.blp") + b:SetPushedTexture("Interface\\HeldP.blp") + "#, + ) + .unwrap(); + s.resolve(); + let shows = |s: &UiScript, path: &str| { + s.extract() + .iter() + .any(|q| matches!(&q.content, QuadContent::Texture { path: Some(p), .. } if p == path)) + }; + + s.mouse_move(50.0, 50.0); + s.mouse_button(50.0, 50.0, "LeftButton", true); + assert!(shows(&s, "Interface\\HeldP.blp"), "the press pushed it"); + + // Walk the cursor right off it, still holding. + s.mouse_move(500.0, 400.0); + assert!( + shows(&s, "Interface\\HeldP.blp"), + "…and it stays pushed off the rect: the leave notify does not write the state" + ); + assert_eq!( + s.eval::("return HeldButton:GetButtonState()") + .unwrap(), + "PUSHED" + ); + + // The release, off the button, still un-presses it — `0x7792d0` runs no hit test of its own. + s.mouse_button(500.0, 400.0, "LeftButton", false); + assert!( + shows(&s, "Interface\\HeldN.blp"), + "the release is the edge, wherever the cursor is" + ); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} + +/// **A button hidden while held comes back NORMAL** — `CSimpleButton`'s hide override +/// (`+0x34` = `0x7791e0`), which un-presses and then tail-jumps the base notify so `` +/// still fires. It hangs off the visibility transition, not off the hover, so it reaches a button +/// hidden nowhere near the cursor too. +#[test] +fn hiding_a_held_button_un_presses_it() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + h = CreateFrame("Button", "HidButton") + h:SetPoint("BOTTOMLEFT", 0, 0); h:SetSize(100, 100) + "#, + ) + .unwrap(); + s.resolve(); + s.mouse_move(50.0, 50.0); + s.mouse_button(50.0, 50.0, "LeftButton", true); + assert_eq!( + s.eval::("return HidButton:GetButtonState()") + .unwrap(), + "PUSHED" + ); + s.run("HidButton:Hide()").unwrap(); + assert_eq!( + s.eval::("return HidButton:GetButtonState()") + .unwrap(), + "NORMAL", + "the hide edge un-pressed it" + ); + s.mouse_button(50.0, 50.0, "LeftButton", false); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} diff --git a/crates/benilla-ui/src/script/tests/input.rs b/crates/benilla-ui/src/script/tests/input.rs index e079aee9..6a9409e2 100644 --- a/crates/benilla-ui/src/script/tests/input.rs +++ b/crates/benilla-ui/src/script/tests/input.rs @@ -617,8 +617,10 @@ fn the_unfired_script_kinds_still_raise_rather_than_silently_accepting() { // delivery walk, so they are fired and therefore accepted — the rule this test enforces, // applied in the other direction. Their law lives in `script::tests::keyboard`. // - // Caret geometry is host-side; its four float args would all be zero. - "OnCursorChanged", + // `OnCursorChanged` left this list with decision 2141: the tick's caret flush fires it + // (`editbox::drain_cursor_changed`), which is the rule applied in the other direction + // again — its law lives in `script::editbox::tests`. + // // 2.0's secure-frame system — no such slot exists in any 1.12 resolver. "OnAttributeChanged", // Real 1.12 slots we do not fire, and zero corpus call sites. (`OnUpdateModel` and diff --git a/crates/benilla-ui/src/script/tests/slider.rs b/crates/benilla-ui/src/script/tests/slider.rs index b6544dd5..085ad4e4 100644 --- a/crates/benilla-ui/src/script/tests/slider.rs +++ b/crates/benilla-ui/src/script/tests/slider.rs @@ -438,3 +438,214 @@ fn a_thumb_with_no_authored_size_takes_its_arts_texel_span_and_still_drags() { "travel is 128−32 = 96, so a 96-unit pull runs the value min→max (got {v})" ); } + +#[test] +fn slider_setvalue_quantises_onto_the_min_anchored_lattice() { + // `SetValue 0x789930` rounds the clamped value onto `min + n·step` (round-half-away-from-zero, + // truncated by `__ftol`) BEFORE it stores — so `GetValue` on a stepped slider can only ever + // answer a lattice point, and the lattice is anchored at `min`, not at zero. Decision 2095 + // named this divergence and left it; this is it closed. + let s = script(); + s.run( + r#" + q = CreateFrame("Slider", "SlQuant") + q:SetMinMaxValues(0.2, 1.2) + q:SetValueStep(0.5) + "#, + ) + .unwrap(); + // The lattice is 0.2 / 0.7 / 1.2 — 0.5 and 1.0 are NOT on it, which is the whole point of + // anchoring at `min`. + for (set, want) in [ + (0.6, 0.7), + (0.95, 0.7), + (0.4, 0.2), + (1.2, 1.2), + (0.2, 0.2), + (0.0, 0.2), // clamped to min first, then quantised + ] { + s.run(&format!("SlQuant:SetValue({set})")).unwrap(); + let v: f32 = s.eval("return SlQuant:GetValue()").unwrap(); + assert!( + (v - want).abs() < 1e-5, + "SetValue({set}) settles on {want}, got {v}" + ); + } +} + +#[test] +fn a_zero_step_leaves_the_value_continuous() { + // The gate that keeps every scrollbar working: `UIPanelScrollBarTemplate` declares no + // `valueStep`, so `[+0x324]` stays at the ctor's 0.0 and `0x78999c`'s `fcomp(step, 0.0)` + // skips the quantiser outright. A stepless slider stores exactly what it was handed. + let s = script(); + s.run( + r#" + c = CreateFrame("Slider", "SlCont") + c:SetMinMaxValues(0, 1) + c:SetValue(0.375) + "#, + ) + .unwrap(); + let v: f32 = s.eval("return SlCont:GetValue()").unwrap(); + assert_eq!(v, 0.375, "no step means no lattice"); + let step: f32 = s.eval("return SlCont:GetValueStep()").unwrap(); + assert_eq!(step, 0.0, "the ctor's step is 0.0, and nothing set one"); +} + +#[test] +fn the_quantised_value_is_not_re_clamped_and_may_pass_max() { + // The clamp runs BEFORE the quantiser and there is no second clamp after it (`0x789a06` + // stores whatever `fild n; fmul step; fadd min` produced). So a range that is not a whole + // number of steps rounds *past* its own max at the top of the travel. This looks like a bug + // and is the client's arithmetic; it is unreachable in the shipped UI because every stepped + // slider the reference ships is an exact multiple of its step. + let s = script(); + s.run( + r#" + o = CreateFrame("Slider", "SlOver") + o:SetMinMaxValues(0, 2.5) + o:SetValueStep(1) + o:SetValue(2.5) + "#, + ) + .unwrap(); + let v: f32 = s.eval("return SlOver:GetValue()").unwrap(); + assert_eq!(v, 3.0, "2.5 clamps to 2.5, then rounds half-away to 3·step"); + let (min, max): (f32, f32) = s.eval("return SlOver:GetMinMaxValues()").unwrap(); + assert_eq!( + (min, max), + (0.0, 2.5), + "and the range itself is untouched — the overshoot is in the value alone" + ); +} + +#[test] +fn a_faux_scrollframes_rows_snap_and_its_bottom_is_exact() { + // The falsifier decision 2095 named for this change, run on the shape it names. + // `FauxScrollFrame_Update` builds its range as `(numItems − numToDisplay) · valueStep` + // (`UIPanelTemplates.lua:180`) and then sets that same step, so the range IS a whole number + // of steps: every drag lands on a row boundary and the bottom of the travel is exactly max. + // 20 items, 10 on screen, 16 units a row ⇒ range 0..160, step 16. + let s = script(); + s.run( + r#" + f = CreateFrame("Slider", "SlFaux") + f:SetMinMaxValues(0, 160) + f:SetValueStep(16) + "#, + ) + .unwrap(); + for (set, want) in [ + (23.9, 16.0), // just under the half-step boundary: the row below + (24.1, 32.0), // just over it: the row above + (37.0, 32.0), + (160.0, 160.0), // the bottom is exact — no overshoot, the range is 10 steps + (0.0, 0.0), + ] { + s.run(&format!("SlFaux:SetValue({set})")).unwrap(); + let v: f32 = s.eval("return SlFaux:GetValue()").unwrap(); + assert_eq!(v, want, "SetValue({set}) snaps to the row at {want}"); + // FauxScrollFrame_GetOffset's own arithmetic, which is what the row list reads. + let off: f32 = s.eval("return floor(SlFaux:GetValue() / 16)").unwrap(); + assert_eq!(off, (want / 16.0).floor(), "and the row offset follows it"); + } +} + +#[test] +fn a_move_inside_one_step_fires_nothing() { + // The quantiser sits INSIDE the change-gate (`0x789a0b`'s `fcom` is against the stored value, + // and what is stored is already quantised), so a drag that stays within one step's band + // resolves to the same lattice point and never reaches `OnValueChanged`. Before this, every + // pixel of a stepped drag fired. + let s = script(); + s.run( + r#" + fires = 0 + g = CreateFrame("Slider", "SlGate") + g:SetScript("OnValueChanged", function() fires = fires + 1 end) + g:SetMinMaxValues(0, 160) + g:SetValueStep(16) + g:SetValue(32) -- first-ever value: always fires + "#, + ) + .unwrap(); + assert_eq!(s.eval::("return fires").unwrap(), 1); + for v in [30.0, 34.0, 39.9, 24.1] { + s.run(&format!("SlGate:SetValue({v})")).unwrap(); + } + assert_eq!( + s.eval::("return fires").unwrap(), + 1, + "four moves inside the 24..40 band are one lattice point: no further fire" + ); + s.run("SlGate:SetValue(40.1)").unwrap(); + assert_eq!( + s.eval::("return fires").unwrap(), + 2, + "crossing into the next band fires once" + ); +} + +#[test] +fn set_value_step_re_quantises_the_held_value_and_can_fire() { + // `SetValueStep 0x789a60` stores the step and then re-pushes the range through + // `SetMinMaxValues`, which re-clamps the held value through `SetValue` — onto the NEW lattice + // (decision 2143). A step handed to a slider that already holds a value moves it and fires. + let s = script(); + s.run( + r#" + fires = 0 + v = CreateFrame("Slider", "SlStep") + v:SetScript("OnValueChanged", function() fires = fires + 1 end) + v:SetMinMaxValues(0, 100) + v:SetValue(37) -- first-ever value: always fires, no step yet, so raw + "#, + ) + .unwrap(); + assert_eq!(s.eval::("return SlStep:GetValue()").unwrap(), 37.0); + assert_eq!(s.eval::("return fires").unwrap(), 1); + + // A step arriving AFTER the value snaps it: 37 → 40 on a 10-lattice, and the move fires. + s.run("SlStep:SetValueStep(10)").unwrap(); + assert_eq!( + s.eval::("return SlStep:GetValue()").unwrap(), + 40.0, + "the step re-quantised a value that was already stored" + ); + assert_eq!( + s.eval::("return fires").unwrap(), + 2, + "…and the move fired OnValueChanged, exactly as a SetValue would" + ); + + // A step that moves nothing fires nothing — the same change gate, one level up. + s.run("SlStep:SetValueStep(20)").unwrap(); + assert_eq!(s.eval::("return SlStep:GetValue()").unwrap(), 40.0); + assert_eq!(s.eval::("return fires").unwrap(), 2); +} + +#[test] +fn a_step_before_any_range_is_the_whole_call() { + // `0x789a6c test byte [+0x314],2` — bit1 clear ⇒ the step is stored and the function returns. + // No range is pushed, nothing is clamped, nothing fires. It is what keeps an `` that + // sets a step before a range from running a handler the addon has not armed (2095's subject, + // one verb over). + let s = script(); + s.run( + r#" + fires = 0 + n = CreateFrame("Slider", "SlNoRange") + n:SetScript("OnValueChanged", function() fires = fires + 1 end) + n:SetValueStep(0.25) + "#, + ) + .unwrap(); + assert_eq!( + s.eval::("return SlNoRange:GetValueStep()").unwrap(), + 0.25 + ); + assert_eq!(s.eval::("return fires").unwrap(), 0); + let (min, max): (f32, f32) = s.eval("return SlNoRange:GetMinMaxValues()").unwrap(); + assert_eq!((min, max), (0.0, 0.0), "no range was pushed"); +} diff --git a/crates/benilla-ui/src/script/tick.rs b/crates/benilla-ui/src/script/tick.rs index 71139ae4..54279851 100644 --- a/crates/benilla-ui/src/script/tick.rs +++ b/crates/benilla-ui/src/script/tick.rs @@ -147,6 +147,11 @@ impl super::UiScript { // OnUpdate sweep, matching the reference's order: the box's own OnUpdate override drains // before the FrameXML handlers that read the box get their turn. editbox::drain_text_changed(&self.lua); + // …and the caret flush's own fire, the second half of `0x77a790`'s update walk + // (`0x77d3e0` → `0x77da80`): `OnCursorChanged` when the caret has moved. It is what + // `ScrollingEdit_OnCursorChanged` records and `ScrollingEdit_OnUpdate` then scrolls by, + // so this is the mail body and the GM ticket box following the caret as you type. + editbox::drain_cursor_changed(&self.lua); // Events queued by Lua bindings last tick (`Model::pending_events` — e.g. `SetMapZoom` → // `WORLD_MAP_UPDATE`; the cursor arc's `CURSOR_UPDATE`/`ITEM_LOCK_CHANGED`/ // `DELETE_ITEM_CONFIRM`, decision 0216) fire first, so handlers see them before this diff --git a/crates/benilla-ui/src/script/tooltip/verbs.rs b/crates/benilla-ui/src/script/tooltip/verbs.rs index d42f1643..bb954eb0 100644 --- a/crates/benilla-ui/src/script/tooltip/verbs.rs +++ b/crates/benilla-ui/src/script/tooltip/verbs.rs @@ -55,7 +55,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "ANCHOR_BOTTOMLEFT" => Some((Point::TopRight, Point::BottomLeft)), "ANCHOR_NONE" | "ANCHOR_PRESERVE" => None, other => { - model.warnings.push(format!( + model.record_warning(format!( "SetOwner: unknown anchor '{other}' (kept ANCHOR_RIGHT)" )); Some((Point::BottomLeft, Point::TopRight)) diff --git a/crates/benilla-ui/src/widget/kinds/editbox.rs b/crates/benilla-ui/src/widget/kinds/editbox.rs index 037400ca..ca6f8e8a 100644 --- a/crates/benilla-ui/src/widget/kinds/editbox.rs +++ b/crates/benilla-ui/src/widget/kinds/editbox.rs @@ -186,6 +186,11 @@ pub struct EditBoxState { /// The row pitch in px (the snapped font em — the same `N·S` block law the host's measure /// uses), answered with [`Self::advances`]. `0.0` until answered. pub cell_h: f32, + /// The `(row, x)` the last `OnCursorChanged` was fired with — the caret-flush edge's memory + /// (`0x77da80`, whose one caller `0x77d475` is gated on dirty bit 2, so the fire is per + /// *change*, not per frame). `None` = never fired, which is what makes the first flush after + /// a focus fire even though the caret is at the home position. + pub cursor_fired: Option<(usize, f32)>, /// First visible byte of the display window (`E+0x348` display/scroll start index) — the /// char-granular h-scroll. Clamped each read so the cursor stays inside the window /// (`0x77da80`'s early-out hides the caret outside `[start, start+visible]`). @@ -310,6 +315,7 @@ impl Default for EditBoxState { advances_key: 0, rows: vec![0], cell_h: 0.0, + cursor_fired: None, scroll_start: 0, drag_active: false, blink_period: 0.5, diff --git a/crates/benilla-ui/src/widget/kinds/mod.rs b/crates/benilla-ui/src/widget/kinds/mod.rs index 31097332..c3877add 100644 --- a/crates/benilla-ui/src/widget/kinds/mod.rs +++ b/crates/benilla-ui/src/widget/kinds/mod.rs @@ -1029,21 +1029,19 @@ pub enum ButtonVisualState { /// the pointer at extract time. #[derive(Clone, Debug, PartialEq)] pub struct ButtonState { - /// `Enable`/`Disable`. A disabled button shows its DisabledTexture and fires no clicks. - pub enabled: bool, - /// The scripted PUSHED state — `SetButtonState("PUSHED"/"NORMAL") 0x780270` / - /// `GetButtonState 0x780180`, the keybind visual's engine half (ref `ActionButtonDown/Up`, - /// `ActionButton.lua:15-28`). ORs with the mouse-derived held+hovered press in - /// [`Self::input_state`]; the mouse press itself stays outside the widget (the app's capture), - /// which is why the state machine takes it as an argument. + /// **`[CSimpleButton+0x32c]` — the LOCK**, written unconditionally by every + /// `SetButtonState(state, locked) 0x779790` and read by exactly four guards, all of them the + /// engine's own edges. While it is set the button **ignores the mouse press/release + /// transitions entirely**; while it is clear, any engine edge that fires re-writes it to + /// `false`, because every engine call site passes `locked = 0` (wow-re + /// `system/ui/scratch/button-state-edge-set.md`, VERIFIED — including `Enable`/`Disable`, + /// which therefore CLEAR a lock a script set). /// - /// **The reference has one variable where we have two**, and the difference is stated rather - /// than implied: `SetButtonState(state, locked)` writes `[+0x328]` *and* the lock at `+0x32c`, - /// and an unlocked scripted push is therefore cleared by the next mouse press/release - /// (`0x7793c2`'s `SetState(NORMAL)`, gated on `locked == 0`). Ours keeps the flag until Lua - /// clears it. No 1.12 caller pushes without meaning it to stick: `ActionButtonDown` pairs - /// every push with its own `ActionButtonUp`, and the micro buttons pass `locked = 1`. - pub pushed_state: bool, + /// `MainMenuBarMicroButtons.lua` is the reference's own customer: `SetButtonState("PUSHED", 1)` + /// when a panel opens, `SetButtonState("NORMAL")` — the flag defaulting to 0 — when it closes. + /// The ordinary action bars pass no flag at all, and Tablet-2.0 pushes its clicked rows + /// unlocked and never un-pushes them, which is the whole reason this field exists (2134). + pub locked: bool, /// [`FrameKind::LootButton`]'s own field — `CLootButton +0x4dc`, the **0-based** loot slot /// this row takes when clicked. `None` until `SetSlot` writes it (the ctor's zero is a slot /// id in the reference; ours is `None`, so a row that was never given one takes nothing @@ -1146,10 +1144,17 @@ pub struct ButtonState { /// [`Self::set_state`] and [`Self::set_state_slot`] alone, which is what makes the transition /// rule un-bypassable. shown: Option, - /// The state [`Self::shown`] was last resolved for — the client's `[+0x328]`. Latched from the - /// three inputs ([`Self::enabled`], [`Self::pushed_state`] and the mouse's held+hovered) by - /// [`Self::settle`], so a *transition* can be detected at all; the client keeps the same one - /// variable for the same reason. + /// **`[CSimpleButton+0x328]` — THE state**, and the client really does have only this one + /// variable: `SetButtonState 0x779790` is its only writer past the ctor, and `IsEnabled + /// 0x7800b0`, `GetButtonState 0x780180` and every texture decision read it back. + /// + /// Private, because it is a **latch driven by edges**, not a value derived from inputs. That + /// distinction is the whole of decision 2134: benilla used to carry `enabled` and + /// `pushed_state` beside it and re-derive `state` from them plus the live mouse + /// (`(held && hovered) || pushed_state`), and the reference's press state **has no hover term + /// at all** — the enter and leave notifies (`0x779490`/`0x7794e0`) read `[+0x328]` only as a + /// DISABLED guard and never write it. A derived model un-presses when the cursor leaves a held + /// button and re-presses when it comes back; the reference does neither. state: ButtonVisualState, } @@ -1157,8 +1162,7 @@ impl Default for ButtonState { fn default() -> Self { ButtonState { loot_slot: None, - enabled: true, - pushed_state: false, + locked: false, checked: false, normal: None, pushed: None, @@ -1195,18 +1199,6 @@ impl ButtonState { } } - /// The state the three inputs put the button in — disabled wins, then the press (the mouse's - /// held+hovered, or the scripted [`Self::pushed_state`]), else resting. - fn input_state(&self, hovered: bool, held: bool) -> ButtonVisualState { - if !self.enabled { - ButtonVisualState::Disabled - } else if (held && hovered) || self.pushed_state { - ButtonVisualState::Pushed - } else { - ButtonVisualState::Normal - } - } - /// **`CSimpleButton::SetState 0x779790`** — the transition, and the whole of why the shown /// texture is state rather than a lookup. /// @@ -1251,16 +1243,111 @@ impl ButtonState { } } - /// Run the state machine over the current inputs — the caller's job at every point one of them - /// can have moved (`script::button::settle`, which reads the mouse's two off the model). + /// **`CSimpleButton::SetButtonState(state, locked) 0x779790` — the ONE writer**, and the door + /// every edge below goes through. /// - /// The client has no such call because it has no derived inputs: its mouse handlers call - /// `SetState` directly (`0x7791ed` enter, `0x7793f0` leave, `0x7792ad` down, `0x7793c2` up, - /// each gated on `locked == 0`) and so do `Enable`/`Disable`. Ours keeps the inputs as fields - /// and latches [`Self::state`] from them here; the transitions that result are the same ones, - /// in the same order. - pub fn settle(&mut self, hovered: bool, held: bool) { - self.set_state(self.input_state(hovered, held)); + /// `0x77979d mov [this+0x32c], locked` is **unconditional**, before and independent of the + /// state's own equality early-out at `0x7797a3` — so a `SetButtonState("NORMAL", 1)` on a + /// button that is already NORMAL still arms the lock, and every engine edge (all of which pass + /// `locked = 0`) disarms it even when the state does not move. + pub fn set_button_state(&mut self, new: ButtonVisualState, locked: bool) { + self.locked = locked; + self.set_state(new); + } + + /// `IsEnabled 0x7800b0` — which reads `[+0x328]` and nothing else, because there is no + /// separate enabled flag in the class. + pub fn enabled(&self) -> bool { + self.state != ButtonVisualState::Disabled + } + + /// `GetButtonState 0x780180` — the same one variable, un-derived. A button held down by the + /// mouse answers `"PUSHED"` because the press *wrote* it, not because a getter re-checks the + /// cursor (which is what ours used to do, and why it could disagree with the art). + pub fn button_state(&self) -> ButtonVisualState { + self.state + } + + // ── The engine's edges (wow-re `scratch/button-state-edge-set.md`, VERIFIED) ───────────── + // + // Seven call sites of `0x779790` exist image-wide; six are below and the seventh is the Lua + // binding ([`Self::set_button_state`]). Every one of the six passes `locked = 0`. There is + // **no enter edge and no leave edge** — `0x779490`/`0x7794e0` re-apply the label style and + // never touch `[+0x328]` — which is the correction 2134 is built on. + + /// **`SetEnabled(bool) 0x779160`** (vtable `+0x90`) — `Enable 0x77fef0` / `Disable 0x77ffd0`. + /// + /// Two asymmetries the byte read settles, both early-outs on the state it already has: + /// `Enable()` on a button that is not DISABLED does **nothing at all** (`0x779175`) — so it + /// does not un-push a PUSHED button and does not clear a lock; `Disable()` on an + /// already-DISABLED one likewise (`0x77919c`). When it does run it passes `locked = 0` + /// (`0x779179`/`0x7791c2`), so enabling or disabling a script-locked button **unlocks** it. + /// + /// Returns whether the transition ran — the caller owes the HIGHLIGHT draw-layer half only + /// then (`script::button::set_enabled`). + pub fn set_enabled(&mut self, on: bool) -> bool { + if on == self.enabled() { + return false; + } + self.set_button_state( + if on { + ButtonVisualState::Normal + } else { + ButtonVisualState::Disabled + }, + false, + ); + true + } + + /// **The hide edge** — `CSimpleButton`'s `+0x34` override `0x7791e0`, which un-presses and then + /// tail-jumps the base notify so the frame's `` still fires. Guard at `0x7791ed`: + /// `state != DISABLED && locked == 0`. + pub fn on_hide(&mut self) { + if self.state != ButtonVisualState::Disabled && !self.locked { + self.set_button_state(ButtonVisualState::Normal, false); + } + } + + /// **The press edge** — the mouse-down dispatcher `0x779210`'s `0x7792b1` + /// `SetButtonState(PUSHED, 0)`, guarded at `0x779295` on `locked == 0 && state != DISABLED`. + /// Unconditional past the registration and hit gates above it, which is why a right-click on a + /// slot that registered `RightButtonUp` lights up even when the click does nothing. + pub fn on_mouse_down(&mut self) { + if !self.locked && self.state != ButtonVisualState::Disabled { + self.set_button_state(ButtonVisualState::Pushed, false); + } + } + + /// **The release edge** — the mouse-up dispatcher `0x7792d0`'s `0x7793de` + /// `SetButtonState(NORMAL, 0)`, guarded at `0x7793c2` on `locked == 0` and at `0x779307` on the + /// state actually being PUSHED. + /// + /// **It does not ask where the cursor is.** A press held over a button and released anywhere + /// on screen un-presses it; the release is the edge, the hover is not. (The caller skips this + /// when the base finalized a drag — `0x7792df`'s `0x76c040` test — which is our + /// `cursor::take_drag(..).started`.) + /// + /// This is the edge that fixes Tablet-2.0: its rows call `SetButtonState("PUSHED")` with no + /// lock argument and never call `SetButtonState("NORMAL")` at all, because in the reference + /// this release does it for them. + pub fn on_mouse_up(&mut self) { + if !self.locked && self.state == ButtonVisualState::Pushed { + self.set_button_state(ButtonVisualState::Normal, false); + } + } + + /// **The drag-start edge** — `+0x74`'s override `0x7793f0`, `0x779410` + /// `SetButtonState(NORMAL, 0)` under the same `0x779400` guard, before forwarding to the base + /// `0x76c1e0` that fires ``. + /// + /// It fires on the drag THRESHOLD crossing (a link-time `0.01` frame units, `0x81c468`), not on + /// the rect edge, and only for a frame that called `RegisterForDrag` — so a button you drag off + /// un-presses well before the cursor leaves it, and one you merely slide off does not. + pub fn on_drag_start(&mut self) { + if !self.locked && self.state != ButtonVisualState::Disabled { + self.set_button_state(ButtonVisualState::Normal, false); + } } /// **`SetNormalTexture`/`SetPushedTexture`/`SetDisabledTexture 0x778fd0`** — the slot store, @@ -1308,10 +1395,10 @@ impl ButtonState { return hovered || self.locked_highlight; } if some == self.checked_tex { - return self.checked && (self.enabled || self.disabled_checked.is_none()); + return self.checked && (self.enabled() || self.disabled_checked.is_none()); } if some == self.disabled_checked { - return self.checked && !self.enabled; + return self.checked && !self.enabled(); } true } @@ -1382,11 +1469,13 @@ pub struct SliderState { /// Atlas's option sliders raised `attempt to index global 'AtlasOptions'` from their own /// ``, a handler the reference never runs at that point. pub has_value: bool, - /// `SetValueStep` (XML `valueStep`) — the step the arrow keys / step buttons move by. Stored and - /// returned. **Known divergence:** the client's `SetValue` (`0x789930`) round-half quantises the - /// value by this step before the change compare (wow-re `item9-firing34-merge.md`, `ui.md` - /// "clamped/step-quantized"); ours stores the raw value. Not yet reconciled — every scrollbar - /// rides this path, so it is its own change with its own falsifier. + /// `SetValueStep` (XML `valueStep`) — the client's `[+0x324]`, and **the quantiser every + /// stored value passes through**, not merely the arrow keys' stride: `SetValue 0x789930` + /// rounds the clamped value onto the `min + n·step` lattice before the change compare, so + /// `GetValue` on a stepped slider can only ever answer a lattice point ([`slider_set_value`], + /// which is the whole kernel transcribed). A `step` of exactly `0.0` — the ctor default, and + /// what every scrollbar template leaves it at — skips the quantiser outright, which is the + /// gate that keeps `UIPanelScrollBarTemplate` continuous. pub step: f32, /// `true` = VERTICAL (the ctor default; value maps along the track's height, min at the top), /// `false` = HORIZONTAL (`orientation`; shared enum `0x811b00` HORIZONTAL=0/VERTICAL=1). @@ -1473,6 +1562,80 @@ pub fn slider_fraction( (travel > 0.0).then(|| ((cursor - grab) / travel).clamp(0.0, 1.0)) } +/// **`CSimpleSlider::SetValue`'s arithmetic kernel — `0x789930`, clamp then quantise.** +/// +/// The value a `SetValue(v)` actually stores, given the widget's range fields as the client holds +/// them: `min` = `[+0x318]`, `span` = `[+0x31c]` (the client keeps the *span*, not the max — the +/// range setter `0x7898f0` stores `end − start` and `0x789a60` re-derives `max = span + min`), and +/// `step` = `[+0x324]`. +/// +/// Transcribed op-for-op from wow-re's bit-exact primitive `glue_789930_set_value` +/// (`crates/ui/src/glue_geom_789.rs`, difftest `glue_geom_789.rs::glue_789930_set_value`, +/// ledger `PRIMITIVE:glue_789930_set_value`, VERIFIED), whose own doc reads the emitted jccs: +/// +/// 1. **clamp-low** (`0x789947`..`0x78995f`) — `fld min; fcomp v; test ah,0x41; jne`: `max(min, v)`, +/// with an unordered compare (NaN) taking the `v` arm. +/// 2. **`bound = span + min`** (`0x789962`/`0x789968`), spilled to the f32 slot `[ebp-4]`. +/// 3. **clamp-high** (`0x78996e`..`0x78999c`) — `hi = bound` *reloaded as f32* iff `bound < lo` +/// ordered-strict; otherwise `hi` is the clamp-low again. Net: `clamp(v, min, bound)`, except +/// that a **degenerate range pins to `bound`, not to `min`** — which is the one place this +/// replaced an assumption, our old `v.clamp(min, max.max(min))` having pinned the other way. +/// 4. **quantise** (`0x78999c`..`0x789a06`), skipped entirely when `step == 0.0`: +/// `diff = hi − min`, `halfstep = step · 0.5`, then **round-half-away-from-zero** — +/// `(diff + halfstep) / step` when `diff > 0` ordered, `(diff − halfstep) / step` otherwise +/// (`≤ 0` and NaN both take the round-down arm) — truncated toward zero by `__ftol`, and +/// `hi = n · step + min`. +/// +/// Two consequences worth stating because they look like bugs and are not: +/// +/// * **The quantised value is NOT re-clamped.** With a range that is not a whole number of steps, +/// the top of the travel rounds *past* `max` by up to half a step. Every stepped slider the +/// reference ships is an exact multiple (`FauxScrollFrame_Update` builds its range as +/// `(numItems − numToDisplay) · valueStep`; the option sliders are all 10/20/100 steps), so the +/// overshoot is unreachable there — but an addon's odd range really does produce it. +/// * **The lattice is anchored at `min`, not at zero**, so a slider over `[0.25, 1.0]` stepping by +/// `0.1` settles on `0.25, 0.35, …`, never on `0.3`. +/// +/// The f64 intermediates are the x87 register stack at PC_53, and the two `as f32` narrowings are +/// its two `fst`/`fstp` spills — not an approximation of them. +pub fn slider_set_value(v: f32, min: f32, span: f32, step: f32) -> f32 { + let minv = f64::from(min); + let vv = f64::from(v); + + // clamp-low: `fcomp(min, v)` falls through to `min` only on `min > v` ordered-strict. + let clamp_low = |v: f64| { + if matches!(minv.partial_cmp(&v), Some(core::cmp::Ordering::Greater)) { + minv + } else { + v + } + }; + let lo = clamp_low(vv); + + // `bound = span + min`, live, and spilled to f32 at `[ebp-4]`. + let bound = f64::from(span) + minv; + let mut hi = if matches!(bound.partial_cmp(&lo), Some(core::cmp::Ordering::Less)) { + f64::from(bound as f32) // `fld [ebp-4]` — the f32 reload, not the live value + } else { + clamp_low(vv) + }; + + // The step test is `fcomp(step, 0.0)`: exactly zero (either sign) skips the quantiser. + if f64::from(step) != 0.0 { + let stepv = f64::from(step); + let diff = hi - minv; + let halfstep = stepv * 0.5; + let scaled = if diff > 0.0 { + (halfstep + diff) / stepv + } else { + (diff - halfstep) / stepv + }; + let n = scaled.trunc() as i32; // `__ftol`, truncate toward zero + hi = (f64::from(n) * stepv) + minv; + } + hi as f32 +} + impl SliderState { /// The value fraction `(value − min) / (max − min)`, clamped to `[0, 1]`; a degenerate range /// (`max <= min`, an unscrollable slider) is `0.0` — the thumb sits at the track's start. @@ -1484,26 +1647,71 @@ impl SliderState { } } - /// `SetValue` (`0x789930`): clamp `v` into the live range and store it; returns `Some(value)` - /// when the caller must fire `OnValueChanged`. The fire law is the client's, emitted-jcc read - /// in wow-re `slider-mouse-law.md` §6: **no range yet → a complete no-op**; **first-ever value → - /// always store and fire**; after that, **fire iff the clamped value differs** from the stored - /// one. The change-gate is load-bearing, not an optimisation: the reference scrollbar wires - /// `OnValueChanged → SetVerticalScroll → scrollbar:SetValue` back on itself, and the gate is - /// what stops that after one hop. A degenerate range (`max <= min`) pins to `min`. + /// `SetValue` (`0x789930`): clamp **and quantise** `v` ([`slider_set_value`]) and store it; + /// returns `Some(value)` when the caller must fire `OnValueChanged`. The fire law is the + /// client's, emitted-jcc read in wow-re `slider-mouse-law.md` §6: **no range yet → a complete + /// no-op**; **first-ever value → always store and fire**; after that, **fire iff the + /// clamped/step-quantised value differs** from the stored one. The change-gate is load-bearing, + /// not an optimisation: the reference scrollbar wires `OnValueChanged → SetVerticalScroll → + /// scrollbar:SetValue` back on itself, and the gate is what stops that after one hop. + /// + /// **The quantiser is inside the gate, not after it**, and that is what makes a stepped + /// slider's drag cheap as well as faithful: a thumb moved a pixel within one step's band + /// resolves to the same lattice point and fires nothing at all, exactly as the reference's + /// `fcom` against `[+0x320]` decides. + /// + /// The client holds the range as `min` + `span`; we hold `min` + `max`, so the span is derived + /// here the way `0x7898f0` computes it (`end − start`, at f64 and narrowed). That keeps + /// `GetMinMaxValues` answering the exact pair that was set — the reference's own `max` is + /// re-derived as `span + min` and can differ from it in the last ulp — at the cost of the + /// clamp bound being that same re-derivation. A degenerate range (`max < min`) pins to the + /// bound, which is the client's arm, not to `min`. pub fn store_value(&mut self, v: f32) -> Option { if !self.range_valid { return None; } - let clamped = v.clamp(self.min, self.max.max(self.min)); + let span = (f64::from(self.max) - f64::from(self.min)) as f32; + let settled = slider_set_value(v, self.min, span, self.step); let first = !self.has_value; self.has_value = true; - (first || clamped != self.value).then(|| { - self.value = clamped; - clamped + (first || settled != self.value).then(|| { + self.value = settled; + settled }) } + /// **`SetValueStep` (`0x789a60`) — which is not a field write** (decision 2143). + /// + /// The byte read, VERIFIED (wow-re `slider-mouse-law.md` §10, a scoped round dispatched from + /// this repo, cross-derived twice and pinned by an executed probe on the binary's own bytes): + /// + /// 1. `0x789a66` stores the step — **unconditional**. + /// 2. `0x789a6c test byte [+0x314],2` — **no range yet ⇒ return, and nothing else happens.** + /// 3. Otherwise `0x789a91 call 0x7898f0(min, f32(span + min))` — [`Self::set_min_max`] with the + /// max **re-derived**, which re-clamps the held value through `SetValue` when one exists, + /// re-quantising it onto the NEW lattice and firing `OnValueChanged` if it moved. + /// + /// So a step handed to a slider that already holds a value can move that value and fire — and, + /// because `span′ = f32(f32(span + min) − min)` is **not the identity**, it can also change the + /// range (`min = 1.0`, `span = 1e-8` collapses it to zero). Ours stored the step and stopped, + /// which diverged on all three counts and on the next `GetMinMaxValues` besides. + /// + /// Inert for every caller the reference ships, and **not for the reason decision 2133 gave** + /// (that they set the range and the step before any value exists — true only of each site's + /// first run; `FauxScrollFrame_Update` re-runs on a live bar from fourteen update functions and + /// never calls `SetValue` at all). It is inert because the only writer of the value is + /// `SetValue`, which quantises on the way in, so the value is always already on the lattice + /// being re-imposed. + pub fn set_value_step(&mut self, step: f32) -> Option { + self.step = step; + if !self.range_valid { + return None; + } + let span = (f64::from(self.max) - f64::from(self.min)) as f32; + let max = (f64::from(span) + f64::from(self.min)) as f32; + self.set_min_max(self.min, max) + } + /// `SetMinMaxValues` (`0x7898f0`): set the range, mark it valid, and re-clamp the held value /// **only if one exists** (bit2) — a fresh slider's zero-init value is not a value, so a range /// that excludes zero fires nothing. Returns what [`Self::store_value`] would for the re-clamp. diff --git a/crates/benilla-ui/src/widget/mod.rs b/crates/benilla-ui/src/widget/mod.rs index 9216aa6a..71c7e17f 100644 --- a/crates/benilla-ui/src/widget/mod.rs +++ b/crates/benilla-ui/src/widget/mod.rs @@ -173,10 +173,10 @@ impl Arena { mod kinds; pub use kinds::{ - model_key, slider_fraction, slider_grab, ArmedSequence, ButtonFont, ButtonState, - ButtonVisualState, ColorSelectState, EditAction, EditBoxState, EditOutcome, EditUnit, - FrameKind, InsertMode, KindState, MessageFrameState, MessageLine, MinimapState, ModelFileFacts, - ModelFog, ModelLight, ModelPlayHead, ModelState, RegionKind, ScrollFrameState, + model_key, slider_fraction, slider_grab, slider_set_value, ArmedSequence, ButtonFont, + ButtonState, ButtonVisualState, ColorSelectState, EditAction, EditBoxState, EditOutcome, + EditUnit, FrameKind, InsertMode, KindState, MessageFrameState, MessageLine, MinimapState, + ModelFileFacts, ModelFog, ModelLight, ModelPlayHead, ModelState, RegionKind, ScrollFrameState, ScrollingMessageState, SequenceFacts, SliderState, StatusBarState, TooltipState, MINIMAP_DEFAULT_ARROW_MODEL, MINIMAP_DEFAULT_MASK, MINIMAP_DEFAULT_PLAYER_MODEL, MINIMAP_DEFAULT_ZOOM, MINIMAP_ENGINE_CHILDREN, MINIMAP_ZOOM_LEVELS, TOOLTIP_DOUBLE_GAP,