Decouple Fuel System using GameSettingsResource and Signals#534
Merged
Decouple Fuel System using GameSettingsResource and Signals#534
Conversation
Introduce a centralized fuel subsystem via GameSettingsResource: adds max_fuel, current_fuel, base_consumption_rate, a refuel() helper, and signals (setting_changed, fuel_depleted). Persist/load fuel settings in globals.gd and validate types when loading. Update player.gd to read/write fuel from Globals.settings, connect to setting_changed and fuel_depleted, and move consumption calculations to use the new resource values; UI updates are driven by the resource signals. Add GUT tests covering the resource behavior, UI reactivity, and integration/persistence edge cases (including signal emission and invalid config handling). Clamping and one-time depletion signaling are enforced.
ikostan
added this to the
Milestone 15: Resource-Driven Componentization & UI Standardization milestone
Contributor
Reviewer's GuideCentralizes the fuel system in GameSettingsResource, rewires player/main_scene and globals persistence to use it as the single source of truth, and adds GUT/GdUnit tests for fuel behavior, UI reactivity, lifecycle cleanup, and persistence edge cases. Sequence diagram for fuel consumption, depletion, and player/MainScene reactionssequenceDiagram
participant FuelTimer as FuelTimer
participant Player as Player
participant Settings as GameSettingsResource
participant Globals as Globals
participant UI as FuelUI
participant MainScene as MainScene
FuelTimer->>Player: _on_fuel_timer_timeout()
activate Player
Player->>Settings: current_fuel -= consumption
deactivate Player
alt current_fuel clamped and changed
Settings-->>Player: setting_changed(current_fuel, value)
activate Player
Player->>UI: update_fuel_bar()
Player->>UI: check_fuel_warning()
deactivate Player
end
alt fuel reached 0 from positive value
Settings-->>Player: fuel_depleted
activate Player
Player->>Player: _on_player_out_of_fuel()
Player->>UI: stop rotors, stop fuel_timer
deactivate Player
end
MainScene->>Globals: read settings
Globals-->>MainScene: settings reference
MainScene->>Settings: read current_fuel and difficulty
alt current_fuel <= 0
MainScene->>MainScene: stop background scroll
else current_fuel > 0
MainScene->>MainScene: scroll background based on speed and difficulty
end
Class diagram for centralized fuel system in GameSettingsResource and player integrationclassDiagram
class GameSettingsResource {
<<Resource>>
+signal setting_changed(setting_name: String, new_value: Variant)
+signal fuel_depleted
+max_fuel: float
+current_fuel: float
+base_consumption_rate: float
+high_fuel_threshold: float
+medium_fuel_threshold: float
+low_fuel_threshold: float
+no_fuel_threshold: float
+current_log_level: int
+difficulty: float
+enable_debug_logging: bool
+key_mapping_scene: PackedScene
+options_scene: PackedScene
-_max_fuel: float
-_current_fuel: float
-_high_fuel_threshold: float
-_medium_fuel_threshold: float
-_low_fuel_threshold: float
-_no_fuel_threshold: float
-_current_log_level: int
-_difficulty: float
-_enable_debug_logging: bool
+_init(): void
+refuel(amount: float): void
}
class Globals {
<<Autoload>>
+settings: GameSettingsResource
+_is_loading_settings: bool
+_on_setting_changed(setting_name: String, new_value: Variant): void
+_load_settings(path: String): void
+_save_settings(path: String): void
+log_message(message: String, level: int): void
+_notification(what: int): void
}
class Player {
<<Node2D>>
+lateral_speed: float
+acceleration: float
+deceleration: float
+screen_size: Vector2
+player_x_min: float
+player_x_max: float
+player_y_min: float
+player_y_max: float
+fuel: Dictionary
+speed: Dictionary
-_settings: GameSettingsResource
+_ready(): void
+_exit_tree(): void
+_input(event: InputEvent): void
+_physics_process(delta: float): void
+_on_setting_changed(setting_name: String, new_value: Variant): void
+_on_player_out_of_fuel(): void
+_on_fuel_timer_timeout(): void
+update_fuel_bar(): void
+check_fuel_warning(): void
+update_speed_bar(): void
+check_speed_warning(): void
+rotor_start(rotor: Node2D, rotor_sfx: AudioStreamPlayer2D): void
+rotor_stop(rotor: Node2D, rotor_sfx: AudioStreamPlayer2D): void
}
class MainScene {
<<Node>>
+player: Player
+background: ParallaxBackground
+setup_decor_layer(viewport: Vector2): void
+_process(delta: float): void
}
GameSettingsResource <.. Globals : uses
Globals o-- GameSettingsResource : settings
Player ..> Globals : reads_writes_settings
Player ..> GameSettingsResource : caches__settings
Player ..> GameSettingsResource : connects_setting_changed
Player ..> GameSettingsResource : connects_fuel_depleted
MainScene ..> Globals : reads_settings
MainScene ..> GameSettingsResource : reads_current_fuel_and_difficulty
Flow diagram for settings change, logging, and persistence with fuel filteringflowchart TD
subgraph SettingsChangeFlow
A["GameSettingsResource\nproperty change"] --> B["Emit setting_changed"]
B --> C["Globals._on_setting_changed"]
C --> D{Setting_name_is_current_fuel?}
D -- Yes --> E["Skip_logging_and_saving\navoid_log_spam_and_disk_IO"]
D -- No --> F["log_message at DEBUG"]
F --> G["_save_settings to disk"]
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- When
max_fuelchanges, consider re-clampingcurrent_fuel(e.g., in themax_fuelsetter) so that lowering the max capacity cannot leavecurrent_fueltemporarily above the new maximum. - The player’s
_on_setting_changedhandler only reacts tocurrent_fuel; you might also handlemax_fuelthere (or elsewhere) to keepfuel_bar.max_valuein sync with upgrades or settings changes instead of only initializing it in_ready. - The
fueldictionary inplayer.gdstill containsfuel/maxentries that are no longer updated or used consistently now that fuel comes fromGlobals.settings; consider either removing these keys or keeping them in sync to avoid confusion and accidental use of stale values.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When `max_fuel` changes, consider re-clamping `current_fuel` (e.g., in the `max_fuel` setter) so that lowering the max capacity cannot leave `current_fuel` temporarily above the new maximum.
- The player’s `_on_setting_changed` handler only reacts to `current_fuel`; you might also handle `max_fuel` there (or elsewhere) to keep `fuel_bar.max_value` in sync with upgrades or settings changes instead of only initializing it in `_ready`.
- The `fuel` dictionary in `player.gd` still contains `fuel`/`max` entries that are no longer updated or used consistently now that fuel comes from `Globals.settings`; consider either removing these keys or keeping them in sync to avoid confusion and accidental use of stale values.
## Individual Comments
### Comment 1
<location path="scripts/player.gd" line_range="221-224" />
<code_context>
+# NEW: Observer pattern handler to react when GameSettingsResource properties (like fuel) are updated externally.
+func _on_setting_changed(setting_name: String, _value: Variant) -> void:
+ if setting_name == "current_fuel":
+ update_fuel_bar()
+ check_fuel_warning()
+
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Handle `max_fuel` changes so the fuel bar’s max stays in sync with the settings resource.
`_on_setting_changed` only handles `"current_fuel"`, so changes to `max_fuel` (from options, save/load, cheats, etc.) leave `fuel_bar.max_value` stale. Either handle `"max_fuel"` here to update `fuel_bar.max_value`, or have `update_fuel_bar()` always read `Globals.settings.max_fuel` and apply it so the bar stays in sync.
Suggested implementation:
```
func _on_setting_changed(setting_name: String, _value: Variant) -> void:
if setting_name == "current_fuel":
update_fuel_bar()
check_fuel_warning()
elif setting_name == "max_fuel":
# Keep the UI fuel bar max in sync with the GameSettingsResource.
fuel_bar.max_value = Globals.settings.max_fuel
# Re-run the standard UI update so the current fuel is represented correctly
# relative to the new maximum.
update_fuel_bar()
```
If there are other settings that can affect how the fuel bar is displayed (e.g. a percentage/scale setting), they can also be handled in `_on_setting_changed` using additional `elif` branches, reusing `update_fuel_bar()` to keep the behavior centralized.
</issue_to_address>
### Comment 2
<location path="scripts/player.gd" line_range="317" />
<code_context>
+ var m_fuel: float = Globals.settings.max_fuel
+
+ fuel["bar"].value = cur_fuel
+ var fuel_percent: float = (cur_fuel / m_fuel) * 100.0
if fuel_percent > HIGH_FUEL_THRESHOLD:
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against division by zero when computing `fuel_percent` if `max_fuel` is 0.
If `Globals.settings.max_fuel` is ever `0.0` (e.g., via config or test setup), this will raise a division-by-zero error. Consider returning early when `m_fuel <= 0.0` or explicitly setting `fuel_percent` to `0.0` in that case.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMoves fuel system from local player state to global GameSettingsResource with persistence and signaling. Implements fuel_depleted signal, threshold exports, clamping logic, and refuel helper. Updates Player to consume global fuel, adjusts Globals persistence to load/save fuel, and adds extensive test coverage for signal emission, edge cases, and persistence behavior. Changes
Sequence DiagramsequenceDiagram
participant Player
participant GameSettingsResource as GlobalSettings
participant Globals
participant UINode as UI/ProgressBar
Player->>GameSettingsResource: _on_fuel_timer_timeout()<br/>current_fuel -= depletion
GameSettingsResource->>GameSettingsResource: Clamp to [0, max_fuel]
alt current_fuel crosses 0.0
GameSettingsResource->>Player: emit fuel_depleted
Player->>Player: _on_player_out_of_fuel()<br/>stop engines
else current_fuel changes (not zero)
GameSettingsResource->>Player: emit setting_changed("current_fuel")
Player->>Player: Update UI colors/bars
end
GameSettingsResource->>UINode: emit setting_changed("current_fuel")
UINode->>UINode: Update progress bar value
rect rgba(100, 150, 200, 0.5)
Note over Globals,Globals: On Quit or Manual Save
Globals->>Globals: _save_settings()
Globals->>Globals: Write current_fuel to config
Globals->>Globals: Write max_fuel to config
end
rect rgba(100, 150, 200, 0.5)
Note over Globals,GameSettingsResource: On Game Load
Globals->>Globals: _load_settings()
Globals->>GameSettingsResource: settings.current_fuel = loaded_value
Globals->>GameSettingsResource: settings.max_fuel = loaded_value
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python |  |
Apr 10, 2026 12:52a.m. | Review ↗ |
| JavaScript | |
Apr 10, 2026 12:52a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Read the fuel bar max value from the centralized GameSettingsResource (Globals.settings.max_fuel) instead of a hardcoded MAX_FUEL, ensuring UI capacity stays in sync with game settings. Also reset Globals.settings.current_fuel to max on player spawn so a new game always starts with a full tank. Minor comment formatting adjusted for clarity.
Ensure the player stops moving when fuel is depleted by resetting speed["speed"] to 0.0 inside _on_player_out_of_fuel (migrated from the previous fuel-timer location). Also update the GDUnit helper: rename the player param to player_root, compute a normalized_speed variable, and use Globals.settings.base_consumption_rate instead of a removed local base_fuel_drain so tests reflect the new global consumption resource.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Owner
Author
|
This one is also addressed: #278 |
Introduce GUT unit tests covering fuel system edge cases: verify the 'fuel_depleted' signal emits exactly once when current_fuel transitions to zero, and ensure current_fuel is correctly saved to and loaded from a config file. Tests isolate filesystem using a user:// test path, stub Globals.log_message, reset Globals.settings between tests, and clean up the temporary config. Also add a UID file for the new test.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
scripts/main_scene.gd (1)
192-196: Handle fuel exhaustion once instead of resetting the parallax every frame.
_on_player_out_of_fuel()already zeroes the player's speed, so_process()will naturally stop scrolling. This branch snapsscroll_offsetback to(0, 0)and keeps reapplying it for the rest of the empty-fuel state. If the reset is intentional, drive it offGlobals.settings.fuel_depleted; otherwise remove it so the background freezes in place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/main_scene.gd` around lines 192 - 196, The branch in _process() that forces background.scroll_offset = Vector2(0, 0) whenever Globals.settings.current_fuel <= 0 repeatedly snaps the parallax every frame; either remove this line so the background naturally stops when _on_player_out_of_fuel() zeroes player speed, or change the condition to check a one-time flag (Globals.settings.fuel_depleted) so the reset runs only once. Locate the check using Globals.settings.current_fuel and modify it accordingly (or delete the scroll_offset assignment) and ensure _on_player_out_of_fuel() sets Globals.settings.fuel_depleted if you choose the one-time reset approach.test/gut/test_fuel_integration.gd (1)
44-55: Also exercise the invalidcurrent_fuelbranch.
Globals._load_settings()now has a separate type guard forcurrent_fuel, but this test only proves the fallback behavior formax_fuel. Adding the corruptcurrent_fuelcase would lock down the second branch too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/gut/test_fuel_integration.gd` around lines 44 - 55, Extend the test_persistence_invalid_types_fallback test to also corrupt the current_fuel entry and assert fallback behavior: after writing "corrupt_string_value" for "current_fuel" into the same ConfigFile and saving at TEST_CONFIG_PATH, call Globals._load_settings(TEST_CONFIG_PATH) and assert that Globals.settings.current_fuel equals its default (capture it before corrupting) to exercise the separate type guard for current_fuel in Globals._load_settings.scripts/player.gd (1)
228-231: Also refresh the fuel bar whenmax_fuelchanges.
fuel_bar.max_valueis initialized from the resource in_ready(), but this observer ignores"max_fuel". If capacity is reloaded or tuned while the player is alive, the bar fill and value drift out of sync.Possible adjustment
func _on_setting_changed(setting_name: String, _value: Variant) -> void: - if setting_name == "current_fuel": + if setting_name == "max_fuel": + fuel_bar.max_value = Globals.settings.max_fuel + update_fuel_bar() + check_fuel_warning() + elif setting_name == "current_fuel": update_fuel_bar() check_fuel_warning()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/player.gd` around lines 228 - 231, The observer _on_setting_changed currently only reacts to "current_fuel" changes; also handle "max_fuel" so the UI stays in sync by updating fuel_bar.max_value and refreshing the display. In _on_setting_changed (and anywhere settings reload), detect when setting_name == "max_fuel", set fuel_bar.max_value from the updated resource or Settings value, then call update_fuel_bar() (and optionally check_fuel_warning()) to recompute the filled fraction; reference the existing functions _on_setting_changed, update_fuel_bar, check_fuel_warning, and the fuel_bar control to locate where to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/game_settings_resource.gd`:
- Around line 27-30: When lowering max_fuel in the setter for max_fuel, clamp
current_fuel to the new capacity and emit the appropriate events: capture the
previous current_fuel, set max_fuel = max(0.0, value), and if previous_current >
max_fuel then set current_fuel = max_fuel and emit setting_changed for
"current_fuel"; additionally if max_fuel == 0 and previous_current > 0 then emit
fuel_depleted. This ensures the capacity invariant and that fuel_depleted fires
when capacity is reduced to zero.
In `@scripts/globals.gd`:
- Around line 249-251: The change persists current_fuel on every settings write,
causing excessive I/O because Globals._on_setting_changed() is triggered by
runtime fuel ticks; remove or stop writing settings.current_fuel in the generic
save path (the two calls config.set_value("Settings","current_fuel",
settings.current_fuel)) and instead persist fuel only from an explicit
checkpoint/quit handler or via a debounced save routine; update Globals (e.g.,
_on_setting_changed or save_settings) to only write static settings like
max_fuel, and add a new checkpoint_save_current_fuel() or a debounced save tied
to player.gd fuel events to handle runtime current_fuel persistence.
In `@scripts/player.gd`:
- Around line 324-329: Protect the fuel percentage calculation from
division-by-zero by checking m_fuel before dividing: in the block that reads
Globals.settings.current_fuel into cur_fuel and Globals.settings.max_fuel into
m_fuel, compute fuel_percent only if m_fuel > 0 (e.g., set fuel_percent =
(cur_fuel / m_fuel) * 100.0); otherwise set fuel_percent to 0. Also clamp
fuel_percent into the 0..100 range before it is used by the color-threshold
logic so negative or overflow values cannot propagate (refer to variables
cur_fuel, m_fuel, fuel["bar"].value and fuel_percent).
In `@test/gdunit4/test_difficulty_integration.gd`:
- Around line 35-37: The test alters the global fuel state
(Globals.settings.current_fuel) but only restores difficulty, risking leakage
into other tests; update the teardown in test_difficulty_integration.gd to
snapshot and restore the fuel fields (e.g., save Globals.settings.current_fuel
and any related fuel fields before the test and restore them after), or instead
reset current_fuel deterministically using Globals.settings.max_fuel (e.g., set
current_fuel = min(previous_or_default, Globals.settings.max_fuel)) so the test
does not depend on max_fuel >= 100.0.
In `@test/gdunit4/test_difficulty.gd`:
- Around line 33-35: The test mutates the singleton
Globals.settings.current_fuel without restoring it and uses a hard-coded 100.0
baseline, which makes tests order-dependent; fix by snapshotting and restoring
the relevant fuel fields (e.g., save old_current = Globals.settings.current_fuel
and old_max = Globals.settings.max_fuel at test start and restore them in
teardown) or derive the reset value from Globals.settings.max_fuel (e.g., set
Globals.settings.current_fuel = Globals.settings.max_fuel) instead of using
100.0; apply the same change for the other occurrences that set
Globals.settings.current_fuel.
In `@test/gdunit4/test_player.gd`:
- Around line 86-99: The tests set Globals.settings.current_fuel to hardcoded
values (e.g., 95.0, 10.0) which assume a 100-unit tank; change these to scale
from the centralized capacity by using Globals.settings.max_fuel (e.g.,
Globals.settings.current_fuel = Globals.settings.max_fuel * 0.95, *0.7, *0.3, or
= Globals.settings.max_fuel - expected_depletion) before calling
player_root.update_fuel_bar(), and keep the same fuel_bar/StyleBoxFlat color
assertions (style_1/style_2) so the tests remain correct regardless of max_fuel;
apply the same replacement pattern to the other occurrences mentioned (around
the blocks using player_root.update_fuel_bar(), fuel_bar, style_1/style_2).
In `@test/gut/test_fuel_edge_cases.gd`:
- Around line 14-29: The tests overwrite the autoload Globals.settings in
before_each, breaking observer wiring from Globals._ready() for later suites;
save the original Globals.settings at the start of before_each (e.g.,
orig_settings = Globals.settings) and then restore it in after_each by assigning
Globals.settings = orig_settings and cleaning any stubs; ensure the saved symbol
is used to rewire the autoload rather than leaving a new GameSettingsResource
permanently in place.
In `@test/gut/test_fuel_resource.gd`:
- Around line 14-19: The test fixture is masking the resource's defaults by
setting both max_fuel and current_fuel in before_each(); to let
test_fuel_initialization() actually verify defaults, stop overriding
current_fuel in before_each() (only set max_fuel or necessary properties) or
initialize current_fuel to nil/unset so GameSettingsResource can provide its
default; update any other fixtures/tests that repeat this (the similar
assignments at lines referenced 25-27) to avoid pre-setting current_fuel so the
tests validate the resource's real default behavior (refer to before_each(),
GameSettingsResource, and test_fuel_initialization()).
In `@test/gut/test_fuel_ui.gd`:
- Around line 9-21: In test_ui_updates_on_fuel_change_signal capture the
previous fuel value (var prev_fuel = Globals.settings.current_fuel) before
assigning 45.5 and restore it at the end of the test
(Globals.settings.current_fuel = prev_fuel); place the restore in a
finally-style block (or just after the assert) so the global is always reset
even if the test fails; reference the test function name
test_ui_updates_on_fuel_change_signal and the Globals.settings.current_fuel
symbol to locate where to add the save-and-restore logic.
---
Nitpick comments:
In `@scripts/main_scene.gd`:
- Around line 192-196: The branch in _process() that forces
background.scroll_offset = Vector2(0, 0) whenever Globals.settings.current_fuel
<= 0 repeatedly snaps the parallax every frame; either remove this line so the
background naturally stops when _on_player_out_of_fuel() zeroes player speed, or
change the condition to check a one-time flag (Globals.settings.fuel_depleted)
so the reset runs only once. Locate the check using
Globals.settings.current_fuel and modify it accordingly (or delete the
scroll_offset assignment) and ensure _on_player_out_of_fuel() sets
Globals.settings.fuel_depleted if you choose the one-time reset approach.
In `@scripts/player.gd`:
- Around line 228-231: The observer _on_setting_changed currently only reacts to
"current_fuel" changes; also handle "max_fuel" so the UI stays in sync by
updating fuel_bar.max_value and refreshing the display. In _on_setting_changed
(and anywhere settings reload), detect when setting_name == "max_fuel", set
fuel_bar.max_value from the updated resource or Settings value, then call
update_fuel_bar() (and optionally check_fuel_warning()) to recompute the filled
fraction; reference the existing functions _on_setting_changed, update_fuel_bar,
check_fuel_warning, and the fuel_bar control to locate where to apply the
change.
In `@test/gut/test_fuel_integration.gd`:
- Around line 44-55: Extend the test_persistence_invalid_types_fallback test to
also corrupt the current_fuel entry and assert fallback behavior: after writing
"corrupt_string_value" for "current_fuel" into the same ConfigFile and saving at
TEST_CONFIG_PATH, call Globals._load_settings(TEST_CONFIG_PATH) and assert that
Globals.settings.current_fuel equals its default (capture it before corrupting)
to exercise the separate type guard for current_fuel in Globals._load_settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3e7690cd-b820-4de8-8449-35bd20dcef31
📒 Files selected for processing (16)
scripts/game_settings_resource.gdscripts/globals.gdscripts/main_scene.gdscripts/player.gdtest/gdunit4/test_difficulty.gdtest/gdunit4/test_difficulty_integration.gdtest/gdunit4/test_helpers.gdtest/gdunit4/test_player.gdtest/gut/test_fuel_edge_cases.gdtest/gut/test_fuel_edge_cases.gd.uidtest/gut/test_fuel_integration.gdtest/gut/test_fuel_integration.gd.uidtest/gut/test_fuel_resource.gdtest/gut/test_fuel_resource.gd.uidtest/gut/test_fuel_ui.gdtest/gut/test_fuel_ui.gd.uid
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Browser Functional Tests / test
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-30T04:02:23.747Z
Learnt from: ikostan
Repo: ikostan/SkyLockAssault PR: 500
File: test/gut/test_audio_web_bridge.gd:131-145
Timestamp: 2026-03-30T04:02:23.747Z
Learning: In GUT (Godot Unit Test) for Godot 4, when using `assert_called` / `assert_called_count` with parameter matching, include *every* argument the mocked method accepts, including parameters with default values. GUT does not auto-fill default arguments during call matching. For example, if `JavaScriptBridgeWrapper.eval(script: String, global_exec: bool = false)` is invoked as `eval(js_string)`, the actual call recorded by GUT includes the default (`eval(js_string, false)`), so your assertion must match both arguments (e.g., `.bind(js_string, false)`, not `.bind(js_string)`). Apply this rule to GUT assertions in `test/gut` tests.
Applied to files:
test/gut/test_fuel_ui.gdtest/gut/test_fuel_resource.gdtest/gut/test_fuel_edge_cases.gdtest/gut/test_fuel_integration.gd
🔇 Additional comments (6)
scripts/globals.gd (1)
205-223: Nice: the fuel load path now rejects malformed persisted values.The numeric guards here make the bad-config fallback behavior much safer.
test/gdunit4/test_helpers.gd (1)
13-17: LGTM: the helper now mirrors the centralized drain formula.Using
Globals.settings.base_consumption_ratekeeps the shared expectation aligned with the production refactor.test/gdunit4/test_player.gd (1)
34-37: Nice: this expectation now follows the same fuel-rate source as production.The helper check stays aligned with the new global resource model.
test/gut/test_fuel_resource.gd (1)
33-71: Nice resource-level coverage for clamp and refuel behavior.These cases exercise the core fuel invariants without pulling a full scene into the test.
test/gut/test_fuel_edge_cases.gd (1)
32-74: Good edge-case coverage.The once-only depletion assertion and explicit save/load round-trip hit the fragile parts of the new fuel behavior.
scripts/player.gd (1)
142-165: Nice move to a signal-driven fuel source.Initializing from
Globals.settingsand subscribing tofuel_depleted/setting_changedremoves the remaining player-local fuel coupling cleanly.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
SourceryAI
reviewed
Apr 9, 2026
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The fallback path in
player.gdthat creates a newGameSettingsResourcewhenGlobals.settingsis missing means the player can end up using a private fuel resource while the rest of the game (e.g.main_scene.gdbackground scroll logic) still readsGlobals.settings, which can desynchronize behavior; consider either enforcing thatGlobals.settingsalways exists in runtime scenes or wiring the fallback instance back intoGlobals.settingsso there is a single source of truth. - The fuel threshold exports (
high_fuel_threshold,medium_fuel_threshold,low_fuel_threshold,no_fuel_threshold) are fully user-configurable but not validated against each other, so misordered or out-of-range values could break both color bands and warning logic; it may be worth clamping and enforcing a monotonic ordering inside the setters (or in a validation helper) to keep the UI behavior predictable. - In
_on_fuel_timer_timeout()you still log"Fuel left"on every tick while you’ve suppressed log spam forcurrent_fuelin_on_setting_changed; consider gating this log behindenable_debug_loggingor removing it to avoid reintroducing high-frequency log noise under normal play.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The fallback path in `player.gd` that creates a new `GameSettingsResource` when `Globals.settings` is missing means the player can end up using a private fuel resource while the rest of the game (e.g. `main_scene.gd` background scroll logic) still reads `Globals.settings`, which can desynchronize behavior; consider either enforcing that `Globals.settings` always exists in runtime scenes or wiring the fallback instance back into `Globals.settings` so there is a single source of truth.
- The fuel threshold exports (`high_fuel_threshold`, `medium_fuel_threshold`, `low_fuel_threshold`, `no_fuel_threshold`) are fully user-configurable but not validated against each other, so misordered or out-of-range values could break both color bands and warning logic; it may be worth clamping and enforcing a monotonic ordering inside the setters (or in a validation helper) to keep the UI behavior predictable.
- In `_on_fuel_timer_timeout()` you still log `"Fuel left"` on every tick while you’ve suppressed log spam for `current_fuel` in `_on_setting_changed`; consider gating this log behind `enable_debug_logging` or removing it to avoid reintroducing high-frequency log noise under normal play.
## Individual Comments
### Comment 1
<location path="scripts/player.gd" line_range="77-83" />
<code_context>
func _ready() -> void:
+ # Safely cache the settings resource
+ _settings = Globals.settings if is_instance_valid(Globals) else null
+
+ if not is_instance_valid(_settings):
+ # NEW: Log the error, but generate a fallback resource so the player
+ # fully initializes and doesn't become a game-crashing "zombie" node.
+ push_error("Player couldn't find Globals.settings! Using fallback defaults.")
+ _settings = GameSettingsResource.new()
+
# Auto-start rotors (overrides editor if needed)
</code_context>
<issue_to_address>
**issue (bug_risk):** Fallback GameSettingsResource can desync player fuel from the global settings used elsewhere.
If `Globals`/`Globals.settings` are invalid and you create a local `GameSettingsResource.new()`, the player ends up using a different settings instance than the rest of the game. For example, `main_scene.gd` reads `Globals.settings.current_fuel`, while the player uses `_settings.current_fuel` from the fallback. This can cause inconsistent fuel behavior. Either fail fast when `Globals.settings` is missing, or initialize `Globals.settings` to the new resource and ensure all code uses that single instance.
</issue_to_address>
### Comment 2
<location path="scripts/player.gd" line_range="264" />
<code_context>
+
+ rotor_stop(rotor_right, rotor_right_sfx)
+ rotor_stop(rotor_left, rotor_left_sfx)
+ fuel_timer.stop()
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Fuel timer never restarts after refuel, so fuel consumption stops permanently once depleted.
`_on_player_out_of_fuel()` stops `fuel_timer`, but it’s never restarted when fuel is refilled via `GameSettingsResource.refuel()` or other `current_fuel` updates. Because consumption now depends on resource signals, refueling won’t resume fuel drain unless something explicitly restarts `fuel_timer`, letting the plane fly indefinitely. Consider either restarting the timer when `setting_changed("current_fuel", ...)` detects a transition from 0 to >0, or having the refuel logic explicitly restart `fuel_timer` on the player.
</issue_to_address>
### Comment 3
<location path="scripts/player.gd" line_range="238-246" />
<code_context>
+ if setting_name == "current_fuel":
+ update_fuel_bar()
+ check_fuel_warning()
+ elif setting_name == "max_fuel":
+ # Keep the UI fuel bar max in sync with the GameSettingsResource.
+ fuel_bar.max_value = _settings.max_fuel
+ # Re-run the standard UI update so the current fuel is represented correctly
+ # relative to the new maximum.
+ update_fuel_bar()
+
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Fuel warning blinking isn’t re-evaluated when max_fuel changes, which can leave the warning out of sync.
In `_on_setting_changed`, when `max_fuel` changes you resync `fuel_bar.max_value` and call `update_fuel_bar()`, but don’t re-run `check_fuel_warning()`. If `max_fuel` changes, the fuel percentage can cross the low-fuel threshold without updating the blinking state. Please call `check_fuel_warning()` here so the warning stays consistent with the new capacity.
```suggestion
# NEW: Observer pattern handler to react when GameSettingsResource
# properties (like fuel) are updated externally.
func _on_setting_changed(setting_name: String, _value: Variant) -> void:
if not is_instance_valid(_settings):
return
if setting_name == "current_fuel":
update_fuel_bar()
check_fuel_warning()
elif setting_name == "max_fuel":
# Keep the UI fuel bar max in sync with the GameSettingsResource.
fuel_bar.max_value = _settings.max_fuel
# Re-run the standard UI update so the current fuel is represented correctly
# relative to the new maximum.
update_fuel_bar()
# Re-evaluate the fuel warning in case the fuel percentage crossed the threshold.
check_fuel_warning()
```
</issue_to_address>Hi @ikostan! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Caching
Globals.settingsinto_settingsinplayer.gdmeans the player will keep using the old resource ifGlobals.settingsis ever swapped at runtime (e.g., in tests or a future settings reload flow); consider either not caching or reacting toGlobals.settingschanges so the reference stays in sync. - The fuel color/threshold logic is now split between
GameSettingsResource(threshold data) andplayer.gd(percent and color computation); if you expect thresholds to be reused elsewhere, consider moving the percentage/lerp calculation into a helper onGameSettingsResourceto avoid duplicating this logic in future consumers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Caching `Globals.settings` into `_settings` in `player.gd` means the player will keep using the old resource if `Globals.settings` is ever swapped at runtime (e.g., in tests or a future settings reload flow); consider either not caching or reacting to `Globals.settings` changes so the reference stays in sync.
- The fuel color/threshold logic is now split between `GameSettingsResource` (threshold data) and `player.gd` (percent and color computation); if you expect thresholds to be reused elsewhere, consider moving the percentage/lerp calculation into a helper on `GameSettingsResource` to avoid duplicating this logic in future consumers.
## Individual Comments
### Comment 1
<location path="scripts/game_settings_resource.gd" line_range="72-81" />
<code_context>
+@export var high_fuel_threshold: float = 90.0:
</code_context>
<issue_to_address>
**issue (bug_risk):** New fuel threshold settings are not persisted alongside `max_fuel`/`current_fuel`.
These new exported thresholds aren’t included in `_save_settings()` / `_load_settings()`, so changes made in the inspector or at runtime won’t persist across restarts. If they’re meant to be user/difficulty-tunable, please persist them via `Globals._save_settings` / `_load_settings` like `difficulty`, `max_fuel`, and `current_fuel`.
</issue_to_address>
### Comment 2
<location path="scripts/main_scene.gd" line_range="193-195" />
<code_context>
- if player.fuel["fuel"] <= 0:
+
+ # OLD: if player.fuel["fuel"] <= 0:
+ # NEW: Check the global settings resource for current fuel,
+ # since the local dictionary key was removed.
+ if Globals.settings.current_fuel <= 0:
background.scroll_offset = Vector2(0, 0)
</code_context>
<issue_to_address>
**issue (bug_risk):** Direct access to `Globals.settings.current_fuel` assumes the settings resource is always valid.
Here `_process` directly dereferences `Globals.settings.current_fuel`, unlike `player.gd` which caches `_settings` and guards it with `is_instance_valid`. If the settings resource fails to load, is replaced, or set to `null` (e.g., in tests or tools), this will error. Please add a validity check (e.g. `if Globals.settings and is_instance_valid(Globals.settings)`) before accessing `current_fuel`, and use a safe default when it’s unavailable to match `Player`’s behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
test/gut/test_fuel_persistence_integration.gd (1)
65-65: Assert config save success in test setup.At Line 65 and Line 87,
config.save(...)return values are ignored. If save fails, later assertions fail for the wrong reason.Suggested patch
- config.save(TEST_CONFIG_PATH) + assert_eq(config.save(TEST_CONFIG_PATH), OK, "Failed to write test config file.")Also applies to: 87-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/gut/test_fuel_persistence_integration.gd` at line 65, The test currently ignores the boolean return from config.save(TEST_CONFIG_PATH) (and the other save call) which can hide setup failures; update the test to capture and assert the save succeeded by checking the return value of config.save(TEST_CONFIG_PATH) (and the second config.save) and failing the test if it returns false (e.g., assert config.save(TEST_CONFIG_PATH) or raise an error), so the test setup fails immediately with a clear message when saving the config fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/player.gd`:
- Around line 146-149: The unconditional reset of _settings.current_fuel to
_settings.max_fuel in the player scene init overwrites persisted fuel; remove
that unconditional assignment and instead only initialize current_fuel when
starting a fresh run (e.g., in your new-game path) or when no persisted value
exists—check for a saved value (null/undefined or a saved-flag) in the player
scene initialization function (e.g., _ready/_init) before assigning from
_settings.max_fuel so that _settings.current_fuel loaded from persistence is
preserved.
- Around line 244-253: When handling setting_name == "max_fuel" in the player.gd
diff, also refresh the warning/blink state and handle any threshold changes:
after setting fuel_bar.max_value = _settings.max_fuel and calling
update_fuel_bar(), also call check_fuel_warning(); additionally, if there are
other fuel-related keys (e.g., a fuel warning threshold property in _settings),
ensure the same sequence (update_fuel_bar() then check_fuel_warning()) runs when
that setting_name is updated so the UI warning/blink state stays in sync with
max/current/threshold changes.
In `@test/gut/test_fuel_additional_edge_cases.gd`:
- Around line 31-33: After instantiating the scene and grabbing player_root,
stop the Player's live fuel timer so background ticks won't race with manual
_on_fuel_timer_timeout() calls; locate the Timer started in Player (e.g., a
child node "FuelTimer" or a field like fuel_timer) and call stop() (or
set_autostart = false) on it immediately after player_root =
main_scene.get_node("Player") so tests become deterministic.
---
Nitpick comments:
In `@test/gut/test_fuel_persistence_integration.gd`:
- Line 65: The test currently ignores the boolean return from
config.save(TEST_CONFIG_PATH) (and the other save call) which can hide setup
failures; update the test to capture and assert the save succeeded by checking
the return value of config.save(TEST_CONFIG_PATH) (and the second config.save)
and failing the test if it returns false (e.g., assert
config.save(TEST_CONFIG_PATH) or raise an error), so the test setup fails
immediately with a clear message when saving the config fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7ba78d2e-ec1c-41f2-a41a-553b218e97e0
📒 Files selected for processing (10)
scenes/main_scene.tscnscripts/game_settings_resource.gdscripts/globals.gdscripts/player.gdtest/gut/test_fuel_additional_edge_cases.gdtest/gut/test_fuel_additional_edge_cases.gd.uidtest/gut/test_fuel_persistence_integration.gdtest/gut/test_fuel_persistence_integration.gd.uidtest/gut/test_player_lifecycle.gdtest/gut/test_player_lifecycle.gd.uid
✅ Files skipped from review due to trivial changes (4)
- test/gut/test_player_lifecycle.gd.uid
- test/gut/test_fuel_persistence_integration.gd.uid
- test/gut/test_fuel_additional_edge_cases.gd.uid
- scripts/globals.gd
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/game_settings_resource.gd
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Browser Functional Tests / test
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-30T04:02:23.747Z
Learnt from: ikostan
Repo: ikostan/SkyLockAssault PR: 500
File: test/gut/test_audio_web_bridge.gd:131-145
Timestamp: 2026-03-30T04:02:23.747Z
Learning: In GUT (Godot Unit Test) for Godot 4, when using `assert_called` / `assert_called_count` with parameter matching, include *every* argument the mocked method accepts, including parameters with default values. GUT does not auto-fill default arguments during call matching. For example, if `JavaScriptBridgeWrapper.eval(script: String, global_exec: bool = false)` is invoked as `eval(js_string)`, the actual call recorded by GUT includes the default (`eval(js_string, false)`), so your assertion must match both arguments (e.g., `.bind(js_string, false)`, not `.bind(js_string)`). Apply this rule to GUT assertions in `test/gut` tests.
Applied to files:
test/gut/test_fuel_additional_edge_cases.gdtest/gut/test_player_lifecycle.gdtest/gut/test_fuel_persistence_integration.gd
🔇 Additional comments (5)
scripts/player.gd (1)
347-347: Good defensive math guards in fuel UI computation.The checks around
m_fuel <= 0.0and non-positive threshold spans prevent invalid interpolation factors and UI breakage under misconfiguration.Also applies to: 360-373
scenes/main_scene.tscn (1)
7-7: Scene reference update looks correct.Using
res://scenes/Player.tscnis a clean fix for case-sensitive path consistency.test/gut/test_fuel_additional_edge_cases.gd (1)
15-22: Good global-state isolation in test setup/teardown.Snapshotting and restoring
Globals.settingshere prevents cross-test leakage.test/gut/test_player_lifecycle.gd (1)
31-74: Lifecycle coverage is solid here.These tests validate both signal disconnection behavior and null-settings safety around
_exit_tree().test/gut/test_fuel_persistence_integration.gd (1)
132-135: Good explicit signal cleanup at test end.Disconnecting the test listener prevents cross-test signal leakage.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Gemini said This is a classic case of an AI reviewer lacking the context of your specific game design! The reviewer is technically correct about the code logic, but you are completely right about the game design. The Contradiction The reviewer is pointing out a conflict between two scripts: In globals.gd, you are carefully saving and loading current_fuel to your settings.cfg file on the player's hard drive. In player.gd (Line 149 / 206), you specifically added this comment: # Restored the unconditional fuel reset. Since the game doesn't use mid-run resumes, the player MUST spawn with a full tank to prevent infinite death loops from previous 0-fuel saves. The reviewer is saying: "Why are you saving the fuel to the hard drive if you just instantly refill the tank the moment the scene loads?" Who is right? You are. If your game is an arcade-style shooter or level-based (no mid-run saves), spawning with a 0-fuel save file would create an infinite instant-death loop. The unconditional reset to max_fuel in player.gd is a vital safety net. The Real Fix To satisfy the reviewer and clean up the architecture, you shouldn't change player.gd. Instead, you should stop saving current_fuel to the hard drive entirely. Since current_fuel is volatile session data (it only matters while the plane is flying), it doesn't belong in a permanent settings.cfg file next to things like Volume or Controls. To fix this perfectly, just open globals.gd and delete the current_fuel lines from your load/save functions Once you do this, max_fuel (the capacity upgrade) is still saved forever, but current_fuel is just a temporary number that always safely resets to maximum when the player spawns. This completely resolves the reviewer's warning while protecting your game's logic!
…mes the settings resource is always valid. issue (bug_risk): Direct access to Globals.settings.current_fuel assumes the settings resource is always valid. Here _process directly dereferences Globals.settings.current_fuel, unlike player.gd which caches _settings and guards it with is_instance_valid. If the settings resource fails to load, is replaced, or set to null (e.g., in tests or tools), this will error. Please add a validity check (e.g. if Globals.settings and is_instance_valid(Globals.settings)) before accessing current_fuel, and use a safe default when it’s unavailable to match Player’s behavior.
Owner
Author
Thanks for the suggestions! We are going to intentionally keep the current implementation for the following architectural reasons:
Marking this as resolved! |
…l from the global settings used elsewhere. issue (bug_risk): Fallback GameSettingsResource can desync player fuel from the global settings used elsewhere. If Globals/Globals.settings are invalid and you create a local GameSettingsResource.new(), the player ends up using a different settings instance than the rest of the game. For example, main_scene.gd reads Globals.settings.current_fuel, while the player uses _settings.current_fuel from the fallback. This can cause inconsistent fuel behavior. Either fail fast when Globals.settings is missing, or initialize Globals.settings to the new resource and ensure all code uses that single instance.
…sumption stops permanently once depleted.
issue (bug_risk): Fuel timer never restarts after refuel, so fuel consumption stops permanently once depleted.
_on_player_out_of_fuel() stops fuel_timer, but it’s never restarted when fuel is refilled via GameSettingsResource.refuel() or other current_fuel updates. Because consumption now depends on resource signals, refueling won’t resume fuel drain unless something explicitly restarts fuel_timer, letting the plane fly indefinitely. Consider either restarting the timer when setting_changed("current_fuel", ...) detects a transition from 0 to >0, or having the refuel logic explicitly restart fuel_timer on the player.
Owner
Author
|
Working on:
|
The fuel threshold exports (high_fuel_threshold, medium_fuel_threshold, low_fuel_threshold, no_fuel_threshold) are fully user-configurable but not validated against each other, so misordered or out-of-range values could break both color bands and warning logic; it may be worth clamping and enforcing a monotonic ordering inside the setters (or in a validation helper) to keep the UI behavior predictable.
Owner
Author
|
Working on:
|
In _on_fuel_timer_timeout() you still log "Fuel left" on every tick while you’ve suppressed log spam for current_fuel in _on_setting_changed; consider gating this log behind enable_debug_logging or removing it to avoid reintroducing high-frequency log noise under normal play.
Owner
Author
All addressed. |
Owner
Author
|
@sourcery-ai guide |
ikostan
changed the title
github-project-automation
Bot
moved this from In Progress
to Done
in Sky Lock Assault Project
This was referenced Apr 11, 2026
8 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
📦 PR Summary
Introduce a centralized fuel subsystem via GameSettingsResource: adds max_fuel, current_fuel, base_consumption_rate, a refuel() helper, and signals (setting_changed, fuel_depleted). Persist/load fuel settings in globals.gd and validate types when loading. Update player.gd to read/write fuel from Globals.settings, connect to setting_changed and fuel_depleted, and move consumption calculations to use the new resource values; UI updates are driven by the resource signals. Add GUT tests covering the resource behavior, UI reactivity, and integration/persistence edge cases (including signal emission and invalid config handling). Clamping and one-time depletion signaling are enforced.
Overview
This PR migrates the fuel system from a hard-coded local state into a robust, globally accessible Godot Resource (
GameSettingsResource). In doing so, it establishes a new standard for data management, UI reactivity, and defensive programming across the project.Key Features & Architectural Changes:
_processloop. UI elements and player states now update instantly and automatically by listening to thesetting_changedandfuel_depletedsignals emitted by the global resource.is_instance_valid()null guards across hot paths (_physics_processinplayer.gd,_processinmain_scene.gd) to prevent engine crashes during scene transitions or isolated tests.GameSettingsResourceinto theGlobalssingleton if the primary settings fail to load, ensuring all systems stay synchronized.max_fuelanddifficultyare saved to the player's disk, whilecurrent_fuelcorrectly resets on load to prevent inescapable 0-fuel death loops.test_fuel_edge_cases.gd,test_fuel_persistence_integration.gd, etc.) to definitively prove signal constraints, persistence fallbacks, and UI reactivity.name: Default Pull Request Template
about: Suggesting changes to SkyLockAssault
title: ''
labels: ''
assignees: ''
Description
What does this PR do? (e.g., "Fixes player jump physics in level 2" or "Adds
new enemy AI script")
Related Issue
Closes #ISSUE_NUMBER (if applicable)
Changes
system")
Testing
works on Win10 with 60 FPS")
Checklist
Additional Notes
Anything else? (e.g., "Tested on Win10 64-bit; needs Linux validation")
🤖 Bot & AI Contributions (Updated)
This pull request features extensive automated support from AI agents and development bots, with DeepSource specifically ensuring code health through rigorous static analysis.
Summary of Activity
GameSettingsResource. Automated summaries provided clear visibility into how the fuel system integration impacts player behavior, UI reactivity, and state persistence.GameSettingsResource.current_fuelwhere a recursive setter would have caused a stack overflow at runtime.player.gd, forcing the system to read directly fromGlobals.settingsto prevent UI/logic desynchronization.Globals.settingsin hot paths (like_physics_process) to prevent crashes during initialization.test_fuel_persistence_integration.gdand member variable declaration issues in edge case tests.AI & Bot Contributors
The following automated entities provided significant technical contributions and quality assurance for this PR:
📝 Contributors List
To include these automated contributors in your GitHub attribution, you can use the following handles:
Summary by Sourcery
Centralize the fuel system in a shared GameSettingsResource and integrate it across player behavior, UI, persistence, and tests.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by Sourcery
Centralize the fuel system in a shared GameSettingsResource and integrate it with the player, globals persistence, and tests.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit