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 <noreply@anthropic.com>
This commit is contained in:
Sam
2026-09-11 20:52:20 +07:00
co-authored by Claude Fable 5.1
parent 94945e8339
commit 5432a8bfbd
44 changed files with 1525 additions and 265 deletions
@@ -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);
@@ -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);
@@ -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();
@@ -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::<i64>("return TestRepaints").unwrap();
s.run("TestScrollScrollBar:SetValue(51)").unwrap();
assert_eq!(
s.eval::<f64>("return TestScrollScrollBar:GetValue()")
.unwrap(),
48.0,
"51px is not a lattice point; the bar stays on row 3's"
);
assert_eq!(
s.eval::<i64>("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::<i64>("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::<i64>("return TestRepaints").unwrap() > settled,
"the reference repaints on every scroll, changed row or not"
@@ -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::<String>(
"return OptionsFrameContainerBodyGraphicsRowFarclipControlValue:GetText()"
)
.unwrap(),
"350"
"357"
);
assert_eq!(
s.eval::<String>(
@@ -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::<f64>("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),
@@ -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::<Option<String>>("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<String> = (1..=13)
.filter_map(|i| {
script
.eval::<Option<String>>(&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::<Option<String>>("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::<crate::ui_chat::ChatLog>();
log_in_as(&mut world, "Onehunter", 1);
let script = world
.get_non_send_resource::<benilla_ui::script::UiScript>()
.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<String> = 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:#?}"
);
}