Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
8a26313
[BUG] Orphan Node Leak from Placeholder Sprites in main_scene.gd #540
ikostan Apr 16, 2026
faa58f0
[QA] Orphan Node Leak Fix – Test Plan #549
ikostan Apr 16, 2026
4782dba
Update test/gut/test_main_scene_orphan_nodes.gd
ikostan Apr 16, 2026
49ac64a
Update main_scene.gd
ikostan Apr 16, 2026
7bfa831
Add orphan leak helper and update tests
ikostan Apr 16, 2026
1bfd05f
Update main_scene.gd
ikostan Apr 16, 2026
2a647aa
suggestion: Consider tightening the type of stats_panel instead of us…
ikostan Apr 16, 2026
3713757
Update test_main_scene_orphan_nodes.gd
ikostan Apr 16, 2026
437369f
Update test_player_lifecycle.gd
ikostan Apr 16, 2026
c36a488
Update main_scene.gd
ikostan Apr 16, 2026
5822274
Update test_player_lifecycle.gd
ikostan Apr 16, 2026
608020e
[FEATURE] Randomize rotation for organic decor sprites in main scene …
ikostan Apr 16, 2026
8eb7436
Update main_scene.gd
ikostan Apr 16, 2026
237b33b
Make the background look as random and organic as possible
ikostan Apr 16, 2026
4d999f4
Update main_scene.gd
ikostan Apr 16, 2026
95b2b9e
[FEATURE] Increase parallax background chunk size to prevent visual r…
ikostan Apr 16, 2026
d63252e
Add GUT tests for parallax chunk sizes
ikostan Apr 16, 2026
26f42a0
GUT test file dedicated specifically to verifying the randomized tran…
ikostan Apr 16, 2026
f38072c
Adjust bushes/decor layer height and density
ikostan Apr 16, 2026
cb04302
assertions that lock in the "Goldilocks Zone"
ikostan Apr 16, 2026
f7df782
Update test_main_scene_parallax_chunks.gd
ikostan Apr 16, 2026
221cbd6
Update scripts/main_scene.gd
ikostan Apr 16, 2026
60e24c2
Update main_scene.gd
ikostan Apr 16, 2026
909d4a1
Merge branch 'orphan-node-leak-from-placeholder-sprites-in-main_scene…
ikostan Apr 16, 2026
aaddd37
Update test_decor_layer_transformations.gd
ikostan Apr 16, 2026
7127f19
Rename performance test & add parallax checks
ikostan Apr 16, 2026
3740a4c
Improve test teardown and prevent orphan nodes
ikostan Apr 16, 2026
deb1ae2
Performance assertion is likely to be CI-flaky, and _process side eff…
ikostan Apr 16, 2026
2588460
suggestion: The hardcoded screens_tall = 8.0 is duplicated and would …
ikostan Apr 16, 2026
2f94791
Update gut_test_helper.gd
ikostan Apr 16, 2026
a1c6821
Update test_main_scene_parallax_and_performance.gd
ikostan Apr 16, 2026
b9cfc17
Use GutHelper.safe_hard_free in tests
ikostan Apr 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions scripts/main_scene.gd
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ var _showing_unbound_warning: bool = false
var _showing_unbound_key_message: bool = false

@onready var player: Node2D = $Player
# @onready var stats_panel: Panel = $PlayerStatsPanel
@onready var stats_panel: Variant = $PlayerStatsPanel
@onready var stats_panel: Panel = $PlayerStatsPanel
@onready var background: ParallaxBackground = $Background
@onready var bushes_layer: ParallaxLayer = $Background/Bushes # Reference to the bushes layer
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
@onready var decor_layer: ParallaxLayer = $Background/Decor # Reference to the decor layer
Expand Down Expand Up @@ -118,14 +117,12 @@ func setup_bushes_layer(viewport: Vector2) -> void:

# Clear existing children
for child in bushes_layer.get_children():
bushes_layer.remove_child(child)
child.queue_free()

# Get bush IDs from preloader (Array[String])
var bush_ids: Array = Array(texture_preloader.get_resource_list()).filter(
func(id: String) -> bool: return id.begins_with("bush_")
)
print("Loaded ", bush_ids.size(), " bush textures")

