feat!: event queue-based keyboard input handling #51

Merged
abart27 merged 7 commits from better-key-handling into main 2026-03-01 15:55:16 +00:00
abart27 commented 2026-03-01 11:49:42 +00:00 (Migrated from github.com)

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_keys

The mechanism for passing keyboard info to ugui via held_keys has been removed.

Provide a list of key events instead.

If you're using the Mupen64 Lua API, this is how your code would change:

-emu.atdrawd2d(function()
-    local keys = input.get()
-    ugui.begin_frame({
-        held_keys = keys,
-        ...
-    })
-    ugui.end_frame()
-end)

+local key_events = {}
+
+emu.atdrawd2d(function()
+    ugui.begin_frame({
+        key_events = key_events,
+        ...
+    })
+    ugui.end_frame()
+    key_events = {}
+end)
+
+emu.atkey(function(args)
+    key_events[#key_events + 1] = args
+end)
# 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_keys` The mechanism for passing keyboard info to ugui via `held_keys` has been removed. Provide a list of key events instead. If you're using the Mupen64 Lua API, this is how your code would change: ```diff -emu.atdrawd2d(function() - local keys = input.get() - ugui.begin_frame({ - held_keys = keys, - ... - }) - ugui.end_frame() -end) +local key_events = {} + +emu.atdrawd2d(function() + ugui.begin_frame({ + key_events = key_events, + ... + }) + ugui.end_frame() + key_events = {} +end) + +emu.atkey(function(args) + key_events[#key_events + 1] = args +end) ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-03-01 14:34:24 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

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:

  • Replaced held_keys plumbing with key_events across demos and tests.
  • Updated textbox/numberbox/listbox keyboard handling to consume environment.key_events.
  • Introduced ugui.keycodes (Virtual-Key code enum) and documented UguiKeyEventArgs.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/ugui/types.lua Adds UguiKeyEventArgs, Environment.key_events, and ugui.keycodes VK enum.
src/ugui/internal.lua Removes old “just pressed keys” helpers and applies minor formatting changes.
src/ugui/controls/textbox.lua Switches textbox editing/navigation to process key_events.
src/ugui/controls/numberbox.lua Switches numberbox navigation/digit entry to process key_events.
src/ugui/controls/listbox.lua Switches listbox navigation/scrolling key handling to key_events.
demos/base.lua Implements event collection via emu.atkey and feeds key_events into ugui.begin_frame.
demos/breitbandgraphics_drawtext_fit.lua Updates input usage and F1 handling to use key_events and VK codes.
test/core.lua Replaces held_keys with empty key_events in frame setup.
test/button.lua Replaces held_keys with empty key_events.
test/toggle_button.lua Replaces held_keys with empty key_events.
test/carrousel_button.lua Replaces held_keys with empty key_events.
test/combobox.lua Replaces held_keys with empty key_events.
test/joystick.lua Replaces held_keys with empty key_events.
test/listbox.lua Updates navigation tests to use key_events + ugui.keycodes.
test/spinner.lua Replaces held_keys with empty key_events and trims trailing whitespace.
test/tabcontrol.lua Replaces held_keys with empty key_events.
test/textbox.lua Replaces held_keys with key_events and introduces key event stubs in tests.
test/tooltip.lua Replaces held_keys with empty key_events and minor formatting change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## 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:** - Replaced `held_keys` plumbing with `key_events` across demos and tests. - Updated textbox/numberbox/listbox keyboard handling to consume `environment.key_events`. - Introduced `ugui.keycodes` (Virtual-Key code enum) and documented `UguiKeyEventArgs`. ### Reviewed changes Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | `src/ugui/types.lua` | Adds `UguiKeyEventArgs`, `Environment.key_events`, and `ugui.keycodes` VK enum. | | `src/ugui/internal.lua` | Removes old “just pressed keys” helpers and applies minor formatting changes. | | `src/ugui/controls/textbox.lua` | Switches textbox editing/navigation to process `key_events`. | | `src/ugui/controls/numberbox.lua` | Switches numberbox navigation/digit entry to process `key_events`. | | `src/ugui/controls/listbox.lua` | Switches listbox navigation/scrolling key handling to `key_events`. | | `demos/base.lua` | Implements event collection via `emu.atkey` and feeds `key_events` into `ugui.begin_frame`. | | `demos/breitbandgraphics_drawtext_fit.lua` | Updates input usage and F1 handling to use `key_events` and VK codes. | | `test/core.lua` | Replaces `held_keys` with empty `key_events` in frame setup. | | `test/button.lua` | Replaces `held_keys` with empty `key_events`. | | `test/toggle_button.lua` | Replaces `held_keys` with empty `key_events`. | | `test/carrousel_button.lua` | Replaces `held_keys` with empty `key_events`. | | `test/combobox.lua` | Replaces `held_keys` with empty `key_events`. | | `test/joystick.lua` | Replaces `held_keys` with empty `key_events`. | | `test/listbox.lua` | Updates navigation tests to use `key_events` + `ugui.keycodes`. | | `test/spinner.lua` | Replaces `held_keys` with empty `key_events` and trims trailing whitespace. | | `test/tabcontrol.lua` | Replaces `held_keys` with empty `key_events`. | | `test/textbox.lua` | Replaces `held_keys` with `key_events` and introduces key event stubs in tests. | | `test/tooltip.lua` | Replaces `held_keys` with empty `key_events` and minor formatting change. | </details> --- 💡 <a href="/mupen64/ugui/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
@ -54,19 +54,21 @@ ugui.registry.listbox = {
-- Keyboard-based selection. FIXME: Why is this based on the mouse being inside it???
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:23 +00:00

key_events is an ordered list of events, but this code iterates with pairs(), which doesn't guarantee order for array-like tables. Also pairs(nil) will error if environment.key_events isn't supplied. Prefer ipairs(ugui.internal.environment.key_events or {}) in both loops.

            for _, e in ipairs(ugui.internal.environment.key_events or {}) do
`key_events` is an ordered list of events, but this code iterates with `pairs()`, which doesn't guarantee order for array-like tables. Also `pairs(nil)` will error if `environment.key_events` isn't supplied. Prefer `ipairs(ugui.internal.environment.key_events or {})` in both loops. ```suggestion for _, e in ipairs(ugui.internal.environment.key_events or {}) do ```
@ -63,27 +63,28 @@ ugui.registry.numberbox = {
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:22 +00:00

key_events is a list where order matters (e.g., arrow + digit in same frame), but this loop uses pairs(), which doesn't guarantee iteration order. Also pairs(nil) errors if environment.key_events isn't provided. Prefer ipairs(ugui.internal.environment.key_events or {}) here.

            for _, e in ipairs(ugui.internal.environment.key_events or {}) do
`key_events` is a list where order matters (e.g., arrow + digit in same frame), but this loop uses `pairs()`, which doesn't guarantee iteration order. Also `pairs(nil)` errors if `environment.key_events` isn't provided. Prefer `ipairs(ugui.internal.environment.key_events or {})` here. ```suggestion for _, e in ipairs(ugui.internal.environment.key_events or {}) do ```
@ -105,7 +106,7 @@ ugui.registry.numberbox = {
return {
primary = data.value,
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:22 +00:00

The keyboard interaction behavior added/changed here (arrow navigation, incrementing digits, typing digits via e.text) currently has no automated test coverage in test/numberbox.lua (it contains only a TODO). Adding at least a basic test that feeds key_events and asserts caret/value updates would help prevent regressions.

The keyboard interaction behavior added/changed here (arrow navigation, incrementing digits, typing digits via `e.text`) currently has no automated test coverage in `test/numberbox.lua` (it contains only a TODO). Adding at least a basic test that feeds `key_events` and asserts caret/value updates would help prevent regressions.
@ -44,52 +44,57 @@ ugui.registry.textbox = {
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:21 +00:00

VK_BACK handling deletes the last character (data.text:sub(1, -2)) and moves the caret to the end, regardless of caret_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 on data.caret_index and updating selection/caret consistently.

                            -- Delete the character before the caret, if any.
                            if data.caret_index > 1 then
                                local delete_index = data.caret_index - 1
                                data.text = ugui.internal.remove_at(data.text, delete_index)

                                data.caret_index = delete_index
                                data.selection_start = delete_index
                                data.selection_end = delete_index
                            end
`VK_BACK` handling deletes the last character (`data.text:sub(1, -2)`) and moves the caret to the end, regardless of `caret_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 on `data.caret_index` and updating selection/caret consistently. ```suggestion -- Delete the character before the caret, if any. if data.caret_index > 1 then local delete_index = data.caret_index - 1 data.text = ugui.internal.remove_at(data.text, delete_index) data.caret_index = delete_index data.selection_start = delete_index data.selection_end = delete_index end ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:23 +00:00

has_selection is computed once before iterating events, but the loop mutates selection_start/selection_end. This can cause subsequent events in the same frame to treat the textbox as still having a selection (or not) incorrectly. Recompute has_selection per event (or after selection mutations) to keep behavior consistent when multiple key events occur in one frame.

`has_selection` is computed once before iterating events, but the loop mutates `selection_start/selection_end`. This can cause subsequent events in the same frame to treat the textbox as still having a selection (or not) incorrectly. Recompute `has_selection` per event (or after selection mutations) to keep behavior consistent when multiple key events occur in one frame.
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:23 +00:00

key_events is a chronological list, but the code iterates it with pairs(), which does not guarantee order for array-like tables. Also, if callers omit environment.key_events, pairs(nil) will error. Prefer iterating ipairs(ugui.internal.environment.key_events or {}) to preserve event ordering and make key_events effectively optional/default-empty.

            for _, e in ipairs(ugui.internal.environment.key_events or {}) do
`key_events` is a chronological list, but the code iterates it with `pairs()`, which does not guarantee order for array-like tables. Also, if callers omit `environment.key_events`, `pairs(nil)` will error. Prefer iterating `ipairs(ugui.internal.environment.key_events or {})` to preserve event ordering and make `key_events` effectively optional/default-empty. ```suggestion for _, e in ipairs(ugui.internal.environment.key_events or {}) do ```
@ -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.
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:22 +00:00

UguiKeyEventArgs (and the key_events API) doesn't match the PR description: the description mentions scancode and repeat state, but the type only documents keycode, pressed, and text. Either update the type/docs to include the additional fields, or adjust the PR description so consumers know what data is actually available.

`UguiKeyEventArgs` (and the `key_events` API) doesn't match the PR description: the description mentions scancode and repeat state, but the type only documents `keycode`, `pressed`, and `text`. Either update the type/docs to include the additional fields, or adjust the PR description so consumers know what data is actually available.
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:23 +00:00

These tests now pass keycode = ctx.data.key where ctx.data.key is still a string like 'left'/'right', but the updated textbox logic compares e.keycode against ugui.keycodes.VK_LEFT/VK_RIGHT numeric 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.

These tests now pass `keycode = ctx.data.key` where `ctx.data.key` is still a string like `'left'`/`'right'`, but the updated textbox logic compares `e.keycode` against `ugui.keycodes.VK_LEFT/VK_RIGHT` numeric 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 = {
{
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:34:23 +00:00

This event uses only keycode with values like 'A'/'backspace', but the new textbox implementation inserts characters from e.text and handles backspace via ugui.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 update ctx.data.key values accordingly.

This event uses only `keycode` with values like `'A'`/`'backspace'`, but the new textbox implementation inserts characters from `e.text` and handles backspace via `ugui.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 update `ctx.data.key` values accordingly.
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-03-01 14:48:04 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

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.

## Pull request overview Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment. --- 💡 <a href="/mupen64/ugui/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-03-01 14:48:04 +00:00

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.

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.
abart27 (Migrated from github.com) reviewed 2026-03-01 14:49:16 +00:00
abart27 (Migrated from github.com) commented 2026-03-01 14:49:16 +00:00

Mid-frame events do not exist.

Mid-frame events do not exist.
Sign in to join this conversation.
No description provided.