fix: stabilize controller menu navigation

tests/controller_focus_recovery_validation.gd

@@ -1,184 +0,0 @@
-extends SceneTree
-
-const ControllerFocusRecoveryType = preload(
-	"res://ui/controller_focus_recovery.gd"
-)
-
-var _failures: Array[String] = []
-
-
-func _initialize() -> void:
-	_run.call_deferred()
-
-
-func _run() -> void:
-	var stage := Control.new()
-	stage.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT)
-	root.add_child(stage)
-	var recovery := ControllerFocusRecoveryType.new()
-	stage.add_child(recovery)
-	var option_list := VBoxContainer.new()
-	option_list.position = Vector2(100.0, 100.0)
-	stage.add_child(option_list)
-	var original := _make_button("cute", "cute (show variants)")
-	option_list.add_child(original)
-	await process_frame
-	original.grab_focus()
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == null,
-		"programmatic focus stays pending outside navigation mode",
-	)
-	var keyboard_navigation := InputEventKey.new()
-	keyboard_navigation.keycode = KEY_DOWN
-	keyboard_navigation.pressed = true
-	recovery._input(keyboard_navigation)
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == original,
-		"keyboard navigation restores pending initial focus",
-	)
-	var menu_shortcut := InputEventKey.new()
-	menu_shortcut.physical_keycode = KEY_TAB
-	menu_shortcut.pressed = true
-	recovery._input(menu_shortcut)
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == null,
-		"the menu shortcut does not leave a pseudo-focused option",
-	)
-
-	var controller_event := InputEventJoypadButton.new()
-	controller_event.button_index = JOY_BUTTON_A
-	controller_event.pressed = true
-	recovery._input(controller_event)
-	await process_frame
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == original,
-		"controller navigation restores pending initial focus",
-	)
-
-	option_list.remove_child(original)
-	original.queue_free()
-	var replacement := _make_button("cute", "cute (hide variants)")
-	option_list.add_child(replacement)
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == replacement,
-		"focus follows a rebuilt semantic option",
-	)
-
-	var explicit_target := _make_button("explicit", "explicit")
-	option_list.add_child(explicit_target)
-	option_list.remove_child(replacement)
-	replacement.queue_free()
-	explicit_target.grab_focus()
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == explicit_target,
-		"explicit focus changes take precedence over recovery",
-	)
-	var pointer_motion := InputEventMouseMotion.new()
-	pointer_motion.position = Vector2(4.0, 4.0)
-	recovery._input(pointer_motion)
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == null,
-		"mouse motion releases controller focus",
-	)
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == null,
-		"mouse motion prevents automatic focus recovery",
-	)
-	recovery._input(controller_event)
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == explicit_target,
-		"controller navigation can resume from pointer mode",
-	)
-	var pointer_press := InputEventMouseButton.new()
-	pointer_press.button_index = MOUSE_BUTTON_LEFT
-	pointer_press.pressed = true
-	recovery._input(pointer_press)
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == explicit_target,
-		"mouse-down preserves focus through button activation",
-	)
-	var pointer_release := InputEventMouseButton.new()
-	pointer_release.button_index = MOUSE_BUTTON_LEFT
-	pointer_release.pressed = false
-	recovery._input(pointer_release)
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == null,
-		"mouse-up clears the completed button focus",
-	)
-	recovery._input(controller_event)
-	await process_frame
-	await process_frame
-
-	var outside_button := _make_button("outside", "outside")
-	stage.add_child(outside_button)
-	option_list.hide()
-	root.gui_release_focus()
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == null,
-		"focus recovery never escapes a hidden menu scope",
-	)
-	option_list.show()
-	explicit_target.grab_focus()
-	await process_frame
-	var leave_world_ui_event := InputEventJoypadButton.new()
-	leave_world_ui_event.button_index = JOY_BUTTON_LEFT_SHOULDER
-	leave_world_ui_event.pressed = true
-	recovery._input(leave_world_ui_event)
-	root.gui_release_focus()
-	await process_frame
-	await process_frame
-	_expect(
-		root.gui_get_focus_owner() == null,
-		"LB intentionally leaving world UI is never recovered",
-	)
-	var accessibility_only := Control.new()
-	accessibility_only.focus_mode = Control.FOCUS_ACCESSIBILITY
-	stage.add_child(accessibility_only)
-	await process_frame
-	_expect(
-		not bool(recovery.call("_is_focusable", accessibility_only)),
-		"accessibility-only controls are not recovery targets",
-	)
-
-	stage.queue_free()
-	if _failures.is_empty():
-		print("Controller focus recovery validation: PASS")
-		quit(0)
-		return
-	for failure: String in _failures:
-		push_error(failure)
-	quit(1)
-
-
-func _make_button(text: String, tooltip: String) -> Button:
-	var button := Button.new()
-	button.text = text
-	button.tooltip_text = tooltip
-	button.custom_minimum_size = Vector2(120.0, 48.0)
-	return button
-
-
-func _expect(condition: bool, message: String) -> void:
-	if not condition:
-		_failures.append(message)

