diff --git a/scripts/advanced_settings.gd b/scripts/advanced_settings.gd index 1684aefd..722fe3dd 100644 --- a/scripts/advanced_settings.gd +++ b/scripts/advanced_settings.gd @@ -264,11 +264,12 @@ func _on_log_level_item_selected(index: int) -> void: var selected_enum: Globals.LogLevel = log_level_display_to_enum.get( selected_name, Globals.LogLevel.INFO ) + # Only update the resource; the Observer handles the rest Globals.settings.current_log_level = selected_enum log_lvl_option.selected = Globals.LogLevel.values().find(selected_enum) # Temporary raw print to bypass log_message Globals.log_message("Log level changed to: " + selected_name, Globals.LogLevel.DEBUG) - Globals._save_settings() + # Globals._save_settings() # New: JS-specific callback (exactly one Array arg, no default) diff --git a/scripts/game_settings_resource.gd b/scripts/game_settings_resource.gd index 80a35c72..58983bf2 100644 --- a/scripts/game_settings_resource.gd +++ b/scripts/game_settings_resource.gd @@ -3,25 +3,49 @@ ## game_settings_resource.gd ## ## DATA CONTAINER: This Resource serves as the central "Source of Truth" for game configuration. -## It decouples static data (difficulty, paths, log levels) from logic found in Globals.gd. -## By using a Resource instead of hard-coded variables, settings can be swapped at runtime -## or modified visually via the Godot Inspector without touching the codebase. - +## It decouples static data from logic found in Globals.gd. class_name GameSettingsResource extends Resource +## SIGNAL: setting_changed(setting_name: String, new_value: Variant) +## +## This signal is the core of the Observer Pattern for game settings. +## It is automatically emitted by property setters whenever a value is updated. +## This allows external systems (like Globals.gd) to react to data changes +## without the UI having to explicitly call persistence or logging methods. +signal setting_changed(setting_name: String, new_value: Variant) + @export_group("Logging") # Current log level: 0=DEBUG, 1=INFO, 2=WARNING, 3=ERROR, 4=NONE -@export_range(0, 4, 1) var current_log_level: int = 1 -@export var enable_debug_logging: bool = false +@export_range(0, 4, 1) var current_log_level: int = 1: + set(value): + var new_value: int = clampi(value, 0, 4) + if _current_log_level == new_value: + return + _current_log_level = new_value + setting_changed.emit("current_log_level", new_value) + get: + return _current_log_level + +@export var enable_debug_logging: bool = false: + set(value): + var new_value: bool = bool(value) + if _enable_debug_logging == new_value: + return + _enable_debug_logging = new_value + setting_changed.emit("enable_debug_logging", new_value) + get: + return _enable_debug_logging @export_group("Gameplay") # Multiplier: 1.0=Normal, <1=Easy, >1=Hard -# In globals.gd, change the difficulty variable in the Resource script: -# game_settings_resource.gd @export var difficulty: float = 1.0: set(value): - _difficulty = clamp(value, 0.5, 2.0) + var new_val: float = clamp(value, 0.5, 2.0) + if _difficulty == new_val: + return # Break the recursion here + _difficulty = new_val + setting_changed.emit("difficulty", _difficulty) get: return _difficulty @@ -31,4 +55,7 @@ extends Resource @export var key_mapping_scene: PackedScene = preload("res://scenes/key_mapping_menu.tscn") @export var options_scene: PackedScene = preload("res://scenes/options_menu.tscn") +# Private member variables moved to bottom to satisfy class-definitions-order +var _current_log_level: int = 1 var _difficulty: float = 1.0 +var _enable_debug_logging: bool = false diff --git a/scripts/gameplay_settings.gd b/scripts/gameplay_settings.gd index 42a52fcb..fee481b6 100644 --- a/scripts/gameplay_settings.gd +++ b/scripts/gameplay_settings.gd @@ -40,6 +40,10 @@ func _ready() -> void: # NEW: Attach tree_exited for unexpected removal cleanup (like other settings scripts) 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 os_wrapper.has_feature("web"): # Toggle overlays... ( @@ -80,12 +84,25 @@ func _ready() -> void: Globals.log_message("Gameplay Settings menu loaded.", Globals.LogLevel.DEBUG) +func _on_external_setting_changed(setting_name: String, new_value: Variant) -> void: + 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. + difficulty_slider.set_value_no_signal(float(new_value)) + difficulty_label.text = "{" + str(new_value) + "}" + + 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) + # Disconnect the global resource observer to prevent stale references + if Globals.settings.setting_changed.is_connected(_on_external_setting_changed): + Globals.settings.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) @@ -238,11 +255,11 @@ func _on_difficulty_value_changed(value: float) -> void: ## :param value: The new slider value. ## :type value: float ## :rtype: void + # Update the resource first (this triggers clamping in the setter) 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(value) + "}" - Globals.log_message("Difficulty changed to: " + str(value), Globals.LogLevel.DEBUG) - Globals._save_settings() + difficulty_label.text = "{" + str(Globals.settings.difficulty) + "}" # New: JS-specific callback (exactly one Array arg, no default) diff --git a/scripts/globals.gd b/scripts/globals.gd index c8396aca..7b969aa4 100644 --- a/scripts/globals.gd +++ b/scripts/globals.gd @@ -33,6 +33,7 @@ var next_scene: String = "" # Path to the next scene to load via loading screen ## Last selected input device for UI messages and labels. ## Updated when player toggles Keyboard/Gamepad in Key Mapping. var current_input_device: String = "keyboard" # "keyboard" or "gamepad" +var _is_loading_settings: bool = false # Guard flag func _ready() -> void: @@ -49,10 +50,26 @@ func _ready() -> void: settings.current_log_level = LogLevel.DEBUG log_message("Log level set to: " + LogLevel.keys()[settings.current_log_level], LogLevel.DEBUG) _load_settings() # Load persisted settings first - # Load last input device early to fix unbound warning on first load when - # gamepad is saved preference. - # Ensures has_unbound_critical_actions_for_current_device() uses correct device from config. - # Settings.load_last_input_device() + + # Connect to the resource signal to centralize side effects [cite: 151] + if settings: + settings.setting_changed.connect(_on_setting_changed) + + +## Reactive handler for the Observer Pattern [cite: 141] +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: + return + + # 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] + log_message(log_msg, LogLevel.DEBUG) + + # Automatically persist to disk [cite: 53] + _save_settings() ## Centralized "ensure initial focus" helper. @@ -129,6 +146,9 @@ func _load_settings(path: String = Settings.CONFIG_PATH) -> void: var config: ConfigFile = ConfigFile.new() var err: int = config.load(path) if err == OK: + # Enable the guard before starting bulk updates + _is_loading_settings = true + if config.has_section_key("Settings", "log_level"): var loaded_log_level: Variant = config.get_value("Settings", "log_level") if ( @@ -158,6 +178,21 @@ func _load_settings(path: String = Settings.CONFIG_PATH) -> void: "Invalid type for difficulty: " + str(typeof(loaded_difficulty)), LogLevel.WARNING ) + + # NEW: Load the debug logging flag + if config.has_section_key("Settings", "enable_debug_logging"): + var loaded_debug: Variant = config.get_value("Settings", "enable_debug_logging") + if loaded_debug is bool: + settings.enable_debug_logging = loaded_debug + log_message( + "Loaded saved debug logging: " + str(settings.enable_debug_logging), + LogLevel.DEBUG + ) + + # Disable the guard and log a single summary instead + _is_loading_settings = false + log_message("All settings loaded and synchronized.", LogLevel.DEBUG) + elif err == ERR_FILE_NOT_FOUND: log_message("No settings config found, using defaults.", LogLevel.DEBUG) else: @@ -176,6 +211,8 @@ func _save_settings(path: String = Settings.CONFIG_PATH) -> void: config.set_value("Settings", "log_level", settings.current_log_level) config.set_value("Settings", "difficulty", settings.difficulty) + # NEW: Persist the debug logging flag + config.set_value("Settings", "enable_debug_logging", settings.enable_debug_logging) err = config.save(path) if err != OK: log_message("Failed to save settings: " + str(err), LogLevel.ERROR) diff --git a/test/gut/test_settings_observer.gd b/test/gut/test_settings_observer.gd new file mode 100644 index 00000000..10a87d55 --- /dev/null +++ b/test/gut/test_settings_observer.gd @@ -0,0 +1,156 @@ +## Copyright (C) 2026 Egor Kostan +## SPDX-License-Identifier: GPL-3.0-or-later +## test_settings_observer.gd +## +## TEST SUITE: Verifies the Observer Pattern implementation for game settings. +## This suite ensures that UI-driven changes to GameSettingsResource correctly +## emit signals and that Globals.gd reacts by persisting data, thereby +## decoupling UI logic from the persistence layer. +extends GutTest + +var _resource: GameSettingsResource +var _test_config_path: String = "user://test_settings.cfg" + + +func before_each() -> void: + _resource = GameSettingsResource.new() + # Ensure the Singleton uses our local test resource to avoid + # cross-test contamination and production file overwrites. + Globals.settings = _resource + + if FileAccess.file_exists(_test_config_path): + DirAccess.remove_absolute(_test_config_path) + + +## PHASE 1: Signal Integrity (The "Subject") +func test_resource_emits_signal_on_difficulty_change() -> void: + watch_signals(_resource) + _resource.difficulty = 1.5 + + assert_signal_emitted(_resource, "setting_changed", "Signal should fire when difficulty is set.") + assert_signal_emitted_with_parameters(_resource, "setting_changed", ["difficulty", 1.5], 0) + + +func test_resource_emits_signal_on_log_level_change() -> void: + watch_signals(_resource) + _resource.current_log_level = Globals.LogLevel.DEBUG + + assert_signal_emitted(_resource, "setting_changed", "Signal should fire when log level is set.") + assert_signal_emitted_with_parameters(_resource, "setting_changed", ["current_log_level", Globals.LogLevel.DEBUG], 0) + + +## PHASE 2: Data Validation & Clamping +func test_difficulty_clamping_emits_correct_value() -> void: + watch_signals(_resource) + # Difficulty is clamped between 0.5 and 2.0 + _resource.difficulty = 5.0 + + # Fix: Use exactly 4 arguments. + # Parameters are: (object, signal_name, expected_params, emission_index) + assert_signal_emitted_with_parameters(_resource, "setting_changed", ["difficulty", 2.0], 0) + + +## PHASE 3: Persistence (The "Observer") +func test_globals_saves_to_disk_on_signal() -> void: + # Use the test-specific path consistently + _resource.setting_changed.connect( + func(key: String, val: Variant) -> void: + Globals._save_settings(_test_config_path) + ) + + # Force reset state before changing + _resource.difficulty = 0.85 + + var config := ConfigFile.new() + var err := config.load(_test_config_path) + assert_eq(err, OK, "Config file should be created.") + assert_eq(config.get_value("Settings", "difficulty"), 0.85) + + +## PHASE 3.1: Verify Globals connection (The "Observer") +func test_globals_saves_when_resource_changes() -> void: + # Manually connect the observer logic to the test resource + # This bypasses the _ready() logic that would overwrite our test setup + _resource.setting_changed.connect( + func(_k: String, _v: Variant) -> void: + Globals._save_settings(_test_config_path) + ) + + # Trigger the change + _resource.difficulty = 0.8 + + # Verify persistence to the test config path + var config := ConfigFile.new() + var err := config.load(_test_config_path) + assert_eq(err, OK, "Config file should be created by the observer.") + assert_eq(config.get_value("Settings", "difficulty"), 0.8, "Value on disk should be updated.") + + +## PHASE 3.2: Persistence Verification +func test_difficulty_persists_to_config_file() -> void: + # We must intercept the signal to force the TEST path, + # otherwise Globals uses the production path by default. + _resource.setting_changed.connect( + func(_k: String, _v: Variant) -> void: + Globals._save_settings(_test_config_path) + ) + + _resource.difficulty = 0.75 + + var config := ConfigFile.new() + var err := config.load(_test_config_path) + assert_eq(err, OK, "Config file should exist after change.") + assert_eq(config.get_value("Settings", "difficulty"), 0.75) + + +## 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] + # 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.") + + +## PHASE 5: UI Reactivity +func test_ui_slider_syncs_with_resource() -> void: + var gameplay_menu: Node = load("res://scenes/gameplay_settings.tscn").instantiate() + add_child_autofree(gameplay_menu) + + Globals.settings.difficulty = 1.8 # Change resource directly + assert_eq(gameplay_menu.difficulty_slider.value, 1.8, "UI Slider should update automatically.") + + +## PHASE 6: Debug Logging Persistence +func test_enable_debug_logging_emits_signal() -> void: + watch_signals(_resource) + _resource.enable_debug_logging = true + + assert_signal_emitted_with_parameters(_resource, "setting_changed", ["enable_debug_logging", true], 0) + +func test_enable_debug_logging_persists_to_disk() -> void: + # Connect signal to the test path for verification + _resource.setting_changed.connect( + func(_k: String, _v: Variant) -> void: + Globals._save_settings(_test_config_path) + ) + + # Act: Toggle the flag + _resource.enable_debug_logging = true + + # Assert: Verify file contents + var config := ConfigFile.new() + var err := config.load(_test_config_path) + assert_eq(err, OK, "Config file should be created for debug_logging change.") + assert_eq(config.get_value("Settings", "enable_debug_logging"), true, "Flag should persist as true.") + +func test_enable_debug_logging_restores_from_disk() -> void: + # Setup: Manually create a config with the flag enabled + var config := ConfigFile.new() + config.set_value("Settings", "enable_debug_logging", true) + config.save(_test_config_path) + + # Act: Load via Globals logic + Globals._load_settings(_test_config_path) + + # Assert: Resource should now match the disk state + assert_eq(_resource.enable_debug_logging, true, "Resource should reflect loaded debug flag.") diff --git a/test/gut/test_settings_observer.gd.uid b/test/gut/test_settings_observer.gd.uid new file mode 100644 index 00000000..60992a73 --- /dev/null +++ b/test/gut/test_settings_observer.gd.uid @@ -0,0 +1 @@ +uid://dgvy06sdtxf6l diff --git a/tests/difficulty_flow_test.py b/tests/difficulty_flow_test.py index d7dea2b9..239a2742 100644 --- a/tests/difficulty_flow_test.py +++ b/tests/difficulty_flow_test.py @@ -172,7 +172,8 @@ def on_console(msg: Any) -> None: page.wait_for_timeout(2500) new_logs = logs[pre_change_log_count:] assert any( - "difficulty changed to: 2.0" in log["text"].lower() for log in new_logs + "setting 'difficulty' updated to: 2.0" in log["text"].lower() + for log in new_logs ), "Failed to set difficulty to 2.0" assert any( "settings saved" in log["text"].lower() for log in new_logs