mirror of
https://github.com/horizon-rust/horizon.git
synced 2026-11-07 11:06:11 -10:00
Controllers: icon buttons and two checkboxes take the focus, and a screen with no targets fails a test
An icon-only button recorded nothing, so the pad could not reach the pause menu's Report Bugs, Feedback and Player Reporting icons, or Realms' icons. An active button with an empty label is now a focus target. So are the Multiplayer warning's "Do not show again" box and Optimize World's Erase Cached Data box, which A now ticks. tools/pad_menu_e2e.py gains every_screen_has_targets, which walks the title's submenus and every options page and fails on a focus screen with no targets. Replay Studio is allowlisted because it registers nothing yet. New pause_icon and erase_checkbox cases press the new targets. The pause menu now has unlabelled widgets, so splitscreen_e2e's setup scenario reads labels with get. controller_settings_pad's check for a Navigation page now matches exactly: it tripped on the General page's Navigation Repeat rows. docs/controllers.md no longer says every rectangle registers itself or that both modes reach every screen. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
19f6c2c382
commit
e58ee9c714
6 files changed
+193
-21
No files matched your search
@@ -33,6 +33,7 @@ Feature → files → harness for the most-changed areas: [docs/architecture.md]
|
||||
- **No hardcoded user-facing strings.** Keep UI translation-ready from the start. In the launcher (`launcher/`), wrap visible text with `tr("...")` or `trf("... {arg} ...", &[("arg", &val)])` from `crate::i18n` instead of raw string literals or `format!`. In the game, use translation keys (`Text::tr`, `lang::get`), never hardcoded English strings.
|
||||
- **Claim before you start**: comment `/claim` on the issue (no issue yet: open one, then claim it). An assigned
|
||||
issue is taken: pick another. `/unclaim` when you stop; a claim with no open PR linking it is released after 5 days.
|
||||
- **Controllers reach what registers.** Draw controls with a registering helper (`ui::button`, `settings_shell::foot_button`); `graphics_style::button` and `screen_draw::button` only paint, and registering one part of a screen ends its crosshair, so register all of it.
|
||||
|
||||
## Crates
|
||||
|
||||
|
||||
@@ -614,7 +614,12 @@ fn button_with_room(
|
||||
room: f32,
|
||||
narration: impl FnOnce([f32; 4]) -> crate::widgets::Narratable,
|
||||
) {
|
||||
crate::widgets::record(&label.plain(), [x, y, w, h], active);
|
||||
let plain = label.plain();
|
||||
crate::widgets::record(&plain, [x, y, w, h], active);
|
||||
// An icon button has no label to record, and A presses it all the same.
|
||||
if active && plain.trim().is_empty() {
|
||||
crate::widgets::target([x, y, w, h]);
|
||||
}
|
||||
crate::widgets::narratable(|| narration([x, y, w, h]).hovered(hover));
|
||||
crate::widgets::widget_cursor(hover, active);
|
||||
let sprite = if !active {
|
||||
@@ -1846,6 +1851,43 @@ mod tests {
|
||||
assert_eq!(scrolled_left(10.0, 40.0, 1_000), 10.0 - offset);
|
||||
}
|
||||
|
||||
/// A controller reaches an icon button only through its target, and can't press a disabled one.
|
||||
#[test]
|
||||
fn an_active_icon_button_is_a_focus_target_and_a_disabled_one_is_not() {
|
||||
let atlas = atlas();
|
||||
let mut batch = HudBatch::new([640, 480]);
|
||||
// Whatever this thread drew before is the last frame's, then gone.
|
||||
crate::widgets::end_frame(u64::MAX - 1);
|
||||
button(
|
||||
&mut batch,
|
||||
&atlas,
|
||||
10.0,
|
||||
20.0,
|
||||
20.0,
|
||||
20.0,
|
||||
&Text::empty(),
|
||||
true,
|
||||
false,
|
||||
);
|
||||
button(
|
||||
&mut batch,
|
||||
&atlas,
|
||||
40.0,
|
||||
20.0,
|
||||
20.0,
|
||||
20.0,
|
||||
&Text::empty(),
|
||||
false,
|
||||
false,
|
||||
);
|
||||
crate::widgets::end_frame(u64::MAX);
|
||||
let targets: Vec<[f32; 4]> = crate::widgets::last_targets()
|
||||
.iter()
|
||||
.map(|t| t.rect)
|
||||
.collect();
|
||||
assert_eq!(targets, [[10.0, 20.0, 20.0, 20.0]]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_glyph_leaves_a_label_that_fits_centred() {
|
||||
let atlas = atlas();
|
||||
|
||||
@@ -2985,6 +2985,7 @@ Would you like to enable the Narrator or visit the Accessibility Settings?",
|
||||
let x = (w / 2.0).floor() - 155.0 + 80.0;
|
||||
let y = 76.0 + (lines.len() as f32 + 1.0) * 9.0;
|
||||
self.erase_check.set([x, y, width, BOX]);
|
||||
crate::widgets::target([x, y, width, BOX]);
|
||||
let hover = (x..x + width).contains(&cursor.0) && (y..y + BOX).contains(&cursor.1);
|
||||
let sprite = match (*erase_cache, hover) {
|
||||
(true, true) => "gui/sprites/widget/checkbox_selected_highlighted",
|
||||
@@ -3155,6 +3156,7 @@ Would you like to enable the Narrator or visit the Accessibility Settings?",
|
||||
let x = (w / 2.0).floor() - (width / 2.0).floor();
|
||||
let y = top + 57.0 + lines.len() as f32 * 9.0;
|
||||
self.warning_check.set([x, y, width, BOX]);
|
||||
crate::widgets::target([x, y, width, BOX]);
|
||||
let hover = (x..x + width).contains(&cursor.0) && (y..y + BOX).contains(&cursor.1);
|
||||
crate::widgets::narratable(|| {
|
||||
crate::widgets::Narratable::checkbox(
|
||||
@@ -3219,6 +3221,10 @@ Would you like to enable the Narrator or visit the Accessibility Settings?",
|
||||
}
|
||||
for b in self.buttons(w, h) {
|
||||
crate::widgets::record(&b.label, [b.x, b.y, b.w, 20.0], b.enabled);
|
||||
// Realms' icon buttons have no label to record.
|
||||
if b.enabled && b.label.trim().is_empty() {
|
||||
crate::widgets::target([b.x, b.y, b.w, 20.0]);
|
||||
}
|
||||
if let Some(action) = b.action.filter(|_| !self.preparing) {
|
||||
crate::widgets::declare_action(action, [b.x, b.y, b.w, 20.0], b.enabled);
|
||||
}
|
||||
|
||||
+18
-10
@@ -94,6 +94,13 @@ Draw the screen's Done, Back, Cancel or Apply with `ui::action_button` and the
|
||||
`widgets::Action` it performs, or call `widgets::declare_action` if the screen
|
||||
draws its own buttons. That is all: nothing else names the screen.
|
||||
|
||||
Every control also needs a focus target, or the pad can't reach it. `ui::button` and its kin register
|
||||
themselves, as does `settings_shell::foot_button`. `graphics_style::button` and `screen_draw::button` only
|
||||
paint, so anything drawn with them, or by hand, needs its own
|
||||
`widgets::record` or `widgets::target_as`. A focus screen with any target has no crosshair, so register all
|
||||
of it. The `every_screen_has_targets` case of `tools/pad_menu_e2e.py` fails on a focus screen that
|
||||
registers nothing, unless its allowlist names it.
|
||||
|
||||
| Action | Pad button | With the button disabled |
|
||||
|---|---|---|
|
||||
| Back, Cancel, Done (the first the screen has that can be pressed, in that order) | B | B falls back to Escape |
|
||||
@@ -140,18 +147,19 @@ that player's options and bindings.
|
||||
Both modes feed the screens the same pointer events a mouse sends: in focus
|
||||
mode the (hidden) pointer sits on the focused widget. So every screen keeps
|
||||
its mouse behaviour (drags, sliders, scrollbars, hover highlights) with no
|
||||
controller code of its own, and both modes reach every screen. Moving the
|
||||
real mouse hands control back at once.
|
||||
controller code of its own, but focus only lands on what a screen registers,
|
||||
and a focus screen with any target has no crosshair: a control with no target
|
||||
on it is out of the pad's reach. Moving the real mouse hands control back at once.
|
||||
|
||||
### Focus navigation
|
||||
|
||||
Every interactive rectangle a screen draws registers itself as a focus
|
||||
target with its kind (`widgets::target_as`): classic buttons, sliders, edit
|
||||
boxes and list rows (worlds, servers, languages, superflat layers and presets, biomes), icon buttons (the title's
|
||||
and pause menu's Friends button), inventory and container slots, creative and recipe-book
|
||||
A screen registers what it draws as focus targets with their kind
|
||||
(`widgets::target_as`): classic buttons, sliders, edit
|
||||
boxes and list rows (worlds, servers, languages, superflat layers and presets, biomes), icon buttons (the title's,
|
||||
the pause menu's and Realms'), inventory and container slots, creative and recipe-book
|
||||
tabs, and the stations' own buttons (trades, loom patterns, beacon effects,
|
||||
enchantments, stonecutter recipes, the recipe book's recipes, pages and
|
||||
filter). A screen that registers nothing falls back to the pointer.
|
||||
filter). Not every control registers yet (see "Adding a screen"). A screen that registers nothing falls back to the pointer.
|
||||
|
||||
The step itself is **vanilla's arrow-key navigation**
|
||||
(`ContainerEventHandler.nextFocusPathInDirection`, 26.3), applied to every
|
||||
@@ -307,10 +315,10 @@ Steam + X opens it at any time. "On-Screen Keyboard: Auto / Horizon / Steam"
|
||||
|
||||
### Screens covered
|
||||
|
||||
Both modes reach every screen through the pointer events. Focus mode
|
||||
Both modes send the same pointer events, but neither reaches everything. Focus mode
|
||||
registers targets on: the title menus (main, world list and its search, create
|
||||
and edit world, delete, multiplayer list and its edit screens), every options
|
||||
page (vanilla's and Horizon Settings with the pages it opens, key and
|
||||
page (vanilla's and Horizon Settings with the pages it opens except Replay Studio, key and
|
||||
controller binds, language list), pause and Play with Friends, death, chat, signs and books (their buttons), the
|
||||
survival and creative inventories, every container and station (chests,
|
||||
furnaces, brewing, crafter, crafting table with its recipe book, anvil,
|
||||
@@ -318,7 +326,7 @@ grindstone, smithing, stonecutter, loom, cartography, enchanting, beacon,
|
||||
villager and wandering-trader trades, mount inventories), and the LAN /
|
||||
world options. Where a screen draws something clickable no target names
|
||||
(the advancements map, the statistics lists, the pack screen's rows, the
|
||||
book's page text), the pad drives the pointer instead.
|
||||
book's page text), the pad drives the pointer instead, as it does on Replay Studio, which registers nothing yet.
|
||||
|
||||
## Glyphs
|
||||
|
||||
|
||||
+124
-9
@@ -30,6 +30,9 @@ Cases:
|
||||
- glyph_on_field: A on a focused text field and B on Cancel; A opens one keyboard, which hides them; B closes it.
|
||||
- settings_apply: X presses World Options' Apply once something changed.
|
||||
- long_label: a label too wide for its button slides, beside its glyph.
|
||||
- every_screen_has_targets: the title's submenus and every options page give the focus something to land on.
|
||||
- pause_icon: the pause menu's icons are targets, and A presses one.
|
||||
- erase_checkbox: the D-pad reaches Optimize World's Erase Cached Data box, and A ticks it.
|
||||
Needs built fast binaries (`cargo build --profile fast -p horizon-client -p horizon-client-lib --bins`)
|
||||
and prepared resources in .local/horizon-client/26.3. Exit status 0 when every case passes.
|
||||
"""
|
||||
@@ -362,6 +365,17 @@ def lock_question(args, name):
|
||||
return check.failures
|
||||
|
||||
|
||||
def open_optimize_confirm(game):
|
||||
"""From the title, with one saved world, to the screen with the Erase Cached Data box."""
|
||||
game.pad('south')
|
||||
game.pad('south')
|
||||
game.wait_targets()
|
||||
game.pad('down')
|
||||
click_widget(game, game.rows()[0])
|
||||
click_widget(game, next(w for w in game.widgets() if w.get('label') == 'Edit'))
|
||||
click_widget(game, next(w for w in game.widgets() if w.get('label') == 'Optimize World'))
|
||||
|
||||
|
||||
def optimize_world(args, name):
|
||||
game = Game(args, name, world=True)
|
||||
check = Check()
|
||||
@@ -375,13 +389,7 @@ def optimize_world(args, name):
|
||||
source = max((game.game_home / 'saves' / 'Menus').rglob('region/*.mca'), key=lambda f: f.stat().st_size)
|
||||
for i in range(400):
|
||||
shutil.copy(source, source.parent / f'r.{100 + i}.100.mca')
|
||||
game.pad('south')
|
||||
game.pad('south')
|
||||
game.wait_targets()
|
||||
game.pad('down')
|
||||
click_widget(game, game.rows()[0])
|
||||
click_widget(game, next(w for w in game.widgets() if w.get('label') == 'Edit'))
|
||||
click_widget(game, next(w for w in game.widgets() if w.get('label') == 'Optimize World'))
|
||||
open_optimize_confirm(game)
|
||||
click_widget(game, next(w for w in game.widgets() if w.get('label') == "I Know What I'm Doing!"))
|
||||
check(game.widgets()[0].get('label') == 'Cancel', f'not on Optimize World: {labels(game)}')
|
||||
game.pad('down')
|
||||
@@ -1318,7 +1326,8 @@ def controller_settings_pad(args, name):
|
||||
for page in ('General', 'Bindings', 'Device'):
|
||||
check(has_label(game, page), f'no page {page} in {labels(game)}')
|
||||
check(has_label(game, 'Editing:'), 'the screen does not say whose profile it edits')
|
||||
check(not has_label(game, 'Navigation'), 'a navigation page contradicts menus always focusing')
|
||||
# Exactly: the General page has its Navigation Repeat rows.
|
||||
check('Navigation' not in labels(game), 'a navigation page contradicts menus always focusing')
|
||||
shot(game, 'controller-general')
|
||||
# The D-pad walks the whole list, scrolling it.
|
||||
game.pad('right')
|
||||
@@ -1440,6 +1449,74 @@ def controller_settings_keys(args, name):
|
||||
return check.failures
|
||||
|
||||
|
||||
OPTIONS_PAGES = ('main', 'video', 'sound', 'controls', 'keybinds', 'controller', 'controller-binds', 'mouse', 'chat',
|
||||
'accessibility', 'language', 'font', 'packs', 'mods', 'shader-packs', 'graphics', 'graphics-advanced',
|
||||
'graphics-hdr', 'extras', 'pvp', 'credits', 'horizon', 'agent')
|
||||
# Screens allowed to draw no target, and why. The pad falls back to the crosshair on these.
|
||||
NO_TARGETS = {'studio': 'its buttons and rows only paint, nothing registers them yet'}
|
||||
TARGETS_WAIT = 5
|
||||
|
||||
|
||||
def check_has_targets(game, check, screen):
|
||||
"""A focus-mode screen draws a target soon after it opens; pointer screens (packs) use the crosshair by design."""
|
||||
if screen in NO_TARGETS:
|
||||
return
|
||||
end = time.time() + TARGETS_WAIT
|
||||
while True:
|
||||
menu = game.menu()
|
||||
if menu['mode'] != 'focus' or menu['targets']:
|
||||
return
|
||||
if time.time() > end:
|
||||
check(False, f'{screen} has no targets: a controller is left with the crosshair')
|
||||
return
|
||||
time.sleep(0.2)
|
||||
|
||||
|
||||
@title_case
|
||||
def every_screen_has_targets(game, check, single, options, quit_game):
|
||||
for screen, steps in (('create world', ['singleplayer']), ('multiplayer warning', ['multiplayer']),
|
||||
('multiplayer', ['multiplayer', 'proceed']), ('realms', ['realms'])):
|
||||
for step in steps:
|
||||
click_widget(game, widget(game, step))
|
||||
check(quit_game not in game.rects(), f'{screen} did not open')
|
||||
check_has_targets(game, check, screen)
|
||||
for _ in range(4):
|
||||
if quit_game in game.rects():
|
||||
break
|
||||
# Not the Escape key: horizon-ctl's needs a world open.
|
||||
game.pad('east')
|
||||
time.sleep(SETTLE)
|
||||
else:
|
||||
raise RuntimeError(f'B did not come back to the title from {screen}')
|
||||
game.pad('west')
|
||||
for page in OPTIONS_PAGES:
|
||||
game.ctl('options', page)
|
||||
time.sleep(SETTLE)
|
||||
check_has_targets(game, check, page)
|
||||
game.ctl('options', 'horizon')
|
||||
time.sleep(SETTLE)
|
||||
click_widget(game, widget(game, 'studio'))
|
||||
check_has_targets(game, check, 'studio')
|
||||
|
||||
|
||||
def pause_icon(args, name):
|
||||
game = Game(args, name, world=True)
|
||||
check = Check()
|
||||
try:
|
||||
game.ctl('key', 'escape', 'tap')
|
||||
game.wait_targets()
|
||||
# Report Bugs, Feedback and Player Reporting, and Friends when it is live.
|
||||
icons = sorted(r for r in game.rects() if r[2:] == (20, 20))
|
||||
check(len(icons) >= 3, f'pause icons with targets: {icons}')
|
||||
game.focus_on(next(w['id'] for w in game.widgets() if tuple(w['bounds']) == icons[-1]))
|
||||
before = game.rects()
|
||||
game.pad('south')
|
||||
check(game.rects() != before, 'A did not press Player Reporting')
|
||||
finally:
|
||||
game.quit()
|
||||
return check.failures
|
||||
|
||||
|
||||
def controller_notices(args, name):
|
||||
game = Game(args, name, world=True)
|
||||
check = Check()
|
||||
@@ -1463,6 +1540,41 @@ def controller_notices(args, name):
|
||||
return check.failures
|
||||
|
||||
|
||||
def erase_checkbox(args, name):
|
||||
game = Game(args, name, world=True)
|
||||
check = Check()
|
||||
try:
|
||||
game.ctl('key', 'escape', 'tap')
|
||||
game.wait_targets()
|
||||
click_widget(game, next(w for w in game.widgets() if w.get('label') == 'Save and Quit to Title'))
|
||||
game.wait_targets()
|
||||
game.title_buttons()
|
||||
open_optimize_confirm(game)
|
||||
box = next(w for w in game.widgets() if 'label' not in w)
|
||||
# The box is the screen's topmost target.
|
||||
for _ in range(4):
|
||||
if game.focus() == tuple(box['bounds']):
|
||||
break
|
||||
game.pad('up')
|
||||
check(game.focus() == tuple(box['bounds']), f"the D-pad stopped on {game.focus()}, not {box['bounds']}")
|
||||
path = game.home / 'box.png'
|
||||
|
||||
def shot():
|
||||
game.ctl('screenshot', path)
|
||||
rows, width, channels = png_rows(path)
|
||||
scale = width / game.ctl('ui')['gui_size'][0]
|
||||
x, y, h = (int(box['bounds'][i] * scale) for i in (0, 1, 3))
|
||||
return [row[x * channels:(x + h) * channels] for row in rows[y:y + h]]
|
||||
|
||||
# Ticked, its sprite changes under the same hover.
|
||||
unticked = shot()
|
||||
game.pad('south')
|
||||
check(shot() != unticked, 'A did not tick Erase Cached Data')
|
||||
finally:
|
||||
game.quit()
|
||||
return check.failures
|
||||
|
||||
|
||||
|
||||
CASES = {'idle_a': idle_a, 'idle_b': idle_b, 'idle_x': idle_x, 'release': release, 'move_cancels': move_cancels,
|
||||
'pad_release': pad_release, 'pause_y': pause_y, 'settings': settings, 'create_world': create_world,
|
||||
@@ -1494,7 +1606,10 @@ CASES = {'idle_a': idle_a, 'idle_b': idle_b, 'idle_x': idle_x, 'release': releas
|
||||
'controller_settings_pad': controller_settings_pad,
|
||||
'controller_bind': controller_bind,
|
||||
'controller_settings_keys': controller_settings_keys,
|
||||
'controller_notices': controller_notices}
|
||||
'controller_notices': controller_notices,
|
||||
'every_screen_has_targets': every_screen_has_targets,
|
||||
'pause_icon': pause_icon,
|
||||
'erase_checkbox': erase_checkbox}
|
||||
|
||||
|
||||
def main():
|
||||
|
||||
@@ -461,7 +461,7 @@ def scenario_setup():
|
||||
print('setup: preview, Revert, Apply, Done, and solo return while the page stays open')
|
||||
g = Game(2, saved_split=True)
|
||||
def labels():
|
||||
return [w['label'] for w in g.ctl(1, 'ui')['widgets']]
|
||||
return [w.get('label', '') for w in g.ctl(1, 'ui')['widgets']]
|
||||
def enabled(label):
|
||||
return next(w['enabled'] for w in g.ctl(1, 'ui')['widgets'] if w['label'] == label)
|
||||
def page():
|
||||
|
||||
Reference in new issue
Block a user