tests/controller_menu_accessibility_validation.gd

@@ -44,6 +44,7 @@ func _run() -> void:
 	await _validate_join_game_navigation()
 	await _validate_data_settings_navigation()
 	await _validate_settings_adjustment_navigation()
+	await _validate_settings_presentation_requires_apply()
 	await _validate_mail_navigation()
 	await _validate_profile_confirmation_focus()
 	await _validate_profile_voice_navigation()
@@ -163,6 +164,47 @@ func _validate_join_game_navigation() -> void:
 	)
 	await process_frame
 
+	var saved_entries: Array[SavedServerEntry] = []
+	server_list.clear()
+	for index: int in 3:
+		var entry := SavedServerEntry.new()
+		entry.entry_id = "controller-saved-%d" % index
+		entry.display_name = "Saved server %d" % index
+		saved_entries.append(entry)
+		server_list.add_item(entry.display_name)
+	page.set("_mode", 2)
+	page.set("_visible_entries", saved_entries)
+	server_list.show()
+	page.call("_configure_controller_navigation")
+	page.call("_restore_entry_selection_and_focus", 1)
+	await process_frame
+	_expect(
+		server_list.is_selected(1)
+		and root.gui_get_focus_owner() == server_list,
+		"Saved-server refresh did not restore the same list position.",
+	)
+	saved_entries.remove_at(1)
+	server_list.remove_item(1)
+	page.set("_visible_entries", saved_entries)
+	page.call("_restore_entry_selection_and_focus", 1)
+	await process_frame
+	_expect(
+		server_list.is_selected(1)
+		and (
+			page.get("_selected_entry") as SavedServerEntry
+		).entry_id == "controller-saved-2",
+		"Deleting a saved server did not select the row that replaced it.",
+	)
+	saved_entries.clear()
+	server_list.clear()
+	page.set("_visible_entries", saved_entries)
+	page.call("_restore_entry_selection_and_focus", 0)
+	await process_frame
+	_expect(
+		root.gui_get_focus_owner() == saved,
+		"An empty saved-server list did not return focus to its mode tab.",
+	)
+
 	page.queue_free()
 	await process_frame
 
@@ -190,13 +232,13 @@ func _validate_data_settings_navigation() -> void:
 		open_activation["count"] = int(open_activation["count"]) + 1
 	)
 	data_tab.grab_focus()
-	var accept_press := InputEventAction.new()
-	accept_press.action = &"ui_accept"
+	var accept_press := InputEventJoypadButton.new()
+	accept_press.button_index = JOY_BUTTON_A
 	accept_press.pressed = true
 	Input.parse_input_event(accept_press)
 	await process_frame