if bush_ids.is_empty():
return
Expand Down Expand Up @@ -161,14 +158,12 @@ func setup_decor_layer(viewport: Vector2) -> void:

# Clear existing children
for child in decor_layer.get_children():
decor_layer.remove_child(child)
child.queue_free()

# Get decor IDs from preloader (Array[String])
var decor_ids: Array = Array(texture_preloader.get_resource_list()).filter(
func(id: String) -> bool: return id.begins_with("decor_")
)
print("Loaded ", decor_ids.size(), " decor textures")

if decor_ids.is_empty():
return
Expand Down Expand Up @@ -248,10 +243,6 @@ func show_message(text: String, type: MessageType = MessageType.CRITICAL_UNBOUND
match type:
MessageType.KEY_PRESS_UNBOUND:
_showing_unbound_key_message = false
# CRITICAL_UNBOUND: Do NOT reset here (once-per-session intent)
# Reset only when bindings are fixed
# (e.g., in key_mapping.gd _on_conflict_confirmed or reset)
# _showing_unbound_warning = false # ← commented out


## Public: Clears the unbound warning flag after fixes.
Expand Down
189 changes: 189 additions & 0 deletions test/gut/test_main_scene_orphan_nodes.gd
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
## Copyright (C) 2026 Egor Kostan
## SPDX-License-Identifier: GPL-3.0-or-later
## test_main_scene_orphan_nodes.gd
##
## GUT unit tests for verifying the absence of orphan node leaks in MainScene.
## Covers the Orphan Node Leak Fix Test Plan (Issue #549).

extends "res://addons/gut/test.gd"

var main_scene: MainScene
var viewport_mock: Vector2 = Vector2(1920, 1080)


## Per-test setup: Flush global garbage, instantiate MainScene, and initialize.
## :rtype: void
func before_each() -> void:
# CRITICAL ISOLATION: Flush the frame *before* spawning our scene to ensure
# any delayed queue_free() calls from completely unrelated test scripts
# have finished resolving before we take our baseline orphan count.
await get_tree().process_frame

main_scene = preload("res://scenes/main_scene.tscn").instantiate()
add_child_autofree(main_scene)

# Allow the scene to initialize (_ready, etc.) before running tests
await get_tree().process_frame


## Per-test teardown: Ensure aggressive cleanup to protect subsequent tests.
## :rtype: void
func after_each() -> void:
# If the test didn't already queue the scene for deletion, GUT's autofree will.
# We force two frame flushes here to guarantee that the SceneTree is
# completely swept clean of this test's garbage before the next test begins.
await get_tree().process_frame
await get_tree().process_frame


## Custom assertion to check if any new orphan nodes leaked during the test.
## :param baseline_orphans: The initial orphan count taken before the test logic.
## :param context: A description of the scenario being tested for the log output.
## :rtype: void
func verify_no_orphan_leaks(baseline_orphans: int, context: String) -> void:
var current_orphans: int = Performance.get_monitor(Performance.OBJECT_ORPHAN_NODE_COUNT)
assert_eq(current_orphans, baseline_orphans, context)


## Manual Orphan Node Check & GUT Teardown Memory Test (Frame Sync) |
## Instantiate MainScene, call setup methods multiple times, flush the frame,
## free the scene, and verify no orphan nodes exist.
## :rtype: void
func test_teardown_memory_sync() -> void:
var baseline_orphans: int = Performance.get_monitor(Performance.OBJECT_ORPHAN_NODE_COUNT)

main_scene.setup_bushes_layer(viewport_mock)
main_scene.setup_decor_layer(viewport_mock)

# Re-trigger to execute the clearing logic on existing sprites
main_scene.setup_bushes_layer(viewport_mock)
main_scene.setup_decor_layer(viewport_mock)

# CRITICAL: Flush the frame to allow queue_free() to complete its cleanup
await get_tree().process_frame
Comment thread
ikostan marked this conversation as resolved.
Outdated

# Free the scene explicitly to test teardown
main_scene.queue_free()
await get_tree().process_frame

verify_no_orphan_leaks(baseline_orphans, "Expected orphan nodes to return to baseline after frame sync and teardown.")


## Repeated Setup Call Stability Test |
## Call setup methods 50 times in a tight loop to simulate heavy reset load,
## then await a frame and check for memory leaks or node accumulation.
## :rtype: void
func test_repeated_setup_call_stability() -> void:
var baseline_orphans: int = Performance.get_monitor(Performance.OBJECT_ORPHAN_NODE_COUNT)

# 1. Call setup_bushes_layer() 50 times in a loop
for i in range(50):
main_scene.setup_bushes_layer(viewport_mock)

# 2. Await one frame after loop
await get_tree().process_frame

verify_no_orphan_leaks(baseline_orphans, "No accumulated orphan nodes after 50 rapid setup calls.")


## Immediate Rebuild Integrity Test |
## Call setup, then immediately repopulate the layer in the exact same frame.
## Verifies old nodes do not double-up with new nodes by filtering out queued items.
## :rtype: void
func test_immediate_rebuild_integrity() -> void:
# Flush out any leftover nodes queued by _ready() first
await get_tree().process_frame

main_scene.setup_bushes_layer(viewport_mock)

# Count only nodes that are NOT queued for deletion
var initial_active_count: int = main_scene.bushes_layer.get_children().filter(func(c: Node) -> bool: return not c.is_queued_for_deletion()).size()

# Immediately repopulate in the same frame
main_scene.setup_bushes_layer(viewport_mock)

var new_active_count: int = main_scene.bushes_layer.get_children().filter(func(c: Node) -> bool: return not c.is_queued_for_deletion()).size()

# The active count should remain consistent, confirming old nodes aren't interfering
assert_eq(new_active_count, initial_active_count, "Child count should remain stable during rapid repopulation.")


## Layer Isolation Test |
## Runs setup on one layer and inspects the other to ensure no unintended
## cross-layer deletions occur.
## :rtype: void
func test_layer_isolation() -> void:
gut.p("Testing: Layers should operate independently without cross-deletions.")

# Step 0: Populate both layers first to establish a baseline
main_scene.setup_bushes_layer(viewport_mock)
main_scene.setup_decor_layer(viewport_mock)

# CRITICAL FIX: Flush the frame so the initial nodes spawned by _ready() are fully swept
# before we take our baseline counts.
await get_tree().process_frame

var initial_decor_count: int = main_scene.decor_layer.get_child_count()
var initial_bushes_count: int = main_scene.bushes_layer.get_child_count()

# Step 1: Run setup_bushes_layer() only.
main_scene.setup_bushes_layer(viewport_mock)
await get_tree().process_frame

# Step 2: Verify decor layer operates independently
var final_decor_count: int = main_scene.decor_layer.get_child_count()
assert_eq(final_decor_count, initial_decor_count, "Decor layer child count should not change when bushes layer is reset.")
assert_gt(final_decor_count, 0, "Decor layer should not be empty.")

# Step 3: Run setup_decor_layer() only.
main_scene.setup_decor_layer(viewport_mock)
await get_tree().process_frame

# Step 4: Verify bushes layer operates independently
var final_bushes_count: int = main_scene.bushes_layer.get_child_count()
assert_eq(final_bushes_count, initial_bushes_count, "Bushes layer child count should not change when decor layer is reset.")
assert_gt(final_bushes_count, 0, "Bushes layer should not be empty.")


## Scene Reload Lifecycle Test |
## Simulates a full scene reload via tree structure replacements.
## Monitors orphan nodes before and after to ensure clean teardown.
## :rtype: void
func test_scene_reload_lifecycle() -> void:
var baseline_orphans: int = Performance.get_monitor(Performance.OBJECT_ORPHAN_NODE_COUNT)

main_scene.setup_bushes_layer(viewport_mock)
main_scene.setup_decor_layer(viewport_mock)

# Simulate change_scene by tearing down and instantiating a new one
main_scene.queue_free()
await get_tree().process_frame

var reloaded_scene: MainScene = preload("res://scenes/main_scene.tscn").instantiate()
add_child_autofree(reloaded_scene)
await get_tree().process_frame

reloaded_scene.setup_bushes_layer(viewport_mock)
reloaded_scene.setup_decor_layer(viewport_mock)
await get_tree().process_frame

verify_no_orphan_leaks(baseline_orphans, "No orphan nodes should persist across scene reload simulation.")


## Stress Input Test (Runtime Simulation) |
## Simulates a user rapidly spamming a debug key across multiple frames
## to ensure no compounding leaks or engine crashes happen.
## :rtype: void
func test_stress_input_simulation() -> void:
var baseline_orphans: int = Performance.get_monitor(Performance.OBJECT_ORPHAN_NODE_COUNT)

# Spam setups across consecutive frames
for i in range(10):
main_scene.setup_bushes_layer(viewport_mock)
main_scene.setup_decor_layer(viewport_mock)
await get_tree().process_frame

# Final flush and check
await get_tree().process_frame

verify_no_orphan_leaks(baseline_orphans, "Memory must remain completely stable after sustained stress input.")
1 change: 1 addition & 0 deletions test/gut/test_main_scene_orphan_nodes.gd.uid
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
uid://c63067f3sp0l4
26 changes: 17 additions & 9 deletions test/gut/test_player_lifecycle.gd
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,22 @@ func before_each() -> void:
original_settings = Globals.settings
Globals.settings = GameSettingsResource.new()

## Per-test cleanup: Restore global state.
## Per-test cleanup: Restore global state and aggressively free the scene.
## :rtype: void
func after_each() -> void:
Globals.settings = original_settings

# NEW: Reverted to a hard free().
# queue_free() delays deletion, causing GUT to falsely report the entire scene as orphans.
# A hard free() executes instantly, ensuring 0 lingering scene nodes when GUT checks memory.
# CRITICAL FIX: Use a hard free() instead of queue_free() or GUT's autofree.
# This instantly incinerates the scene, ensuring 0 lingering orphan nodes
# are left behind to pollute subsequent tests.
if is_instance_valid(main_scene):
main_scene.free()

# Flush the frame just to be absolutely certain the tree is stable for the next test
await get_tree().process_frame

## test_exit_tree_disconnects_signals | Lifecycle | Verify clean signal severing
## test_exit_tree_disconnects_signals |
## Lifecycle | Verify clean signal severing without breaking the SceneTree
## :rtype: void
func test_exit_tree_disconnects_signals() -> void:
gut.p("Testing: Player _exit_tree properly disconnects global signals.")
Expand All @@ -45,20 +49,23 @@ func test_exit_tree_disconnects_signals() -> void:
"fuel_depleted must be connected after player enters the tree."
)

