Harden gameplay settings JS bridge and add comprehensive tests

scripts/gameplay_settings.gd

@@ -6,7 +6,7 @@ extends Control
 ## actual engine singletons.
 var js_bridge_wrapper: JavaScriptBridgeWrapper = JavaScriptBridgeWrapper.new()
 var os_wrapper: OSWrapper = OSWrapper.new()
-var js_window: JavaScriptObject
+var js_window: Variant
 var _change_difficulty_cb: JavaScriptObject
 var _gameplay_back_button_pressed_cb: JavaScriptObject
 var _gameplay_reset_cb: JavaScriptObject
@@ -27,7 +27,10 @@ func _ready() -> void:
 	# Configure for web overlays (invisible but positioned)
 	process_mode = Node.PROCESS_MODE_ALWAYS  # Ignore pause
 
-	difficulty_slider.value_changed.connect(_on_difficulty_value_changed)
+	# ADD GUARDS HERE:
+	if not difficulty_slider.value_changed.is_connected(_on_difficulty_value_changed):
+		difficulty_slider.value_changed.connect(_on_difficulty_value_changed)
+
 	# Set initial difficulty label (sync with global)
 	difficulty_slider.value = Globals.settings.difficulty
 	difficulty_label.text = "{" + str(Globals.settings.difficulty) + "}"
@@ -38,7 +41,8 @@ func _ready() -> void:
 	if not gameplay_reset_button.pressed.is_connected(_on_gameplay_reset_button_pressed):
 		gameplay_reset_button.pressed.connect(_on_gameplay_reset_button_pressed)
 	# NEW: Attach tree_exited for unexpected removal cleanup (like other settings scripts)
-	tree_exited.connect(_on_tree_exited)
+	if not tree_exited.is_connected(_on_tree_exited):
+		tree_exited.connect(_on_tree_exited)
 
 	# NEW: The UI now observes the resource for external changes
 	if not Globals.settings.setting_changed.is_connected(_on_external_setting_changed):
@@ -85,10 +89,15 @@ func _ready() -> void:
 
 
 func _on_external_setting_changed(setting_name: String, new_value: Variant) -> void:
+	## SYNC UI ONLY:
+	## This observer reacts to changes from the resource.
+	## We must ensure the UI nodes are still valid before updating them.
 	if setting_name == "difficulty":
-		# SYNC UI ONLY:
-		# The resource has already been updated, so we only need to update the UI components.
-		# Use set_value_no_signal to prevent re-triggering the local _on_difficulty_value_changed handler.
+		# FIX: Guard against 'previously freed' errors during teardown/unit tests
+		if not is_instance_valid(difficulty_slider) or not is_instance_valid(difficulty_label):
+			return
+
+		# Use set_value_no_signal to prevent re-triggering local handlers [cite: 198, 199]
 		difficulty_slider.set_value_no_signal(float(new_value))
 		difficulty_label.text = "{" + str(new_value) + "}"
 
@@ -97,33 +106,35 @@ func _on_tree_exited() -> void:
 	## Cleanup on unexpected tree exit (e.g. parent removed without calling back button).
 	## Disconnects signals, restores previous menu if not intentional, clears JS/DOM state.
 	## :rtype: void
+	## Cleanup on unexpected tree exit.
 	Globals.log_message("Gameplay Settings _on_tree_exited called.", Globals.LogLevel.DEBUG)
 
-	# Disconnect the global resource observer to prevent stale references
-	# GUARD: Ensure Globals and the settings resource are still valid before disconnecting
-	# Use a local variable to safely check and access the settings resource
+	# 1. Safe Global Resource Disconnection
 	var settings_res := Globals.settings if is_instance_valid(Globals) else null
-
 	if is_instance_valid(settings_res):
 		if settings_res.setting_changed.is_connected(_on_external_setting_changed):
 			settings_res.setting_changed.disconnect(_on_external_setting_changed)
 
