fix(project): keep comments out of project.godot and guard the settings

Godot's ConfigFile writer does not round-trip comments in project.godot. An
observed rewrite deleted both `;` blocks outright and spliced the three-line
`#` block above run/main_scene.dedicated_server onto the setting's own line,
leaving it commented out — which would send dedicated builds to the
interactive main menu instead of server_boot.tscn, with nothing failing until
someone noticed a server process rendering a menu.

Move the explanations into the code that owns the settings (server_boot.gd for
the dedicated-server override, video_settings.gd for stretch mode and vsync)
so they cannot be destroyed by a rewrite, and leave project.godot holding only
assignments plus Godot's own regenerated header.

Add tests/cases/test_project_settings.gd as the backstop: the feature-override
assertions read project.godot as text and reject a line that has been folded
into a comment, since ProjectSettings resolves `key.<feature>` overrides at
load time and never exposes the suffixed key. Verified by reproducing the
exact corruption, which fails the test, and it also covers the Jolt physics
engine, the required autoloads, and that no test-hook autoload is ever shipped
registered.
This commit is contained in:
Josh Creek
2026-08-24 08:40:16 +01:00
parent 46fe696a58
commit 6320b982a8
4 changed files with 124 additions and 17 deletions
-17
View File
@@ -21,9 +21,6 @@ run/main_scene="uid://bcq14356s3e2i"
config/features=PackedStringArray("4.7", "Forward Plus")
config/icon="res://icon.svg"
run/main_scene.training="res://scenes/training.tscn"
# Dedicated exports select the server boot scene before the interactive menu
# is loaded. This is the same project-setting feature override used above by
# the training export.
run/main_scene.dedicated_server="res://scenes/server_boot.tscn"
[autoload]
@@ -59,22 +56,8 @@ NetDebugOverlay="*res://scripts/net_debug_overlay.gd"
window/size/viewport_width=1920
window/size/viewport_height=1080
window/size/mode=2
; Task 0.17c: kept fixed at "viewport" + 1080p rather than moved to
; "disabled", deliberately. A player on a 1440p/4K display cannot render
; native this way, and a 1080p player cannot render lower than 1080p through
; window scaling alone — but task 0.17b's Viewport.scaling_3d_scale already
; covers "render lower than the window" independently of stretch mode (it
; scales the 3D viewport's own internal resolution before this blit, not the
; window itself), and task 0.15b found an unexplained ~6% non-uniform width
; scaling on this project's one tested (Mac/Retina) machine — see
; multiplayer-todo.md §5.5.1 — that needs understanding before stretch mode
; is touched, not blindly carried into a resolution-dependent change.
window/stretch/mode="viewport"
window/stretch/aspect="expand"
; Task 0.17: default matches VideoSettings.gd's VsyncMode.ADAPTIVE default —
; VideoSettings.apply_vsync() overwrites this at runtime via DisplayServer as
; soon as the autoload initializes, so this is only what's in effect for the
; brief pre-autoload window and if VideoSettings ever fails to load.
window/vsync/vsync_mode=2
[editor_plugins]
+9
View File
@@ -9,6 +9,15 @@ extends Node
#
# Run: godot --headless --path Game res://scenes/server_boot.tscn -- --port=7777
#
# Dedicated *exports* reach this scene without the CLI argument above, via the
# `run/main_scene.dedicated_server` feature override in project.godot — the same
# project-setting mechanism the training export uses for training.tscn. That
# override is deliberately uncommented in project.godot: Godot's ConfigFile
# writer does not round-trip comments, and a `#` block directly above a setting
# can be spliced into the setting's own line on rewrite, silently commenting it
# out and sending dedicated builds to the interactive main menu instead of here.
# `tests/cases/test_project_settings.gd` fails loudly if that ever happens.
#
# Deliberately does not spawn a match yet — that's Phase 2's networked_match
# scene. This is just the process shell: listen, log, idle cheaply.
+25
View File
@@ -8,6 +8,31 @@ extends Node
# load) rather than overwriting them outright, so the per-arena bloom tuning
# in arena_01/02/03.tscn survives underneath the user's preference.
# The two display project settings this autoload sits on top of are documented
# here rather than in project.godot, because Godot's ConfigFile writer does not
# round-trip comments — it drops `;` blocks outright and can splice `#` blocks
# into the following line, silently commenting the setting out. Anything in
# project.godot that needs an explanation must therefore be explained from the
# code that owns it. `tests/cases/test_project_settings.gd` guards the settings
# themselves against exactly that corruption.
#
# `window/stretch/mode="viewport"` + 1080p (task 0.17c): kept fixed rather than
# moved to "disabled", deliberately. A player on a 1440p/4K display cannot
# render native this way, and a 1080p player cannot render lower than 1080p
# through window scaling alone — but task 0.17b's Viewport.scaling_3d_scale
# (resolution_scale below) already covers "render lower than the window"
# independently of stretch mode, since it scales the 3D viewport's own internal
# resolution before this blit rather than the window itself. Task 0.15b also
# found an unexplained ~6% non-uniform width scaling on this project's one
# tested (Mac/Retina) machine — see multiplayer-todo.md §5.5.1 — which needs
# understanding before stretch mode is touched, not blindly carrying into a
# resolution-dependent change.
#
# `window/vsync/vsync_mode=2` (task 0.17): matches VsyncMode.ADAPTIVE below.
# apply_vsync() overwrites it at runtime via DisplayServer as soon as this
# autoload initializes, so the project setting is only in effect for the brief
# pre-autoload window, and as a fallback if VideoSettings ever fails to load.
signal settings_changed # Arenas re-apply preset-gated Environment/light state live.
# MSAA_2X appended at the end, not inserted, so existing user://settings.cfg
+90
View File
@@ -0,0 +1,90 @@
extends "res://tests/test_case.gd"
# Guards the project settings that are load-bearing but easy to destroy
# silently. Godot's ConfigFile writer does not round-trip comments in
# project.godot: it drops `;` blocks outright, and a `#` block sitting directly
# above a setting can be spliced onto that setting's own line on rewrite, which
# comments the setting out. A dedicated build would then boot the interactive
# main menu instead of the server, and nothing would fail until someone noticed
# a server process rendering a menu.
#
# The explanations that used to live as comments beside these settings are now
# in the code that owns them — server_boot.gd and video_settings.gd.
#
# The feature-override assertions read project.godot as TEXT rather than through
# ProjectSettings. Godot resolves `key.<feature>` overrides at load time against
# the running build's own feature tags and does not expose the suffixed key, so
# get_setting("run/main_scene.dedicated_server") returns "" in a normal editor/
# headless run even when the line is perfectly intact. Reading the file also
# matches the actual threat, which is textual corruption of the file.
func _project_godot_lines() -> PackedStringArray:
var file := FileAccess.open("res://project.godot", FileAccess.READ)
if file == null:
return PackedStringArray()
return file.get_as_text().split("\n")
# True only if `key="value"` appears as a real, uncommented assignment. A line
# that got spliced into a `#`/`;` comment is deliberately NOT a match — that is
# precisely the corruption being guarded against.
func _has_setting_line(key: String, value: String) -> bool:
var wanted := "%s=\"%s\"" % [key, value]
for raw_line in _project_godot_lines():
var line := raw_line.strip_edges()
if line.begins_with("#") or line.begins_with(";"):
continue
if line == wanted:
return true
return false
func test_project_godot_is_readable() -> void:
# Everything below is vacuously true if the file could not be opened.
assert_true(not _project_godot_lines().is_empty(), "project.godot readable and non-empty")
func test_dedicated_server_feature_override_is_set() -> void:
# Consumed by dedicated exports; see server_boot.gd.
assert_true(
_has_setting_line("run/main_scene.dedicated_server", "res://scenes/server_boot.tscn"),
"run/main_scene.dedicated_server present and uncommented"
)
func test_training_feature_override_is_set() -> void:
assert_true(
_has_setting_line("run/main_scene.training", "res://scenes/training.tscn"),
"run/main_scene.training present and uncommented"
)
func test_physics_engine_is_jolt() -> void:
# The whole flight model and every trained policy assume Jolt. Silently
# reverting to Godot Physics would change ship/ball behaviour under bots
# trained against Jolt, without any other test failing on its own.
var engine: String = ProjectSettings.get_setting("physics/3d/physics_engine", "")
assert_eq(engine, "Jolt Physics", "3D physics engine")
func test_required_autoloads_are_registered() -> void:
# NetworkManager in particular is reached by name from many scripts; losing
# it from [autoload] fails only at the point of use, deep in a smoke test.
for autoload_name in ["GameSettings", "VideoSettings", "NetworkManager", "MatchNet", "MatchSim"]:
assert_true(
ProjectSettings.has_setting("autoload/" + autoload_name),
"autoload/%s registered" % autoload_name
)
func test_test_hook_autoloads_are_not_shipped() -> void:
# main_menu_test_hooks / lobby_test_hooks are added to [autoload] by hand
# when running those scene-level smoke tests, and must be removed again —
# see CLAUDE.md. Shipping one registered would run test code in the real
# game, so fail here rather than discovering it in a build.
for hook_name in ["MainMenuTestHooks", "LobbyTestHooks", "NetworkedMatchTestHooks"]:
assert_true(
not ProjectSettings.has_setting("autoload/" + hook_name),
"test hook autoload/%s must not be registered" % hook_name
)