feat!: event queue-based keyboard input handling #51
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
mupen64/ugui!51
Loading…
Reference in a new issue
No description provided.
Delete branch "better-key-handling"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
This PR improves keyboard input handling by leveraging two mechanisms:
Event-based input
Keyboard inputs are no longer passed in as temporally discrete maps, but as lists of keyboard events between frames.
This allows correct handling of multiple key interactions between frames, greatly improving interactivity.
Detailed key events
Keyboard events now contain information about the scancode, press/release state, repeat state, and the produced text.
Breaking Changes
Removed
Environment::held_keysThe mechanism for passing keyboard info to ugui via
held_keyshas been removed.Provide a list of key events instead.
If you're using the Mupen64 Lua API, this is how your code would change:
Pull request overview
This PR migrates ugui keyboard input from a per-frame “held keys” map to an event-list (
key_events) model, enabling more accurate multi-key interactions between frames and richer key event data.Changes:
held_keysplumbing withkey_eventsacross demos and tests.environment.key_events.ugui.keycodes(Virtual-Key code enum) and documentedUguiKeyEventArgs.Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
src/ugui/types.luaUguiKeyEventArgs,Environment.key_events, andugui.keycodesVK enum.src/ugui/internal.luasrc/ugui/controls/textbox.luakey_events.src/ugui/controls/numberbox.luakey_events.src/ugui/controls/listbox.luakey_events.demos/base.luaemu.atkeyand feedskey_eventsintougui.begin_frame.demos/breitbandgraphics_drawtext_fit.luakey_eventsand VK codes.test/core.luaheld_keyswith emptykey_eventsin frame setup.test/button.luaheld_keyswith emptykey_events.test/toggle_button.luaheld_keyswith emptykey_events.test/carrousel_button.luaheld_keyswith emptykey_events.test/combobox.luaheld_keyswith emptykey_events.test/joystick.luaheld_keyswith emptykey_events.test/listbox.luakey_events+ugui.keycodes.test/spinner.luaheld_keyswith emptykey_eventsand trims trailing whitespace.test/tabcontrol.luaheld_keyswith emptykey_events.test/textbox.luaheld_keyswithkey_eventsand introduces key event stubs in tests.test/tooltip.luaheld_keyswith emptykey_eventsand minor formatting change.💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@ -54,19 +54,21 @@ ugui.registry.listbox = {-- Keyboard-based selection. FIXME: Why is this based on the mouse being inside it???key_eventsis an ordered list of events, but this code iterates withpairs(), which doesn't guarantee order for array-like tables. Alsopairs(nil)will error ifenvironment.key_eventsisn't supplied. Preferipairs(ugui.internal.environment.key_events or {})in both loops.@ -63,27 +63,28 @@ ugui.registry.numberbox = {key_eventsis a list where order matters (e.g., arrow + digit in same frame), but this loop usespairs(), which doesn't guarantee iteration order. Alsopairs(nil)errors ifenvironment.key_eventsisn't provided. Preferipairs(ugui.internal.environment.key_events or {})here.@ -105,7 +106,7 @@ ugui.registry.numberbox = {return {primary = data.value,The keyboard interaction behavior added/changed here (arrow navigation, incrementing digits, typing digits via
e.text) currently has no automated test coverage intest/numberbox.lua(it contains only a TODO). Adding at least a basic test that feedskey_eventsand asserts caret/value updates would help prevent regressions.@ -44,52 +44,57 @@ ugui.registry.textbox = {VK_BACKhandling deletes the last character (data.text:sub(1, -2)) and moves the caret to the end, regardless ofcaret_index. This is a behavioral regression vs deleting the character before the caret (and should be a no-op at caret_index == 1). Consider using the existing string helpers (e.g., remove_at/remove_range) based ondata.caret_indexand updating selection/caret consistently.has_selectionis computed once before iterating events, but the loop mutatesselection_start/selection_end. This can cause subsequent events in the same frame to treat the textbox as still having a selection (or not) incorrectly. Recomputehas_selectionper event (or after selection mutations) to keep behavior consistent when multiple key events occur in one frame.key_eventsis a chronological list, but the code iterates it withpairs(), which does not guarantee order for array-like tables. Also, if callers omitenvironment.key_events,pairs(nil)will error. Prefer iteratingipairs(ugui.internal.environment.key_events or {})to preserve event ordering and makekey_eventseffectively optional/default-empty.@ -24,0 +25,4 @@---@field keycode UguiVKeycodes? The virtual keycode, if the event is a key event.---@field pressed boolean? Whether the key was pressed or released, if the event is a key event.---@field text string? The typed character, if the event is a char event and the key corresponds to a character.UguiKeyEventArgs(and thekey_eventsAPI) doesn't match the PR description: the description mentions scancode and repeat state, but the type only documentskeycode,pressed, andtext. Either update the type/docs to include the additional fields, or adjust the PR description so consumers know what data is actually available.These tests now pass
keycode = ctx.data.keywherectx.data.keyis still a string like'left'/'right', but the updated textbox logic comparese.keycodeagainstugui.keycodes.VK_LEFT/VK_RIGHTnumeric values. Update the test inputs/events to use the new keycode enum (and iterate events in order if needed) so the assertions actually exercise the new code path.@ -262,2 +309,3 @@{key = 'O',key_events = {{This event uses only
keycodewith values like'A'/'backspace', but the new textbox implementation inserts characters frome.textand handles backspace viaugui.keycodes.VK_BACK. To keep the test meaningful, emit{ text = 'A' }for character input and{ keycode = ugui.keycodes.VK_BACK, pressed = true }(etc.) for special keys, and updatectx.data.keyvalues accordingly.Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
end_frame() resets key_events (line 77), but ugui.begin_frame() deep-clones the environment, so any emu.atkey events that arrive during the frame won’t be seen by ugui.internal.environment.key_events and will then be dropped when key_events is cleared. To avoid losing input, swap buffers at the start of begin_frame() (capture current key_events into a local, set key_events = {} immediately, and pass the captured list into ugui.begin_frame) so mid-frame events are preserved for the next frame.
Mid-frame events do not exist.