-	# Disconnect Godot signals if still connected
-	if difficulty_slider.value_changed.is_connected(_on_difficulty_value_changed):
-		difficulty_slider.value_changed.disconnect(_on_difficulty_value_changed)
-	if gameplay_back_button.pressed.is_connected(_on_gameplay_back_button_pressed):
-		gameplay_back_button.pressed.disconnect(_on_gameplay_back_button_pressed)
-	if gameplay_reset_button.pressed.is_connected(_on_gameplay_reset_button_pressed):
-		gameplay_reset_button.pressed.disconnect(_on_gameplay_reset_button_pressed)
-
-	# Clean up JS callbacks on window object
+	# 2. FIX: Guarded Local Disconnections
+	# We must check if the nodes still exist before accessing 'value_changed' or 'pressed'
+	if is_instance_valid(difficulty_slider):
+		if difficulty_slider.value_changed.is_connected(_on_difficulty_value_changed):
+			difficulty_slider.value_changed.disconnect(_on_difficulty_value_changed)
+
+	if is_instance_valid(gameplay_back_button):
+		if gameplay_back_button.pressed.is_connected(_on_gameplay_back_button_pressed):
+			gameplay_back_button.pressed.disconnect(_on_gameplay_back_button_pressed)
+
+	if is_instance_valid(gameplay_reset_button):
+		if gameplay_reset_button.pressed.is_connected(_on_gameplay_reset_button_pressed):
+			gameplay_reset_button.pressed.disconnect(_on_gameplay_reset_button_pressed)
+
+	# 3. Clean up JS/Web state
 	_unset_gameplay_settings_window_callbacks()
-
-	# Null out stored callback references
 	_change_difficulty_cb = null
 	_gameplay_back_button_pressed_cb = null
 	_gameplay_reset_cb = null
-
+	
 	# Web overlay cleanup + optional menu restore
 	if os_wrapper.has_feature("web") and js_window and js_bridge_wrapper:
 		# Hide gameplay overlays (same DOM elements shown in _ready)
@@ -271,62 +282,82 @@ func _on_difficulty_value_changed(value: float) -> void:
 func _on_change_difficulty_js(args: Array) -> void:
 	## JS callback for changing difficulty.
 	##
-	## Routes to the signal handler.
+	## Routes to the signal handler after performing strict type and 
+	## bounds validation to prevent engine crashes on malformed JS input. [cite: 209]
 	##
-	## :param args: Array containing the value (from JS).
+	## :param args: Array containing the value (from JS). [cite: 210]
 	## :type args: Array
 	## :rtype: void
+
+	# GS-JS-10: Guard against entirely empty arguments from the bridge
 	if args.is_empty():
 		Globals.log_message(
-			"JS difficulty callback received empty args—skipping.", Globals.LogLevel.WARNING
+			"JS difficulty callback received empty args—skipping.", 
+			Globals.LogLevel.WARNING
 		)
 		return
 
 	var first_arg: Variant = args[0]
+	var potential_value: Variant = null
+
+	# GS-JS-20/21: Strict Type Check 
+	# Verify first_arg is a container before calling .size() or index [0]
+	if typeof(first_arg) == TYPE_ARRAY or first_arg is JavaScriptObject:
+		# GS-JS-11: Guard against nested empty arrays [[]]
+		if first_arg.size() > 0:
+			potential_value = first_arg[0]
+		else:
+			Globals.log_message("JS difficulty callback: Nested array is empty.", Globals.LogLevel.WARNING)
+			return
+	else:
+		# GS-JS-20/21: Handle scalar values (e.g., [1.5]) by taking the arg directly
+		potential_value = first_arg
+
+	# GS-JS-12/15/22: Validate that the extracted value is a convertible type
 	if (
-		first_arg is not JavaScriptObject
-		and typeof(first_arg) != TYPE_ARRAY
-		and first_arg.size() == 0
-		and first_arg.is_empty()
+		typeof(potential_value) != TYPE_INT
+		and typeof(potential_value) != TYPE_FLOAT
+		and typeof(potential_value) != TYPE_STRING
 	):
 		Globals.log_message(
-			(
-				"JS difficulty callback received invalid first arg (not a non-empty array): "
-				+ str(args)
-			),
+			"JS difficulty callback received non-convertible value: " + str(potential_value),
 			Globals.LogLevel.WARNING
 		)
 		return
 