-	var accept_release := InputEventAction.new()
-	accept_release.action = &"ui_accept"
+	var accept_release := InputEventJoypadButton.new()
+	accept_release.button_index = JOY_BUTTON_A
 	accept_release.pressed = false
 	Input.parse_input_event(accept_release)
 	for _frame: int in 2:
@@ -213,10 +255,6 @@ func _validate_data_settings_navigation() -> void:
 		root.gui_get_focus_owner() == data_tab,
 		"Selecting the Data tab transferred focus into its actions.",
 	)
-	var test_data_root := PlayerDataRoot.new()
-	root.add_child(test_data_root)
-	test_data_root.root_path = "/tmp/netfishing-controller-data"
-	panel.set("_data_root", test_data_root)
 	open_data_folder.grab_focus()
 	_expect(
 		root.gui_get_focus_owner() == open_data_folder,
@@ -226,13 +264,13 @@ func _validate_data_settings_navigation() -> void:
 		not open_data_folder.disabled,
 		"Open Data Folder is unexpectedly disabled.",
 	)
-	var action_press := InputEventAction.new()
-	action_press.action = &"ui_accept"
+	var action_press := InputEventJoypadButton.new()
+	action_press.button_index = JOY_BUTTON_A
 	action_press.pressed = true
 	Input.parse_input_event(action_press)
 	await process_frame
-	var action_release := InputEventAction.new()
-	action_release.action = &"ui_accept"
+	var action_release := InputEventJoypadButton.new()
+	action_release.button_index = JOY_BUTTON_A
 	action_release.pressed = false
 	Input.parse_input_event(action_release)
 	for _frame: int in 2:
@@ -244,31 +282,27 @@ func _validate_data_settings_navigation() -> void:
 			% int(open_activation["count"])
 		),
 	)
-	var confirmations: Array[Node] = panel.find_children(
-		"*", "ConfirmationDialog", true, false
-	)
 	_expect(
-		confirmations.size() == 1
-		and (confirmations.front() as ConfirmationDialog).visible,
-		"Explicitly accepting Open Data Folder did not request confirmation.",
+		panel.find_children(
+			"*", "ConfirmationDialog", true, false
+		).is_empty(),
+		"Open Data Folder left a controller-activatable dialog behind.",
 	)
-	if not confirmations.is_empty():
-		var confirmation := confirmations.front() as ConfirmationDialog
-		_expect(
-			confirmation.gui_get_focus_owner()
-				== confirmation.get_cancel_button(),
-			"Open Data Folder confirmation did not default to Cancel.",
-		)
-		confirmation.queue_free()
+	var change_data_folder := panel.get_node("%ChangeDataFolder") as Button
+	var copy_fingerprint := panel.get_node("%CopyPlayerFingerprint") as Button
+	var export_player := panel.get_node("%ExportPlayerIdentity") as Button
+	var import_player := panel.get_node("%ImportPlayerIdentity") as Button
+	var export_host := panel.get_node("%ExportHostIdentity") as Button
+	var import_host := panel.get_node("%ImportHostIdentity") as Button
 	var controls: Array[Control] = [
 		data_tab,
 		open_data_folder,
-		panel.get_node("%ChangeDataFolder") as Control,
-		panel.get_node("%CopyPlayerFingerprint") as Control,
-		panel.get_node("%ExportPlayerIdentity") as Control,
-		panel.get_node("%ImportPlayerIdentity") as Control,
-		panel.get_node("%ExportHostIdentity") as Control,
-		panel.get_node("%ImportHostIdentity") as Control,
+		change_data_folder,
+		copy_fingerprint,
+		export_player,
+		import_player,
+		export_host,
+		import_host,
 		panel.get_node("%ApplySettingsButton") as Control,
 		panel.get_node("%SettingsBackButton") as Control,
 	]
@@ -278,13 +312,113 @@ func _validate_data_settings_navigation() -> void:
 			"Data & Identity control %s is not controller-focusable."
 			% control.name,
 		)
