diff --git a/scripts/gameplay_settings.gd b/scripts/gameplay_settings.gd index 871378de..1e1bd22a 100644 --- a/scripts/gameplay_settings.gd +++ b/scripts/gameplay_settings.gd @@ -6,7 +6,7 @@ extends Control ## actual engine singletons. var js_bridge_wrapper: JavaScriptBridgeWrapper = JavaScriptBridgeWrapper.new() var os_wrapper: OSWrapper = OSWrapper.new() -var js_window: JavaScriptObject +var js_window: Variant var _change_difficulty_cb: JavaScriptObject var _gameplay_back_button_pressed_cb: JavaScriptObject var _gameplay_reset_cb: JavaScriptObject @@ -27,10 +27,21 @@ func _ready() -> void: # Configure for web overlays (invisible but positioned) process_mode = Node.PROCESS_MODE_ALWAYS # Ignore pause - difficulty_slider.value_changed.connect(_on_difficulty_value_changed) - # Set initial difficulty label (sync with global) - difficulty_slider.value = Globals.settings.difficulty - difficulty_label.text = "{" + str(Globals.settings.difficulty) + "}" + var settings_res := Globals.settings if is_instance_valid(Globals) else null + + # ADD GUARDS HERE: + if not difficulty_slider.value_changed.is_connected(_on_difficulty_value_changed): + difficulty_slider.value_changed.connect(_on_difficulty_value_changed) + + # Set initial difficulty label (sync with global if available) + # FIX: Use the local reference for consistency + if is_instance_valid(settings_res): + difficulty_slider.value = settings_res.difficulty + difficulty_label.text = "{" + str(settings_res.difficulty) + "}" + else: + difficulty_slider.value = _default_difficulty + difficulty_label.text = "{" + str(_default_difficulty) + "}" + # Back button if not gameplay_back_button.pressed.is_connected(_on_gameplay_back_button_pressed): gameplay_back_button.pressed.connect(_on_gameplay_back_button_pressed) @@ -38,11 +49,17 @@ func _ready() -> void: if not gameplay_reset_button.pressed.is_connected(_on_gameplay_reset_button_pressed): gameplay_reset_button.pressed.connect(_on_gameplay_reset_button_pressed) # NEW: Attach tree_exited for unexpected removal cleanup (like other settings scripts) - tree_exited.connect(_on_tree_exited) + if not tree_exited.is_connected(_on_tree_exited): + tree_exited.connect(_on_tree_exited) # NEW: The UI now observes the resource for external changes - if not Globals.settings.setting_changed.is_connected(_on_external_setting_changed): - Globals.settings.setting_changed.connect(_on_external_setting_changed) + # if not Globals.settings.setting_changed.is_connected(_on_external_setting_changed): + # Globals.settings.setting_changed.connect(_on_external_setting_changed) + if ( + is_instance_valid(settings_res) + and not settings_res.setting_changed.is_connected(_on_external_setting_changed) + ): + settings_res.setting_changed.connect(_on_external_setting_changed) if os_wrapper.has_feature("web"): # Toggle overlays... @@ -85,10 +102,15 @@ func _ready() -> void: func _on_external_setting_changed(setting_name: String, new_value: Variant) -> void: + ## SYNC UI ONLY: + ## This observer reacts to changes from the resource. + ## We must ensure the UI nodes are still valid before updating them. if setting_name == "difficulty": - # SYNC UI ONLY: - # The resource has already been updated, so we only need to update the UI components. - # Use set_value_no_signal to prevent re-triggering the local _on_difficulty_value_changed handler. + # FIX: Guard against 'previously freed' errors during teardown/unit tests + if not is_instance_valid(difficulty_slider) or not is_instance_valid(difficulty_label): + return + + # Use set_value_no_signal to prevent re-triggering local handlers difficulty_slider.set_value_no_signal(float(new_value)) difficulty_label.text = "{" + str(new_value) + "}" @@ -97,29 +119,34 @@ func _on_tree_exited() -> void: ## Cleanup on unexpected tree exit (e.g. parent removed without calling back button). ## Disconnects signals, restores previous menu if not intentional, clears JS/DOM state. ## :rtype: void - Globals.log_message("Gameplay Settings _on_tree_exited called.", Globals.LogLevel.DEBUG) + ## Cleanup on unexpected tree exit. - # Disconnect the global resource observer to prevent stale references - # GUARD: Ensure Globals and the settings resource are still valid before disconnecting - # Use a local variable to safely check and access the settings resource - var settings_res := Globals.settings if is_instance_valid(Globals) else null + # FIX: Guard the initial log message against a torn-down Globals singleton + if is_instance_valid(Globals): + Globals.log_message("Gameplay Settings _on_tree_exited called.", Globals.LogLevel.DEBUG) + # 1. Safe Global Resource Disconnection + var settings_res := Globals.settings if is_instance_valid(Globals) else null if is_instance_valid(settings_res): if settings_res.setting_changed.is_connected(_on_external_setting_changed): settings_res.setting_changed.disconnect(_on_external_setting_changed) - # Disconnect Godot signals if still connected - if difficulty_slider.value_changed.is_connected(_on_difficulty_value_changed): - difficulty_slider.value_changed.disconnect(_on_difficulty_value_changed) - if gameplay_back_button.pressed.is_connected(_on_gameplay_back_button_pressed): - gameplay_back_button.pressed.disconnect(_on_gameplay_back_button_pressed) - if gameplay_reset_button.pressed.is_connected(_on_gameplay_reset_button_pressed): - gameplay_reset_button.pressed.disconnect(_on_gameplay_reset_button_pressed) + # 2. FIX: Guarded Local Disconnections + # We must check if the nodes still exist before accessing 'value_changed' or 'pressed' + if is_instance_valid(difficulty_slider): + if difficulty_slider.value_changed.is_connected(_on_difficulty_value_changed): + difficulty_slider.value_changed.disconnect(_on_difficulty_value_changed) - # Clean up JS callbacks on window object - _unset_gameplay_settings_window_callbacks() + if is_instance_valid(gameplay_back_button): + if gameplay_back_button.pressed.is_connected(_on_gameplay_back_button_pressed): + gameplay_back_button.pressed.disconnect(_on_gameplay_back_button_pressed) + + if is_instance_valid(gameplay_reset_button): + if gameplay_reset_button.pressed.is_connected(_on_gameplay_reset_button_pressed): + gameplay_reset_button.pressed.disconnect(_on_gameplay_reset_button_pressed) - # Null out stored callback references + # 3. Clean up JS/Web state + _unset_gameplay_settings_window_callbacks() _change_difficulty_cb = null _gameplay_back_button_pressed_cb = null _gameplay_reset_cb = null @@ -133,7 +160,12 @@ func _on_tree_exited() -> void: document.getElementById('gameplay-reset-button').style.display = 'none'; """ - if not _intentional_exit and not Globals.hidden_menus.is_empty(): + # FIX: Guard the hidden_menus array check against a torn-down Globals singleton + if ( + not _intentional_exit + and is_instance_valid(Globals) + and not Globals.hidden_menus.is_empty() + ): # Unexpected exit → restore previous menu and options overlays var prev_menu: Node = Globals.hidden_menus.pop_back() if is_instance_valid(prev_menu): @@ -261,75 +293,132 @@ func _on_difficulty_value_changed(value: float) -> void: ## :type value: float ## :rtype: void # Update the resource first (this triggers clamping in the setter) - Globals.settings.difficulty = value + #Globals.settings.difficulty = value # Update the UI components using the ALREADY CLAMPED value from the resource - difficulty_slider.value = Globals.settings.difficulty - difficulty_label.text = "{" + str(Globals.settings.difficulty) + "}" + #difficulty_slider.value = Globals.settings.difficulty + #difficulty_label.text = "{" + str(Globals.settings.difficulty) + "}" + var settings_res := Globals.settings if is_instance_valid(Globals) else null + + # FIX: Use the local reference exclusively + if not is_instance_valid(settings_res): + Globals.log_message( + "Gameplay Settings: settings_res unavailable; skipping difficulty update.", + Globals.LogLevel.WARNING + ) + return + + settings_res.difficulty = value + if is_instance_valid(difficulty_slider): + difficulty_slider.set_value_no_signal(settings_res.difficulty) + if is_instance_valid(difficulty_label): + difficulty_label.text = "{" + str(settings_res.difficulty) + "}" # New: JS-specific callback (exactly one Array arg, no default) func _on_change_difficulty_js(args: Array) -> void: ## JS callback for changing difficulty. ## - ## Routes to the signal handler. + ## Routes to the signal handler after performing strict type and + ## bounds validation to prevent engine crashes on malformed JS input. ## ## :param args: Array containing the value (from JS). ## :type args: Array ## :rtype: void - if args.is_empty(): - Globals.log_message( - "JS difficulty callback received empty args—skipping.", Globals.LogLevel.WARNING - ) + + var potential_value: Variant = _extract_js_difficulty(args) + + if potential_value == null: return - var first_arg: Variant = args[0] + # GS-JS-12/15/22: Validate that the extracted value is a convertible type if ( - first_arg is not JavaScriptObject - and typeof(first_arg) != TYPE_ARRAY - and first_arg.size() == 0 - and first_arg.is_empty() + typeof(potential_value) != TYPE_INT + and typeof(potential_value) != TYPE_FLOAT + and typeof(potential_value) != TYPE_STRING ): Globals.log_message( - ( - "JS difficulty callback received invalid first arg (not a non-empty array): " - + str(args) - ), + "JS difficulty callback received non-convertible value: " + str(potential_value), Globals.LogLevel.WARNING ) return - var potential_value: Variant = first_arg[0] - if ( - typeof(potential_value) != TYPE_INT - and typeof(potential_value) != TYPE_FLOAT - and typeof(potential_value) != TYPE_STRING - ): + # GS-JS-03: Coerce to float (e.g., "1.5" becomes 1.5) + # FIX: Ensure strings are numeric before conversion to prevent 0.0/clamping reset + if typeof(potential_value) == TYPE_STRING and not potential_value.is_valid_float(): Globals.log_message( - "JS difficulty callback received non-convertible value: " + str(args), + "JS difficulty callback: Rejected non-numeric string: " + str(potential_value), Globals.LogLevel.WARNING ) return var value: float = float(potential_value) + + # GS-JS-30: Guard against missing UI nodes during callback + if not is_instance_valid(difficulty_slider): + Globals.log_message( + "JS difficulty callback: Slider node is invalid/freed.", Globals.LogLevel.WARNING + ) + + # FIX: Safely check for Globals and Settings before falling back + var settings_res := Globals.settings if is_instance_valid(Globals) else null + if is_instance_valid(settings_res): + settings_res.difficulty = value # Update resource even if UI is gone + return + + # GS-JS-04/05: Validate bounds against the UI constraints if value < difficulty_slider.min_value or value > difficulty_slider.max_value: Globals.log_message( - ( - "JS difficulty callback received out-of-bounds value: " - + str(value) - + " (args: " - + str(args) - + ")" - ), + "JS difficulty callback received out-of-bounds value: " + str(value), Globals.LogLevel.WARNING ) - return Globals.log_message( "JS difficulty callback called with valid value: " + str(value), Globals.LogLevel.DEBUG ) + + # Pass the validated value to the standard handler _on_difficulty_value_changed(value) +## GS-JS: Helper to extract a potential value from diverse JS bridge payloads. +## Isolates branching logic for standard Arrays, JavaScriptObjects, and scalars. +func _extract_js_difficulty(args: Array) -> Variant: + # GS-JS-10: Guard against entirely empty arguments from the bridge + if args.is_empty(): + Globals.log_message( + "JS difficulty callback received empty args—skipping.", Globals.LogLevel.WARNING + ) + return null + + var first_arg: Variant = args[0] + + # GS-JS-20/21: Branch logic to handle TYPE_ARRAY and JavaScriptObject separately + if typeof(first_arg) == TYPE_ARRAY: + # Safe to use .size() and indexing on standard GDScript Arrays + if first_arg.size() > 0: + return first_arg[0] + + Globals.log_message("JS callback: Array is empty.", Globals.LogLevel.WARNING) + return null + + if first_arg is JavaScriptObject: + # BUG RISK FIX: Validate the 'length' property exists and is numeric + # before treating the object as an array. + # Note: Must use dot notation, as .get() attempts to call a JS method. + var js_length: Variant = first_arg.length + + if js_length != null and typeof(js_length) in [TYPE_INT, TYPE_FLOAT] and js_length > 0: + # JS-FIX: If we receive a JS Object (like from Playwright), + # we must index it to get the raw value before the type check. + return first_arg[0] + + # It is a generic JS object or a non-array; treat as a scalar reference + return first_arg + + # Handle scalar values (e.g., [1.5]) directly + return first_arg + + ## Grabs initial focus on the difficulty slider using the global helper. ## Ensures the slider is focused when the menu opens. ## Falls back to other controls if needed. diff --git a/scripts/globals.gd b/scripts/globals.gd index 7b969aa4..7a12c76e 100644 --- a/scripts/globals.gd +++ b/scripts/globals.gd @@ -51,12 +51,12 @@ func _ready() -> void: log_message("Log level set to: " + LogLevel.keys()[settings.current_log_level], LogLevel.DEBUG) _load_settings() # Load persisted settings first - # Connect to the resource signal to centralize side effects [cite: 151] + # Connect to the resource signal to centralize side effects if settings: settings.setting_changed.connect(_on_setting_changed) -## Reactive handler for the Observer Pattern [cite: 141] +## Reactive handler for the Observer Pattern func _on_setting_changed(setting_name: String, new_value: Variant) -> void: # Skip persistence and logging if we are in a bulk-loading state if _is_loading_settings: @@ -65,10 +65,10 @@ func _on_setting_changed(setting_name: String, new_value: Variant) -> void: # FIX: Ensure we are comparing String to String or using correct types var log_msg: String = "Setting '%s' updated to: %s" % [setting_name, str(new_value)] - # Automatically log the change [cite: 59] + # Automatically log the change log_message(log_msg, LogLevel.DEBUG) - # Automatically persist to disk [cite: 53] + # Automatically persist to disk _save_settings() diff --git a/test/gut/test_game_settings_resource.gd b/test/gut/test_game_settings_resource.gd new file mode 100644 index 00000000..80b5219e --- /dev/null +++ b/test/gut/test_game_settings_resource.gd @@ -0,0 +1,133 @@ +## Copyright (C) 2026 Egor Kostan +## SPDX-License-Identifier: GPL-3.0-or-later +## test_game_settings_resource.gd +## +## GUT unit tests for GameSettingsResource and GameplaySettings initialization. +## +## It ensures the GameSettingsResource acts as a reliable source of truth and +## that the GameplaySettings scene correctly synchronizes its UI components during +## the _ready() sequence. + +extends "res://addons/gut/test.gd" + +const GameplaySettings = preload("res://scripts/gameplay_settings.gd") +var gameplay_menu: Control +var _resource: GameSettingsResource + +func before_each() -> void: + # Initialize a fresh resource for every test to ensure isolation + _resource = GameSettingsResource.new() + Globals.settings = _resource + + # Instantiate the menu for initialization tests + gameplay_menu = load("res://scenes/gameplay_settings.tscn").instantiate() + # Inject mock wrapper to avoid real JS/OS calls during unit tests + gameplay_menu.os_wrapper = OSWrapper.new() + + add_child_autofree(gameplay_menu) + await get_tree().process_frame + + +# --- SECTION 2: RESOURCE CONTRACT TESTS (GS-RES) --- + +## GS-RES-01 | Validate signal emission on valid update +func test_gs_res_01_signal_on_valid_change() -> void: + watch_signals(_resource) + _resource.difficulty = 1.5 + assert_signal_emitted_with_parameters(_resource, "setting_changed", ["difficulty", 1.5], 0) + + +## GS-RES-02/03 | Validate clamping logic +func test_gs_res_02_03_clamping_behavior() -> void: + # Test Upper Bound Clamping + _resource.difficulty = 5.0 + assert_eq(_resource.difficulty, 2.0, "Difficulty should clamp to 2.0") + + # Test Lower Bound Clamping + _resource.difficulty = 0.1 + assert_eq(_resource.difficulty, 0.5, "Difficulty should clamp to 0.5") + + +## GS-RES-04/05/06 | Validate boundary and default values +func test_gs_res_04_05_06_boundary_values() -> void: + var values_to_test: Array = [0.5, 1.0, 2.0] + for val: float in values_to_test: + _resource.difficulty = val + assert_eq(_resource.difficulty, val, "Value %s should be accepted exactly" % val) + + +## GS-RES-07 | Validate stability on redundant assignments +func test_gs_res_07_redundant_assignment() -> void: + _resource.difficulty = 1.2 + watch_signals(_resource) + _resource.difficulty = 1.2 + # Corrected function name from assert_signal_emission_count to assert_signal_emit_count + assert_signal_emit_count(_resource, "setting_changed", 0, "Redundant assignment should not re-emit") + + +# --- SECTION 3: MENU INITIALIZATION TESTS (GS-READY) --- + +## GS-READY-01/02 | Confirm UI syncs to resource state on load +func test_gs_ready_01_02_ui_initialization_sync() -> void: + # Pre-condition: Set resource to non-default value before menu _ready() + # Note: Requires re-instantiating or checking logic after add_child + var test_difficulty: float = 1.7 + _resource.difficulty = test_difficulty + + var new_menu: Variant = load("res://scenes/gameplay_settings.tscn").instantiate() + add_child_autofree(new_menu) + await get_tree().process_frame + + assert_eq(new_menu.difficulty_slider.value, test_difficulty, "Slider must match resource on init") + assert_eq(new_menu.difficulty_label.text, "{" + str(test_difficulty) + "}", "Label must match resource on init") + + +## GS-READY-03/04 | Confirm signal connections +func test_gs_ready_03_04_signal_connections() -> void: + assert_true(_resource.setting_changed.is_connected(gameplay_menu._on_external_setting_changed), + "UI must connect to resource observer on ready") + assert_true(gameplay_menu.difficulty_slider.value_changed.is_connected(gameplay_menu._on_difficulty_value_changed), + "Slider signal should be connected") + + +## GS-READY-05 | Prevent duplicate connections +func test_gs_ready_05_no_duplicate_connections() -> void: + # Ensure the menu is in the tree + if not gameplay_menu.is_inside_tree(): + add_child(gameplay_menu) + await get_tree().process_frame + + # Manually call _ready again to test idempotency/guards + # The production code guards should prevent double-connection + gameplay_menu._ready() + + # Check connections on the GLOBAL resource + var connections: Array = Globals.settings.setting_changed.get_connections() + var count: int = 0 + + for conn: Dictionary in connections: + # We must verify the callable points to our specific instance + if conn["callable"].get_object() == gameplay_menu and \ + conn["callable"].get_method() == "_on_external_setting_changed": + count += 1 + + assert_eq(count, 1, "Observer should only be connected once even after redundant ready calls") + + +## GS-READY-06 | Robustness against missing web features +func test_gs_ready_06_safe_init_non_web() -> void: + # Simulate non-web environment + var mock_os: Variant = double(OSWrapper).new() + stub(mock_os, "has_feature").to_return(false) + + # FIX: Instantiate from the SCENE, not just the script + var menu: Control = load("res://scenes/gameplay_settings.tscn").instantiate() + menu.os_wrapper = mock_os + + add_child_autofree(menu) + await get_tree().process_frame + + assert_true(is_instance_valid(menu.difficulty_slider), "Slider should be valid when initialized from scene") + assert_true(true, "Menu initialized safely in non-web environment") + assert_null(menu._change_difficulty_cb, "JS callbacks should NOT be created in non-web env") + assert_false(menu.js_window != null, "JS window interface should remain null") diff --git a/test/gut/test_game_settings_resource.gd.uid b/test/gut/test_game_settings_resource.gd.uid new file mode 100644 index 00000000..4642ae5a --- /dev/null +++ b/test/gut/test_game_settings_resource.gd.uid @@ -0,0 +1 @@ +uid://ccmx64y6uy3dk diff --git a/test/gut/test_gameplay_settings_js.gd b/test/gut/test_gameplay_settings_js.gd new file mode 100644 index 00000000..c7a7f5fd --- /dev/null +++ b/test/gut/test_gameplay_settings_js.gd @@ -0,0 +1,104 @@ +## Copyright (C) 2026 Egor Kostan +## SPDX-License-Identifier: GPL-3.0-or-later +## test_gameplay_settings_js.gd +## +## GS-JS: Defensive regression suite for JavaScriptBridge communication. +## Focuses on payload shapes, malformed input safety, and primitive regressions. + +extends "res://addons/gut/test.gd" + +const GameplaySettings = preload("res://scripts/gameplay_settings.gd") +var gameplay_menu: Control + +func before_each() -> void: + # Fresh resource to isolate state + Globals.settings = GameSettingsResource.new() + gameplay_menu = load("res://scenes/gameplay_settings.tscn").instantiate() + # Inject mock wrapper to simulate web environment + gameplay_menu.os_wrapper = OSWrapper.new() + add_child_autofree(gameplay_menu) + await get_tree().process_frame + + +# --- SECTION 6.1: SUPPORTED PAYLOAD SHAPES (GS-JS-01 to 05) --- + +## GS-JS-01/02 | Valid nested array extraction +func test_gs_js_01_02_nested_array_success() -> void: + # Standard JS Bridge format: [ [value] ] + gameplay_menu._on_change_difficulty_js([[1.5]]) + assert_eq(Globals.settings.difficulty, 1.5, "Should extract 1.5 from nested array") + + gameplay_menu._on_change_difficulty_js([[0.8]]) + assert_eq(Globals.settings.difficulty, 0.8, "Should extract 0.8 from nested array") + + +## GS-JS-03 | Nested numeric string coercion +func test_gs_js_03_numeric_string_coercion() -> void: + # Test if "1.5" is correctly cast to float 1.5 + gameplay_menu._on_change_difficulty_js([["1.5"]]) + assert_eq(Globals.settings.difficulty, 1.5, "Should coerce numeric string to float") + + +## GS-JS-04/05 | JS-originated values are clamped by resource +func test_gs_js_04_05_out_of_range_clamping() -> void: + gameplay_menu._on_change_difficulty_js([[5.0]]) + assert_eq(Globals.settings.difficulty, 2.0, "Input 5.0 should be clamped to max 2.0") + + gameplay_menu._on_change_difficulty_js([[0.1]]) + assert_eq(Globals.settings.difficulty, 0.5, "Input 0.1 should be clamped to min 0.5") + + +# --- SECTION 6.2 & 6.3: MALFORMED INPUT & PRIMITIVE SAFETY (GS-JS-10 to 25) --- + +## GS-JS-10/11 | Handle empty arrays safely +func test_gs_js_10_11_empty_array_safety() -> void: + var initial_val: float = Globals.settings.difficulty + gameplay_menu._on_change_difficulty_js([]) + gameplay_menu._on_change_difficulty_js([[]]) + assert_eq(Globals.settings.difficulty, initial_val, "Empty arrays should not change state or crash") + + +## GS-JS-12/14 | Handle non-numeric and whitespace strings +func test_gs_js_12_14_malformed_string_safety() -> void: + var initial_val: float = Globals.settings.difficulty + gameplay_menu._on_change_difficulty_js([["abc"]]) + gameplay_menu._on_change_difficulty_js([[" "]]) + assert_eq(Globals.settings.difficulty, initial_val, "Malformed strings should be rejected safely") + + +## GS-JS-20/21 | SCALAR REGRESSION - CRITICAL FIX FOR ISSUE #471 +func test_gs_js_20_21_scalar_safety() -> void: + var initial_val: float = Globals.settings.difficulty + gameplay_menu._on_change_difficulty_js([1.5]) + # Precise Assertion: The value should be accepted, not just "not crash" + assert_eq(Globals.settings.difficulty, 1.5, "Scalar float should be accepted") + + gameplay_menu._on_change_difficulty_js(["invalid"]) + # Precise Assertion: Malformed scalar should be rejected, leaving value at 1.5 + assert_eq(Globals.settings.difficulty, 1.5, "Malformed scalar string should be rejected") + + +## GS-JS-22/25 | Unsupported primitives and objects +func test_gs_js_22_25_unsupported_type_safety() -> void: + var initial_val: float = Globals.settings.difficulty + gameplay_menu._on_change_difficulty_js([null]) + gameplay_menu._on_change_difficulty_js([true]) + # Precise Assertion: Ensure the state is untouched + assert_eq(Globals.settings.difficulty, initial_val, "Unsupported types should not modify state") + + +# --- SECTION 6.4: MISSING NODE SAFETY (GS-JS-30 to 32) --- + +## GS-JS-30 | JS callback handles missing slider node +func test_gs_js_30_missing_node_safety() -> void: + # Force the slider to be null to simulate a late callback during teardown + gameplay_menu.difficulty_slider.free() + + # Function should check 'is_instance_valid' before accessing slider properties + gameplay_menu._on_change_difficulty_js([[1.2]]) + # assert_true(true, "Handled missing node reference safely without crash") + assert_eq( + Globals.settings.difficulty, + 1.2, + "When slider is missing, callback should still update resource via fallback path" + ) diff --git a/test/gut/test_gameplay_settings_js.gd.uid b/test/gut/test_gameplay_settings_js.gd.uid new file mode 100644 index 00000000..1d1e6ca3 --- /dev/null +++ b/test/gut/test_gameplay_settings_js.gd.uid @@ -0,0 +1 @@ +uid://dugpu2v4a5t4d diff --git a/test/gut/test_gameplay_settings_lifecycle.gd b/test/gut/test_gameplay_settings_lifecycle.gd new file mode 100644 index 00000000..aec87143 --- /dev/null +++ b/test/gut/test_gameplay_settings_lifecycle.gd @@ -0,0 +1,135 @@ +## Copyright (C) 2026 Egor Kostan +## SPDX-License-Identifier: GPL-3.0-or-later +## test_gameplay_settings_lifecycle.gd +## +## GS-LIFE: Verifies cleanup, menu restoration, and web overlay teardown. + +extends "res://addons/gut/test.gd" + +const GameplaySettings = preload("res://scripts/gameplay_settings.gd") +var gameplay_menu: Control + +func before_each() -> void: + Globals.settings = GameSettingsResource.new() + gameplay_menu = load("res://scenes/gameplay_settings.tscn").instantiate() + gameplay_menu.os_wrapper = OSWrapper.new() + add_child_autofree(gameplay_menu) + await get_tree().process_frame + +# --- SECTION 7: LIFECYCLE AND CLEANUP TESTS --- + +## GS-LIFE-01 | Cleanup on tree exit verifies signals and ALL callbacks +func test_gs_life_01_cleanup_on_exit() -> void: + # 1. Setup: Ensure everything is connected first + assert_true(Globals.settings.setting_changed.is_connected(gameplay_menu._on_external_setting_changed)) + assert_true(gameplay_menu.difficulty_slider.value_changed.is_connected(gameplay_menu._on_difficulty_value_changed)) + + # 2. Act: Trigger exit + gameplay_menu._on_tree_exited() + + # 3. Assert Signal Disconnections (The missing piece) + assert_false(Globals.settings.setting_changed.is_connected(gameplay_menu._on_external_setting_changed), + "Global resource signal should be disconnected") + assert_false(gameplay_menu.difficulty_slider.value_changed.is_connected(gameplay_menu._on_difficulty_value_changed), + "Local UI signals should be disconnected") + + # 4. Assert ALL Callbacks (Including the missing reset callback) + assert_null(gameplay_menu._change_difficulty_cb, "Difficulty callback nullified") + assert_null(gameplay_menu._gameplay_back_button_pressed_cb, "Back button callback nullified") + assert_null(gameplay_menu._gameplay_reset_cb, "Reset callback nullified") + + +## GS-LIFE-02 | Back button restores previous menu from stack +func test_gs_life_02_back_button_restoration() -> void: + # Mock a previous menu + var mock_prev: Control = Control.new() + autofree(mock_prev) # Ensures cleanup even on test failure + mock_prev.name = "MockOptionsMenu" + mock_prev.visible = false + Globals.hidden_menus.push_back(mock_prev) + + gameplay_menu._on_gameplay_back_button_pressed() + + assert_true(mock_prev.visible, "Previous menu should be visible again") + assert_true(gameplay_menu.is_queued_for_deletion(), "Menu should be freed") + + +## GS-LIFE-08 | Web overlay visibility cleanup +func test_gs_life_08_web_overlay_cleanup() -> void: + var test_menu: Control = load("res://scenes/gameplay_settings.tscn").instantiate() + var mock_js_bridge: Variant = double(JavaScriptBridgeWrapper).new() + var mock_os: Variant = double(OSWrapper).new() + + stub(mock_os, "has_feature").to_return(true) + + # FIX: Use a Dictionary. It allows the script to call + # js_window.changeDifficulty = ... without crashing. + var mock_window: Dictionary = {} + stub(mock_js_bridge, "get_interface").to_return(mock_window) + + test_menu.os_wrapper = mock_os + test_menu.js_bridge_wrapper = mock_js_bridge + + add_child_autofree(test_menu) + await get_tree().process_frame + + test_menu._on_tree_exited() + assert_called(mock_js_bridge, "eval") + + +## GS-LIFE-05 | Cleanup handles null Globals gracefully +func test_gs_life_05_null_globals_safety() -> void: + # FIX: Wrap the lambda in a JavaScriptObject callback + var dummy_callable := func(_args: Array) -> void: pass + gameplay_menu._change_difficulty_cb = JavaScriptBridge.create_callback(dummy_callable) + + # Act: Call the cleanup function directly + gameplay_menu._on_tree_exited() + + # Assert: Verify the side effects of the cleanup logic + assert_null(gameplay_menu._change_difficulty_cb, "Callback must be nullified even if Globals are shaky") + + +## GS-LIFE-09 | Unexpected removal (unintentional exit) restores previous menu +func test_gs_life_09_unexpected_removal_restoration() -> void: + # 1. Setup: Create fresh instance but do not parent yet + var test_menu: Control = load("res://scenes/gameplay_settings.tscn").instantiate() + + # 2. Mock a previous menu in the stack + var mock_prev: Control = Control.new() + mock_prev.name = "MockOptionsMenu" + mock_prev.visible = false + Globals.hidden_menus.push_back(mock_prev) + + # 3. Inject Mock Wrappers + var mock_js_bridge: Variant = double(JavaScriptBridgeWrapper).new() + var mock_os: Variant = double(OSWrapper).new() + stub(mock_os, "has_feature").to_return(true) + + # 4. Use a Dictionary for the JS window mock to allow property setting + var mock_window: Dictionary = {"valid": true} + stub(mock_js_bridge, "get_interface").to_return(mock_window) + + test_menu.os_wrapper = mock_os + test_menu.js_bridge_wrapper = mock_js_bridge + + # 5. Add to tree to trigger _ready() + add_child_autofree(test_menu) + await get_tree().process_frame + + # --- THE CRITICAL FIX --- + # Manually force the script's internal 'js_window' to our mock. + # This ensures the 'if js_window' check in _on_tree_exited passes. + test_menu.js_window = mock_window + # ------------------------ + + # 6. Act: Trigger unexpected exit directly (_intentional_exit remains false) + test_menu._on_tree_exited() + + # 7. Assertions + assert_true(mock_prev.visible, "Unexpected exit must restore previous menu visibility") + assert_null(test_menu._change_difficulty_cb, "Callbacks must still be nullified on unexpected exit") + assert_called(mock_js_bridge, "eval") + + # Cleanup mock menu + mock_prev.free() diff --git a/test/gut/test_gameplay_settings_lifecycle.gd.uid b/test/gut/test_gameplay_settings_lifecycle.gd.uid new file mode 100644 index 00000000..7756c224 --- /dev/null +++ b/test/gut/test_gameplay_settings_lifecycle.gd.uid @@ -0,0 +1 @@ +uid://daht74benffiu diff --git a/test/gut/test_gameplay_settings_ui.gd b/test/gut/test_gameplay_settings_ui.gd new file mode 100644 index 00000000..3e789ef1 --- /dev/null +++ b/test/gut/test_gameplay_settings_ui.gd @@ -0,0 +1,87 @@ +## Copyright (C) 2026 Egor Kostan +## SPDX-License-Identifier: GPL-3.0-or-later +## test_gameplay_settings_ui.gd +## +## GUT unit tests for GameplaySettings UI interactions and Observer reactivity. +## Covers GS-UI-01 to GS-UI-06 and GS-OBS-01 to GS-OBS-05. + +extends "res://addons/gut/test.gd" + +const GameplaySettings = preload("res://scripts/gameplay_settings.gd") +var gameplay_menu: Control +var _resource: GameSettingsResource + +func before_each() -> void: + # Ensure a fresh resource for every test to isolate state + _resource = GameSettingsResource.new() + Globals.settings = _resource + + gameplay_menu = load("res://scenes/gameplay_settings.tscn").instantiate() + # Inject mock wrapper to bypass real web/OS calls + gameplay_menu.os_wrapper = OSWrapper.new() + + add_child_autofree(gameplay_menu) + await get_tree().process_frame + + +# --- SECTION 4: LOCAL UI INTERACTION TESTS (GS-UI) --- + +## GS-UI-01/03 | User changes slider updates resource and label +func test_gs_ui_01_03_slider_updates_resource_and_label() -> void: + var test_val: float = 1.5 + # Simulate user sliding the control + gameplay_menu.difficulty_slider.value = test_val + gameplay_menu._on_difficulty_value_changed(test_val) + + assert_eq(_resource.difficulty, test_val, "Resource should update from slider input") + assert_eq(gameplay_menu.difficulty_label.text, "{" + str(test_val) + "}", "Label should reflect new value") + + +## GS-UI-04/05 | Reset button returns to default state +func test_gs_ui_04_05_reset_functionality() -> void: + # Pre-condition: Set to non-default + gameplay_menu.difficulty_slider.value = 2.0 + gameplay_menu._on_gameplay_reset_button_pressed() + + assert_eq(_resource.difficulty, 1.0, "Resource should reset to 1.0") + assert_eq(gameplay_menu.difficulty_slider.value, 1.0, "Slider UI should reset to 1.0") + + +## GS-UI-06 | Change propagation occurs exactly once +func test_gs_ui_06_no_duplicate_propagation() -> void: + watch_signals(_resource) + gameplay_menu._on_difficulty_value_changed(1.8) + + # Verify the resource was only updated once to prevent event loops + assert_signal_emit_count(_resource, "setting_changed", 1, "Change should propagate exactly once") + + +# --- SECTION 5: EXTERNAL RESOURCE REACTIVITY TESTS (GS-OBS) --- + +## GS-OBS-01/02/04 | UI tracks external resource changes (Observer Pattern) +func test_gs_obs_01_02_04_external_reactivity() -> void: + var external_val: float = 1.3 + # Simulate external code changing the global resource + _resource.setting_changed.emit("difficulty", external_val) + + assert_eq(gameplay_menu.difficulty_slider.value, external_val, "Slider must sync with external changes") + assert_eq(gameplay_menu.difficulty_label.text, "{" + str(external_val) + "}", "Label must sync with external changes") + + +## GS-OBS-03 | set_value_no_signal prevents feedback loops +func test_gs_obs_03_no_recursion_on_external_update() -> void: + watch_signals(gameplay_menu.difficulty_slider) + # Trigger the observer + _resource.setting_changed.emit("difficulty", 0.7) + + # Verify the slider updated without re-emitting its own value_changed signal + assert_signal_emit_count(gameplay_menu.difficulty_slider, "value_changed", 0, "Observer must use set_value_no_signal to avoid loops") + + +## GS-OBS-05 | Observer filters unrelated keys +func test_gs_obs_05_filters_unrelated_settings() -> void: + var initial_val: float = gameplay_menu.difficulty_slider.value + # Emit a signal for a different setting + _resource.setting_changed.emit("sfx_volume", 0.1) + + assert_eq(gameplay_menu.difficulty_slider.value, initial_val, "Difficulty UI should ignore unrelated setting signals") diff --git a/test/gut/test_gameplay_settings_ui.gd.uid b/test/gut/test_gameplay_settings_ui.gd.uid new file mode 100644 index 00000000..a7ad574a --- /dev/null +++ b/test/gut/test_gameplay_settings_ui.gd.uid @@ -0,0 +1 @@ +uid://crubwx87ecwdp diff --git a/test/gut/test_globals_resource.gd b/test/gut/test_globals_resource.gd index 889e28ed..9d1ba609 100644 --- a/test/gut/test_globals_resource.gd +++ b/test/gut/test_globals_resource.gd @@ -65,13 +65,13 @@ func test_difficulty_clamping() -> void: func test_scene_resource_validity() -> void: gut.p("Testing: PackedScenes in Resource are valid and preloaded.") - # Verifies that migrating paths to Resources doesn't break preloading [cite: 4] + # Verifies that migrating paths to Resources doesn't break preloading assert_not_null(Globals.settings.key_mapping_scene, "Key mapping scene must be assigned in Resource") assert_true(Globals.settings.key_mapping_scene is PackedScene, "Key mapping should be a PackedScene") func test_remap_prompt_strings() -> void: gut.p("Testing: Remap prompt strings are correctly retrieved from Resource.") - # Verifies migration of hard-coded constants [cite: 3] + # Verifies migration of hard-coded constants assert_eq(Globals.settings.remap_prompt_keyboard, "Press a key...", "Keyboard prompt mismatch") assert_string_contains(Globals.settings.remap_prompt_gamepad, "gamepad", "Gamepad prompt should mention device") diff --git a/test/gut/test_settings_observer.gd b/test/gut/test_settings_observer.gd index 10a87d55..975a68d9 100644 --- a/test/gut/test_settings_observer.gd +++ b/test/gut/test_settings_observer.gd @@ -105,7 +105,7 @@ func test_difficulty_persists_to_config_file() -> void: ## PHASE 4: UI Synchronization (Mocking the UI Layer) func test_ui_logic_can_update_resource_without_globals_call() -> void: - # This simulates what advanced_settings.gd or gameplay_settings.gd will do [cite: 14, 33] + # This simulates what advanced_settings.gd or gameplay_settings.gd will do # The goal is to verify that setting the value is the ONLY thing the UI needs to do. _resource.difficulty = 1.2 assert_eq(_resource.difficulty, 1.2, "UI should successfully update the resource state.") diff --git a/tests/difficulty_flow_test.py b/tests/difficulty_flow_test.py index 239a2742..b144ac15 100644 --- a/tests/difficulty_flow_test.py +++ b/tests/difficulty_flow_test.py @@ -171,10 +171,12 @@ def on_console(msg: Any) -> None: page.evaluate("window.changeDifficulty([2.0])") page.wait_for_timeout(2500) new_logs = logs[pre_change_log_count:] + assert any( - "setting 'difficulty' updated to: 2.0" in log["text"].lower() + "js difficulty callback called with valid value: 2.0" in log["text"].lower() for log in new_logs - ), "Failed to set difficulty to 2.0" + ), "Failed to extract/validate difficulty 2.0 from JS payload" + assert any( "settings saved" in log["text"].lower() for log in new_logs ), "Failed to save the settings" @@ -187,12 +189,12 @@ def on_console(msg: Any) -> None: page.evaluate("window.gameplayResetPressed([])") page.wait_for_timeout(2500) reset_logs: List[Dict[str, str]] = logs[pre_reset_log_count:] + # Verify that difficulty was reset to the expected default assert any( - "difficulty" in log["text"].lower() - and ("default" in log["text"].lower() or "1.0" in log["text"]) + "setting 'difficulty' updated to: 1" in log["text"].lower() for log in reset_logs - ), "Difficulty reset to default was not observed in logs" + ), "Resource did not reset difficulty to 1.0 after reset button press" # Set difficulty to 2.0 again page.evaluate("window.changeDifficulty([2.0])") @@ -216,7 +218,8 @@ def on_console(msg: Any) -> None: # Gameplay UI hidden page.wait_for_selector("#difficulty-slider", state="hidden", timeout=2500) assert page.evaluate( - "document.getElementById('difficulty-slider') === null || document.getElementById('difficulty-slider').offsetParent === null" + "document.getElementById('difficulty-slider') === null || document.getElementById(" + "'difficulty-slider').offsetParent === null" ) # Check element present @@ -230,7 +233,8 @@ def on_console(msg: Any) -> None: assert page.evaluate("document.getElementById('start-button') !== null") page.wait_for_selector("#options-back-button", state="hidden", timeout=2500) assert page.evaluate( - "document.getElementById('options-back-button') === null || document.getElementById('options-back-button').offsetParent === null" + "document.getElementById('options-back-button') === null || document.getElementById(" + "'options-back-button').offsetParent === null" ) # Start game