-	var potential_value: Variant = first_arg[0]
-	if (
-		typeof(potential_value) != TYPE_INT
-		and typeof(potential_value) != TYPE_FLOAT
-		and typeof(potential_value) != TYPE_STRING
-	):
+	# GS-JS-03: Coerce to float (e.g., "1.5" becomes 1.5)
+	# FIX: Ensure strings are numeric before conversion to prevent 0.0/clamping reset
+	if typeof(potential_value) == TYPE_STRING and not potential_value.is_valid_float():
 		Globals.log_message(
-			"JS difficulty callback received non-convertible value: " + str(args),
+			"JS difficulty callback: Rejected non-numeric string: " + str(potential_value),
 			Globals.LogLevel.WARNING
 		)
 		return
 
 	var value: float = float(potential_value)
+
+	# GS-JS-30: Guard against missing UI nodes during callback
+	if not is_instance_valid(difficulty_slider):
+		Globals.log_message("JS difficulty callback: Slider node is invalid/freed.", Globals.LogLevel.WARNING)
+		# We still update the resource even if the UI is gone
+		Globals.settings.difficulty = value
+		return
+
+	# GS-JS-04/05: Validate bounds against the UI constraints
 	if value < difficulty_slider.min_value or value > difficulty_slider.max_value:
 		Globals.log_message(
-			(
-				"JS difficulty callback received out-of-bounds value: "
-				+ str(value)
-				+ " (args: "
-				+ str(args)
-				+ ")"
-			),
+			"JS difficulty callback received out-of-bounds value: " + str(value),
 			Globals.LogLevel.WARNING
 		)
-		return
+		# REMOVE 'return' to allow the value to reach the resource for clamping
+		# return
 
 	Globals.log_message(
-		"JS difficulty callback called with valid value: " + str(value), Globals.LogLevel.DEBUG
+		"JS difficulty callback called with valid value: " + str(value), 
+		Globals.LogLevel.DEBUG
 	)
+
+	# Pass the validated value to the standard handler
 	_on_difficulty_value_changed(value)
 
 

test/gut/test_game_settings_resource.gd