+	_assert_neighbor(data_tab, &"focus_neighbor_bottom", open_data_folder)
+	_assert_neighbor(
+		open_data_folder,
+		&"focus_neighbor_right",
+		change_data_folder,
+	)
+	_assert_neighbor(
+		open_data_folder,
+		&"focus_neighbor_bottom",
+		copy_fingerprint,
+	)
+	_assert_neighbor(
+		change_data_folder,
+		&"focus_neighbor_left",
+		open_data_folder,
+	)
+	_assert_neighbor(
+		change_data_folder,
+		&"focus_neighbor_bottom",
+		copy_fingerprint,
+	)
+	_assert_neighbor(
+		copy_fingerprint,
+		&"focus_neighbor_top",
+		open_data_folder,
+	)
+	_assert_neighbor(
+		copy_fingerprint,
+		&"focus_neighbor_bottom",
+		export_player,
+	)
+	_assert_neighbor(export_player, &"focus_neighbor_right", import_player)
+	_assert_neighbor(export_player, &"focus_neighbor_bottom", export_host)
+	_assert_neighbor(import_player, &"focus_neighbor_left", export_player)
+	_assert_neighbor(import_player, &"focus_neighbor_bottom", import_host)
+	_assert_neighbor(export_host, &"focus_neighbor_top", export_player)
+	_assert_neighbor(export_host, &"focus_neighbor_right", import_host)
+	_assert_neighbor(import_host, &"focus_neighbor_top", import_player)
+	_assert_neighbor(import_host, &"focus_neighbor_left", export_host)
 	_assert_directionally_reachable(controls.front(), controls)
+	await process_frame
+	open_data_folder.grab_focus()
+	panel.call("_select_page", &"sound", true)
+	_expect(
+		panel.get_active_page_id() == &"sound",
+		"Settings did not leave the Data page.",
+	)
+	_expect(
+		root.gui_get_focus_owner() == panel.get_node("%SoundTab"),
+		"Leaving Data retained focus on a hidden Data action.",
+	)
+	var sound_down: Control = (
+		(panel.get_node("%SoundTab") as Control).find_valid_focus_neighbor(
+			SIDE_BOTTOM
+		)
+	)
+	_expect(
+		sound_down != null and not panel.get_node("%DataPage").is_ancestor_of(
+			sound_down
+		),
+		"Sound navigation still pointed into the hidden Data page.",
+	)
+	var hidden_data_activation_count: int = int(open_activation["count"])
+	Input.parse_input_event(action_press)
+	await process_frame
+	Input.parse_input_event(action_release)
+	for _frame: int in 2:
+		await process_frame
+	_expect(
+		int(open_activation["count"]) == hidden_data_activation_count,
+		"Controller A activated Open Data Folder from another settings page.",
+	)
+	open_data_folder.pressed.emit()
+	await process_frame
+	_expect(
+		panel.find_children(
+			"*", "ConfirmationDialog", true, false
+		).is_empty(),
+		"A hidden Data action created a folder dialog.",
+	)
+	panel.call("_select_page", &"data", true)
+	await process_frame
+	copy_fingerprint.grab_focus()
+	var feedback := panel.get_node("%SettingsFeedback") as Label
+	feedback.text = "activation boundary intact"
+	open_data_folder.pressed.emit()
+	await process_frame
+	_expect(
+		feedback.text == "activation boundary intact",
+		(
+			"Open Data Folder crossed its activation boundary without owning "
+			+ "controller focus."
+		),
+	)
+	panel.hide()
+	await process_frame
+	var hidden_focus_owner: Control = root.gui_get_focus_owner()
+	_expect(
+		hidden_focus_owner == null
+		or not panel.is_ancestor_of(hidden_focus_owner),
+		"Hiding Settings retained focus on one of its controls.",
+	)
 	_expect(
 		panel.find_children("*", "BubbleButton", true, false).is_empty(),
 		"Settings children still contain bubble controls.",
 	)
 	panel.queue_free()
