From 8f658cb5169ffe8cf88fd009414e651dbd947c62 Mon Sep 17 00:00:00 2001 From: Sam Sung <51686767+samwhosung@users.noreply.github.com> Date: Mon, 19 Oct 2026 13:31:11 +0700 Subject: [PATCH] Key-binding confirm dialogs no longer raise a Lua error each frame (#1972) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unticking Character Specific Key Bindings, or pressing Defaults, raised `StaticPopup.lua:1713: attempt to compare number with nil` every frame the confirm stayed up. 1.12.1: `StaticPopup_Show` copies an entry's `timeout` into `dialog.timeleft`, and `StaticPopup_OnUpdate` compares it with 0 each frame; every stock entry declares one, the stock key-binding confirm included (`Blizzard_BindingUI.lua:10-24`, `timeout = 0`, `whileDead = 1`, `showAlert = 1`). Our key-bindings page's two confirms declared none. Both now do, the character-set one with the stock entry's three fields. The checkbox test now runs a frame with the confirm up, and a new test fails on any dialog the loaded interface defines without a numeric timeout; both fail without the fix. Fixes #1971 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 881152bda6d48cc3fd1a47679a7465add212ae05) --- .../benilla-app/assets/ui/KeyBindingsPage.xml | 7 ++++++ .../src/ui_script/keybindings_tests.rs | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/crates/benilla-app/assets/ui/KeyBindingsPage.xml b/crates/benilla-app/assets/ui/KeyBindingsPage.xml index a89fa332..92297b23 100644 --- a/crates/benilla-app/assets/ui/KeyBindingsPage.xml +++ b/crates/benilla-app/assets/ui/KeyBindingsPage.xml @@ -408,6 +408,11 @@ KeyBindings_Output("") KeyBindingsPage_Refresh() end, + -- The stock entry's own (`Blizzard_BindingUI.lua:21-23`): `StaticPopup_OnUpdate` + -- compares the timeout with 0 every frame (`StaticPopup.lua:1713`). + timeout = 0, + whileDead = 1, + showAlert = 1, } -- Reset To Default confirms, as the era's does; our string, since 1.12 has none. StaticPopupDialogs["CONFIRM_RESET_KEYBINDINGS"] = { @@ -422,6 +427,8 @@ KeyBindings_Output("") KeyBindingsPage_Refresh() end, + timeout = 0, + whileDead = 1, } -- ── Search ── a binding's tag is its display name uppercased, as the era's. diff --git a/crates/benilla-app/src/ui_script/keybindings_tests.rs b/crates/benilla-app/src/ui_script/keybindings_tests.rs index ab529494..c1c2ae23 100644 --- a/crates/benilla-app/src/ui_script/keybindings_tests.rs +++ b/crates/benilla-app/src/ui_script/keybindings_tests.rs @@ -336,6 +336,9 @@ fn the_esc_ladder_closes_the_window_and_the_checkbox_switches_sets() { .unwrap(), "the box springs back until the popup decides" ); + // A frame with the popup up runs `StaticPopup_OnUpdate`, which compares its timeout. + s.tick(0.1); + assert!(s.errors().is_empty(), "{:?}", s.errors()); s.run("StaticPopup1Button2:Click()").unwrap(); assert_eq!(s.current_binding_set(), 2); assert!(s.character_bindings_exist()); @@ -352,6 +355,25 @@ fn the_esc_ladder_closes_the_window_and_the_checkbox_switches_sets() { assert!(s.errors().is_empty(), "{:?}", s.errors()); } +/// `StaticPopup_Show` copies an entry's `timeout` into `timeleft`, and `StaticPopup_OnUpdate` +/// compares it with 0 every frame (`StaticPopup.lua:1713`), so every entry declares one, as each +/// stock entry does. +#[test] +fn every_dialog_declares_a_timeout() { + benilla_formats::wow_data_or_skip!(); + let s = harness(); + let missing = s + .eval::( + "local out = {} \ + for which, d in pairs(StaticPopupDialogs) do \ + if type(d.timeout) ~= 'number' then table.insert(out, which) end \ + end \ + table.sort(out) return table.concat(out, ' ')", + ) + .unwrap(); + assert_eq!(missing, "", "dialogs with no numeric timeout"); +} + #[test] fn search_surfaces_bindings_as_live_rows_under_the_redirect_head() { benilla_formats::wow_data_or_skip!();