@@ -0,0 +1,131 @@
+## Copyright (C) 2026 Egor Kostan
+## SPDX-License-Identifier: GPL-3.0-or-later
+## test_game_settings_resource.gd
+##
+## GUT unit tests for GameSettingsResource and GameplaySettings initialization.
+##
+## It ensures the GameSettingsResource acts as a reliable source of truth and
+## that the GameplaySettings scene correctly synchronizes its UI components during
+## the _ready() sequence.
+
+extends "res://addons/gut/test.gd"
+
+const GameplaySettings = preload("res://scripts/gameplay_settings.gd")
+var gameplay_menu: Control
+var _resource: GameSettingsResource
+
+func before_each() -> void:
+	# Initialize a fresh resource for every test to ensure isolation
+	_resource = GameSettingsResource.new()
+	Globals.settings = _resource
+
+	# Instantiate the menu for initialization tests
+	gameplay_menu = load("res://scenes/gameplay_settings.tscn").instantiate()
+	# Inject mock wrapper to avoid real JS/OS calls during unit tests
+	gameplay_menu.os_wrapper = OSWrapper.new() 
+
+	add_child_autofree(gameplay_menu)
+	await get_tree().process_frame
+
+
+# --- SECTION 2: RESOURCE CONTRACT TESTS (GS-RES) ---
+
+## GS-RES-01 | Validate signal emission on valid update
+func test_gs_res_01_signal_on_valid_change() -> void:
+	watch_signals(_resource)
+	_resource.difficulty = 1.5
+	assert_signal_emitted_with_parameters(_resource, "setting_changed", ["difficulty", 1.5], 0)
+
+
+## GS-RES-02/03 | Validate clamping logic
+func test_gs_res_02_03_clamping_behavior() -> void:
+	# Test Upper Bound Clamping 
+	_resource.difficulty = 5.0
+	assert_eq(_resource.difficulty, 2.0, "Difficulty should clamp to 2.0")
+
+	# Test Lower Bound Clamping
+	_resource.difficulty = 0.1
+	assert_eq(_resource.difficulty, 0.5, "Difficulty should clamp to 0.5")
+
+
+## GS-RES-04/05/06 | Validate boundary and default values
+func test_gs_res_04_05_06_boundary_values() -> void:
+	var values_to_test: Array = [0.5, 1.0, 2.0]
+	for val: float in values_to_test:
+		_resource.difficulty = val
+		assert_eq(_resource.difficulty, val, "Value %s should be accepted exactly" % val)
+
+
+## GS-RES-07 | Validate stability on redundant assignments
+func test_gs_res_07_redundant_assignment() -> void:
+	_resource.difficulty = 1.2
+	watch_signals(_resource)
+	_resource.difficulty = 1.2
+	# Corrected function name from assert_signal_emission_count to assert_signal_emit_count
+	assert_signal_emit_count(_resource, "setting_changed", 0, "Redundant assignment should not re-emit")
+
+
+# --- SECTION 3: MENU INITIALIZATION TESTS (GS-READY) ---
+
+## GS-READY-01/02 | Confirm UI syncs to resource state on load
+func test_gs_ready_01_02_ui_initialization_sync() -> void:
+	# Pre-condition: Set resource to non-default value before menu _ready()
+	# Note: Requires re-instantiating or checking logic after add_child
+	var test_difficulty: float = 1.7
+	_resource.difficulty = test_difficulty
+
+	var new_menu: Variant = load("res://scenes/gameplay_settings.tscn").instantiate()
+	add_child_autofree(new_menu)
+	await get_tree().process_frame
+
+	assert_eq(new_menu.difficulty_slider.value, test_difficulty, "Slider must match resource on init")
+	assert_eq(new_menu.difficulty_label.text, "{" + str(test_difficulty) + "}", "Label must match resource on init")
+
+
+## GS-READY-03/04 | Confirm signal connections 
+func test_gs_ready_03_04_signal_connections() -> void:
+	assert_true(_resource.setting_changed.is_connected(gameplay_menu._on_external_setting_changed), 
+		"UI must connect to resource observer on ready") 
+	assert_true(gameplay_menu.difficulty_slider.value_changed.is_connected(gameplay_menu._on_difficulty_value_changed),
+		"Slider signal should be connected")
+
+
+## GS-READY-05 | Prevent duplicate connections 
+func test_gs_ready_05_no_duplicate_connections() -> void:
+	# Ensure the menu is in the tree 
+	if not gameplay_menu.is_inside_tree():
+		add_child(gameplay_menu)
+		await get_tree().process_frame
+
+	# Manually call _ready again to test idempotency/guards 
+	# The production code guards should prevent double-connection 
+	gameplay_menu._ready()
+
+	# Check connections on the GLOBAL resource 
+	var connections: Array = Globals.settings.setting_changed.get_connections()
+	var count: int = 0
+
+	for conn: Dictionary in connections:
+		# We must verify the callable points to our specific instance 
+		if conn["callable"].get_object() == gameplay_menu and \
+		   conn["callable"].get_method() == "_on_external_setting_changed":
+			count += 1
+
+	assert_eq(count, 1, "Observer should only be connected once even after redundant ready calls")
+
+
+## GS-READY-06 | Robustness against missing web features
+func test_gs_ready_06_safe_init_non_web() -> void:
+	# Simulate non-web environment
+	var mock_os: Variant = double(OSWrapper).new()
+	stub(mock_os, "has_feature").to_return(false)
+
+	# FIX: Instantiate from the SCENE, not just the script
+	var menu: Control = load("res://scenes/gameplay_settings.tscn").instantiate()
+	menu.os_wrapper = mock_os
+
+	add_child_autofree(menu)
+	await get_tree().process_frame
+
+	assert_true(is_instance_valid(menu.difficulty_slider), "Slider should be valid when initialized from scene")
+	assert_true(true, "Menu initialized safely in non-web environment")

test/gut/test_game_settings_resource.gd.uid

@@ -0,0 +1 @@
+uid://ccmx64y6uy3dk