-	test_data_root.queue_free()
 	await process_frame
 
 
@@ -313,6 +447,11 @@ func _validate_settings_adjustment_navigation() -> void:
 	var controller_slider := panel.get_node(
 		"%ControllerSensitivitySlider"
 	) as HSlider
+	var on_screen_keyboard := panel.get_node(
+		"%OnScreenKeyboardToggle"
+	) as Button
+	var controller_binds := panel.get_node("%ControllerMapping") as Button
+	var keyboard_binds := panel.get_node("%KeyboardMapping") as Button
 	_expect(
 		mouse_slider.focus_mode == Control.FOCUS_ALL
 		and controller_slider.focus_mode == Control.FOCUS_ALL,
@@ -323,14 +462,39 @@ func _validate_settings_adjustment_navigation() -> void:
 		and is_equal_approx(controller_slider.step, 0.1),
 		"Sensitivity sliders do not retain their authored increments.",
 	)
+	_assert_neighbor(
+		on_screen_keyboard,
+		&"focus_neighbor_bottom",
+		controller_binds,
+	)
+	_assert_neighbor(
+		controller_binds,
+		&"focus_neighbor_top",
+		on_screen_keyboard,
+	)
+	_assert_neighbor(
+		controller_binds,
+		&"focus_neighbor_right",
+		keyboard_binds,
+	)
+	_assert_neighbor(
+		keyboard_binds,
+		&"focus_neighbor_top",
+		on_screen_keyboard,
+	)
+	_assert_neighbor(
+		keyboard_binds,
+		&"focus_neighbor_left",
+		controller_binds,
+	)
 	var control_inputs: Array[Control] = [
 		panel.get_node("%ControlsTab") as Control,
 		mouse_slider,
 		controller_slider,
 		panel.get_node("%InvertYToggle") as Control,
-		panel.get_node("%OnScreenKeyboardToggle") as Control,
-		panel.get_node("%ControllerMapping") as Control,
-		panel.get_node("%KeyboardMapping") as Control,
+		on_screen_keyboard,
+		controller_binds,
+		keyboard_binds,
 		panel.get_node("%ApplySettingsButton") as Control,
 		panel.get_node("%SettingsBackButton") as Control,
 	]
@@ -339,6 +503,38 @@ func _validate_settings_adjustment_navigation() -> void:
 	await process_frame
 
 
+func _validate_settings_presentation_requires_apply() -> void:
+	var panel := SettingsPanelScene.instantiate() as SettingsPanel
+	var settings_manager := PlayerSettingsManager.new()
+	root.add_child(settings_manager)
+	root.add_child(panel)
+	await process_frame
+	panel.open_panel(settings_manager)
+	await process_frame
+	var chat_dock := panel.get_node("%ChatDockSelector") as OptionButton
+	var chat_mode := panel.get_node("%ChatModeSelector") as OptionButton
+	var paint_dock := panel.get_node("%PaintDockSelector") as OptionButton
+	chat_dock.select(1)
+	chat_dock.item_selected.emit(1)
+	chat_mode.select(1)
+	chat_mode.item_selected.emit(1)
+	paint_dock.select(0)
+	paint_dock.item_selected.emit(0)
+	_expect(
+		not settings_manager.current_settings.chat_dock_right
+		and not settings_manager.current_settings.chat_mobile_mode
+		and settings_manager.current_settings.paint_dock_right,
+		(
+			"Navigating presentation choices changed saved docking settings "
+			+ "before Apply."
+		),
+	)
+	panel.close_panel(true)
+	panel.queue_free()
+	settings_manager.queue_free()
+	await process_frame
+
+
 func _validate_mail_navigation() -> void:
 	var page := MailPageType.new() as Control
 	root.add_child(page)