# NEW: 2. Instead of remove_child() (which breaks the tree), manually call the lifecycle function
# 2. CRITICAL FIX: Manually trigger the lifecycle function instead of using remove_child().
# remove_child() instantly orphans the node and triggers cascading tree updates
# that cause false-positive memory leaks during testing.
player_root._exit_tree()

# 3. Assert the signals were cleanly severed
assert_false(
Globals.settings.setting_changed.is_connected(player_root._on_setting_changed),
"setting_changed must be completely disconnected after player leaves the tree."
"setting_changed must be completely disconnected after _exit_tree is called."
)
assert_false(
Globals.settings.fuel_depleted.is_connected(player_root._on_player_out_of_fuel),
"fuel_depleted must be completely disconnected after player leaves the tree."
"fuel_depleted must be completely disconnected after _exit_tree is called."
)

## test_exit_tree_safe_without_globals | Safety | Verify no crashes on early exit
## test_exit_tree_safe_without_globals |
## Safety | Verify no crashes on early exit
## :rtype: void
func test_exit_tree_safe_without_globals() -> void:
gut.p("Testing: Player _exit_tree does not crash if Globals.settings is null.")
Expand All @@ -69,6 +76,7 @@ func test_exit_tree_safe_without_globals() -> void:
main_scene = load("res://scenes/main_scene.tscn").instantiate()
player_root = main_scene.get_node("Player")

# Safely call _exit_tree in isolation
player_root._exit_tree()

assert_true(true, "_exit_tree handled a null Globals.settings state gracefully.")
Loading