-
-
Notifications
You must be signed in to change notification settings - Fork 1
Adopt observer-based settings with automatic save and UI sync #469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
8bd8398
test_settings_observer.gd
ikostan f99af33
Observer Pattern
ikostan 7b04056
Update game_settings_resource.gd
ikostan 94b2b23
Update gameplay_settings.gd
ikostan 9b57178
Update globals.gd
ikostan 5a467e6
issue (bug_risk): New connection to Globals.settings.setting_changed …
ikostan 37f56b3
issue (bug_risk): Setter for current_log_level is recursively assigni…
ikostan 6172e59
Update game_settings_resource.gd
ikostan aa56a73
Update game_settings_resource.gd
ikostan 7ba43f1
The enable_debug_logging setter has the same self-recursive assignmen…
ikostan 1babb51
Bug: Label displays unclamped value when input exceeds bounds.
ikostan b7da8d0
Test does not verify the intended observer behavior.
ikostan d176792
Update difficulty_flow_test.py
ikostan 3430bda
suggestion (bug_risk): UI updates for difficulty are now split betwee…
ikostan 68ef758
Update game_settings_resource.gd
ikostan b786831
suggestion: Log messages for log-level changes are now duplicated: on…
ikostan ebca539
Update advanced_settings.gd
ikostan 831ac40
suggestion: The write-back to Globals.settings.difficulty in the exte…
ikostan 96cf7c9
Update gameplay_settings.gd
ikostan b2b2e3b
Update scripts/game_settings_resource.gd
ikostan 224fa02
Update scripts/gameplay_settings.gd
ikostan a474c1a
Update game_settings_resource.gd
ikostan 657a9d2
enable_debug_logging still does not persist across reloads.
ikostan a83f2f2
Update scripts/game_settings_resource.gd
ikostan aeddbfe
suggestion (performance): Observer handler may cause excessive disk w…
ikostan 07cb212
Merge branch 'migrate-to-resource-persistence' of https://github.com/…
ikostan 39ad83b
Update difficulty_flow_test.py
ikostan d5d21b2
Update difficulty_flow_test.py
ikostan 038800a
style: format code with Black and isort
deepsource-autofix[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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.") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| uid://dgvy06sdtxf6l |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.