@@ -1334,13 +1530,8 @@ func _assert_directionally_reachable(
 	var pending: Array[Control] = [start]
 	while not pending.is_empty():
 		var current: Control = pending.pop_front()
-		for path: NodePath in [
-			current.focus_neighbor_left,
-			current.focus_neighbor_right,
-			current.focus_neighbor_top,
-			current.focus_neighbor_bottom,
-		]:
-			var neighbor := current.get_node_or_null(path) as Control
+		for side: Side in [SIDE_LEFT, SIDE_RIGHT, SIDE_TOP, SIDE_BOTTOM]:
+			var neighbor: Control = current.find_valid_focus_neighbor(side)
 			if (
 				neighbor == null
 				or not expected.has(neighbor.get_instance_id())

tests/controller_ui_navigation_validation.gd

@@ -151,7 +151,11 @@ func _validate_player_page_zone_contracts() -> void:
 	var net_source: String = FileAccess.get_file_as_string(
 		"res://ui/the_net_page.gd"
 	)
-	assert(net_source.contains("ROLE_RIGHT_STICK_Y"))
+	var game_ui_source: String = FileAccess.get_file_as_string(
+		"res://ui/game_ui.gd"
+	)
+	assert(not net_source.contains("ROLE_RIGHT_STICK_Y"))
+	assert(game_ui_source.contains("_update_controller_menu_scroll"))
 
 
 func _validate_shop_navigation_contract() -> void:

tests/file_dialog_controller_navigation_validation.gd

@@ -172,7 +172,15 @@ func _run() -> void:
 		)
 	)
 	assert(dialog.gui_get_focus_owner() == select_button)
-	assert(FileDialogControllerNavigationType.activate_control(sort_button))
+	sort_button.grab_focus()
+	var accept_press := InputEventAction.new()
+	accept_press.action = &"ui_accept"
+	accept_press.pressed = true
+	Input.parse_input_event(accept_press)
+	var accept_release := InputEventAction.new()
+	accept_release.action = &"ui_accept"
+	accept_release.pressed = false
+	Input.parse_input_event(accept_release)
 	await process_frame
 	assert(sort_button.get_popup().visible)
 	sort_button.get_popup().hide()
@@ -283,13 +291,16 @@ func _validate_compact_dialog() -> void:
 	var down_event := InputEventJoypadButton.new()
 	down_event.button_index = JOY_BUTTON_DPAD_DOWN
 	down_event.pressed = true
-	font_controller.call("_input", down_event)
+	var dialog_controller := font_controller.get(
+		"_file_dialog_controller"
+	) as FileDialogController
+	dialog_controller.call("_input", down_event)
 	assert(scope.gui_get_focus_owner() == select_button)
 	path_edit.grab_focus()
 	var accept_event := InputEventJoypadButton.new()
 	accept_event.button_index = JOY_BUTTON_A
 	accept_event.pressed = true
-	font_controller.call("_input", accept_event)
+	dialog_controller.call("_input", accept_event)
 	assert(keyboard.is_open())
 	keyboard.call("_close_keyboard", true)
 	dialog.queue_free()

tests/fishing_shop_controller_validation.gd

@@ -32,6 +32,9 @@ func _run() -> void:
 	var accept := InputEventJoypadButton.new()
 	accept.button_index = JOY_BUTTON_A
 	accept.pressed = true
+	var accept_release := InputEventJoypadButton.new()
+	accept_release.button_index = JOY_BUTTON_A
+	accept_release.pressed = false
 	var cancel := InputEventJoypadButton.new()
 	cancel.button_index = JOY_BUTTON_B
 	cancel.pressed = true
@@ -72,7 +75,10 @@ func _run() -> void:
 		))
 		if section_index == FishingShop.ShopSection.BAIT:
 			assert(root.gui_get_focus_owner() == dummy_stock)
-			shop.call("_input", accept)
+			Input.parse_input_event(accept)
+			await process_frame
+			Input.parse_input_event(accept_release)
+			await process_frame
 			assert(activations[0] == 1)
 		shop.call("_input", cancel)
 		for _frame: int in 2: