From 67cffa90238e1065afcaca6d9b7f2511f4a147bf Mon Sep 17 00:00:00 2001 From: "Derek J. Clark" Date: Thu, 20 Jun 2024 12:25:34 -0700 Subject: [PATCH] fix(StateMachine): Prevent global state stack from going null. - Adds @export var minimum_states: int = 1 to StateMachine. state_machine.pop_state() will never go below this value. - Sets all non-global StateMachines minimum states to 0. - Remove code no longer needed in overlay_input_manager that prevented dropping to null states. - While at it, change method comments in StateMachine to be used in automatic documentation formatting. --- .../first_boot_network_state_machine.tres | 1 + .../first_boot_state_machine.tres | 1 + .../game_settings_state_machine.tres | 1 + .../gamepad_settings_state_machine.tres | 1 + .../plugin_settings_state_machine.tres | 3 +- .../quick_bar_state_machine.tres | 1 + .../settings_state_machine.tres | 3 +- core/global/launch_manager.gd | 1 - core/systems/input/back_input_handler.gd | 4 +- .../input/overlay_mode_input_manager.gd | 59 ++++--------------- core/systems/state/state_machine.gd | 51 +++++++++++----- core/systems/state/state_machine_test.gd | 1 + core/systems/state/state_updater_test.gd | 1 + core/systems/state/state_watcher_test.gd | 1 + core/ui/card_ui/main-menu/main_menu_test.gd | 3 +- .../card_ui_overlay_mode.gd | 13 ++-- 16 files changed, 71 insertions(+), 74 deletions(-) diff --git a/assets/state/state_machines/first_boot_network_state_machine.tres b/assets/state/state_machines/first_boot_network_state_machine.tres index e035ffe8..bad28e8e 100644 --- a/assets/state/state_machines/first_boot_network_state_machine.tres +++ b/assets/state/state_machines/first_boot_network_state_machine.tres @@ -5,3 +5,4 @@ [resource] script = ExtResource("1_iwio6") logger_name = "NetworkOOBE" +minimum_states = 0 diff --git a/assets/state/state_machines/first_boot_state_machine.tres b/assets/state/state_machines/first_boot_state_machine.tres index 265a73f7..7da45e9f 100644 --- a/assets/state/state_machines/first_boot_state_machine.tres +++ b/assets/state/state_machines/first_boot_state_machine.tres @@ -5,3 +5,4 @@ [resource] script = ExtResource("1_8ewnv") logger_name = "FirstBoot" +minimum_states = 0 diff --git a/assets/state/state_machines/game_settings_state_machine.tres b/assets/state/state_machines/game_settings_state_machine.tres index 7c68c07d..9622242e 100644 --- a/assets/state/state_machines/game_settings_state_machine.tres +++ b/assets/state/state_machines/game_settings_state_machine.tres @@ -5,3 +5,4 @@ [resource] script = ExtResource("1_ilq52") logger_name = "GameSettingsStateMachine" +minimum_states = 0 diff --git a/assets/state/state_machines/gamepad_settings_state_machine.tres b/assets/state/state_machines/gamepad_settings_state_machine.tres index 7a72b610..aeae7b4d 100644 --- a/assets/state/state_machines/gamepad_settings_state_machine.tres +++ b/assets/state/state_machines/gamepad_settings_state_machine.tres @@ -5,3 +5,4 @@ [resource] script = ExtResource("1_hgufk") logger_name = "StateMachine" +minimum_states = 0 diff --git a/assets/state/state_machines/plugin_settings_state_machine.tres b/assets/state/state_machines/plugin_settings_state_machine.tres index 33ecebea..b6027729 100644 --- a/assets/state/state_machines/plugin_settings_state_machine.tres +++ b/assets/state/state_machines/plugin_settings_state_machine.tres @@ -1,7 +1,8 @@ -[gd_resource type="Resource" load_steps=2 format=3 uid="uid://bnwdyr66wnxaw"] +[gd_resource type="Resource" script_class="StateMachine" load_steps=2 format=3 uid="uid://bnwdyr66wnxaw"] [ext_resource type="Script" path="res://core/systems/state/state_machine.gd" id="1_20omy"] [resource] script = ExtResource("1_20omy") logger_name = "PluginSettingsStateMachine" +minimum_states = 0 diff --git a/assets/state/state_machines/quick_bar_state_machine.tres b/assets/state/state_machines/quick_bar_state_machine.tres index 933a6cf7..5c750cc9 100644 --- a/assets/state/state_machines/quick_bar_state_machine.tres +++ b/assets/state/state_machines/quick_bar_state_machine.tres @@ -5,3 +5,4 @@ [resource] script = ExtResource("1_k2cku") logger_name = "QuickBarStateMachine" +minimum_states = 0 diff --git a/assets/state/state_machines/settings_state_machine.tres b/assets/state/state_machines/settings_state_machine.tres index 8cfbf1cd..649a6e34 100644 --- a/assets/state/state_machines/settings_state_machine.tres +++ b/assets/state/state_machines/settings_state_machine.tres @@ -1,7 +1,8 @@ -[gd_resource type="Resource" load_steps=2 format=3 uid="uid://iqrotrmq62i6"] +[gd_resource type="Resource" script_class="StateMachine" load_steps=2 format=3 uid="uid://iqrotrmq62i6"] [ext_resource type="Script" path="res://core/systems/state/state_machine.gd" id="1_7ufyt"] [resource] script = ExtResource("1_7ufyt") logger_name = "SettingsStateMachine" +minimum_states = 0 diff --git a/core/global/launch_manager.gd b/core/global/launch_manager.gd index a1162580..eb07d663 100644 --- a/core/global/launch_manager.gd +++ b/core/global/launch_manager.gd @@ -60,7 +60,6 @@ var should_manage_overlay := true # Connect to Gamescope signals func _init() -> void: - # When window focus changes, update the current app and gamepad profile var on_focus_changed := func(from: int, to: int): logger.info("Window focus changed from " + str(from) + " to: " + str(to)) diff --git a/core/systems/input/back_input_handler.gd b/core/systems/input/back_input_handler.gd index bf715640..0cadd6ca 100644 --- a/core/systems/input/back_input_handler.gd +++ b/core/systems/input/back_input_handler.gd @@ -42,6 +42,4 @@ func _input(event: InputEvent) -> void: get_viewport().set_input_as_handled() # Pop the state machine stack to go back - if state_machine.stack_length() > minimum_states: - state_machine.pop_state() - return + state_machine.pop_state() diff --git a/core/systems/input/overlay_mode_input_manager.gd b/core/systems/input/overlay_mode_input_manager.gd index 9a48eaf8..e071bd10 100644 --- a/core/systems/input/overlay_mode_input_manager.gd +++ b/core/systems/input/overlay_mode_input_manager.gd @@ -25,7 +25,6 @@ var main_menu_state := preload("res://assets/state/states/main_menu.tres") as St var quick_bar_state := preload("res://assets/state/states/quick_bar_menu.tres") as State var base_state = preload("res://assets/state/states/in_game.tres") as State -var handle_back: bool = false var actions_pressed := {} ## Will show logger events with the prefix InputManager(Overlay Mode) @@ -40,16 +39,6 @@ func _ready() -> void: for device in input_plumber.composite_devices: _watch_dbus_device(device) - state_machine.state_changed.connect(_on_state_changed) - - -# Only process back input when not on the base state -func _on_state_changed(_from: State, to: State) -> void: - if to == base_state: - handle_back = false - return - handle_back = true - ## Queue a release event for the given action func action_release(dbus_path: String, action: String, strength: float = 1.0) -> void: @@ -205,22 +194,6 @@ func _input(event: InputEvent) -> void: get_viewport().set_input_as_handled() return - # Only proceed if this is a back event. - if not event.is_action("ogui_back"): - return - - # If we're on the base state, don't pop the state. - if not handle_back: - return - - # Only handle back on the down event - if not event.is_pressed(): - return - - # Pop the state off the state stack and prevent the event from propagating - state_machine.pop_state() - get_viewport().set_input_as_handled() - ## Handle guide button events and determine whether this is a guide action ## (e.g. guide + A to open the Quick Bar), or if it's just a normal guide button press. @@ -240,9 +213,8 @@ func _guide_input(event: InputEvent) -> void: # Emit the main menu action if this was not a guide action logger.debug("Guide released. Additional events did not use guide action. Sending Guide.") - input_plumber.set_intercept_mode(InputPlumber.INTERCEPT_MODE.PASS) + _close_focused_window() _return_chord(["Gamepad:Button:Guide"]) - _close_quickbar() ## Handle quick bar menu events to open the quick bar menu @@ -256,15 +228,12 @@ func _quick_bar_input(event: InputEvent) -> void: var state := state_machine.current_state() logger.debug("Current State: " +str(state)) - if state == quick_bar_state: + if state != base_state: logger.debug("Close Quick Bar") - state_machine.pop_state() - elif state in [main_menu_state, in_game_menu_state]: - logger.debug("Replace state with Quick Bar") - state_machine.replace_state(quick_bar_state) - else: - logger.debug("Push Quick Bar") - state_machine.push_state(quick_bar_state) + _close_focused_window() + return + logger.debug("Push Quick Bar") + state_machine.push_state(quick_bar_state) ## Handle OSK events for bringing up the on-screen keyboard @@ -272,9 +241,8 @@ func _osk_input(event: InputEvent) -> void: logger.debug("Trigger Steam OSK") if not event.is_pressed(): return - input_plumber.set_intercept_mode(InputPlumber.INTERCEPT_MODE.PASS) + _close_focused_window() _return_chord(["Gamepad:Button:Guide", "Gamepad:Button:East"]) - _close_quickbar() ## Handle QAM events for bringing up the steam QAM @@ -282,9 +250,8 @@ func _qam_input(event: InputEvent) -> void: logger.debug("Trigger Steam QAM") if not event.is_pressed(): return - input_plumber.set_intercept_mode(InputPlumber.INTERCEPT_MODE.PASS) + _close_focused_window() _return_chord(["Gamepad:Button:Guide", "Gamepad:Button:South"]) - _close_quickbar() func _return_chord(actions: PackedStringArray) -> void: @@ -292,7 +259,7 @@ func _return_chord(actions: PackedStringArray) -> void: # Input.parse_input_event so we don't do this terrible loop. This is awful. logger.debug("Return events to InputPlumber: " + str(actions)) for device in input_plumber.composite_devices: - device.intercept_mode = 0 + device.intercept_mode = InputPlumber.INTERCEPT_MODE.PASS device.send_button_chord(actions) @@ -358,9 +325,7 @@ func _on_dbus_input_event(event: String, value: float, dbus_path: String) -> voi action_release(dbus_path, action) -func _close_quickbar() -> void: - var state := state_machine.current_state() - logger.debug("Current State: " +str(state)) - while state != base_state: +# Closes all windows until we return to the base_state +func _close_focused_window() -> void: + while state_machine.stack_length() > state_machine.minimum_states: state_machine.pop_state() - state = state_machine.current_state() diff --git a/core/systems/state/state_machine.gd b/core/systems/state/state_machine.gd index ed5c4cfe..a78424ef 100644 --- a/core/systems/state/state_machine.gd +++ b/core/systems/state/state_machine.gd @@ -9,6 +9,8 @@ class_name StateMachine signal state_changed(from: State, to: State) @export var logger_name := "StateMachine" +@export var minimum_states: int = 1 + var _state_stack: Array[State] = [] var logger := Log.get_logger(logger_name) @@ -17,7 +19,7 @@ func _init() -> void: state_changed.connect(_on_state_changed) -# Emit state changes on the state itself and log state changes +## Emit state changes on the state itself and log state changes func _on_state_changed(from: State, to: State) -> void: var from_str = "" var to_str = "" @@ -36,7 +38,7 @@ func _on_state_changed(from: State, to: State) -> void: logger.info("Stack: " + "-> ".join(state_names)) -# Returns the current state at the end of the state stack +## Returns the current state at the end of the state stack func current_state() -> State: var length = len(_state_stack) if length == 0: @@ -44,8 +46,11 @@ func current_state() -> State: return _state_stack[length-1] -# Set state will set the entire state stack to the given array of states +## Set state will set the entire state stack to the given array of states func set_state(stack: Array[State]) -> void: + if null in stack: + logger.warn("Invalid NULL state pushed.") + return var cur := current_state() var old_stack := _state_stack _state_stack = stack @@ -57,30 +62,41 @@ func set_state(stack: Array[State]) -> void: state_changed.emit(cur, stack[-1]) -# Push state will push the given state to the top of the state stack. +## Push state will push the given state to the top of the state stack. func push_state(state: State) -> void: + if state == null: + logger.warn("Invalid NULL state pushed.") + return var cur := current_state() _push_unique(state) state_changed.emit(cur, state) -# Pushes the given state to the front of the stack +## Pushes the given state to the front of the stack func push_state_front(state: State) -> void: + if state == null: + logger.warn("Invalid NULL state pushed.") + return var cur = current_state() _state_stack.push_front(state) state_changed.emit(cur, current_state()) -# Pop state will remove the last state from the stack and return it. +## Pop state will remove the last state from the stack and return it. func pop_state() -> State: - var popped = _state_stack.pop_back() - var cur = current_state() - state_changed.emit(popped, cur) - return popped + if self.stack_length() > minimum_states: + var popped = _state_stack.pop_back() + var cur = current_state() + state_changed.emit(popped, cur) + return popped + return current_state() -# Replaces the current state at the end of the stack with the given state +## Replaces the current state at the end of the stack with the given state func replace_state(state: State) -> void: + if state == null: + logger.warn("Invalid NULL state pushed.") + return var popped := _state_stack.pop_back() as State _push_unique(state) if popped != null: @@ -88,7 +104,7 @@ func replace_state(state: State) -> void: state_changed.emit(popped, state) -# Removes all instances of the given state from the stack +## Removes all instances of the given state from the stack func remove_state(state: State) -> void: var cur := current_state() var new_state_stack: Array[State] = [] @@ -101,17 +117,22 @@ func remove_state(state: State) -> void: state_changed.emit(cur, current_state()) -# Returns the length of the state stack +## Removes all states +func clear_states() -> void: + _state_stack.clear() + + +## Returns the length of the state stack func stack_length() -> int: return len(_state_stack) -# Returns the current state stack +## Returns the current state stack func stack() -> Array[State]: return _state_stack.duplicate() -# Returns true if the given state exists anywhere in the state stack +## Returns true if the given state exists anywhere in the state stack func has_state(state: State) -> bool: if _state_stack.find(state) != -1: return true diff --git a/core/systems/state/state_machine_test.gd b/core/systems/state/state_machine_test.gd index 4ee06ec0..2041e481 100644 --- a/core/systems/state/state_machine_test.gd +++ b/core/systems/state/state_machine_test.gd @@ -5,6 +5,7 @@ var state_machine: StateMachine func before_each() -> void: state_machine = StateMachine.new() + state_machine.minimum_states = 0 watch_signals(state_machine) diff --git a/core/systems/state/state_updater_test.gd b/core/systems/state/state_updater_test.gd index d0920c29..154efed9 100644 --- a/core/systems/state/state_updater_test.gd +++ b/core/systems/state/state_updater_test.gd @@ -10,6 +10,7 @@ var node: Button func before_each() -> void: # Create a state machine and state state_machine = StateMachine.new() + state_machine.minimum_states = 0 state = State.new() # Create and configure the state updater diff --git a/core/systems/state/state_watcher_test.gd b/core/systems/state/state_watcher_test.gd index 46aa2d8a..0d471220 100644 --- a/core/systems/state/state_watcher_test.gd +++ b/core/systems/state/state_watcher_test.gd @@ -10,6 +10,7 @@ var node: Node func before_each() -> void: # Create a state machine and state state_machine = StateMachine.new() + state_machine.minimum_states = 0 state = State.new() # Create a state watcher diff --git a/core/ui/card_ui/main-menu/main_menu_test.gd b/core/ui/card_ui/main-menu/main_menu_test.gd index 0bbfb7a9..05437a3e 100644 --- a/core/ui/card_ui/main-menu/main_menu_test.gd +++ b/core/ui/card_ui/main-menu/main_menu_test.gd @@ -23,8 +23,7 @@ func before_each() -> void: func after_each() -> void: if headless: return - while state_machine.stack_length() > 0: - state_machine.pop_state() + state_machine.clear_states() # Stop all threads to prevent crashing diff --git a/core/ui/card_ui_overlay_mode/card_ui_overlay_mode.gd b/core/ui/card_ui_overlay_mode/card_ui_overlay_mode.gd index f59fb69b..a80aecad 100644 --- a/core/ui/card_ui_overlay_mode/card_ui_overlay_mode.gd +++ b/core/ui/card_ui_overlay_mode/card_ui_overlay_mode.gd @@ -41,6 +41,7 @@ func _init(): # Back button wont close windows without this. OverlayInputManager prevents poping the last state. state_machine.push_state(base_state) + state_machine.state_changed.connect(_on_no_state) # Ensure LaunchManager doesn't override our custom overlay management l launch_manager.should_manage_overlay = false @@ -237,7 +238,6 @@ func _find_underlay_window_id() -> void: func _on_window_open(from: State) -> void: if from: logger.info("Quick bar open state: " + from.name) - input_plumber.set_intercept_mode(InputPlumber.INTERCEPT_MODE.ALL) if game_running: gamescope.set_overlay(overlay_window_id, 1, display) gamescope.set_overlay(underlay_window_id, 0, display) @@ -247,7 +247,6 @@ func _on_window_open(from: State) -> void: func _on_window_closed(to: State) -> void: if to: logger.info("Quick bar closed state: " + to.name) - input_plumber.set_intercept_mode(InputPlumber.INTERCEPT_MODE.PASS) if game_running: gamescope.set_overlay(overlay_window_id, 0, display) gamescope.set_overlay(underlay_window_id, 1, display) @@ -279,14 +278,20 @@ func _check_exit() -> void: ## Sets gamescope input focus to on so the user can interact with the UI - func _on_base_state_exited(_to: State) -> void: + input_plumber.set_intercept_mode(InputPlumber.INTERCEPT_MODE.ALL) if gamescope.set_input_focus(overlay_window_id, 1) != OK: logger.error("Unable to set STEAM_INPUT_FOCUS atom!") ## Sets gamescope input focus to off so the user can interact with the game func _on_base_state_entered(_from: State) -> void: - + input_plumber.set_intercept_mode(InputPlumber.INTERCEPT_MODE.PASS) if gamescope.set_input_focus(overlay_window_id, 0) != OK: logger.error("Unable to set STEAM_INPUT_FOCUS atom!") + + +## Ensures there is always a state on the state stack. +func _on_no_state(_from: State, to: State) -> void: + if state_machine.stack_length() == 0: + state_machine.push_state(base_state)