UI focus management, virtual cursor, and theming for BGE.UI - #184
Merged
Conversation
…collisions Two changes: 1. Pin `brighterscript` to exact "1.0.0-alpha.52" (no caret) in every package.json - the engine and every example/script project - instead of the floating "^1.0.0-alpha.50" range. That range already happened to resolve to alpha.52 today (confirmed via node_modules), but as a floating prerelease range it could silently drift to a newer alpha later. #175's bslint+rooibos-roku interaction bug is already known to be sensitive to exact alpha generation, so pin it everywhere rather than rely on install-time luck. 2. Fixes #167: every example's bsconfig.json (and the exampleTemplate scaffold used by `create-example`) copied the engine's entire src/ tree - both plain source and SceneGraph component code-behind files - into one shared destination folder. Component files declare same-named subs (redraw, init, onRenderComplete, etc.) that BrighterScript treats as global, so flattening them together caused ~33 duplicate-function errors the moment any example needed the engine's Shapes components (examples/scenegraph was the only example that already split them correctly). Fix: split each example's file copy into separate "source" and "components" destinations, matching examples/scenegraph's existing (correct) config - same pattern, just applied everywhere. controller's prior components-exclusion workaround and special controller-web copy are no longer needed (the split copy already lands controller-web at the right destination) and have been removed. Verified this needed a real device, not just bsc --validate: a first attempt at also nesting each dest under roku_modules/<packagename> (mirroring exactly what a real `ropm install` produces, confirmed by packing and installing the engine into a scratch consumer) validated fine via bsc/npm run check:all but failed to compile on a real Roku with "Install Failure: Compilation Failed. ShapeRenderTask" - a SceneGraph Task node quirk invisible to any static check. Reverted to the flatter destinations (still fully separating source from components, still fixing the collision) and confirmed both examples/scenegraph and examples/controller sideload and run cleanly on the same real device. Verification: - npm run check:all (lint, validate, headless tests, validate every example) passes clean. - npm run build-examples packages all 19 examples with no errors. - Sideloaded examples/scenegraph and examples/controller to a real Roku: both install and run correctly (controller's QR code, ship, and controller-web server all confirmed on screen). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… file (works around known bslint+rooibos-roku new-file bug)
…consumption Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n resolve a container's theme override Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…'s signature Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…and-rolled focusIndex
Game.bs dispatches two events per press frame (the press, then a synthesized 1000+code held event while the button stays down), and UiContainer.onInput() decided setInputEntity/unsetInputEntity independently for each - so the unconsumed held event immediately undid the press's capture, defeating same-frame consumption entirely. The same cause walked the cursor off a focused Slider/Select on every held frame of a Left/Right tap. - Slider/Select onInput() now accept held as well as press, so holding a direction keeps adjusting/cycling and the synthesized held event is consumed rather than leaking. - UiContainer gains a per-frame consumedThisFrame latch (reset at the top of onUpdate(), which Game.processUiUpdate() runs once per frame after all onInput dispatch); capture and cursor-movement suppression both read the latch instead of this one event's consumed flag. - The OK-release branch now consumes too, matching OK-press. - Dropped the always-true `m.currentlyFocused.onInput <> invalid` check (a bound method reference is never invalid). - Restored onInput forwarding to children that are themselves UiContainers (a nested container is never focusable, so focus-only dispatch never reached it) - the modal-dialog case the design cites. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…h hover - Game.Play()'s stale-inputEntityId check skipped getEntityByID() for gameUi/debugUi, which are never registered via addEntity() and so logged a warning every frame a UI container held input capture. - Checkbox defaults width/height to 20 (matching draw()'s own boxSize fallback) so a Checkbox left unsized is still hit-testable - UiWidget.containsPoint() requires both to be > 0. - Theme.hoveredBackgroundColor defaults to Silver instead of Gray, so Button/Checkbox/Select's hover state is visible out of the box. - Theme's doc comment now describes resolution accurately (per-draw(), immediate parent's effectiveTheme() only, no tree walk), notes that font/fontSize are stored but not yet applied by any widget, and records why Theme lives in Style.bs (issue #178 toolchain bug). - Removed Button.activateCallCount, test-only instrumentation that was shipping as public API; the spec counts calls on a test-local subclass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-stick UI capture Three regressions introduced by the previous final-review fix wave. 1. Slider/Select acted on BOTH the press and the same-frame synthesized held event Game.bs dispatches alongside it, double-stepping every tap (a single Right on a 2-option Select looked like nothing happened). They still CONSUME both events - that's what keeps input capture and the cursor from drifting - but only ACT on a press, or on a held event whose heldTimeMs has crossed BGE.UI.WIDGET_REPEAT_DELAY_MS since the last step. 2. UiContainer.onInput()'s nested-container forward ran unconditionally, so one press could fire both the outer container's focused widget and a nested container's focused widget. Now skipped when input.consumed is already true. 3. Game.Play()'s top-of-frame staleness check skipped clearing capture entirely for gameUi/debugUi, making capture sticky across frames and able to starve controller input indefinitely. Extracted to clearStaleInputCapture(), which skips only the warning-producing getEntityByID() lookup for those two ids while still clearing, keeping capture scoped to the single frame it was set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…first FocusManager - onInput() reverts to a plain broadcast to every child; a separate UiWidget.handleInput(input) as boolean (Roku onKeyEvent-style) is the only thing that participates in the focus/consume chain, called on the currently-focused widget only. - Focus is global, not per-container: BGE.UI.FocusManager (Game.focusManager) owns the single focused-widget registry shared by every focusEnabled UiContainer, fixing double-handling when containers nest. GameInput.focusVisited guards against processing one event twice when it reaches nested containers. - FocusNavigationMode.list (default) does discrete next/previous focus movement like a typical d-pad menu, with an off-by-default wrapFocus flag; FocusNavigationMode.pointer (opt-in) keeps the old spatial cursor/hit-test behavior for games that actually want it. - debugUi.focusEnabled = false, so it's simple non-blocking broadcast only. - OK release no longer consumes input, matching the documented contract. - isValid() check between onMouseDown/onClick; seeding repositions the widget before reading its position; Game tracks isContainer() generally instead of special-casing gameUi/debugUi ids; theme-resolution deduped into UiWidget.resolveTheme(). Merged FocusManager into UiContainer.bs rather than its own file - a pre-existing file referencing a brand-new one trips a bslint cannot-find-name bug (confirmed reproducible with bslint alone, not just bslint+rooibos-roku).
- Game.dispatchOnInput() now always bypasses the currentInputEntityId capture gate for debugUi specifically, in both directions - a gameUi capture no longer blocks debugUi's onInput() the same frame. - FocusManager.unregister() now resyncs currentlyFocusedIndex when an earlier, unfocused widget is removed, not just the focused one. - FocusManager.update() is driven exactly once per event by Game.processFocusManagerInput(), not from UiContainer.onInput() during the recursive broadcast - removes the need for GameInput.focusVisited entirely rather than papering over the double-dispatch risk. - FocusManager's pointer-mode seed repositions the whole ancestor chain (repositionAncestorChain), not just the immediate parent. - examples/audio's AudioVolumeSlider now refreshes its label from handleInput() (where Slider actually mutates its value), not onInput() (plain broadcast, runs before the value changes). - Extracted the press/held repeat-throttle duplicated across Slider, Select, and FocusManager's list/pointer navigation into a shared BGE.UI.RepeatThrottle (added to the existing Style.bs, not a new file - see the bslint new-file bug note). - Button/Slider/Select/Checkbox no longer redraw their fill a second time just to layer the focus border underneath it - the border draws first and larger instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements UI focus management, virtual-cursor navigation, and theming for
BGE.UI(closes #133).GameInput.consume()lets a widget stop an input event from also reachingGameEntity.onInput()the same frame.Game.Play()now processesgameUi's input before entity input (was the reverse), which is what makes same-frame consumption possible at all — documented inCLAUDE.md.UiContainerowns a virtual cursor: directional input moves it, hit-testing drives hover/focus (cursor-primary — hovering a widget focuses it), and OK/raw input dispatches to the focused widget.BGE.UI.Theme(default colors/fonts/spacing) with aGame.defaultThemeand per-UiContaineroverride; any per-widget color field left unset resolves from there.Button,Checkbox,Select(inline-cycling), plus aSliderretrofit onto focus/theme.examples/audioretrofitted off its old hand-rolledfocusIndexpattern onto the new system — verified live on a real Roku device.Spec:
specs/2026-08-30-ui-focus-cursor-theme-design.mdPlan (15 tasks, executed via subagent-driven-development, several controller "Ruling:" annotations document real bugs found and fixed mid-execution):
specs/2026-08-30-ui-focus-cursor-theme-plan.mdNotable things found and fixed along the way
This shipped several real bugs beyond the original plan's own text — each is called out in the plan/ledger, summarized here:
UiContainer.draw()calledchild.draw()with no arguments, soUiWidget.draw(parent)'sparentparameter had always been dead code — every widget's theme-from-container-override was unreachable. Fixed at the root (child.draw(m)), which also required givingUiContainer.draw()itself a matchingparentsignature (it was previously 0-arg, which crashed on a nestedUiContainer— e.g.Game.enableStandardDebugUi()'s panels — the moment the fix above started passing an argument).UiContainer.onInput(): cursor movement happened before dispatching to the focused widget, so a widget consuming Left/Right for its own purpose (e.g.Slideradjusting) never got the chance before the cursor had already moved away. Fixed by hit-testing first, dispatching next, moving the cursor only if not consumed.Game.bsdispatches a press and a synthesized held event togameUiin the same frame; a widget consuming the press but not reacting to the unhandled held event released capture before entities were dispatched, defeating the feature's central guarantee. Fixed with a per-frameconsumedThisFramelatch plus makingSlider/Selectreact to (and throttle-repeat on) held input too.UiContainerchildren weren't receivingonInputat all (breaks the modal-dialog case the design cites),setInputEntity(gameUi)spammed a warning log every consuming frame,Checkboxwas unfocusable when onlyheightwas set, andTheme.hoveredBackgroundColordefaulted equal tobackgroundColor(invisible hover feedback).Themelives inStyle.bsrather than its ownTheme.bs— documented inline.Follow-ups filed rather than folded into this PR: #179 (text input widget), #180 (9-patch/image backgrounds), #181 (popup
Selectlist), #182 (analog-stick cursor movement), #183 (Button/Checkboxstill lack their own on-device demo surface — onlySelect/Sliderare exercised viaexamples/audio).Test plan
npm run check— lint, validate, headless tests: 933 passingnpm run check:all— every example project validatesexamples/audioverified live on a real Roku device viarokubot: cursor seeding, row cycling with live label updates, focus moving to the volume slider (confirmed via the cursor visually disappearing into the slider's white fill), single-tap = single-step adjustment (no double-step), held-repeat at the throttled rate, OK play/pause +onAudioEvent, Options/Rewind/Back, and a clean Back-triggered quit🤖 Generated with Claude Code