From 6c8427d81be85f556504aefaf5e317970bb7d7b9 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Thu, 16 Apr 2026 21:56:25 -0700 Subject: [PATCH 01/36] [FEATURE] Implement Parallax Scrolling Background linked to Player Speed #543 Implement a dynamic ParallaxBackground system that automatically adjusts its scrolling speed based on the player's current forward velocity. --- scenes/main_scene.tscn | 4 +++- scripts/main_scene.gd | 16 +++++++++++----- scripts/parallax_manager.gd | 33 +++++++++++++++++++++++++++++++++ scripts/parallax_manager.gd.uid | 1 + 4 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 scripts/parallax_manager.gd create mode 100644 scripts/parallax_manager.gd.uid diff --git a/scenes/main_scene.tscn b/scenes/main_scene.tscn index 6fb7610f4..888ca363d 100644 --- a/scenes/main_scene.tscn +++ b/scenes/main_scene.tscn @@ -1,4 +1,4 @@ -[gd_scene load_steps=82 format=3 uid="uid://nnnc0qhx07i8"] +[gd_scene load_steps=83 format=3 uid="uid://nnnc0qhx07i8"] [ext_resource type="Script" uid="uid://ctm7qg12s2swt" path="res://scripts/main_scene.gd" id="1_7ykc4"] [ext_resource type="PackedScene" uid="uid://cb4n4cqkuddqg" path="res://scenes/pause_menu.tscn" id="1_w2twt"] @@ -77,6 +77,7 @@ [ext_resource type="Texture2D" uid="uid://bvxu5x1awjrjv" path="res://files/random_decor/dirt_001.png" id="69_7tyuc"] [ext_resource type="Texture2D" uid="uid://f4hxu68qa4fi" path="res://files/random_decor/dirt_002.png" id="70_isor2"] [ext_resource type="Script" uid="uid://blu5qujicfa7e" path="res://scripts/hud.gd" id="72_sgkfd"] +[ext_resource type="Script" uid="uid://b5x2ehdthhrla" path="res://scripts/parallax_manager.gd" id="76_qj6t7"] [sub_resource type="StyleBoxFlat" id="StyleBoxFlat_pu3yx"] bg_color = Color(0.2627451, 0.2627451, 0.2627451, 0.5882353) @@ -204,6 +205,7 @@ step = 1.0 [node name="PauseMenu" parent="." instance=ExtResource("1_w2twt")] [node name="Background" type="ParallaxBackground" parent="."] +script = ExtResource("76_qj6t7") [node name="Sand" type="ParallaxLayer" parent="Background"] diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 13afd0cbf..857dfadaa 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -1,4 +1,4 @@ -## Copyright (C) 2025 Egor Kostan +## Copyright (C) 2026 Egor Kostan ## SPDX-License-Identifier: GPL-3.0-or-later ## main_scene.gd ## Main scene script for SkyLockAssault. @@ -52,6 +52,12 @@ func _ready() -> void: # Setup decor layer with random instances setup_decor_layer(viewport_size) + # Wire up the signal architecture for the parallax background + if player.has_signal("speed_changed") and background.has_method("_on_player_speed_changed"): + player.speed_changed.connect(background._on_player_speed_changed) + # Prime the background with the player's initial starting speed + background._on_player_speed_changed(player.speed["speed"], 0.0) + # 2. Detect when player presses a key/button that has NO binding at all # Only significant inputs (axes above deadzone) are checked to prevent @@ -227,12 +233,12 @@ func _process(delta: float) -> void: return # Use the safe local reference for difficulty - var scroll_speed: float = player.speed["speed"] * delta * settings_res.difficulty * 0.8 - background.scroll_offset.y += scroll_speed + # var scroll_speed: float = player.speed["speed"] * delta * settings_res.difficulty * 0.8 + # background.scroll_offset.y += scroll_speed # Use the safe local reference for current_fuel - if settings_res.current_fuel <= 0: - background.scroll_offset = Vector2(0, 0) + # if settings_res.current_fuel <= 0: + # background.scroll_offset = Vector2(0, 0) # 1. Critical unbound controls warning (shown ONCE per session) # Flag stays true until player fixes bindings (e.g., in key_mapping.gd after remap). diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd new file mode 100644 index 000000000..fb77844af --- /dev/null +++ b/scripts/parallax_manager.gd @@ -0,0 +1,33 @@ +## Copyright (C) 2026 Egor Kostan +## SPDX-License-Identifier: GPL-3.0-or-later +## parallax_manager.gd +## Manages the scrolling speed of the parallax background based on player velocity. +## Decoupled from direct physics polling via the Observer Pattern. + +class_name ParallaxManager +extends ParallaxBackground + +var _current_speed: float = 0.0 + + +## Observer callback triggered when the player's speed changes. +## Updates the internal speed used for parallax scrolling. +## @param new_speed: float - The new forward speed of the player. +## @param _max_speed: float - The maximum speed threshold (unused). +## @return: void +func _on_player_speed_changed(new_speed: float, _max_speed: float) -> void: + _current_speed = new_speed + + +## Called every physics/rendering frame. Updates the vertical scroll offset +## based on the cached player speed and current game difficulty. +## @param delta: float - The elapsed time since the previous frame. +## @return: void +func _process(delta: float) -> void: + var difficulty: float = 1.0 + + if is_instance_valid(Globals) and is_instance_valid(Globals.settings): + difficulty = Globals.settings.difficulty + + var scroll_amount: float = _current_speed * delta * difficulty * 0.8 + scroll_offset.y += scroll_amount diff --git a/scripts/parallax_manager.gd.uid b/scripts/parallax_manager.gd.uid new file mode 100644 index 000000000..947ef9ab6 --- /dev/null +++ b/scripts/parallax_manager.gd.uid @@ -0,0 +1 @@ +uid://b5x2ehdthhrla From 1246521e48cfb373f0a5b23903ea9681894a0d89 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Thu, 16 Apr 2026 22:00:51 -0700 Subject: [PATCH 02/36] Update main_scene.gd --- scripts/main_scene.gd | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 857dfadaa..07ead13dc 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -223,7 +223,7 @@ func setup_decor_layer(viewport: Vector2) -> void: decor_layer.motion_mirroring = Vector2(0, layer_height) -func _process(delta: float) -> void: +func _process(_delta: float) -> void: # NEW: Safely grab the settings resource and guard against null crashes # during scene transitions, engine shutdown, or isolated GUT tests. var settings_res: GameSettingsResource = ( @@ -232,14 +232,6 @@ func _process(delta: float) -> void: if not is_instance_valid(settings_res): return - # Use the safe local reference for difficulty - # var scroll_speed: float = player.speed["speed"] * delta * settings_res.difficulty * 0.8 - # background.scroll_offset.y += scroll_speed - - # Use the safe local reference for current_fuel - # if settings_res.current_fuel <= 0: - # background.scroll_offset = Vector2(0, 0) - # 1. Critical unbound controls warning (shown ONCE per session) # Flag stays true until player fixes bindings (e.g., in key_mapping.gd after remap). # Do NOT reset here — that would make it repeat every 4s (bug fixed). From b137b63085ddb26ffec1bd7e8ffe79ea02d12f4a Mon Sep 17 00:00:00 2001 From: Egor Kostan <20955183+ikostan@users.noreply.github.com> Date: Thu, 16 Apr 2026 22:02:55 -0700 Subject: [PATCH 03/36] Update scripts/main_scene.gd Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --- scripts/main_scene.gd | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 07ead13dc..6419b8a8d 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -57,6 +57,12 @@ func _ready() -> void: player.speed_changed.connect(background._on_player_speed_changed) # Prime the background with the player's initial starting speed background._on_player_speed_changed(player.speed["speed"], 0.0) + elif not player.has_signal("speed_changed"): + push_warning("Parallax background not wired: player is missing the `speed_changed` signal. " + + "Verify that the Player node defines and emits `speed_changed`.") + elif not background.has_method("_on_player_speed_changed"): + push_warning("Parallax background not wired: background is missing `_on_player_speed_changed` method. " + + "Ensure the background script implements `_on_player_speed_changed(speed: float, delta: float)`.") # 2. Detect when player presses a key/button that has NO binding at all From e436db6c797e43670a62d9ff8c00e9464455c9e1 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Thu, 16 Apr 2026 22:04:02 -0700 Subject: [PATCH 04/36] Update main_scene.gd --- scripts/main_scene.gd | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 6419b8a8d..3d2feb43b 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -58,11 +58,19 @@ func _ready() -> void: # Prime the background with the player's initial starting speed background._on_player_speed_changed(player.speed["speed"], 0.0) elif not player.has_signal("speed_changed"): - push_warning("Parallax background not wired: player is missing the `speed_changed` signal. " - + "Verify that the Player node defines and emits `speed_changed`.") + push_warning( + ( + "Parallax background not wired: player is missing the `speed_changed` signal. " + + "Verify that the Player node defines and emits `speed_changed`." + ) + ) elif not background.has_method("_on_player_speed_changed"): - push_warning("Parallax background not wired: background is missing `_on_player_speed_changed` method. " - + "Ensure the background script implements `_on_player_speed_changed(speed: float, delta: float)`.") + push_warning( + ( + "Parallax background not wired: background is missing `_on_player_speed_changed` method. " + + "Ensure the background script implements `_on_player_speed_changed(speed: float, delta: float)`." + ) + ) # 2. Detect when player presses a key/button that has NO binding at all From e3a9b70aa47518243d76db614ce9ab97cd232cf3 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Thu, 16 Apr 2026 22:06:15 -0700 Subject: [PATCH 05/36] Update main_scene.gd --- scripts/main_scene.gd | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 3d2feb43b..585ad1c09 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -67,8 +67,10 @@ func _ready() -> void: elif not background.has_method("_on_player_speed_changed"): push_warning( ( - "Parallax background not wired: background is missing `_on_player_speed_changed` method. " - + "Ensure the background script implements `_on_player_speed_changed(speed: float, delta: float)`." + "Parallax background not wired: background is missing" + + " `_on_player_speed_changed` method. " + + "Ensure the background script implements " + + " `_on_player_speed_changed(speed: float, delta: float)`." ) ) From 7040237dfba2973ff3af4ef3fcca76348293f439 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Thu, 16 Apr 2026 22:08:22 -0700 Subject: [PATCH 06/36] Here is a comprehensive GUT unit test suite for the new parallax_manager.gd file. Here is a comprehensive GUT unit test suite for the new parallax_manager.gd file. This test file mirrors the high standards of your existing tests. It includes full static typing, proper setup/teardown encapsulation, file headers, and specific scenario testing (Observer updates, math calculations, and null safety). --- test/gut/test_parallax_manager.gd | 128 ++++++++++++++++++++++++++ test/gut/test_parallax_manager.gd.uid | 1 + 2 files changed, 129 insertions(+) create mode 100644 test/gut/test_parallax_manager.gd create mode 100644 test/gut/test_parallax_manager.gd.uid diff --git a/test/gut/test_parallax_manager.gd b/test/gut/test_parallax_manager.gd new file mode 100644 index 000000000..e678768f7 --- /dev/null +++ b/test/gut/test_parallax_manager.gd @@ -0,0 +1,128 @@ +## Copyright (C) 2026 Egor Kostan +## SPDX-License-Identifier: GPL-3.0-or-later +## test_parallax_manager.gd +## +## GUT unit tests for the ParallaxManager script. +## Validates observer pattern synchronization, scroll offset math, and fallback safety. +extends "res://addons/gut/test.gd" + +var _parallax_manager: ParallaxManager +var _original_settings: GameSettingsResource + + +## Per-test setup: Isolates the global resource state and instantiates the manager. +## :rtype: void +func before_each() -> void: + _original_settings = Globals.settings + Globals.settings = GameSettingsResource.new() + Globals.settings.difficulty = 1.0 + + _parallax_manager = ParallaxManager.new() + add_child_autofree(_parallax_manager) + + +## Post-test cleanup: Restores global state to prevent test leakage. +## :rtype: void +func after_each() -> void: + Globals.settings = _original_settings + + +# ========================================== +# OBSERVER INTEGRATION TESTS +# ========================================== + +## test_speed_update_from_signal | Observer Integration +## :rtype: void +func test_speed_update_from_signal() -> void: + gut.p("Testing: ParallaxManager correctly caches speed from the player's signal.") + + # Simulate the Player broadcasting a new speed of 250.0 + _parallax_manager._on_player_speed_changed(250.0, 500.0) + + assert_eq( + _parallax_manager._current_speed, + 250.0, + "Manager must update _current_speed when signal callback is invoked." + ) + + +# ========================================== +# PROCESS & MATH LOGIC TESTS +# ========================================== + +## test_scroll_offset_math | Process Logic +## :rtype: void +func test_scroll_offset_math() -> void: + gut.p("Testing: Process loop correctly calculates the scroll offset increment based on difficulty.") + + # 1. Setup specific variables for predictable math + Globals.settings.difficulty = 2.0 + _parallax_manager._current_speed = 100.0 + _parallax_manager.scroll_offset.y = 0.0 + + # 2. Simulate one physics frame + var delta: float = 0.5 + _parallax_manager._process(delta) + + # 3. Verify the math + # Expected math: speed(100.0) * delta(0.5) * diff(2.0) * multiplier(0.8) = 80.0 + var expected_offset: float = 100.0 * 0.5 * 2.0 * 0.8 + + assert_almost_eq( + _parallax_manager.scroll_offset.y, + expected_offset, + 0.01, + "Scroll offset must accurately reflect the delta, speed, and difficulty multiplier." + ) + + +## test_zero_speed_stops_scroll | State Management +## :rtype: void +func test_zero_speed_stops_scroll() -> void: + gut.p("Testing: A speed of 0.0 results in a halted background scroll.") + + # 1. Setup flameout/halt state + _parallax_manager._current_speed = 0.0 + var initial_offset: float = 125.5 + _parallax_manager.scroll_offset.y = initial_offset + + # 2. Simulate processing frame + _parallax_manager._process(1.0) + + # 3. Verify no movement + assert_eq( + _parallax_manager.scroll_offset.y, + initial_offset, + "Scroll offset must remain completely unchanged when speed is zero." + ) + + +# ========================================== +# SAFETY & EDGE CASE TESTS +# ========================================== + +## test_process_safe_without_globals | Safety Constraint +## :rtype: void +func test_process_safe_without_globals() -> void: + gut.p("Testing: ParallaxManager defaults to difficulty 1.0 if Globals.settings is missing.") + + # 1. Force a null state (simulating scene transition or engine shutdown) + Globals.settings = null + + _parallax_manager._current_speed = 100.0 + _parallax_manager.scroll_offset.y = 0.0 + + # 2. Simulate processing frame + var delta: float = 1.0 + _parallax_manager._process(delta) + + # 3. Verify the math defaulted safely + # Expected math: speed(100.0) * delta(1.0) * diff(1.0 fallback) * multiplier(0.8) = 80.0 + var expected_offset: float = 100.0 * 1.0 * 1.0 * 0.8 + + assert_almost_eq( + _parallax_manager.scroll_offset.y, + expected_offset, + 0.01, + "Process must fall back to a 1.0 difficulty multiplier without throwing null instance errors." + ) diff --git a/test/gut/test_parallax_manager.gd.uid b/test/gut/test_parallax_manager.gd.uid new file mode 100644 index 000000000..1b743fc69 --- /dev/null +++ b/test/gut/test_parallax_manager.gd.uid @@ -0,0 +1 @@ +uid://8158s2i62s66 From d6b27ed771c9aa66f22d829349edd55dc96fa452 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Fri, 17 Apr 2026 20:42:19 -0700 Subject: [PATCH 07/36] Update parallax_manager.gd The new ParallaxManager drops the previous current_fuel <= 0 handling that reset scroll_offset, so if that behavior is still desired you should migrate that logic into the manager instead of just commenting it out in main_scene.gd. To satisfy the PR reviewer's feedback while maintaining the clean architecture we established, we need to reintroduce the current_fuel <= 0 check into the ParallaxManager. Since we are already safely accessing Globals.settings inside the _process loop to grab the game's difficulty, we can cleanly grab the current_fuel state in that exact same block and enforce the Vector2.ZERO offset reset. --- scripts/parallax_manager.gd | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index fb77844af..af3e97ca3 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -21,13 +21,20 @@ func _on_player_speed_changed(new_speed: float, _max_speed: float) -> void: ## Called every physics/rendering frame. Updates the vertical scroll offset ## based on the cached player speed and current game difficulty. +## Explicitly resets the scroll offset to zero if the player runs out of fuel. ## @param delta: float - The elapsed time since the previous frame. ## @return: void func _process(delta: float) -> void: var difficulty: float = 1.0 + var current_fuel: float = 1.0 # Default to > 0 to prevent accidental resets if Globals is null if is_instance_valid(Globals) and is_instance_valid(Globals.settings): difficulty = Globals.settings.difficulty - - var scroll_amount: float = _current_speed * delta * difficulty * 0.8 - scroll_offset.y += scroll_amount + current_fuel = Globals.settings.current_fuel + + # Enforce the legacy behavior: reset offset immediately on flameout + if current_fuel <= 0.0: + scroll_offset = Vector2.ZERO + else: + var scroll_amount: float = _current_speed * delta * difficulty * 0.8 + scroll_offset.y += scroll_amount From 64b1bbc00e3ba066dd80142e2b2aeaece61d59e1 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Fri, 17 Apr 2026 20:51:15 -0700 Subject: [PATCH 08/36] completely eliminate the _process polling by using Dependency Injection alongside the Observer pattern ParallaxManager pulls difficulty from the global Globals.settings each frame, which introduces tight coupling and repeated lookups; consider injecting difficulty (or settings) via a property or signal from the main scene instead. We can completely eliminate the _process polling by using Dependency Injection alongside the Observer pattern we already set up. main_scene.gd will pass the settings resource into the manager exactly once, and the manager will listen for the difficulty and fuel changes via signals. --- scripts/main_scene.gd | 14 ++++++++ scripts/parallax_manager.gd | 57 +++++++++++++++++++++++-------- test/gut/test_parallax_manager.gd | 3 ++ 3 files changed, 59 insertions(+), 15 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 585ad1c09..a943558a6 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -52,6 +52,20 @@ func _ready() -> void: # Setup decor layer with random instances setup_decor_layer(viewport_size) + # Wire up the signal architecture for the parallax background +# ========================================================= + # DEPENDENCY INJECTION: Parallax Background + # ========================================================= + if background.has_method("setup"): + var settings_res: GameSettingsResource = ( + Globals.settings if is_instance_valid(Globals) else null + ) + background.setup(settings_res) + else: + push_warning( + "Parallax background is missing the `setup` method. Settings injection failed." + ) + # Wire up the signal architecture for the parallax background if player.has_signal("speed_changed") and background.has_method("_on_player_speed_changed"): player.speed_changed.connect(background._on_player_speed_changed) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index af3e97ca3..277afb2a9 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -2,16 +2,34 @@ ## SPDX-License-Identifier: GPL-3.0-or-later ## parallax_manager.gd ## Manages the scrolling speed of the parallax background based on player velocity. -## Decoupled from direct physics polling via the Observer Pattern. +## Decoupled via Dependency Injection and the Observer Pattern. class_name ParallaxManager extends ParallaxBackground var _current_speed: float = 0.0 +var _difficulty: float = 1.0 +var _out_of_fuel: bool = false + + +## Injects the game settings resource and wires up observer signals. +## Prevents tight coupling to global singletons in the process loop. +## @param settings: GameSettingsResource - The configuration resource. +## @return: void +func setup(settings: GameSettingsResource) -> void: + if not is_instance_valid(settings): + return + + _difficulty = settings.difficulty + _out_of_fuel = (settings.current_fuel <= 0.0) + + if not settings.setting_changed.is_connected(_on_setting_changed): + settings.setting_changed.connect(_on_setting_changed) + if not settings.fuel_depleted.is_connected(_on_fuel_depleted): + settings.fuel_depleted.connect(_on_fuel_depleted) ## Observer callback triggered when the player's speed changes. -## Updates the internal speed used for parallax scrolling. ## @param new_speed: float - The new forward speed of the player. ## @param _max_speed: float - The maximum speed threshold (unused). ## @return: void @@ -19,22 +37,31 @@ func _on_player_speed_changed(new_speed: float, _max_speed: float) -> void: _current_speed = new_speed -## Called every physics/rendering frame. Updates the vertical scroll offset -## based on the cached player speed and current game difficulty. -## Explicitly resets the scroll offset to zero if the player runs out of fuel. -## @param delta: float - The elapsed time since the previous frame. +## Observer callback for specific setting updates (difficulty and refueling). +## @param setting_name: String - The name of the changed setting. +## @param new_value: Variant - The updated value. ## @return: void -func _process(delta: float) -> void: - var difficulty: float = 1.0 - var current_fuel: float = 1.0 # Default to > 0 to prevent accidental resets if Globals is null +func _on_setting_changed(setting_name: String, new_value: Variant) -> void: + if setting_name == "difficulty": + _difficulty = float(new_value) + elif setting_name == "current_fuel" and float(new_value) > 0.0: + _out_of_fuel = false - if is_instance_valid(Globals) and is_instance_valid(Globals.settings): - difficulty = Globals.settings.difficulty - current_fuel = Globals.settings.current_fuel - # Enforce the legacy behavior: reset offset immediately on flameout - if current_fuel <= 0.0: +## Observer callback to instantly snap the background when fuel runs out. +## @return: void +func _on_fuel_depleted() -> void: + _out_of_fuel = true + scroll_offset = Vector2.ZERO + + +## Called every physics/rendering frame. +## Updates scroll offset based entirely on cached local variables. +## @param delta: float - The elapsed time since the previous frame. +## @return: void +func _process(delta: float) -> void: + if _out_of_fuel: scroll_offset = Vector2.ZERO else: - var scroll_amount: float = _current_speed * delta * difficulty * 0.8 + var scroll_amount: float = _current_speed * delta * _difficulty * 0.8 scroll_offset.y += scroll_amount diff --git a/test/gut/test_parallax_manager.gd b/test/gut/test_parallax_manager.gd index e678768f7..e608a1fd5 100644 --- a/test/gut/test_parallax_manager.gd +++ b/test/gut/test_parallax_manager.gd @@ -19,6 +19,9 @@ func before_each() -> void: _parallax_manager = ParallaxManager.new() add_child_autofree(_parallax_manager) + + # NEW: Inject the test settings into the manager + _parallax_manager.setup(Globals.settings) ## Post-test cleanup: Restores global state to prevent test leakage. From 1dbb09deba411ef49d8f72d6546a810d48fc024a Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Fri, 17 Apr 2026 20:57:28 -0700 Subject: [PATCH 09/36] Missing coverage for the fuel-depletion reset, and tests implicitly depend on current_fuel's default. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The whole point of migrating logic into ParallaxManager (per the PR discussion) was to preserve the current_fuel <= 0 → scroll_offset = Vector2.ZERO behavior — but there is no test asserting this flameout reset. In addition, before_each() constructs a fresh GameSettingsResource without ever setting current_fuel, so: test_scroll_offset_math only passes if GameSettingsResource.current_fuel happens to default to > 0. If the default is ever changed to 0.0 (common for "fuel to be filled at game start"), the production code would hit the current_fuel <= 0 branch, reset scroll_offset to Vector2.ZERO, and the 80.0 expectation would fail — for reasons unrelated to the math being tested. test_zero_speed_stops_scroll is similarly coupled: with a fuel default of 0, scroll_offset.y would be reset to 0 rather than staying at 125.5, producing a misleading failure. --- test/gut/test_parallax_manager.gd | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/gut/test_parallax_manager.gd b/test/gut/test_parallax_manager.gd index e608a1fd5..95d6f1c69 100644 --- a/test/gut/test_parallax_manager.gd +++ b/test/gut/test_parallax_manager.gd @@ -16,6 +16,7 @@ func before_each() -> void: _original_settings = Globals.settings Globals.settings = GameSettingsResource.new() Globals.settings.difficulty = 1.0 + Globals.settings.current_fuel = 100.0 # Ensure scroll is not gated by flameout reset _parallax_manager = ParallaxManager.new() add_child_autofree(_parallax_manager) @@ -100,6 +101,21 @@ func test_zero_speed_stops_scroll() -> void: ) +## test_flameout_resets_offset | State Management +## :rtype: void +func test_flameout_resets_offset() -> void: + gut.p("Testing: current_fuel <= 0 resets scroll_offset to Vector2.ZERO.") + Globals.settings.current_fuel = 0.0 + _parallax_manager._current_speed = 100.0 + _parallax_manager.scroll_offset = Vector2(42.0, 125.5) + _parallax_manager._process(0.5) + assert_eq( + _parallax_manager.scroll_offset, + Vector2.ZERO, + "Offset must reset to ZERO when fuel is depleted." + ) + + # ========================================== # SAFETY & EDGE CASE TESTS # ========================================== From 6af4227ad93231c4239a59d15b8f1621b30783d3 Mon Sep 17 00:00:00 2001 From: Egor Kostan <20955183+ikostan@users.noreply.github.com> Date: Fri, 17 Apr 2026 20:59:19 -0700 Subject: [PATCH 10/36] Update test/gut/test_parallax_manager.gd Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- test/gut/test_parallax_manager.gd | 1 + 1 file changed, 1 insertion(+) diff --git a/test/gut/test_parallax_manager.gd b/test/gut/test_parallax_manager.gd index 95d6f1c69..10edbda75 100644 --- a/test/gut/test_parallax_manager.gd +++ b/test/gut/test_parallax_manager.gd @@ -20,6 +20,7 @@ func before_each() -> void: _parallax_manager = ParallaxManager.new() add_child_autofree(_parallax_manager) + _parallax_manager.set_process(false) # Only run _process explicitly from tests # NEW: Inject the test settings into the manager _parallax_manager.setup(Globals.settings) From d5cc59a802af002e2f6a41e72d66ea890f0ff655 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Fri, 17 Apr 2026 21:06:18 -0700 Subject: [PATCH 11/36] Update main_scene.gd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In _ready you call background._on_player_speed_changed(player.speed["speed"], 0.0) to prime the parallax, but the second argument is always 0.0 and unused—either remove the unused parameter from the signal/handler or pass the actual max speed if it is intended to be meaningful. The reviewer caught a great detail. We have two options here, but only one is architecturally safe: Remove the parameter: We cannot do this. The speed_changed(new_speed, max_speed) signal is broadcast by player.gd, and the HUD relies heavily on that max_speed parameter to calculate the color lerping (Green -> Yellow -> Red) for the UI progress bars. If we change the signal signature, the HUD breaks. Pass the actual max speed: This is the correct approach. Since parallax_manager.gd connects to that signal, it must accept both parameters to match the signature (even if it only uses the first one). Therefore, when we manually prime it in main_scene.gd, we should pass the real max_speed instead of a lazy 0.0. --- scripts/main_scene.gd | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index a943558a6..98c0733e9 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -52,14 +52,16 @@ func _ready() -> void: # Setup decor layer with random instances setup_decor_layer(viewport_size) - # Wire up the signal architecture for the parallax background -# ========================================================= + # ========================================================= # DEPENDENCY INJECTION: Parallax Background # ========================================================= + # Wire up the signal architecture for the parallax background + # Safely extract settings once to use for both injection and priming + var settings_res: GameSettingsResource = ( + Globals.settings if is_instance_valid(Globals) else null + ) + if background.has_method("setup"): - var settings_res: GameSettingsResource = ( - Globals.settings if is_instance_valid(Globals) else null - ) background.setup(settings_res) else: push_warning( @@ -69,8 +71,13 @@ func _ready() -> void: # Wire up the signal architecture for the parallax background if player.has_signal("speed_changed") and background.has_method("_on_player_speed_changed"): player.speed_changed.connect(background._on_player_speed_changed) - # Prime the background with the player's initial starting speed - background._on_player_speed_changed(player.speed["speed"], 0.0) + + # Prime the background with the player's initial starting speed and actual max speed + var initial_max_speed: float = ( + settings_res.max_speed if is_instance_valid(settings_res) else 800.0 + ) + background._on_player_speed_changed(player.speed["speed"], initial_max_speed) + elif not player.has_signal("speed_changed"): push_warning( ( From 39bd96357441bdb6471f8e090ea729bbb6a10e9c Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Fri, 17 Apr 2026 21:10:48 -0700 Subject: [PATCH 12/36] Update parallax_manager.gd The Encapsulation Leak: Calling _on_player_speed_changed from main_scene.gd forces the main scene to "pretend" to be a signal emitter, which is dirty architecture. Creating a public prime_speed() method fixes this, and as a bonus, it means we don't have to clumsily pass the max_speed parameter just to satisfy the signal signature. --- scripts/parallax_manager.gd | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index 277afb2a9..95149c059 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -29,6 +29,14 @@ func setup(settings: GameSettingsResource) -> void: settings.fuel_depleted.connect(_on_fuel_depleted) +## Public method to prime the background's initial speed. +## Keeps private signal handlers properly encapsulated. +## @param initial_speed: float - The starting forward speed. +## @return: void +func prime_speed(initial_speed: float) -> void: + _current_speed = initial_speed + + ## Observer callback triggered when the player's speed changes. ## @param new_speed: float - The new forward speed of the player. ## @param _max_speed: float - The maximum speed threshold (unused). From ee9263694d06b455cd61fdfdcfdb8e0c2d9d03cc Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Fri, 17 Apr 2026 21:11:00 -0700 Subject: [PATCH 13/36] Update main_scene.gd The Connection Guard: If _ready() runs twice (e.g., moving nodes in and out of the tree, or in certain GUT testing environments), connect() will throw an error if it's already connected. Wrapping it in is_connected is a bulletproof safety net. --- scripts/main_scene.gd | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 98c0733e9..a8db38250 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -70,13 +70,15 @@ func _ready() -> void: # Wire up the signal architecture for the parallax background if player.has_signal("speed_changed") and background.has_method("_on_player_speed_changed"): - player.speed_changed.connect(background._on_player_speed_changed) - - # Prime the background with the player's initial starting speed and actual max speed - var initial_max_speed: float = ( - settings_res.max_speed if is_instance_valid(settings_res) else 800.0 - ) - background._on_player_speed_changed(player.speed["speed"], initial_max_speed) + # 1. Guard against duplicate connections + if not player.speed_changed.is_connected(background._on_player_speed_changed): + player.speed_changed.connect(background._on_player_speed_changed) + + # 2. Prime the background securely via a public method + if background.has_method("prime_speed"): + background.prime_speed(player.speed["speed"]) + else: + push_warning("Parallax background is missing the `prime_speed` method.") elif not player.has_signal("speed_changed"): push_warning( From 43ed4c21b27b543f51e7a7e87e4081aa7680eaff Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Fri, 17 Apr 2026 21:15:53 -0700 Subject: [PATCH 14/36] Update main_scene.gd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Globals is a Godot autoload, so the engine guarantees it exists for the lifetime of the scene tree. The check adds no safety and just makes the intent less clear — simplifying to only guard Globals.settings (which can legitimately be null during transitions per the pattern in scripts/player.gd:50-57) would be cleaner. The reviewer is absolutely correct here. In Godot, an Autoload (like your Globals) is attached to the root of the SceneTree before any of your scenes even load, and it stays there until the application quits entirely. Because the engine guarantees its existence, checking is_instance_valid(Globals) is redundant "paranoid" coding. It clutters the script and makes it look like you expect the singleton to be destroyed. We only need to check if the property inside it (Globals.settings) is valid. --- scripts/main_scene.gd | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index a8db38250..4731cb7f6 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -57,9 +57,9 @@ func _ready() -> void: # ========================================================= # Wire up the signal architecture for the parallax background # Safely extract settings once to use for both injection and priming - var settings_res: GameSettingsResource = ( - Globals.settings if is_instance_valid(Globals) else null - ) + # Note: Because background.setup(settings_res) already has its own internal + # if not is_instance_valid(settings): return check, this is perfectly safe + var settings_res: GameSettingsResource = Globals.settings if background.has_method("setup"): background.setup(settings_res) @@ -263,11 +263,10 @@ func setup_decor_layer(viewport: Vector2) -> void: func _process(_delta: float) -> void: - # NEW: Safely grab the settings resource and guard against null crashes + # Safely grab the settings resource and guard against null crashes # during scene transitions, engine shutdown, or isolated GUT tests. - var settings_res: GameSettingsResource = ( - Globals.settings if is_instance_valid(Globals) else null - ) + var settings_res: GameSettingsResource = Globals.settings + if not is_instance_valid(settings_res): return From f6eeb1fa9e995de64fc8e11a955a91f1ce2ec7fa Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Fri, 17 Apr 2026 21:16:54 -0700 Subject: [PATCH 15/36] Update main_scene.gd --- scripts/main_scene.gd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 4731cb7f6..2e3cd560e 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -266,7 +266,7 @@ func _process(_delta: float) -> void: # Safely grab the settings resource and guard against null crashes # during scene transitions, engine shutdown, or isolated GUT tests. var settings_res: GameSettingsResource = Globals.settings - + if not is_instance_valid(settings_res): return From 19467a2885353153ca792b5f06561f05f7862229 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:03:07 -0700 Subject: [PATCH 16/36] =?UTF-8?q?scroll=5Foffset.y=20grows=20unboundedly?= =?UTF-8?q?=20=E2=80=94=20consider=20wrapping=20to=20preserve=20float=20pr?= =?UTF-8?q?ecision=20over=20long=20sessions.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scroll_offset.y += scroll_amount accumulates indefinitely. At ~250 px/s × 0.8 × difficulty, the value can reach millions after extended play, at which point float32 precision degrades visibly (jitter/stutter in the parallax scroll). Since the ParallaxLayer children use motion_mirroring to tile, the offset can safely be taken modulo the mirroring period. --- scripts/main_scene.gd | 9 ++++++++- scripts/parallax_manager.gd | 17 ++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 2e3cd560e..cb24d8748 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -58,7 +58,6 @@ func _ready() -> void: # Wire up the signal architecture for the parallax background # Safely extract settings once to use for both injection and priming # Note: Because background.setup(settings_res) already has its own internal - # if not is_instance_valid(settings): return check, this is perfectly safe var settings_res: GameSettingsResource = Globals.settings if background.has_method("setup"): @@ -96,6 +95,14 @@ func _ready() -> void: + " `_on_player_speed_changed(speed: float, delta: float)`." ) ) + # ========================================================= + # FLOAT DEGRADATION SAFEGUARD + # ========================================================= + # Calculate global wrap period based on the tallest layer to prevent float degradation + # Formula: motion_mirroring.y / motion_scale.y + var global_bushes_period: float = (viewport_size.y * parallax_screens_tall) / 0.5 + if background.has_method("set_wrap_period"): + background.set_wrap_period(global_bushes_period) # 2. Detect when player presses a key/button that has NO binding at all diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index 95149c059..20f77d00a 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -7,6 +7,10 @@ class_name ParallaxManager extends ParallaxBackground +## Optional wrap limit to prevent float32 precision degradation over long sessions. +## Should be a common multiple of the layers' (motion_mirroring.y / motion_scale.y). +@export var wrap_period: float = 0.0 + var _current_speed: float = 0.0 var _difficulty: float = 1.0 var _out_of_fuel: bool = false @@ -37,6 +41,13 @@ func prime_speed(initial_speed: float) -> void: _current_speed = initial_speed +## Public method to dynamically set the wrap limit to prevent float precision loss. +## @param period: float - The calculated period to wrap the offset around. +## @return: void +func set_wrap_period(period: float) -> void: + wrap_period = period + + ## Observer callback triggered when the player's speed changes. ## @param new_speed: float - The new forward speed of the player. ## @param _max_speed: float - The maximum speed threshold (unused). @@ -64,7 +75,7 @@ func _on_fuel_depleted() -> void: ## Called every physics/rendering frame. -## Updates scroll offset based entirely on cached local variables. +## Updates scroll offset based entirely on cached local variables and wraps to preserve float precision. ## @param delta: float - The elapsed time since the previous frame. ## @return: void func _process(delta: float) -> void: @@ -73,3 +84,7 @@ func _process(delta: float) -> void: else: var scroll_amount: float = _current_speed * delta * _difficulty * 0.8 scroll_offset.y += scroll_amount + + # Prevent float precision degradation by wrapping modulo the period + if wrap_period > 0.0: + scroll_offset.y = wrapf(scroll_offset.y, 0.0, wrap_period) From bc0702ebfe698899c2d4a69610970e52098107c2 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:04:08 -0700 Subject: [PATCH 17/36] Update main_scene.gd --- scripts/main_scene.gd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index cb24d8748..d71f1e2cd 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -100,7 +100,7 @@ func _ready() -> void: # ========================================================= # Calculate global wrap period based on the tallest layer to prevent float degradation # Formula: motion_mirroring.y / motion_scale.y - var global_bushes_period: float = (viewport_size.y * parallax_screens_tall) / 0.5 + var global_bushes_period: float = (viewport_size.y * parallax_screens_tall) / 0.5 if background.has_method("set_wrap_period"): background.set_wrap_period(global_bushes_period) From b101d71ce8872d7bbb4042b826011ba2c2695b78 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:05:36 -0700 Subject: [PATCH 18/36] Update parallax_manager.gd --- scripts/parallax_manager.gd | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index 20f77d00a..91a47017c 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -75,7 +75,8 @@ func _on_fuel_depleted() -> void: ## Called every physics/rendering frame. -## Updates scroll offset based entirely on cached local variables and wraps to preserve float precision. +## Updates scroll offset based entirely on cached local variables +## and wraps to preserve float precision. ## @param delta: float - The elapsed time since the previous frame. ## @return: void func _process(delta: float) -> void: From 9abb76a97ac06de3084d4a5920ccabf4b7c89c5c Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:10:56 -0700 Subject: [PATCH 19/36] 126-148: Nit: test name/assertion message is slightly misleading. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 126-148: Nit: test name/assertion message is slightly misleading. Since _process no longer reads Globals.settings at all (it uses cached _difficulty set by setup() in before_each), this test actually verifies that cached state still drives _process after globals are nulled — not that a 1.0 fallback kicks in. The math passes only because setup() happened to cache difficulty = 1.0 before the null. Consider renaming to something like test_process_safe_with_null_globals_after_setup and updating the comment/assertion so the intent ("doesn't crash / keeps using cached values") matches the actual behavior. To truly exercise the _difficulty = 1.0 initial fallback path, you'd need to instantiate a fresh ParallaxManager without calling setup() and null globals before the first _process. --- test/gut/test_parallax_manager.gd | 42 +++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/test/gut/test_parallax_manager.gd b/test/gut/test_parallax_manager.gd index 10edbda75..462ec1067 100644 --- a/test/gut/test_parallax_manager.gd +++ b/test/gut/test_parallax_manager.gd @@ -121,28 +121,56 @@ func test_flameout_resets_offset() -> void: # SAFETY & EDGE CASE TESTS # ========================================== -## test_process_safe_without_globals | Safety Constraint +## test_process_safe_with_null_globals_after_setup | Safety Constraint ## :rtype: void -func test_process_safe_without_globals() -> void: - gut.p("Testing: ParallaxManager defaults to difficulty 1.0 if Globals.settings is missing.") +func test_process_safe_with_null_globals_after_setup() -> void: + gut.p("Testing: ParallaxManager continues using cached state and does not crash if Globals drop.") # 1. Force a null state (simulating scene transition or engine shutdown) Globals.settings = null - _parallax_manager._current_speed = 100.0 + _parallax_manager.prime_speed(100.0) _parallax_manager.scroll_offset.y = 0.0 # 2. Simulate processing frame var delta: float = 1.0 _parallax_manager._process(delta) - # 3. Verify the math defaulted safely - # Expected math: speed(100.0) * delta(1.0) * diff(1.0 fallback) * multiplier(0.8) = 80.0 + # 3. Verify the math used the cached difficulty (1.0 from before_each) + # Expected math: speed(100.0) * delta(1.0) * cached_diff(1.0) * multiplier(0.8) = 80.0 var expected_offset: float = 100.0 * 1.0 * 1.0 * 0.8 assert_almost_eq( _parallax_manager.scroll_offset.y, expected_offset, 0.01, - "Process must fall back to a 1.0 difficulty multiplier without throwing null instance errors." + "Process must use cached difficulty and avoid null instance errors when Globals are missing." + ) + + +## test_process_uses_default_values_without_setup | Initialization +## :rtype: void +func test_process_uses_default_values_without_setup() -> void: + gut.p("Testing: ParallaxManager uses safe default values (difficulty 1.0) if setup() is never called.") + + # 1. Create a fresh manager without calling setup() + var uninitialized_manager: ParallaxManager = ParallaxManager.new() + add_child_autofree(uninitialized_manager) + + uninitialized_manager.prime_speed(100.0) + uninitialized_manager.scroll_offset.y = 0.0 + + # 2. Simulate processing frame + var delta: float = 1.0 + uninitialized_manager._process(delta) + + # 3. Verify the math used the default initialized difficulty of 1.0 + # Expected math: speed(100.0) * delta(1.0) * default_diff(1.0) * multiplier(0.8) = 80.0 + var expected_offset: float = 100.0 * 1.0 * 1.0 * 0.8 + + assert_almost_eq( + uninitialized_manager.scroll_offset.y, + expected_offset, + 0.01, + "Process must use its baseline difficulty of 1.0 if dependency injection never occurs." ) From 2c99f7c3bb00d4d20c2850856081dcf47029edf4 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:13:40 -0700 Subject: [PATCH 20/36] =?UTF-8?q?63-67:=20LGTM=20=E2=80=94=20refuel=20reco?= =?UTF-8?q?very=20path=20is=20correctly=20wired.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 63-67: LGTM — refuel recovery path is correctly wired. The current_fuel > 0 branch clears _out_of_fuel, and the depletion transition is handled by the dedicated fuel_depleted signal emitted by game_settings_resource.gd (lines 147-148). No redundancy, and the observer chain avoids per-frame polling as intended by the PR. Minor suggestion for test coverage: there's currently no GUT test asserting that setting current_fuel back to a positive value after a flameout clears _out_of_fuel and allows scroll to resume on the next _process. Worth adding as a follow-up. --- test/gut/test_parallax_manager.gd | 41 +++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/gut/test_parallax_manager.gd b/test/gut/test_parallax_manager.gd index 462ec1067..ce3e9842d 100644 --- a/test/gut/test_parallax_manager.gd +++ b/test/gut/test_parallax_manager.gd @@ -117,6 +117,47 @@ func test_flameout_resets_offset() -> void: ) +## test_flameout_recovery_resumes_scroll | State Management +## Tests the exact recovery path of the ParallaxManager after a flameout event. +## Verifies that pushing a positive fuel value via the global Observer pattern +## successfully flips the internal `_out_of_fuel` boolean back to false, allowing +## the `_process` loop to seamlessly resume parallax scrolling without needing a scene reload. +## :rtype: void +func test_flameout_recovery_resumes_scroll() -> void: + gut.p("Testing: Refueling after a flameout clears the _out_of_fuel state and resumes scrolling.") + + # 1. Setup initial speed and force the flameout state + _parallax_manager.prime_speed(100.0) + _parallax_manager._on_fuel_depleted() # Simulates the global fuel_depleted signal + + # Verify the background is hard-stopped (Baseline Assertion) + _parallax_manager._process(1.0) + assert_eq( + _parallax_manager.scroll_offset.y, + 0.0, + "PRE-CONDITION: Scroll must be completely locked to ZERO during a flameout." + ) + + # 2. Simulate Refueling via the Observer Pattern + # This mimics `main_scene.gd` or `player.gd` updating the global resource. + # It triggers the specific `elif` branch in `_on_setting_changed` to clear `_out_of_fuel`. + _parallax_manager._on_setting_changed("current_fuel", 50.0) + + # 3. Simulate the next physics frame post-refuel + _parallax_manager._process(1.0) + + # 4. Verify the math resumed correctly + # Expected math: speed(100.0) * delta(1.0) * diff(1.0) * multiplier(0.8) = 80.0 + var expected_offset: float = 100.0 * 1.0 * 1.0 * 0.8 + + assert_almost_eq( + _parallax_manager.scroll_offset.y, + expected_offset, + 0.01, + "POST-CONDITION: Scroll offset must resume incrementing seamlessly once fuel is restored." + ) + + # ========================================== # SAFETY & EDGE CASE TESTS # ========================================== From 27a09031c2bee89d1c5a619079ace8bda1c744c9 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:18:22 -0700 Subject: [PATCH 21/36] Update main_scene.gd In main_scene.gd, _process(_delta) now only fetches Globals.settings and early-returns without using it; consider removing this method (or the unused settings_res lookup) entirely to avoid confusion and unnecessary work each frame. The reviewer has an eagle eye! They are absolutely right. That settings_res lookup inside main_scene.gd's _process loop is a leftover "ghost" from before we decoupled the parallax background. Back when main_scene.gd was handling the parallax scrolling directly, it needed the settings to check the fuel and difficulty every frame. Now that parallax_manager.gd handles all of that, main_scene.gd doesn't use settings_res in _process at all! Since we still need _process to check for the unbound controls warning, we shouldn't delete the whole function, but we should absolutely delete the useless settings_res fetch. --- scripts/main_scene.gd | 7 ------- 1 file changed, 7 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index d71f1e2cd..362597a70 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -270,13 +270,6 @@ func setup_decor_layer(viewport: Vector2) -> void: func _process(_delta: float) -> void: - # Safely grab the settings resource and guard against null crashes - # during scene transitions, engine shutdown, or isolated GUT tests. - var settings_res: GameSettingsResource = Globals.settings - - if not is_instance_valid(settings_res): - return - # 1. Critical unbound controls warning (shown ONCE per session) # Flag stays true until player fixes bindings (e.g., in key_mapping.gd after remap). # Do NOT reset here — that would make it repeat every 4s (bug fixed). From 0117f9f4fb36b2a0ab43dd45f4040f96f199d67b Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:21:15 -0700 Subject: [PATCH 22/36] Update parallax_manager.gd In parallax_manager.gd, the 0.8 scroll multiplier is a magic number used in _process; consider extracting it to a named constant (e.g., SCROLL_MULTIPLIER) so it is easier to tune and understand its purpose. The reviewer is bringing up a classic clean code principle here! "Magic numbers" (unexplained numbers floating in the middle of equations) make code harder to read and maintain. By extracting 0.8 into a named constant at the top of the script, anyone reading the file immediately knows what that number does, and if you ever need to tweak the background speed, you don't have to go hunting through the math in the _process loop to find it. --- scripts/parallax_manager.gd | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index 91a47017c..64adab944 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -7,6 +7,9 @@ class_name ParallaxManager extends ParallaxBackground +## Base multiplier applied to the final scroll math to scale the speed visually. +const SCROLL_MULTIPLIER: float = 0.8 + ## Optional wrap limit to prevent float32 precision degradation over long sessions. ## Should be a common multiple of the layers' (motion_mirroring.y / motion_scale.y). @export var wrap_period: float = 0.0 @@ -83,7 +86,7 @@ func _process(delta: float) -> void: if _out_of_fuel: scroll_offset = Vector2.ZERO else: - var scroll_amount: float = _current_speed * delta * _difficulty * 0.8 + var scroll_amount: float = _current_speed * delta * _difficulty * SCROLL_MULTIPLIER scroll_offset.y += scroll_amount # Prevent float precision degradation by wrapping modulo the period From 87efef1cf15aef274bb3a64ba6bfaef913b6a725 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:30:12 -0700 Subject: [PATCH 23/36] Update main_scene.gd In main_scene.gd you now access Globals.settings directly in _ready() for parallax setup without the null/validity guard that existed in the old _process, which can reintroduce crashes during scene transitions or GUT tests; consider mirroring the previous is_instance_valid/null safety when grabbing settings_res. Yes, this is 100% valid feedback, and it highlights a very specific quirk about how Godot handles isolated unit testing! While it is true that Globals (as an Autoload) is supposed to exist for the entire lifetime of the game, GUT tests often instantiate scenes in a total vacuum. If a GUT test loads main_scene.gd directly without spinning up the full SceneTree and Autoloads, Globals will technically be a null instance. If your code says var settings_res: GameSettingsResource = Globals.settings, and Globals doesn't exist in that specific test, the engine will throw a fatal "Attempt to access property on a null instance" error and crash the test. To make your code bulletproof and match the exact safety pattern you already established in player.gd, we should restore that guard. --- scripts/main_scene.gd | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 362597a70..3231a23fe 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -52,13 +52,15 @@ func _ready() -> void: # Setup decor layer with random instances setup_decor_layer(viewport_size) - # ========================================================= +# ========================================================= # DEPENDENCY INJECTION: Parallax Background # ========================================================= - # Wire up the signal architecture for the parallax background - # Safely extract settings once to use for both injection and priming - # Note: Because background.setup(settings_res) already has its own internal - var settings_res: GameSettingsResource = Globals.settings + # Safely extract settings once to use for both injection and priming. + # The is_instance_valid(Globals) guard prevents hard crashes during + # isolated GUT tests where Autoloads may not be fully initialized. + var settings_res: GameSettingsResource = ( + Globals.settings if is_instance_valid(Globals) else null + ) if background.has_method("setup"): background.setup(settings_res) From 189ec2189cdf5a23ccfb4017bce178f8a99e5570 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:32:39 -0700 Subject: [PATCH 24/36] Update main_scene.gd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global_bushes_period calculation in main_scene.gd uses a hard-coded 0.5 divisor; if this is tied to a specific layer’s motion_scale.y, it would be clearer and less fragile to derive it from that layer’s actual properties or a named constant rather than a magic number. This is another fantastic piece of feedback from your reviewer. It targets a concept called "Fragile Coupling." Right now, if you or another developer decide later to change the parallax scrolling speed to make the background move slower (e.g., changing the motion_scale to 0.2), you would have to remember to also scroll down to the bottom of the _ready() function and change that random 0.5 divisor. If you forget, the math breaks, and the background will glitch. By dynamically reading the motion_scale.y directly from the layer itself, the code becomes "self-healing." If the layer properties change, the math automatically adapts. --- scripts/main_scene.gd | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 3231a23fe..8b76542ed 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -100,9 +100,15 @@ func _ready() -> void: # ========================================================= # FLOAT DEGRADATION SAFEGUARD # ========================================================= - # Calculate global wrap period based on the tallest layer to prevent float degradation + # Calculate global wrap period based on the tallest layer to prevent float degradation. # Formula: motion_mirroring.y / motion_scale.y - var global_bushes_period: float = (viewport_size.y * parallax_screens_tall) / 0.5 + # Dynamically derived from the layer properties to prevent fragile magic numbers. + var bg_motion_scale: float = 0.5 # Safe fallback + if is_instance_valid(bushes_layer) and bushes_layer.motion_scale.y > 0.0: + bg_motion_scale = bushes_layer.motion_scale.y + + var global_bushes_period: float = (viewport_size.y * parallax_screens_tall) / bg_motion_scale + if background.has_method("set_wrap_period"): background.set_wrap_period(global_bushes_period) From 14084a02e9e9db714acaa591cc69f4508671ea83 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:51:19 -0700 Subject: [PATCH 25/36] This fully resolves the reviewer's concern by creating a proper, public API for the Parallax Manager! MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is another excellent catch by your reviewer, focusing on encapsulation and API design. In GDScript, methods starting with an underscore (like _on_player_speed_changed) are universally understood as "private" or "internal" methods. When main_scene.gd connects directly to a private method on background, it is breaking encapsulation—it "knows too much" about how the background works internally. By replacing the private handler with a public update_speed method, we create a clean, public API that main_scene.gd is allowed to use. --- scripts/main_scene.gd | 14 +++++++------- scripts/parallax_manager.gd | 9 +++++---- test/gut/test_parallax_manager.gd | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 8b76542ed..2b2aa9f36 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -52,7 +52,7 @@ func _ready() -> void: # Setup decor layer with random instances setup_decor_layer(viewport_size) -# ========================================================= + # ========================================================= # DEPENDENCY INJECTION: Parallax Background # ========================================================= # Safely extract settings once to use for both injection and priming. @@ -70,10 +70,10 @@ func _ready() -> void: ) # Wire up the signal architecture for the parallax background - if player.has_signal("speed_changed") and background.has_method("_on_player_speed_changed"): + if player.has_signal("speed_changed") and background.has_method("update_speed"): # 1. Guard against duplicate connections - if not player.speed_changed.is_connected(background._on_player_speed_changed): - player.speed_changed.connect(background._on_player_speed_changed) + if not player.speed_changed.is_connected(background.update_speed): + player.speed_changed.connect(background.update_speed) # 2. Prime the background securely via a public method if background.has_method("prime_speed"): @@ -88,13 +88,13 @@ func _ready() -> void: + "Verify that the Player node defines and emits `speed_changed`." ) ) - elif not background.has_method("_on_player_speed_changed"): + elif not background.has_method("update_speed"): push_warning( ( "Parallax background not wired: background is missing" - + " `_on_player_speed_changed` method. " + + " `update_speed` method. " + "Ensure the background script implements " - + " `_on_player_speed_changed(speed: float, delta: float)`." + + " `update_speed(speed: float, max_speed: float)`." ) ) # ========================================================= diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index 64adab944..3eccc6475 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -51,11 +51,12 @@ func set_wrap_period(period: float) -> void: wrap_period = period -## Observer callback triggered when the player's speed changes. -## @param new_speed: float - The new forward speed of the player. -## @param _max_speed: float - The maximum speed threshold (unused). +## Public method to update the scrolling speed. +## Designed to be safely connected to external speed_changed signals. +## @param new_speed: float - The new forward speed. +## @param _max_speed: float - The maximum speed threshold (unused, defaults to 0.0). ## @return: void -func _on_player_speed_changed(new_speed: float, _max_speed: float) -> void: +func update_speed(new_speed: float, _max_speed: float = 0.0) -> void: _current_speed = new_speed diff --git a/test/gut/test_parallax_manager.gd b/test/gut/test_parallax_manager.gd index ce3e9842d..9b54303ef 100644 --- a/test/gut/test_parallax_manager.gd +++ b/test/gut/test_parallax_manager.gd @@ -42,7 +42,7 @@ func test_speed_update_from_signal() -> void: gut.p("Testing: ParallaxManager correctly caches speed from the player's signal.") # Simulate the Player broadcasting a new speed of 250.0 - _parallax_manager._on_player_speed_changed(250.0, 500.0) + _parallax_manager.update_speed(250.0, 500.0) assert_eq( _parallax_manager._current_speed, From b7d5b259eb95db64a68d7362f234c2d37ec392dc Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 18:57:14 -0700 Subject: [PATCH 26/36] let the ParallaxManager inspect its own children and calculate the maximum wrap period automatically. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wrap period computation in main_scene.gd (based on bushes_layer.motion_scale.y and viewport_size) might be better encapsulated inside ParallaxManager or driven by a configuration value, so main_scene doesn’t need to know about the specific parallax layer structure. This is a fantastic architectural critique from the reviewer. It touches on the principle of Encapsulation. Right now, main_scene.gd is reaching into the background, grabbing a specific layer (bushes_layer), extracting its properties, doing the math, and handing the answer back to the background. main_scene.gd is "knowing too much" about how the background is structured. The most elegant way to solve this is to let the ParallaxManager inspect its own children and calculate the maximum wrap period automatically. main_scene.gd just needs to say: "Hey, I'm done setting up the layers. Please calculate your wrap limits now." Why this is the perfect solution: Zero Coupling: main_scene.gd no longer knows what a motion_scale is, nor does it care. Future Proof: If you add 5 more parallax layers tomorrow with totally different speeds, you won't have to change any math. The ParallaxManager will automatically scan them all, find the one that requires the longest wrap period, and apply it seamlessly! --- scripts/main_scene.gd | 15 ++++----------- scripts/parallax_manager.gd | 22 ++++++++++++++++++---- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 2b2aa9f36..daa386ace 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -100,17 +100,10 @@ func _ready() -> void: # ========================================================= # FLOAT DEGRADATION SAFEGUARD # ========================================================= - # Calculate global wrap period based on the tallest layer to prevent float degradation. - # Formula: motion_mirroring.y / motion_scale.y - # Dynamically derived from the layer properties to prevent fragile magic numbers. - var bg_motion_scale: float = 0.5 # Safe fallback - if is_instance_valid(bushes_layer) and bushes_layer.motion_scale.y > 0.0: - bg_motion_scale = bushes_layer.motion_scale.y - - var global_bushes_period: float = (viewport_size.y * parallax_screens_tall) / bg_motion_scale - - if background.has_method("set_wrap_period"): - background.set_wrap_period(global_bushes_period) + # Delegate wrap period calculation entirely to the ParallaxManager. + # This preserves encapsulation so main_scene doesn't need to know layer specifics. + if background.has_method("auto_calculate_wrap_period"): + background.auto_calculate_wrap_period() # 2. Detect when player presses a key/button that has NO binding at all diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index 3eccc6475..5fe7b6215 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -44,11 +44,25 @@ func prime_speed(initial_speed: float) -> void: _current_speed = initial_speed -## Public method to dynamically set the wrap limit to prevent float precision loss. -## @param period: float - The calculated period to wrap the offset around. +## Public method to dynamically calculate the optimal wrap limit +## based on the properties of its ParallaxLayer children. +## Must be called after all layers have had their mirroring and scale set. ## @return: void -func set_wrap_period(period: float) -> void: - wrap_period = period +func auto_calculate_wrap_period() -> void: + var max_period: float = 0.0 + + # Iterate through all children to find the longest required wrap period + for child in get_children(): + if child is ParallaxLayer: + var layer_scale: float = child.motion_scale.y + var layer_mirror: float = child.motion_mirroring.y + + if layer_scale > 0.0 and layer_mirror > 0.0: + var period: float = layer_mirror / layer_scale + if period > max_period: + max_period = period + + wrap_period = max_period ## Public method to update the scrolling speed. From ba0fb2210633a82dde3d9b3d24b054678a5d9585 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 19:05:54 -0700 Subject: [PATCH 27/36] player's speed is now rigorously typed When priming the manager with player.speed["speed"] in main_scene.gd, consider exposing a typed property or getter on the player instead of pulling from a dictionary key to avoid silent runtime errors if the key or data structure changes. This is an excellent catch by the reviewer. Using a Dictionary (speed["speed"]) for a single numerical value is a "code smell" known as stringly-typed data. It defeats Godot's static typing system. If someone accidentally renames the key or drops the dictionary, the compiler won't warn you, but the game will crash at runtime. By refactoring this into a strongly typed float property (e.g., current_speed: float = 250.0), we gain full autocomplete, compiler safety, and we protect main_scene.gd from silent errors. --- scripts/main_scene.gd | 2 +- scripts/player.gd | 25 ++++++++++++++----------- test/gut/test_player_fuel_logic.gd | 2 +- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index daa386ace..a2c189579 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -77,7 +77,7 @@ func _ready() -> void: # 2. Prime the background securely via a public method if background.has_method("prime_speed"): - background.prime_speed(player.speed["speed"]) + background.prime_speed(player.current_speed) else: push_warning("Parallax background is missing the `prime_speed` method.") diff --git a/scripts/player.gd b/scripts/player.gd index a8a2fe699..06b35e615 100644 --- a/scripts/player.gd +++ b/scripts/player.gd @@ -27,7 +27,10 @@ var rotor_left_sfx: AudioStreamPlayer2D var rotor_right_sfx: AudioStreamPlayer2D # Local state container for physics -var speed: Dictionary = {"speed": 250.0} +# var speed: Dictionary = {"speed": 250.0} + +## The player's current forward speed. +var current_speed: float = 250.0 # Cache the global settings to avoid singleton lookups in hot paths var _settings: GameSettingsResource = null @@ -136,20 +139,20 @@ func _set_speed(target_speed: float) -> void: if not is_instance_valid(_settings): return - var old_speed: float = speed["speed"] + var old_speed: float = current_speed # Clamp current_speed based on fuel state if _settings.current_fuel == 0: - speed["speed"] = clamp(target_speed, 0.0, _settings.max_speed) + current_speed = clamp(target_speed, 0.0, _settings.max_speed) else: - speed["speed"] = clamp(target_speed, _settings.min_speed, _settings.max_speed) + current_speed = clamp(target_speed, _settings.min_speed, _settings.max_speed) # Emit signals if speed actually changed - if old_speed != speed["speed"]: - speed_changed.emit(speed["speed"], _settings.max_speed) + if old_speed != current_speed: + speed_changed.emit(current_speed, _settings.max_speed) # Check for maximum speed limit - if speed["speed"] >= _settings.max_speed: + if current_speed >= _settings.max_speed: speed_maxed.emit() # Check for low speed warning @@ -157,7 +160,7 @@ func _set_speed(target_speed: float) -> void: _settings.min_speed + (_settings.max_speed - _settings.min_speed) * _settings.low_yellow_fraction ) - if speed["speed"] <= low_yellow_thresh: + if current_speed <= low_yellow_thresh: speed_low.emit(low_yellow_thresh) @@ -222,7 +225,7 @@ func _on_fuel_timer_timeout() -> void: if not is_instance_valid(_settings): return - var normalized_speed: float = clamp(speed["speed"] / _settings.max_speed, 0.0, 1.0) + var normalized_speed: float = clamp(current_speed / _settings.max_speed, 0.0, 1.0) var consumption: float = ( _settings.base_consumption_rate * normalized_speed * _settings.difficulty ) @@ -238,7 +241,7 @@ func _physics_process(_delta: float) -> void: if not is_instance_valid(_settings): return - var target_speed: float = speed["speed"] + var target_speed: float = current_speed # Speed changes allowed only if fuel > 0 if Input.is_action_pressed("speed_up") and _settings.current_fuel > 0: @@ -253,7 +256,7 @@ func _physics_process(_delta: float) -> void: # Left/Right movement var lateral_input: float = Input.get_axis("move_left", "move_right") - if lateral_input and _settings.current_fuel > 0 and speed["speed"] > 0: + if lateral_input and _settings.current_fuel > 0 and current_speed > 0: player.velocity.x = lateral_input * _settings.lateral_speed else: player.velocity.x = 0.0 diff --git a/test/gut/test_player_fuel_logic.gd b/test/gut/test_player_fuel_logic.gd index c80e08205..83a346e62 100644 --- a/test/gut/test_player_fuel_logic.gd +++ b/test/gut/test_player_fuel_logic.gd @@ -94,7 +94,7 @@ func test_lateral_movement_blocked_without_fuel() -> void: gut.p("Testing: Lateral turning is disabled if fuel is completely empty.") Globals.settings.current_fuel = 0.0 - _player.speed["speed"] = 150.0 + _player.current_speed = 150.0 _player.player.velocity.x = 0.0 Input.action_press("move_left") From cafaa32cf8c4d6c50ff2e6f3a798f0c9eff0d7e2 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 19:19:21 -0700 Subject: [PATCH 28/36] Unit test updates --- test/gdunit4/test_difficulty.gd | 6 +++--- test/gdunit4/test_difficulty_integration.gd | 2 +- test/gdunit4/test_helpers.gd | 2 +- test/gdunit4/test_player.gd | 17 +++++++++-------- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/test/gdunit4/test_difficulty.gd b/test/gdunit4/test_difficulty.gd index 1e6dc7201..e9a9d6498 100644 --- a/test/gdunit4/test_difficulty.gd +++ b/test/gdunit4/test_difficulty.gd @@ -49,7 +49,7 @@ func test_fuel_depletion_with_difficulty() -> void: Globals.settings.difficulty = 1.0 # NEW: Use Globals.settings.max_speed instead of the removed player_inst.MAX_SPEED - var normalized_speed: float = player_inst.speed["speed"] / Globals.settings.max_speed + var normalized_speed: float = player_inst.current_speed / Globals.settings.max_speed # OLD: var dep_1: float = player_inst.base_fuel_drain * normalized_speed * Globals.settings.difficulty # NEW: Use the global base_consumption_rate instead of the removed local base_fuel_drain @@ -67,7 +67,7 @@ func test_fuel_depletion_with_difficulty() -> void: Globals.settings.difficulty = 2.0 # NEW: Use Globals.settings.max_speed instead of the removed player_inst.MAX_SPEED - normalized_speed = player_inst.speed["speed"] / Globals.settings.max_speed + normalized_speed = player_inst.current_speed / Globals.settings.max_speed # OLD: var dep_2: float = player_inst.base_fuel_drain * normalized_speed * Globals.settings.difficulty # NEW: Use the global base_consumption_rate instead of the removed local base_fuel_drain @@ -85,7 +85,7 @@ func test_fuel_depletion_with_difficulty() -> void: Globals.settings.difficulty = 0.5 # NEW: Use Globals.settings.max_speed instead of the removed player_inst.MAX_SPEED - normalized_speed = player_inst.speed["speed"] / Globals.settings.max_speed + normalized_speed = player_inst.current_speed / Globals.settings.max_speed # OLD: var dep_05: float = player_inst.base_fuel_drain * normalized_speed * Globals.settings.difficulty # NEW: Use the global base_consumption_rate instead of the removed local base_fuel_drain diff --git a/test/gdunit4/test_difficulty_integration.gd b/test/gdunit4/test_difficulty_integration.gd index 603cd01b8..5b2211785 100644 --- a/test/gdunit4/test_difficulty_integration.gd +++ b/test/gdunit4/test_difficulty_integration.gd @@ -52,7 +52,7 @@ func test_difficulty_scales_fuel_and_weapon() -> void: Globals.settings.current_fuel = start_fuel # NEW: Calculate normalized speed using the global max_speed, as MAX_SPEED was removed from player.gd - var normalized_speed: float = player.speed["speed"] / Globals.settings.max_speed + var normalized_speed: float = player.current_speed / Globals.settings.max_speed # OLD: var expected_depletion: float = player.base_fuel_drain * normalized_speed * Globals.settings.difficulty # NEW: Reference base_consumption_rate from the global resource since it was removed from the player script diff --git a/test/gdunit4/test_helpers.gd b/test/gdunit4/test_helpers.gd index 67120c37d..b48ca4a6c 100644 --- a/test/gdunit4/test_helpers.gd +++ b/test/gdunit4/test_helpers.gd @@ -9,7 +9,7 @@ extends RefCounted ## Calculates the expected fuel depletion based on the global GameSettingsResource. static func calculate_expected_depletion(player_root: Node, difficulty: float) -> float: # NEW: Use Globals.settings.max_speed instead of player_root.MAX_SPEED - var normalized_speed: float = player_root.speed["speed"] / Globals.settings.max_speed + var normalized_speed: float = player_root.current_speed / Globals.settings.max_speed # NEW: Use Globals.settings.base_consumption_rate instead of player_root.base_fuel_drain return Globals.settings.base_consumption_rate * normalized_speed * difficulty diff --git a/test/gdunit4/test_player.gd b/test/gdunit4/test_player.gd index ffecfdacb..4f49c0698 100644 --- a/test/gdunit4/test_player.gd +++ b/test/gdunit4/test_player.gd @@ -20,8 +20,6 @@ func after_test() -> void: Globals.settings.difficulty = original_difficulty # Restore after each test -## Tests shared helper calculates depletion correctly. -## @return: void func test_shared_depletion_helper() -> void: var main_scene: Node = auto_free(load("res://scenes/main_scene.tscn").instantiate()) add_child(main_scene) @@ -30,8 +28,8 @@ func test_shared_depletion_helper() -> void: var player_root: Node = main_scene.get_node("Player") Globals.settings.difficulty = 2.0 - # NEW: Use global max_speed - var expected: float = Globals.settings.base_consumption_rate * (player_root.speed["speed"] / Globals.settings.max_speed) * Globals.settings.difficulty + # CHANGED: Use current_speed instead of speed["speed"] + var expected: float = Globals.settings.base_consumption_rate * (player_root.current_speed / Globals.settings.max_speed) * Globals.settings.difficulty assert_float(TestHelpers.calculate_expected_depletion(player_root, Globals.settings.difficulty)).is_equal_approx(expected, 0.001) @@ -305,10 +303,11 @@ func test_movement() -> void: Input.action_release("move_left") # Speed up (no velocity change, just speed var) - var initial_speed: float = player_root.speed["speed"] + # CHANGED: Use current_speed + var initial_speed: float = player_root.current_speed Input.action_press("speed_up") player_root._physics_process(1.0/60.0) - assert_float(player_root.speed["speed"]).is_greater(initial_speed) # Increases speed var + assert_float(player_root.current_speed).is_greater(initial_speed) # Increases speed var assert_vector(body.velocity).is_equal(Vector2(0.0, 0.0)) # No y velocity Input.action_release("speed_up") @@ -400,7 +399,8 @@ func test_fuel_depletion() -> void: assert_float(hud.fuel_bar.value).is_equal(Globals.settings.max_fuel) # Simulate one timer tick - var normalized_speed: float = player_root.speed["speed"] / Globals.settings.max_speed + # CHANGED: Use current_speed + var normalized_speed: float = player_root.current_speed / Globals.settings.max_speed var expected_depletion: float = Globals.settings.base_consumption_rate * normalized_speed * Globals.settings.difficulty player_root._on_fuel_timer_timeout() @@ -412,5 +412,6 @@ func test_fuel_depletion() -> void: Globals.settings.current_fuel = 0.0 player_root._on_fuel_timer_timeout() - assert_float(player_root.speed["speed"]).is_equal(0.0) + # CHANGED: Use current_speed + assert_float(player_root.current_speed).is_equal(0.0) assert_bool(player_root.fuel_timer.is_stopped()).is_true() From a8a50fefc5b7e51f2bff9043f6cae811d526a065 Mon Sep 17 00:00:00 2001 From: Egor Kostan <20955183+ikostan@users.noreply.github.com> Date: Sat, 18 Apr 2026 19:20:33 -0700 Subject: [PATCH 29/36] Update test/gut/test_parallax_manager.gd Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- test/gut/test_parallax_manager.gd | 1 + 1 file changed, 1 insertion(+) diff --git a/test/gut/test_parallax_manager.gd b/test/gut/test_parallax_manager.gd index 9b54303ef..7cad17cdb 100644 --- a/test/gut/test_parallax_manager.gd +++ b/test/gut/test_parallax_manager.gd @@ -197,6 +197,7 @@ func test_process_uses_default_values_without_setup() -> void: # 1. Create a fresh manager without calling setup() var uninitialized_manager: ParallaxManager = ParallaxManager.new() add_child_autofree(uninitialized_manager) + uninitialized_manager.set_process(false) # Only run _process explicitly from tests uninitialized_manager.prime_speed(100.0) uninitialized_manager.scroll_offset.y = 0.0 From 539986c2aeb3bd86168222eaa23e513fa79f6693 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 19:27:58 -0700 Subject: [PATCH 30/36] Update GUT unit tests --- test/gut/test_fuel_additional_edge_cases.gd | 10 ++--- test/gut/test_player_movement_signals.gd | 47 +++++++++++++++------ 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/test/gut/test_fuel_additional_edge_cases.gd b/test/gut/test_fuel_additional_edge_cases.gd index 29b040efe..d39c600ae 100644 --- a/test/gut/test_fuel_additional_edge_cases.gd +++ b/test/gut/test_fuel_additional_edge_cases.gd @@ -26,7 +26,7 @@ func after_each() -> void: func test_fuel_consumption_with_scaling() -> void: gut.p("Testing: Fuel consumption scales up when moving at a higher speed.") - # NEW: Instantiate the main scene locally and use GUT's add_child_autoqfree(). + # NEW: Instantiate the main scene locally and use GUT's add_child_autoqfree(). # This ensures the scene and all its dynamically generated Sprite2D children are safely queued for deletion. main_scene = load("res://scenes/main_scene.tscn").instantiate() add_child_autoqfree(main_scene) @@ -37,16 +37,16 @@ func test_fuel_consumption_with_scaling() -> void: Globals.settings.current_fuel = 100.0 Globals.settings.difficulty = 1.0 - # NEW: Simulate consumption at normal (minimum) speed using the updated Resource. - player_root.speed["speed"] = Globals.settings.min_speed + # CHANGED: Use current_speed instead of speed["speed"] + player_root.current_speed = Globals.settings.min_speed player_root._on_fuel_timer_timeout() var base_depletion: float = 100.0 - Globals.settings.current_fuel # NEW: Reset the fuel tank for the second measurement. Globals.settings.current_fuel = 100.0 - # NEW: Simulate consumption at an increased-consumption state (maximum speed) using the updated Resource. - player_root.speed["speed"] = Globals.settings.max_speed + # CHANGED: Use current_speed instead of speed["speed"] + player_root.current_speed = Globals.settings.max_speed player_root._on_fuel_timer_timeout() var high_speed_depletion: float = 100.0 - Globals.settings.current_fuel diff --git a/test/gut/test_player_movement_signals.gd b/test/gut/test_player_movement_signals.gd index 7e28b7b84..b2d09591c 100644 --- a/test/gut/test_player_movement_signals.gd +++ b/test/gut/test_player_movement_signals.gd @@ -7,7 +7,7 @@ extends "res://addons/gut/test.gd" const GutTestHelper = preload("res://test/gut/gut_test_helper.gd") var _mock_root: Node -var _player: Variant # CHANGED: Use Variant to allow dynamic property access to player.gd variables +var _player: Variant var _original_settings: GameSettingsResource var _added_actions: Array[String] = [] @@ -24,7 +24,7 @@ func before_each() -> void: InputMap.add_action(action) _added_actions.append(action) - # NEW: Call the shared static builder + # Call the shared static builder _mock_root = GutTestHelper.build_mock_player_scene() add_child_autoqfree(_mock_root) _player = _mock_root.get_node("Player") @@ -43,29 +43,36 @@ func after_each() -> void: ## test_physics_emits_speed_changed_on_acceleration | Signal Behavior +## Verifies that dynamic speed changes correctly trigger the Observer pattern. +## When the player successfully accelerates, the `speed_changed` signal must be +## broadcast so decoupled systems (like the ParallaxBackground and UI) can react. ## :rtype: void func test_physics_emits_speed_changed_on_acceleration() -> void: gut.p("Testing: _physics_process emits speed_changed exactly once per frame when accelerating.") watch_signals(_player) Globals.settings.current_fuel = 100.0 - _player.speed["speed"] = 100.0 + _player.current_speed = 100.0 # Simulate acceleration input Input.action_press("speed_up") _player._physics_process(1.0) # 1 second delta to cause noticeable change assert_signal_emitted(_player, "speed_changed", "Signal must fire when speed up increases value.") - assert_gt(float(_player.speed["speed"]), 100.0, "Speed logic should have increased current speed.") + assert_gt(_player.current_speed, 100.0, "Speed logic should have increased current speed.") + ## test_physics_does_not_spam_speed_changed | Signal Efficiency +## Verifies performance optimization inside the `_set_speed` helper. +## The physics loop runs 60 times a second; to prevent UI redraw bottlenecks, +## the `speed_changed` signal must ONLY be emitted if the speed value mathematically changes. ## :rtype: void func test_physics_does_not_spam_speed_changed() -> void: gut.p("Testing: _physics_process suppresses speed_changed emissions when cruising.") watch_signals(_player) Globals.settings.current_fuel = 100.0 - _player.speed["speed"] = 250.0 + _player.current_speed = 250.0 # Process multiple frames without active input _player._physics_process(0.1) @@ -74,15 +81,19 @@ func test_physics_does_not_spam_speed_changed() -> void: assert_signal_emit_count(_player, "speed_changed", 0, "Signal must not emit when speed is unchanged.") + ## test_flameout_resets_speed_and_emits_signal | Edge Cases +## Verifies the critical failure state when the player runs out of fuel. +## It ensures the player's speed is instantly hard-locked to 0.0, and verifies +## that this sudden halt is broadcast via signal so the UI and background stop scrolling. ## :rtype: void func test_flameout_resets_speed_and_emits_signal() -> void: gut.p("Testing: Engine flameout halts the plane instantly and notifies UI.") watch_signals(_player) - _player.speed["speed"] = 300.0 + _player.current_speed = 300.0 - # NEW FIX: Use the private backing field `_current_fuel` to bypass the public setter. + # Use the private backing field `_current_fuel` to bypass the public setter. # This sets up the empty tank condition without automatically triggering the fuel_depleted signal, # ensuring our manual call below is actually what we are testing! Globals.settings._current_fuel = 0.0 @@ -90,10 +101,14 @@ func test_flameout_resets_speed_and_emits_signal() -> void: # Manually trigger the flameout handler _player._on_player_out_of_fuel() - assert_eq(float(_player.speed["speed"]), 0.0, "Speed must forcibly reset to 0.0 on zero fuel.") + assert_eq(_player.current_speed, 0.0, "Speed must forcibly reset to 0.0 on zero fuel.") assert_signal_emitted(_player, "speed_changed", "Flameout must broadcast the speed halt to UI.") -## test_ui_updates_on_speed_signal | UI Reactivity + +## test_ui_updates_on_speed_signal | UI Reactivity & Integration +## Verifies the integration between the Player and the HUD. +## Proves that manually emitting the `speed_changed` signal successfully +## forces the PlayerStatsPanel to update its internal progress bar values. ## :rtype: void func test_ui_updates_on_speed_signal() -> void: gut.p("Testing: Target UI updates instantly when speed_changed fires.") @@ -102,14 +117,18 @@ func test_ui_updates_on_speed_signal() -> void: hud_panel.setup_hud(_player) hud_panel.speed_bar.value = 0.0 - _player.speed["speed"] = 500.0 # Force local sync + _player.current_speed = 500.0 # Force local sync # Fire the signal explicitly using the Global Resource setting _player.speed_changed.emit(500.0, Globals.settings.max_speed) assert_eq(hud_panel.speed_bar.value, 500.0, "Progress bar must sync tightly with speed_changed.") + ## test_speed_clamps_to_max_and_min | Constraints +## Verifies that the internal math strictly obeys the Resource configuration limits. +## Prevents logic bugs where a player holding 'accelerate' for too long exceeds +## the physical capabilities of the plane, or achieves negative speeds by decelerating. ## :rtype: void func test_speed_clamps_to_max_and_min() -> void: gut.p("Testing: Speed values obey MIN and MAX constraints.") @@ -120,21 +139,21 @@ func test_speed_clamps_to_max_and_min() -> void: var min_cap: float = Globals.settings.min_speed # --- 1. Test MAX Clamp --- - _player.speed["speed"] = max_cap - 5.0 + _player.current_speed = max_cap - 5.0 Input.action_press("speed_up") # Force an extreme acceleration delta _player._physics_process(10.0) - assert_eq(float(_player.speed["speed"]), max_cap, "Speed must not exceed configured MAX_SPEED.") + assert_eq(_player.current_speed, max_cap, "Speed must not exceed configured MAX_SPEED.") Input.action_release("speed_up") # Release the key for the next test phase # --- 2. Test MIN Clamp --- - _player.speed["speed"] = min_cap + 5.0 + _player.current_speed = min_cap + 5.0 Input.action_press("speed_down") # Force an extreme deceleration delta _player._physics_process(10.0) - assert_eq(float(_player.speed["speed"]), min_cap, "Speed must not fall below configured MIN_SPEED.") + assert_eq(_player.current_speed, min_cap, "Speed must not fall below configured MIN_SPEED.") Input.action_release("speed_down") # Clean up From de072dcc74a4ee86d5bb99abe14f3365d38c7ac7 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 19:31:01 -0700 Subject: [PATCH 31/36] auto_calculate_wrap_period() silently disables wrap when no layers are found. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 51-65: auto_calculate_wrap_period() silently disables wrap when no layers are found. If get_children() contains no ParallaxLayers (or all of them have zero motion_scale.y/motion_mirroring.y), max_period stays 0.0 and Line 65 overwrites the exported wrap_period, silently disabling the float-precision safeguard — even when a caller configured it via the inspector. --- scripts/parallax_manager.gd | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index 5fe7b6215..9e679c161 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -62,7 +62,17 @@ func auto_calculate_wrap_period() -> void: if period > max_period: max_period = period - wrap_period = max_period + # 1. Only overwrite the exported wrap_period if we successfully calculated a new one. + # This protects values set manually via the Godot Inspector. + if max_period > 0.0: + wrap_period = max_period + + # 2. Warn the developer if the background is scrolling forever with no safeguard + if wrap_period <= 0.0: + push_warning( + "ParallaxManager: No valid wrap limit calculated or set. " + + "Float precision degradation may occur during long sessions." + ) ## Public method to update the scrolling speed. From d08fb8027b10cd100812dc0fa1364efc751eb740 Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 19:31:12 -0700 Subject: [PATCH 32/36] Update parallax_manager.gd --- scripts/parallax_manager.gd | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index 9e679c161..b209b6522 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -66,12 +66,14 @@ func auto_calculate_wrap_period() -> void: # This protects values set manually via the Godot Inspector. if max_period > 0.0: wrap_period = max_period - + # 2. Warn the developer if the background is scrolling forever with no safeguard if wrap_period <= 0.0: push_warning( - "ParallaxManager: No valid wrap limit calculated or set. " + - "Float precision degradation may occur during long sessions." + ( + "ParallaxManager: No valid wrap limit calculated or set. " + + "Float precision degradation may occur during long sessions." + ) ) From cac27df64734b4a2a61f856626705dc33567ffce Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 19:33:55 -0700 Subject: [PATCH 33/36] Optional: max-of-periods is only safe when layer periods are commensurate. 51-65: Optional: max-of-periods is only safe when layer periods are commensurate. wrap_period needs to be a common multiple of every layer's motion_mirroring.y / motion_scale.y for all layers to wrap seamlessly. Picking the maximum works only if the other layers' periods evenly divide it. In main_scene.gd, the sand layer uses motion_mirroring.y = 2*tiles_y*tex_size.y while bushes_layer/decor_layer use motion_mirroring.y = viewport.y * parallax_screens_tall, so the periods are generally non-commensurate and a reset at max will cause a visible jump on the shorter-period layer. Consider computing an LCM (over integer-quantized periods) instead, or pad max_period to an integer multiple of each layer's period. --- scripts/parallax_manager.gd | 39 +++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index b209b6522..bfcd920b3 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -44,14 +44,32 @@ func prime_speed(initial_speed: float) -> void: _current_speed = initial_speed +## Helper to find the Greatest Common Divisor for LCM calculations. +func _gcd(a: int, b: int) -> int: + while b != 0: + var temp: int = b + b = a % b + a = temp + return a + + +## Helper to find the Least Common Multiple to sync disparate layer periods. +func _lcm(a: int, b: int) -> int: + if a == 0 or b == 0: + return 0 + return absi((a * b) / _gcd(a, b)) + + ## Public method to dynamically calculate the optimal wrap limit ## based on the properties of its ParallaxLayer children. +## Uses the Least Common Multiple (LCM) to ensure non-commensurate layers don't jump. ## Must be called after all layers have had their mirroring and scale set. ## @return: void func auto_calculate_wrap_period() -> void: - var max_period: float = 0.0 + var computed_lcm: int = 1 + var periods: Array[int] = [] - # Iterate through all children to find the longest required wrap period + # 1. Collect all valid layer periods as integers (pixels) for child in get_children(): if child is ParallaxLayer: var layer_scale: float = child.motion_scale.y @@ -59,15 +77,20 @@ func auto_calculate_wrap_period() -> void: if layer_scale > 0.0 and layer_mirror > 0.0: var period: float = layer_mirror / layer_scale - if period > max_period: - max_period = period + periods.append(roundi(period)) + + # 2. Calculate the universal LCM of all collected periods + if periods.size() > 0: + computed_lcm = periods[0] + for i in range(1, periods.size()): + computed_lcm = _lcm(computed_lcm, periods[i]) - # 1. Only overwrite the exported wrap_period if we successfully calculated a new one. + # 3. Only overwrite the exported wrap_period if we successfully calculated a valid LCM. # This protects values set manually via the Godot Inspector. - if max_period > 0.0: - wrap_period = max_period + if computed_lcm > 1: + wrap_period = float(computed_lcm) - # 2. Warn the developer if the background is scrolling forever with no safeguard + # 4. Warn the developer if the background is scrolling forever with no safeguard if wrap_period <= 0.0: push_warning( ( From 5fdc662c3faee1ba54716ab89a1361d0c0562537 Mon Sep 17 00:00:00 2001 From: Egor Kostan <20955183+ikostan@users.noreply.github.com> Date: Sat, 18 Apr 2026 19:38:12 -0700 Subject: [PATCH 34/36] Update scripts/main_scene.gd Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --- scripts/main_scene.gd | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index a2c189579..93de5a12e 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -58,8 +58,14 @@ func _ready() -> void: # Safely extract settings once to use for both injection and priming. # The is_instance_valid(Globals) guard prevents hard crashes during # isolated GUT tests where Autoloads may not be fully initialized. + # Also guard against a null or freed Globals.settings so background.setup + # only ever receives a valid GameSettingsResource or null. var settings_res: GameSettingsResource = ( - Globals.settings if is_instance_valid(Globals) else null + Globals.settings + if is_instance_valid(Globals) + and Globals.settings != null + and is_instance_valid(Globals.settings) + else null ) if background.has_method("setup"): From 4c3c8ec47edcd64294e1988de4004f843b874bca Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 19:39:12 -0700 Subject: [PATCH 35/36] Update main_scene.gd --- scripts/main_scene.gd | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scripts/main_scene.gd b/scripts/main_scene.gd index 93de5a12e..041c4e3a7 100644 --- a/scripts/main_scene.gd +++ b/scripts/main_scene.gd @@ -62,9 +62,11 @@ func _ready() -> void: # only ever receives a valid GameSettingsResource or null. var settings_res: GameSettingsResource = ( Globals.settings - if is_instance_valid(Globals) - and Globals.settings != null - and is_instance_valid(Globals.settings) + if ( + is_instance_valid(Globals) + and Globals.settings != null + and is_instance_valid(Globals.settings) + ) else null ) From e6cfa37b580a2297eb8ecc07320475157e85ea6f Mon Sep 17 00:00:00 2001 From: Egor Kostan Date: Sat, 18 Apr 2026 19:42:35 -0700 Subject: [PATCH 36/36] Update parallax_manager.gd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > * In `ParallaxManager.auto_calculate_wrap_period()` you're rounding float periods to ints and computing an LCM; if `motion_mirroring.y / motion_scale.y` isn't an exact integer this can introduce subtle drift—consider either documenting the requirement that those ratios be integral or using a float-based approach with an epsilon instead of integer rounding. This feedback is incredibly sharp. The reviewer is pointing out a "silent failure" that happens when math precision slowly degrades the visual quality of the game over time, known as "sub-pixel drift." Here is why it happens: Imagine one of your layers has a calculated period of 1000.5. By using roundi(), we force it to 1001. The LCM math works perfectly using 1001, but the actual graphic on the screen is still wrapping every 1000.5 pixels. Every time the background loops, that layer will visually "slip" by 0.5 pixels. After 10 loops, the graphic has drifted 5 full pixels out of alignment, causing noticeable stuttering or seams. In 2D game development, the best practice is to always use exact integers for texture mirroring to prevent artifacting anyway. Therefore, instead of rewriting the LCM math to use complex float-epsilons, we should do exactly what the reviewer suggested as their first option: document the requirement and throw a warning if the rule is broken. --- scripts/parallax_manager.gd | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/scripts/parallax_manager.gd b/scripts/parallax_manager.gd index bfcd920b3..b5494d1f2 100644 --- a/scripts/parallax_manager.gd +++ b/scripts/parallax_manager.gd @@ -63,6 +63,8 @@ func _lcm(a: int, b: int) -> int: ## Public method to dynamically calculate the optimal wrap limit ## based on the properties of its ParallaxLayer children. ## Uses the Least Common Multiple (LCM) to ensure non-commensurate layers don't jump. +## IMPORTANT: For flawless wrapping, (motion_mirroring.y / motion_scale.y) MUST result +## in a whole number. Fractional periods will be rounded and may cause visual drift over time. ## Must be called after all layers have had their mirroring and scale set. ## @return: void func auto_calculate_wrap_period() -> void: @@ -77,7 +79,24 @@ func auto_calculate_wrap_period() -> void: if layer_scale > 0.0 and layer_mirror > 0.0: var period: float = layer_mirror / layer_scale - periods.append(roundi(period)) + var rounded_period: int = roundi(period) + + # Drift Safeguard: Warn if the period is not a clean integer + if not is_equal_approx(period, float(rounded_period)): + push_warning( + ( + "ParallaxManager: Layer '" + + child.name + + "' has a fractional wrap period (" + + str(period) + + "). Rounding to " + + str(rounded_period) + + " may cause visual drift. " + + "Ensure (motion_mirroring.y / motion_scale.y) yields a whole number." + ) + ) + + periods.append(rounded_period) # 2. Calculate the universal LCM of all collected periods if periods.size() > 0: