From 4533da34e09d4d967c6e0ce344923af1f0c9d36c Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Thu, 20 Aug 2026 08:18:59 +0100 Subject: [PATCH] feat(multiplayer): Phase 1 transport, connection, and lobby Lands tasks 1.0-1.8 of multiplayer-todo.md: the pure-function test runner, net_codec (wire format quantizers/pack-unpack), NetworkManager (ENet transport, manual polling, min-RTT clock sync), MatchNet (handshake, protocol/tick-rate gating, roster with team+ready state), lobby.tscn (team columns, switch team, ready toggle), server_boot.tscn (headless dedicated server with structured logging and an overrun watchdog), and main_menu.gd's Host/Join-by-IP UI (connecting overlay, cancel, bounded failure path). Followed by an adversarial review (Opus subagent) that found and fixed two real bugs - an unvalidated player_name broadcast that let one client's oversized name head-of-line-block the reliable channel for everyone, and a server-side roster leak across a host/re-host cycle - plus three gaps in the test suite itself where a claim of "verified" wasn't actually backed by what the test checked. All five two-process smoke tests plus the pure-function suite are green with the strengthened assertions in place. --- CLAUDE.md | 5 +- Game/project.godot | 8 + Game/scenes/lobby.tscn | 112 +++++++++++ Game/scenes/main_menu.tscn | 96 ++++++++++ Game/scenes/server_boot.tscn | 6 + Game/scripts/lobby.gd | 116 ++++++++++++ Game/scripts/main_menu.gd | 110 +++++++++++ Game/scripts/match_net.gd | 255 +++++++++++++++++++++++++ Game/scripts/net_body_state.gd | 23 +++ Game/scripts/net_codec.gd | 295 +++++++++++++++++++++++++++++ Game/scripts/net_debug_overlay.gd | 43 +++++ Game/scripts/network_manager.gd | 210 ++++++++++++++++++++ Game/scripts/server_boot.gd | 98 ++++++++++ Game/tests/cases/test_match_net.gd | 39 ++++ Game/tests/cases/test_net_codec.gd | 169 +++++++++++++++++ Game/tests/cases/test_smoke.gd | 12 ++ Game/tests/clock_smoke.gd | 148 +++++++++++++++ Game/tests/clock_smoke.tscn | 6 + Game/tests/lobby_smoke.gd | 79 ++++++++ Game/tests/lobby_smoke.tscn | 6 + Game/tests/lobby_test_hooks.gd | 126 ++++++++++++ Game/tests/main_menu_test_hooks.gd | 114 +++++++++++ Game/tests/match_net_smoke.gd | 167 ++++++++++++++++ Game/tests/match_net_smoke.tscn | 6 + Game/tests/net_smoke.gd | 112 +++++++++++ Game/tests/net_smoke.tscn | 6 + Game/tests/test_case.gd | 38 ++++ Game/tests/test_runner.gd | 81 ++++++++ Game/tests/test_runner.tscn | 6 + multiplayer-todo.md | 29 +-- 30 files changed, 2509 insertions(+), 12 deletions(-) create mode 100644 Game/scenes/lobby.tscn create mode 100644 Game/scenes/server_boot.tscn create mode 100644 Game/scripts/lobby.gd create mode 100644 Game/scripts/match_net.gd create mode 100644 Game/scripts/net_body_state.gd create mode 100644 Game/scripts/net_codec.gd create mode 100644 Game/scripts/net_debug_overlay.gd create mode 100644 Game/scripts/network_manager.gd create mode 100644 Game/scripts/server_boot.gd create mode 100644 Game/tests/cases/test_match_net.gd create mode 100644 Game/tests/cases/test_net_codec.gd create mode 100644 Game/tests/cases/test_smoke.gd create mode 100644 Game/tests/clock_smoke.gd create mode 100644 Game/tests/clock_smoke.tscn create mode 100644 Game/tests/lobby_smoke.gd create mode 100644 Game/tests/lobby_smoke.tscn create mode 100644 Game/tests/lobby_test_hooks.gd create mode 100644 Game/tests/main_menu_test_hooks.gd create mode 100644 Game/tests/match_net_smoke.gd create mode 100644 Game/tests/match_net_smoke.tscn create mode 100644 Game/tests/net_smoke.gd create mode 100644 Game/tests/net_smoke.tscn create mode 100644 Game/tests/test_case.gd create mode 100644 Game/tests/test_runner.gd create mode 100644 Game/tests/test_runner.tscn diff --git a/CLAUDE.md b/CLAUDE.md index 13dace60..f9a5e819 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,11 +53,14 @@ Upstream ships telemetry, and there are **two independent switches** — turning ## Commands -There is no build step, linter, or automated test suite for the GDScript project itself — Godot projects run directly from source. +There is no build step or linter for the GDScript project itself — Godot projects run directly from source. - **Open the project**: open `Game/` as a project in the Godot 4.7 editor, or run `godot --path Game` from the repo root. - **Run the game**: press Play in the editor, or `godot --path Game res://scenes/main_menu.tscn`. - **Headless smoke test** (RL/CI precondition — the game must run without rendering): `godot --headless --path Game res://scenes/free_play.tscn`. +- **Unit tests** (pure-function assertions, see `multiplayer-todo.md` task 1.0): `godot --headless --path Game res://tests/test_runner.tscn`. Exits 0/1. Add a test by dropping a `*.gd` file under `Game/tests/cases/` that extends `res://tests/test_case.gd` (path-based `extends`, not the bare `class_name` — see that file for why) with any number of `test_*()` methods; the runner discovers it, no registration needed. +- **Networking smoke tests** (real two-process ENet connect/disconnect, see `multiplayer-todo.md` §7 Phase 1 tasks): each starts a host then a client, each in its own `godot --headless` process, printing `SMOKE PASS/FAIL: ...` and exiting 0/1. Not part of `test_runner.tscn` — a live ENet handshake needs two real processes. `res://tests/net_smoke.tscn` (task 1.2 — `--role=host|client`, now also confirms the host observes `client_disconnected`, not just that each side exits cleanly on its own), `res://tests/match_net_smoke.tscn` (task 1.4 — `--role=host|client|client-badversion|client-longname|host_recycle`; `client-longname` sends an oversized player name and expects rejection, `host_recycle` hosts, lets a client join, leaves, re-hosts, and confirms the roster is actually empty — run a plain `client` role against it), `res://tests/clock_smoke.tscn` (task 1.8 — `--role=host|client`, clock convergence cross-checked against independent OS-wall-clock ground truth, not just self-consistency), `res://tests/lobby_smoke.tscn` (task 1.5 — `--role=host|client`; **both** roles load `lobby.tscn` for real via `change_scene_to_file` now, exercising the server's read-only view as well as the client's interactive one). See `network_manager.gd`'s header comment and `multiplayer-todo.md` §9 gotchas 25–30 for the non-obvious Godot/ENet failure modes these caught (`OfflineMultiplayerPeer` sentinel, premature peer teardown, `change_scene_to_file` off the real `current_scene`, unbounded `connection_failed`, the `is_client`-before-actually-connected race, `load()` not returning null on a broken script). +- **`main_menu.tscn`'s Host/Join flow** (task 1.7) is verified the same way but needs a temporary autoload since it's the real main scene, not a wrapper: add `MainMenuTestHooks="*res://tests/main_menu_test_hooks.gd"` to `project.godot [autoload]`, run `godot --headless --path Game res://scenes/main_menu.tscn -- --role=` (host first, sleep ~1s, then the join role), then remove the autoload line again — it must never ship registered. - The `mcp/godot-mcp` submodule is a separate Node/TypeScript project with its own `npm install` / `npm run build` (see above) — it is tooling, not part of the game itself. ## Architecture diff --git a/Game/project.godot b/Game/project.godot index 3e414e51..58a4744f 100644 --- a/Game/project.godot +++ b/Game/project.godot @@ -44,6 +44,9 @@ GameSettings="*res://scripts/game_settings.gd" VideoSettings="*res://scripts/video_settings.gd" BackgroundFPS="*res://scripts/background_fps.gd" PerfOverlay="*res://scripts/perf_overlay.gd" +NetworkManager="*res://scripts/network_manager.gd" +MatchNet="*res://scripts/match_net.gd" +NetDebugOverlay="*res://scripts/net_debug_overlay.gd" [display] @@ -157,6 +160,11 @@ toggle_perf_overlay={ "events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":0,"physical_keycode":4194334,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null) ] } +toggle_net_overlay={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":0,"physical_keycode":4194335,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null) +] +} [layer_names] diff --git a/Game/scenes/lobby.tscn b/Game/scenes/lobby.tscn new file mode 100644 index 00000000..72a83e4b --- /dev/null +++ b/Game/scenes/lobby.tscn @@ -0,0 +1,112 @@ +[gd_scene load_steps=2 format=3] + +[ext_resource type="Script" path="res://scripts/lobby.gd" id="1_lobby"] + +[node name="Lobby" type="Control"] +layout_mode = 3 +anchors_preset = 15 +anchor_right = 1.0 +anchor_bottom = 1.0 +grow_horizontal = 2 +grow_vertical = 2 +script = ExtResource("1_lobby") + +[node name="CenterContainer" type="CenterContainer" parent="."] +layout_mode = 1 +anchors_preset = 15 +anchor_right = 1.0 +anchor_bottom = 1.0 +grow_horizontal = 2 +grow_vertical = 2 + +[node name="VBoxContainer" type="VBoxContainer" parent="CenterContainer"] +custom_minimum_size = Vector2(520, 0) +layout_mode = 2 +theme_override_constants/separation = 10 + +[node name="TitleLabel" type="Label" parent="CenterContainer/VBoxContainer"] +layout_mode = 2 +theme_override_font_sizes/font_size = 40 +text = "Lobby" +horizontal_alignment = 1 + +[node name="StatusLabel" type="Label" parent="CenterContainer/VBoxContainer"] +unique_name_in_owner = true +modulate = Color(1, 1, 1, 0.65) +layout_mode = 2 +theme_override_font_sizes/font_size = 14 +text = "Connecting..." +horizontal_alignment = 1 +autowrap_mode = 2 + +[node name="TeamsSeparator" type="HSeparator" parent="CenterContainer/VBoxContainer"] +layout_mode = 2 + +[node name="TeamsRow" type="HBoxContainer" parent="CenterContainer/VBoxContainer"] +layout_mode = 2 +theme_override_constants/separation = 20 + +[node name="Team0Panel" type="VBoxContainer" parent="CenterContainer/VBoxContainer/TeamsRow"] +layout_mode = 2 +size_flags_horizontal = 3 +theme_override_constants/separation = 4 + +[node name="Team0Header" type="Label" parent="CenterContainer/VBoxContainer/TeamsRow/Team0Panel"] +layout_mode = 2 +theme_override_font_sizes/font_size = 18 +text = "Team 1" + +[node name="Team0List" type="VBoxContainer" parent="CenterContainer/VBoxContainer/TeamsRow/Team0Panel"] +unique_name_in_owner = true +layout_mode = 2 +theme_override_constants/separation = 2 + +[node name="TeamsVSeparator" type="VSeparator" parent="CenterContainer/VBoxContainer/TeamsRow"] +layout_mode = 2 + +[node name="Team1Panel" type="VBoxContainer" parent="CenterContainer/VBoxContainer/TeamsRow"] +layout_mode = 2 +size_flags_horizontal = 3 +theme_override_constants/separation = 4 + +[node name="Team1Header" type="Label" parent="CenterContainer/VBoxContainer/TeamsRow/Team1Panel"] +layout_mode = 2 +theme_override_font_sizes/font_size = 18 +text = "Team 2" + +[node name="Team1List" type="VBoxContainer" parent="CenterContainer/VBoxContainer/TeamsRow/Team1Panel"] +unique_name_in_owner = true +layout_mode = 2 +theme_override_constants/separation = 2 + +[node name="ControlsSeparator" type="HSeparator" parent="CenterContainer/VBoxContainer"] +layout_mode = 2 + +[node name="ControlsRow" type="HBoxContainer" parent="CenterContainer/VBoxContainer"] +unique_name_in_owner = true +layout_mode = 2 +theme_override_constants/separation = 10 + +[node name="SwitchTeamButton" type="Button" parent="CenterContainer/VBoxContainer/ControlsRow"] +unique_name_in_owner = true +custom_minimum_size = Vector2(0, 48) +layout_mode = 2 +size_flags_horizontal = 3 +text = "Switch Team" + +[node name="ReadyButton" type="CheckButton" parent="CenterContainer/VBoxContainer/ControlsRow"] +unique_name_in_owner = true +custom_minimum_size = Vector2(0, 48) +layout_mode = 2 +size_flags_horizontal = 3 +text = "Ready" + +[node name="LeaveButton" type="Button" parent="CenterContainer/VBoxContainer"] +unique_name_in_owner = true +custom_minimum_size = Vector2(0, 48) +layout_mode = 2 +text = "Leave" + +[connection signal="pressed" from="CenterContainer/VBoxContainer/ControlsRow/SwitchTeamButton" to="." method="_on_switch_team_pressed"] +[connection signal="toggled" from="CenterContainer/VBoxContainer/ControlsRow/ReadyButton" to="." method="_on_ready_toggled"] +[connection signal="pressed" from="CenterContainer/VBoxContainer/LeaveButton" to="." method="_on_leave_pressed"] diff --git a/Game/scenes/main_menu.tscn b/Game/scenes/main_menu.tscn index ce45505f..eeb5fcf3 100644 --- a/Game/scenes/main_menu.tscn +++ b/Game/scenes/main_menu.tscn @@ -100,6 +100,51 @@ custom_minimum_size = Vector2(0, 56) layout_mode = 2 text = "Play Match" +[node name="MultiplayerSeparator" type="HSeparator" parent="CenterContainer/VBoxContainer"] +layout_mode = 2 + +[node name="MultiplayerHeader" type="Label" parent="CenterContainer/VBoxContainer"] +layout_mode = 2 +theme_override_font_sizes/font_size = 22 +text = "Multiplayer" + +[node name="MultiplayerHint" type="Label" parent="CenterContainer/VBoxContainer"] +modulate = Color(1, 1, 1, 0.55) +layout_mode = 2 +theme_override_font_sizes/font_size = 13 +text = "LAN / direct IP — host a match or join one" + +[node name="HostButton" type="Button" parent="CenterContainer/VBoxContainer"] +custom_minimum_size = Vector2(0, 48) +layout_mode = 2 +text = "Host" + +[node name="JoinRow" type="HBoxContainer" parent="CenterContainer/VBoxContainer"] +layout_mode = 2 +theme_override_constants/separation = 10 + +[node name="JoinAddressEdit" type="LineEdit" parent="CenterContainer/VBoxContainer/JoinRow"] +unique_name_in_owner = true +custom_minimum_size = Vector2(0, 40) +layout_mode = 2 +size_flags_horizontal = 3 +text = "127.0.0.1" +placeholder_text = "IP address" + +[node name="JoinButton" type="Button" parent="CenterContainer/VBoxContainer/JoinRow"] +custom_minimum_size = Vector2(96, 40) +layout_mode = 2 +text = "Join" + +[node name="MultiplayerErrorLabel" type="Label" parent="CenterContainer/VBoxContainer"] +unique_name_in_owner = true +modulate = Color(1, 0.5, 0.5, 1) +layout_mode = 2 +theme_override_font_sizes/font_size = 13 +text = "" +autowrap_mode = 2 +visible = false + [node name="SettingsSeparator" type="HSeparator" parent="CenterContainer/VBoxContainer"] layout_mode = 2 @@ -180,7 +225,58 @@ custom_minimum_size = Vector2(0, 56) layout_mode = 2 text = "Watch Match" +[node name="ConnectingOverlay" type="Control" parent="."] +unique_name_in_owner = true +visible = false +layout_mode = 1 +anchors_preset = 15 +anchor_right = 1.0 +anchor_bottom = 1.0 +grow_horizontal = 2 +grow_vertical = 2 +mouse_filter = 1 + +[node name="Backdrop" type="ColorRect" parent="ConnectingOverlay"] +layout_mode = 1 +anchors_preset = 15 +anchor_right = 1.0 +anchor_bottom = 1.0 +grow_horizontal = 2 +grow_vertical = 2 +color = Color(0, 0, 0, 0.7) + +[node name="CenterContainer" type="CenterContainer" parent="ConnectingOverlay"] +layout_mode = 1 +anchors_preset = 15 +anchor_right = 1.0 +anchor_bottom = 1.0 +grow_horizontal = 2 +grow_vertical = 2 + +[node name="VBoxContainer" type="VBoxContainer" parent="ConnectingOverlay/CenterContainer"] +custom_minimum_size = Vector2(360, 0) +layout_mode = 2 +theme_override_constants/separation = 14 + +[node name="ConnectingStatusLabel" type="Label" parent="ConnectingOverlay/CenterContainer/VBoxContainer"] +unique_name_in_owner = true +layout_mode = 2 +theme_override_font_sizes/font_size = 18 +text = "Connecting..." +horizontal_alignment = 1 +autowrap_mode = 2 + +[node name="ConnectingCancelButton" type="Button" parent="ConnectingOverlay/CenterContainer/VBoxContainer"] +unique_name_in_owner = true +custom_minimum_size = Vector2(0, 48) +layout_mode = 2 +text = "Cancel" + [connection signal="pressed" from="CenterContainer/VBoxContainer/FreePlayButton" to="." method="_on_free_play_pressed"] [connection signal="pressed" from="CenterContainer/VBoxContainer/MatchButton" to="." method="_on_match_pressed"] +[connection signal="pressed" from="CenterContainer/VBoxContainer/HostButton" to="." method="_on_host_pressed"] +[connection signal="pressed" from="CenterContainer/VBoxContainer/JoinRow/JoinButton" to="." method="_on_join_pressed"] +[connection signal="text_submitted" from="CenterContainer/VBoxContainer/JoinRow/JoinAddressEdit" to="." method="_on_join_address_submitted"] [connection signal="pressed" from="CenterContainer/VBoxContainer/SettingsButton" to="." method="_on_settings_pressed"] [connection signal="pressed" from="CenterContainer/VBoxContainer/DevSection/SpectateButton" to="." method="_on_spectate_pressed"] +[connection signal="pressed" from="ConnectingOverlay/CenterContainer/VBoxContainer/ConnectingCancelButton" to="." method="_on_connecting_cancel_pressed"] diff --git a/Game/scenes/server_boot.tscn b/Game/scenes/server_boot.tscn new file mode 100644 index 00000000..c3c71acc --- /dev/null +++ b/Game/scenes/server_boot.tscn @@ -0,0 +1,6 @@ +[gd_scene load_steps=2 format=3] + +[ext_resource type="Script" path="res://scripts/server_boot.gd" id="1_sb"] + +[node name="ServerBoot" type="Node"] +script = ExtResource("1_sb") diff --git a/Game/scripts/lobby.gd b/Game/scripts/lobby.gd new file mode 100644 index 00000000..b8c8fce3 --- /dev/null +++ b/Game/scripts/lobby.gd @@ -0,0 +1,116 @@ +extends Control + +# Lobby (task 1.5): roster list split by team, team swap, ready toggle, +# leave. Reads/writes MatchNet.roster — this scene owns no state of its +# own, it's a view over the autoload. Reached via main_menu.gd's Host/Join +# flow (task 1.7) calling change_scene_to_file("res://scenes/lobby.tscn") +# after NetworkManager.host()/join() succeeds — this scene must always be +# loaded that way (as the real current_scene), not instantiated as a child +# of something else: change_scene_to_file() operates on +# get_tree().current_scene, and _on_disconnected_from_server()/_leave() +# below call it themselves, which hangs if this scene isn't actually the +# tree's current_scene when that happens (see multiplayer-todo.md §9 +# gotcha 27 — found the hard way while building tests/lobby_smoke.gd). + +@onready var _status_label: Label = %StatusLabel +@onready var _team0_list: VBoxContainer = %Team0List +@onready var _team1_list: VBoxContainer = %Team1List +@onready var _controls_row: HBoxContainer = %ControlsRow +@onready var _switch_team_button: Button = %SwitchTeamButton +@onready var _ready_button: CheckButton = %ReadyButton +@onready var _leave_button: Button = %LeaveButton + + +func _ready() -> void: + MatchNet.welcomed.connect(_on_welcomed) + MatchNet.player_joined.connect(_on_roster_changed) + MatchNet.player_left.connect(_on_roster_changed) + MatchNet.player_state_changed.connect(_on_roster_changed) + MatchNet.rejected.connect(_on_rejected) + NetworkManager.disconnected_from_server.connect(_on_disconnected_from_server) + + # The server process is never a roster member (§1.1 decision 2) — it + # gets a read-only view, no team/ready controls to operate on itself. + _controls_row.visible = NetworkManager.is_client + + _refresh() + + +func _process(_delta: float) -> void: + NetworkManager.poll() + + +func _physics_process(_delta: float) -> void: + NetworkManager.poll() + + +func _unhandled_input(event: InputEvent) -> void: + if event.is_action_pressed("ui_cancel"): + _leave() + + +func _on_roster_changed(_a = null, _b = null, _c = null) -> void: + _refresh() + + +func _on_welcomed() -> void: + _refresh() + + +func _on_rejected(reason: String) -> void: + _status_label.text = "Connection rejected: %s" % reason + + +func _on_disconnected_from_server() -> void: + get_tree().change_scene_to_file(ScenePaths.MAIN_MENU) + + +func _on_switch_team_pressed() -> void: + var my_id := multiplayer.get_unique_id() + var info: MatchNet.PlayerInfo = MatchNet.roster.get(my_id) + if info == null: + return + MatchNet.request_set_team((info.team + 1) % MatchNet.TEAM_COUNT) + + +func _on_ready_toggled(pressed: bool) -> void: + MatchNet.request_set_ready(pressed) + + +func _on_leave_pressed() -> void: + _leave() + + +func _leave() -> void: + NetworkManager.shutdown() + get_tree().change_scene_to_file(ScenePaths.MAIN_MENU) + + +func _refresh() -> void: + if NetworkManager.is_server: + _status_label.text = "Hosting — %d player(s) connected" % MatchNet.roster.size() + elif NetworkManager.is_client: + _status_label.text = "Connected" if not MatchNet.roster.is_empty() else "Connecting..." + else: + _status_label.text = "Not connected" + + for child in _team0_list.get_children(): + child.queue_free() + for child in _team1_list.get_children(): + child.queue_free() + + var my_id := multiplayer.get_unique_id() + var infos: Array = MatchNet.roster.values() + infos.sort_custom(func(a: MatchNet.PlayerInfo, b: MatchNet.PlayerInfo) -> bool: return a.peer_id < b.peer_id) + for info: MatchNet.PlayerInfo in infos: + var row := Label.new() + var marker := " (you)" if info.peer_id == my_id else "" + var ready_mark := "✓" if info.ready else "…" + row.text = "%s %s%s" % [ready_mark, info.player_name, marker] + var target_list := _team0_list if info.team == 0 else _team1_list + target_list.add_child(row) + + if NetworkManager.is_client: + var my_info: MatchNet.PlayerInfo = MatchNet.roster.get(my_id) + if my_info != null: + _ready_button.set_pressed_no_signal(my_info.ready) diff --git a/Game/scripts/main_menu.gd b/Game/scripts/main_menu.gd index 920a57f9..325848ed 100644 --- a/Game/scripts/main_menu.gd +++ b/Game/scripts/main_menu.gd @@ -31,6 +31,10 @@ const DIFFICULTIES := [ @onready var dev_bot_dropdown: OptionButton = %DevBotDropdown @onready var bot_a_dropdown: OptionButton = %BotADropdown @onready var bot_b_dropdown: OptionButton = %BotBDropdown +@onready var join_address_edit: LineEdit = %JoinAddressEdit +@onready var multiplayer_error_label: Label = %MultiplayerErrorLabel +@onready var connecting_overlay: Control = %ConnectingOverlay +@onready var connecting_status_label: Label = %ConnectingStatusLabel func _ready() -> void: @@ -46,9 +50,27 @@ func _ready() -> void: _populate_dropdown(dev_bot_dropdown, bots, GameSettings.dev_bot_override_path, true) _populate_dropdown(bot_a_dropdown, bots, GameSettings.spectate_bot_a_path) _populate_dropdown(bot_b_dropdown, bots, GameSettings.spectate_bot_b_path) + NetworkManager.connected_to_server.connect(_on_connected_to_server) + NetworkManager.connection_failed.connect(_on_connection_failed) $CenterContainer/VBoxContainer/FreePlayButton.grab_focus() +# main_menu.gd's first async flow (task 1.7): Host is synchronous +# (NetworkManager.host() either succeeds immediately or fails immediately), +# but Join is not — it can take anywhere from a clean local-network round +# trip to ENet's own ~5s connect timeout to resolve, so unlike every other +# handler in this file (GameSettings.x = y; change_scene_to_file(...)) it +# needs a loading state (ConnectingOverlay), a cancel path, and a failure +# path that returns the player to a sane, retryable menu state rather than +# just hanging with no feedback. +func _process(_delta: float) -> void: + NetworkManager.poll() + + +func _physics_process(_delta: float) -> void: + NetworkManager.poll() + + func _populate_difficulty_dropdown() -> void: difficulty_dropdown.clear() for tier in DIFFICULTIES: @@ -158,3 +180,91 @@ func _on_spectate_pressed() -> void: GameSettings.spectate_bot_a_path = _selected_path(bot_a_dropdown) GameSettings.spectate_bot_b_path = _selected_path(bot_b_dropdown) _leave_to_gameplay("res://scenes/spectate.tscn") + + +func _on_host_pressed() -> void: + _clear_multiplayer_error() + var err := NetworkManager.host() + if err != OK: + _show_multiplayer_error("Could not host: %s" % error_string(err)) + return + _leave_to_lobby() + + +func _on_join_pressed() -> void: + _start_join() + + +func _on_join_address_submitted(_new_text: String) -> void: + _start_join() + + +# ENet's own give-up-and-fire-connection_failed schedule is not bounded to +# anything a menu should make a player wait for — verified empirically +# (tests/main_menu_test_hooks.gd's join_refused case) against a genuinely +# refused loopback connection: connection_failed never fired within 14s. +# This timer is what actually guarantees "connection-refused reaches a sane +# UI state" rather than leaving the overlay up indefinitely. +const CONNECT_TIMEOUT_SECONDS := 6.0 + +var _connect_timeout_token := 0 # bumped on every new attempt/cancel/resolution so a stale timer callback is a no-op + + +func _start_join() -> void: + _clear_multiplayer_error() + var address := join_address_edit.text.strip_edges() + if address.is_empty(): + _show_multiplayer_error("Enter an IP address to join") + return + var err := NetworkManager.join(address) + if err != OK: + _show_multiplayer_error("Could not join: %s" % error_string(err)) + return + connecting_status_label.text = "Connecting to %s..." % address + connecting_overlay.visible = true + _connect_timeout_token += 1 + var my_token := _connect_timeout_token + get_tree().create_timer(CONNECT_TIMEOUT_SECONDS).timeout.connect(func(): _on_connect_timeout(my_token)) + + +func _on_connect_timeout(token: int) -> void: + if token != _connect_timeout_token or not connecting_overlay.visible: + return # a newer attempt (or Cancel, or a real success/failure) already resolved this + NetworkManager.shutdown() + connecting_overlay.visible = false + _show_multiplayer_error("Connection timed out — check the address and that a server is hosting on that port") + + +func _on_connecting_cancel_pressed() -> void: + _connect_timeout_token += 1 + NetworkManager.shutdown() + connecting_overlay.visible = false + + +func _on_connected_to_server() -> void: + if not connecting_overlay.visible: + return # e.g. a stray/late signal after Cancel already shut the peer down + _connect_timeout_token += 1 + connecting_overlay.visible = false + _leave_to_lobby() + + +func _on_connection_failed() -> void: + if not connecting_overlay.visible: + return + _connect_timeout_token += 1 + connecting_overlay.visible = false + _show_multiplayer_error("Connection failed — check the address and that a server is hosting on that port") + + +func _leave_to_lobby() -> void: + get_tree().change_scene_to_file("res://scenes/lobby.tscn") + + +func _show_multiplayer_error(message: String) -> void: + multiplayer_error_label.text = message + multiplayer_error_label.visible = true + + +func _clear_multiplayer_error() -> void: + multiplayer_error_label.visible = false diff --git a/Game/scripts/match_net.gd b/Game/scripts/match_net.gd new file mode 100644 index 00000000..db116c3c --- /dev/null +++ b/Game/scripts/match_net.gd @@ -0,0 +1,255 @@ +extends Node + +# Autoload (project.godot [autoload] MatchNet). Handshake + roster layer on +# top of NetworkManager's raw transport (§2.5, §1.3 of multiplayer-todo.md). +# hello/welcome, strict protocol_version and physics_ticks_per_second +# gating, player_joined/player_left, and — since lobby.tscn (task 1.5) needs +# somewhere durable to keep it across the lobby→match scene transition — +# each player's team and ready state. Slot assignment (fixed spawn index +# within a team) is NOT here; that's match spawn's job in Phase 2, derived +# from this roster's team field at spawn time, not stored redundantly here. + +const NetCodec = preload("res://scripts/net_codec.gd") +const SimConstants = preload("res://scripts/sim_constants.gd") + +signal player_joined(peer_id: int, player_name: String) +signal player_left(peer_id: int) +signal player_state_changed(peer_id: int, team: int, ready: bool) +signal rejected(reason: String) # client-side only: the server refused our hello +signal welcomed() # client-side only: our hello was accepted + +const TEAM_COUNT := 2 + +# player_name is the one client-supplied value in _hello that gets broadcast +# verbatim to every other peer (protocol_version/tick_hz are checked, never +# relayed). MAX_INPUT_LENGTH is a reject threshold, checked before touching +# the string at all — a legitimate client only ever sends local_player_name, +# which the UI already keeps short, so anything past this is a bug or an +# attacker, not a real name to truncate politely. Adversarial review found +# an unbounded name relayed to every peer head-of-line-blocks the reliable +# control channel hard enough that a concurrently-joining client's own +# _welcome never arrived — this is what closes that. +const MAX_INPUT_LENGTH := 256 +const MAX_PLAYER_NAME_LENGTH := 24 + + +class PlayerInfo: + var peer_id: int + var player_name: String + var team: int = 0 + var ready: bool = false + + func _init(p_peer_id: int, p_player_name: String, p_team: int = 0, p_ready: bool = false) -> void: + peer_id = p_peer_id + player_name = p_player_name + team = p_team + ready = p_ready + + +var roster: Dictionary = {} # peer_id (int) -> PlayerInfo. Never contains peer 1 (the server; §1.1 decision 2 — dedicated servers are never a player). +var local_player_name := "Player" + +# Test hook (tests/match_net_smoke.gd): set false before connecting to +# suppress the automatic real hello, so a test can send a deliberately +# mismatched one instead to exercise the rejection path. +var _auto_hello := true + + +func _ready() -> void: + NetworkManager.client_disconnected.connect(_on_peer_disconnected) + NetworkManager.connected_to_server.connect(_on_connected_to_server) + NetworkManager.disconnected_from_server.connect(_on_disconnected_from_server) + NetworkManager.shutting_down.connect(_on_shutting_down) + + +func _on_connected_to_server() -> void: + roster.clear() + if _auto_hello: + _hello.rpc_id(1, NetCodec.PROTOCOL_VERSION, SimConstants.TICK_HZ, local_player_name) + + +func _on_disconnected_from_server() -> void: + roster.clear() + + +# Covers the case _on_disconnected_from_server doesn't: a HOST calling +# NetworkManager.shutdown() itself (Leave, or hosting again after already +# hosting) never fires disconnected_from_server — that signal only fires +# from an incoming multiplayer.server_disconnected event, which a server +# never receives about itself. Without this, roster (and every peer's team/ +# ready state in it) would persist forever across a host/re-host cycle in +# the same process. +func _on_shutting_down() -> void: + roster.clear() + + +# Server only: a raw ENet disconnect (crash, timeout) that never sent a +# proper hello just needs its (possibly absent) roster entry cleaned up. +# The normal leave path also goes through here after the server erases it, +# guarded by roster.erase()'s own has-check below. +func _on_peer_disconnected(peer_id: int) -> void: + if not multiplayer.is_server(): + return + _remove_player(peer_id) + + +func _remove_player(peer_id: int) -> void: + if not roster.has(peer_id): + return + roster.erase(peer_id) + player_left.emit(peer_id) + _player_left.rpc(peer_id) + + +# Balances a new joiner onto whichever team currently has fewer players +# (ties go to team 0). Server only. +func _pick_balanced_team() -> int: + var counts := [] + counts.resize(TEAM_COUNT) + counts.fill(0) + for info: PlayerInfo in roster.values(): + counts[info.team] += 1 + var best_team := 0 + for team in range(TEAM_COUNT): + if counts[team] < counts[best_team]: + best_team = team + return best_team + + +@rpc("any_peer", "call_remote", "reliable") +func _hello(protocol_version: int, tick_hz: int, player_name: String) -> void: + if not multiplayer.is_server(): + return + var peer_id := multiplayer.get_remote_sender_id() + if roster.has(peer_id): + return # duplicate hello from an already-accepted peer; ignore + + if protocol_version != NetCodec.PROTOCOL_VERSION: + await _reject(peer_id, "protocol version mismatch: server=%d client=%d" % [NetCodec.PROTOCOL_VERSION, protocol_version]) + return + if tick_hz != SimConstants.TICK_HZ: + await _reject(peer_id, "physics tick rate mismatch: server=%d client=%d" % [SimConstants.TICK_HZ, tick_hz]) + return + if player_name.length() > MAX_INPUT_LENGTH: + await _reject(peer_id, "player name too long") + return + var clean_name := _sanitize_player_name(player_name) + + # Tell the new peer about everyone already here before anyone is told + # about them, so no client ever observes an unknown peer_id in a + # player_joined it didn't get a prior player_joined for. + for existing_id: int in roster.keys(): + var existing: PlayerInfo = roster[existing_id] + _player_joined.rpc_id(peer_id, existing_id, existing.player_name, existing.team, existing.ready) + + var team := _pick_balanced_team() + roster[peer_id] = PlayerInfo.new(peer_id, clean_name, team, false) + player_joined.emit(peer_id, clean_name) # local: the broadcast below is call_remote, never loops back to the server itself + _welcome.rpc_id(peer_id) + _player_joined.rpc(peer_id, clean_name, team, false) # broadcast, includes the new peer itself + + +# Strips control/formatting characters (so a name can't corrupt a log line +# or blow out UI layout with e.g. embedded newlines) and clamps to display +# length. Input is already bounded to MAX_INPUT_LENGTH by the caller before +# this runs, so this never iterates an attacker-sized string. static: pure +# function of its argument, doesn't touch roster/multiplayer — also lets +# tests/cases/test_match_net.gd call it with no Node instantiation. +static func _sanitize_player_name(raw: String) -> String: + var clean := "" + for c in raw: + var code := c.unicode_at(0) + if code >= 0x20 and code != 0x7F: + clean += c + clean = clean.strip_edges() + if clean.length() > MAX_PLAYER_NAME_LENGTH: + clean = clean.substr(0, MAX_PLAYER_NAME_LENGTH) + if clean.is_empty(): + clean = "Player" + return clean + + +func _reject(peer_id: int, reason: String) -> void: + _rejected.rpc_id(peer_id, reason) + # §9 gotcha 26: a reliable RPC just queued still needs a beat of polling + # to actually reach the wire before we pull the connection out from + # under it. + await get_tree().create_timer(0.3).timeout + if multiplayer.multiplayer_peer is ENetMultiplayerPeer: + multiplayer.multiplayer_peer.disconnect_peer(peer_id) + + +# Client-callable requests. Both are fire-and-forget: the authoritative +# change comes back through _state_changed once the server applies it, same +# as everyone else's — a client never mutates its own roster entry directly. +func request_set_team(team: int) -> void: + _set_team.rpc_id(1, team) + + +func request_set_ready(ready: bool) -> void: + _set_ready.rpc_id(1, ready) + + +@rpc("any_peer", "call_remote", "reliable") +func _set_team(team: int) -> void: + if not multiplayer.is_server(): + return + var peer_id := multiplayer.get_remote_sender_id() + if not roster.has(peer_id) or team < 0 or team >= TEAM_COUNT: + return + var info: PlayerInfo = roster[peer_id] + if info.team == team: + return + info.team = team + info.ready = false # switching teams un-readies — the roster you were ready against just changed + player_state_changed.emit(peer_id, info.team, info.ready) + _state_changed.rpc(peer_id, info.team, info.ready) + + +@rpc("any_peer", "call_remote", "reliable") +func _set_ready(ready: bool) -> void: + if not multiplayer.is_server(): + return + var peer_id := multiplayer.get_remote_sender_id() + if not roster.has(peer_id): + return + var info: PlayerInfo = roster[peer_id] + if info.ready == ready: + return + info.ready = ready + player_state_changed.emit(peer_id, info.team, info.ready) + _state_changed.rpc(peer_id, info.team, info.ready) + + +@rpc("authority", "call_remote", "reliable") +func _state_changed(peer_id: int, team: int, ready: bool) -> void: + if not roster.has(peer_id): + return + var info: PlayerInfo = roster[peer_id] + info.team = team + info.ready = ready + player_state_changed.emit(peer_id, team, ready) + + +@rpc("authority", "call_remote", "reliable") +func _welcome() -> void: + welcomed.emit() + + +@rpc("authority", "call_remote", "reliable") +func _rejected(reason: String) -> void: + rejected.emit(reason) + + +@rpc("authority", "call_remote", "reliable") +func _player_joined(peer_id: int, player_name: String, team: int, ready: bool) -> void: + roster[peer_id] = PlayerInfo.new(peer_id, player_name, team, ready) + player_joined.emit(peer_id, player_name) + + +@rpc("authority", "call_remote", "reliable") +func _player_left(peer_id: int) -> void: + if not roster.has(peer_id): + return + roster.erase(peer_id) + player_left.emit(peer_id) diff --git a/Game/scripts/net_body_state.gd b/Game/scripts/net_body_state.gd new file mode 100644 index 00000000..992fb394 --- /dev/null +++ b/Game/scripts/net_body_state.gd @@ -0,0 +1,23 @@ +extends RefCounted + +# Plain data holder for one body's snapshot state (§2.4 of multiplayer-todo.md). +# Deliberately not Ship/Ball themselves, and deliberately not a scene-tree +# node — NetCodec's pack/unpack must stay callable from pure-function tests +# with no live scene. Phase 2's snapshot writer fills one of these per body +# per tick from the real RigidBody3D state; Phase 2's interpolator does the +# reverse. +# +# avel_range must match what the sender quantised with (SHIP_AVEL_RANGE vs +# BALL_AVEL_RANGE in net_codec.gd) — it is not carried on the wire, because +# slot order already tells both peers which body is which (§1.3: "entities +# are addressed by integer slot, never by path"). + +var position := Vector3.ZERO +var rotation := Quaternion.IDENTITY +var linear_velocity := Vector3.ZERO +var angular_velocity := Vector3.ZERO +var frozen := false +var turbo := false +var thrust_z := 0.0 # -1..1; re-quantised to a 3-bit bin on the wire +var stalled := false +var avel_range := 4.0 # NetCodec.SHIP_AVEL_RANGE; set to BALL_AVEL_RANGE for the ball diff --git a/Game/scripts/net_codec.gd b/Game/scripts/net_codec.gd new file mode 100644 index 00000000..f5cf70c7 --- /dev/null +++ b/Game/scripts/net_codec.gd @@ -0,0 +1,295 @@ +class_name NetCodec + +# Wire-format constants, quantisers, and pack/unpack for the two hot-path +# packets (§2 of multiplayer-todo.md). Pure functions only — no networking, +# no autoload state — so they're testable head-on by tests/test_runner.tscn +# without a live connection. +# +# Referenced from elsewhere via preload(), not the bare class_name, per the +# same global-script-class-cache caveat documented in tests/test_case.gd and +# sim_constants.gd. + +const SimConstants = preload("res://scripts/sim_constants.gd") +const ShipAction = preload("res://scripts/ship_action.gd") +const NetBodyState = preload("res://scripts/net_body_state.gd") + +# --- Protocol --- +const PROTOCOL_VERSION := 1 +const TICK_HZ: int = SimConstants.TICK_HZ + +# --- Channels (logical intent; NetworkManager may need to offset these on +# top of ENet's own reserved channels — verify empirically, see §2.1) --- +const CHANNEL_CONTROL := 0 +const CHANNEL_INPUT := 1 +const CHANNEL_SNAPSHOT := 2 + +# --- Packet type/version byte: high nibble = type, low nibble = protocol version --- +enum PacketType { INPUT = 0, SNAPSHOT = 1 } + +# --- Input packet (§2.3) --- +const MAX_REDUNDANCY := 4 +# type_version u8 + seq u32 + count u8 + ack_snapshot_tick u32 + client_send_ms u16 +const INPUT_HEADER_SIZE := 12 +const INPUT_ENTRY_SIZE := 7 # thrust i8x3 + rotation i8x3 + flags u8 +const INPUT_FLAG_TURBO := 1 << 0 + +# --- Snapshot packet (§2.4) --- +# last_input_seq u32 + input_buffer_depth i8 + echo_client_send_ms u16 +const SNAPSHOT_CLIENT_HEADER_SIZE := 7 +# type_version u8 + server_tick u32 + match_state u8 + reset_gen u8 + body_count u8 +const SNAPSHOT_BODY_HEADER_SIZE := 8 +const SNAPSHOT_BODY_SIZE := 22 + +const BODY_FLAG_FROZEN := 1 << 0 +const BODY_FLAG_TURBO := 1 << 1 +const BODY_FLAG_THRUST_Z_SHIFT := 2 +const BODY_FLAG_THRUST_Z_MASK := 0x1C # bits 2-4 +const BODY_FLAG_STALLED := 1 << 5 +const BODY_FLAG_QUAT_W_SIGN := 1 << 6 + +# --- Quantisation ranges (§2.4 — derived from arena/gameplay constants, not +# restated prose; see multiplayer-todo.md for the ArenaBoundary/Ship/Ball +# constants these are sized against) --- +const POS_RANGE := 64.0 # metres, ± +const VEL_RANGE := 64.0 # m/s, ± +const QUAT_COMPONENT_RANGE := 1.0 +const SHIP_AVEL_RANGE := 4.0 # rad/s, ± +const BALL_AVEL_RANGE := 32.0 # rad/s, ± + +const I16_MAX := 32767 +const I8_MAX := 127 +const THRUST_Z_BIN_MAX := 7 # 3 bits + + +# ============================================================ +# Quantisers — pure, reusable, independently testable. +# ============================================================ + +static func quantize_i16(value: float, range_max: float) -> int: + var scaled := clampf(value / range_max, -1.0, 1.0) * I16_MAX + return clampi(roundi(scaled), -I16_MAX, I16_MAX) + +static func dequantize_i16(raw: int, range_max: float) -> float: + return (float(raw) / I16_MAX) * range_max + +static func quantize_i8(value: float, range_max: float) -> int: + var scaled := clampf(value / range_max, -1.0, 1.0) * I8_MAX + return clampi(roundi(scaled), -I8_MAX, I8_MAX) + +static func dequantize_i8(raw: int, range_max: float) -> float: + return (float(raw) / I8_MAX) * range_max + +static func quantize_thrust_z_bin(thrust_z: float) -> int: + var t := clampf((thrust_z + 1.0) * 0.5, 0.0, 1.0) + return clampi(roundi(t * THRUST_Z_BIN_MAX), 0, THRUST_Z_BIN_MAX) + +static func dequantize_thrust_z_bin(bin_value: int) -> float: + return (float(bin_value) / THRUST_Z_BIN_MAX) * 2.0 - 1.0 + + +static func type_version_byte(type: PacketType) -> int: + return ((int(type) & 0x0F) << 4) | (PROTOCOL_VERSION & 0x0F) + +static func packet_type_of(type_version: int) -> int: + return (type_version >> 4) & 0x0F + +static func protocol_version_of(type_version: int) -> int: + return type_version & 0x0F + + +# ============================================================ +# Input packet — client -> server, channel 1 (§2.3) +# ============================================================ + +# actions: newest-first, 1..MAX_REDUNDANCY ShipAction instances. +static func pack_input(seq: int, ack_snapshot_tick: int, client_send_ms: int, actions: Array) -> PackedByteArray: + var count: int = clampi(actions.size(), 1, MAX_REDUNDANCY) + var buf := StreamPeerBuffer.new() + buf.put_u8(type_version_byte(PacketType.INPUT)) + buf.put_u32(seq) + buf.put_u8(count) + buf.put_u32(ack_snapshot_tick) + buf.put_u16(client_send_ms & 0xFFFF) + for i in count: + var action: ShipAction = actions[i] + buf.put_8(quantize_i8(action.thrust.x, 1.0)) + buf.put_8(quantize_i8(action.thrust.y, 1.0)) + buf.put_8(quantize_i8(action.thrust.z, 1.0)) + buf.put_8(quantize_i8(action.rotation.x, 1.0)) + buf.put_8(quantize_i8(action.rotation.y, 1.0)) + buf.put_8(quantize_i8(action.rotation.z, 1.0)) + var flags := 0 + if action.turbo: + flags |= INPUT_FLAG_TURBO + buf.put_u8(flags) + return buf.data_array + + +# Returns a Dictionary: type_version, seq, count, ack_snapshot_tick, +# client_send_ms, actions (Array[ShipAction], newest first). +static func unpack_input(bytes: PackedByteArray) -> Dictionary: + var buf := StreamPeerBuffer.new() + buf.data_array = bytes + var type_version := buf.get_u8() + var seq := buf.get_u32() + var count := buf.get_u8() + var ack_snapshot_tick := buf.get_u32() + var client_send_ms := buf.get_u16() + var actions: Array[ShipAction] = [] + for i in count: + var a := ShipAction.new() + a.thrust = Vector3( + dequantize_i8(buf.get_8(), 1.0), + dequantize_i8(buf.get_8(), 1.0), + dequantize_i8(buf.get_8(), 1.0) + ) + a.rotation = Vector3( + dequantize_i8(buf.get_8(), 1.0), + dequantize_i8(buf.get_8(), 1.0), + dequantize_i8(buf.get_8(), 1.0) + ) + var flags := buf.get_u8() + a.turbo = (flags & INPUT_FLAG_TURBO) != 0 + actions.append(a) + return { + "type_version": type_version, + "seq": seq, + "count": count, + "ack_snapshot_tick": ack_snapshot_tick, + "client_send_ms": client_send_ms, + "actions": actions, + } + + +# ============================================================ +# Snapshot packet — server -> client, channel 2 (§2.4) +# ============================================================ + +# Shared across every peer this tick — build once, reuse (§2.4's stated +# intent). Returns type_version + server_tick + match_state + reset_gen + +# body_count + body_count * SNAPSHOT_BODY_SIZE bytes. +static func pack_snapshot_body_segment(server_tick: int, match_state: int, reset_gen: int, bodies: Array) -> PackedByteArray: + var buf := StreamPeerBuffer.new() + buf.put_u8(type_version_byte(PacketType.SNAPSHOT)) + buf.put_u32(server_tick) + buf.put_u8(match_state & 0xFF) + buf.put_u8(reset_gen & 0xFF) + buf.put_u8(bodies.size()) + for body in bodies: + var b: NetBodyState = body + buf.put_16(quantize_i16(b.position.x, POS_RANGE)) + buf.put_16(quantize_i16(b.position.y, POS_RANGE)) + buf.put_16(quantize_i16(b.position.z, POS_RANGE)) + buf.put_16(quantize_i16(b.rotation.x, QUAT_COMPONENT_RANGE)) + buf.put_16(quantize_i16(b.rotation.y, QUAT_COMPONENT_RANGE)) + buf.put_16(quantize_i16(b.rotation.z, QUAT_COMPONENT_RANGE)) + buf.put_16(quantize_i16(b.linear_velocity.x, VEL_RANGE)) + buf.put_16(quantize_i16(b.linear_velocity.y, VEL_RANGE)) + buf.put_16(quantize_i16(b.linear_velocity.z, VEL_RANGE)) + buf.put_8(quantize_i8(b.angular_velocity.x, b.avel_range)) + buf.put_8(quantize_i8(b.angular_velocity.y, b.avel_range)) + buf.put_8(quantize_i8(b.angular_velocity.z, b.avel_range)) + var flags := 0 + if b.frozen: + flags |= BODY_FLAG_FROZEN + if b.turbo: + flags |= BODY_FLAG_TURBO + flags |= (quantize_thrust_z_bin(b.thrust_z) << BODY_FLAG_THRUST_Z_SHIFT) & BODY_FLAG_THRUST_Z_MASK + if b.stalled: + flags |= BODY_FLAG_STALLED + if b.rotation.w < 0.0: + flags |= BODY_FLAG_QUAT_W_SIGN + buf.put_u8(flags) + return buf.data_array + + +static func pack_snapshot_client_header(last_input_seq: int, input_buffer_depth: int, echo_client_send_ms: int) -> PackedByteArray: + var buf := StreamPeerBuffer.new() + buf.put_u32(last_input_seq) + buf.put_8(clampi(input_buffer_depth, -128, 127)) + buf.put_u16(echo_client_send_ms & 0xFFFF) + return buf.data_array + + +# Convenience: one full per-client packet = per-client header + shared body segment. +static func pack_snapshot(last_input_seq: int, input_buffer_depth: int, echo_client_send_ms: int, body_segment: PackedByteArray) -> PackedByteArray: + var header := pack_snapshot_client_header(last_input_seq, input_buffer_depth, echo_client_send_ms) + var out := PackedByteArray() + out.append_array(header) + out.append_array(body_segment) + return out + + +# Returns a Dictionary: last_input_seq, input_buffer_depth, echo_client_send_ms, +# type_version, server_tick, match_state, reset_gen, bodies (Array[NetBodyState]). +static func unpack_snapshot(bytes: PackedByteArray) -> Dictionary: + var buf := StreamPeerBuffer.new() + buf.data_array = bytes + var last_input_seq := buf.get_u32() + var input_buffer_depth := buf.get_8() + var echo_client_send_ms := buf.get_u16() + var type_version := buf.get_u8() + var server_tick := buf.get_u32() + var match_state := buf.get_u8() + var reset_gen := buf.get_u8() + var body_count := buf.get_u8() + var bodies: Array[NetBodyState] = [] + for i in body_count: + var b := NetBodyState.new() + b.position = Vector3( + dequantize_i16(buf.get_16(), POS_RANGE), + dequantize_i16(buf.get_16(), POS_RANGE), + dequantize_i16(buf.get_16(), POS_RANGE) + ) + var qx := dequantize_i16(buf.get_16(), QUAT_COMPONENT_RANGE) + var qy := dequantize_i16(buf.get_16(), QUAT_COMPONENT_RANGE) + var qz := dequantize_i16(buf.get_16(), QUAT_COMPONENT_RANGE) + b.linear_velocity = Vector3( + dequantize_i16(buf.get_16(), VEL_RANGE), + dequantize_i16(buf.get_16(), VEL_RANGE), + dequantize_i16(buf.get_16(), VEL_RANGE) + ) + # avel_range is unknown to the codec at this point (it isn't on the + # wire — see net_body_state.gd) — decode at SHIP_AVEL_RANGE and let + # the caller, which knows this slot's body kind, rescale if it's the + # ball's slot. Storing the raw i8 would avoid this, but every other + # field in this struct is already physical units; consistency wins. + b.angular_velocity = Vector3( + dequantize_i8(buf.get_8(), SHIP_AVEL_RANGE), + dequantize_i8(buf.get_8(), SHIP_AVEL_RANGE), + dequantize_i8(buf.get_8(), SHIP_AVEL_RANGE) + ) + var flags := buf.get_u8() + b.frozen = (flags & BODY_FLAG_FROZEN) != 0 + b.turbo = (flags & BODY_FLAG_TURBO) != 0 + var bin_value := (flags & BODY_FLAG_THRUST_Z_MASK) >> BODY_FLAG_THRUST_Z_SHIFT + b.thrust_z = dequantize_thrust_z_bin(bin_value) + b.stalled = (flags & BODY_FLAG_STALLED) != 0 + var w_sq := 1.0 - qx * qx - qy * qy - qz * qz + var w := sqrt(maxf(w_sq, 0.0)) + if (flags & BODY_FLAG_QUAT_W_SIGN) != 0: + w = -w + b.rotation = Quaternion(qx, qy, qz, w) + bodies.append(b) + return { + "last_input_seq": last_input_seq, + "input_buffer_depth": input_buffer_depth, + "echo_client_send_ms": echo_client_send_ms, + "type_version": type_version, + "server_tick": server_tick, + "match_state": match_state, + "reset_gen": reset_gen, + "bodies": bodies, + } + + +# Rescales an already-decoded body's angular_velocity from the SHIP_AVEL_RANGE +# assumption unpack_snapshot() decoded it with to the range it was actually +# quantised at (BALL_AVEL_RANGE for the ball). Call once per non-ship body +# immediately after unpack_snapshot(), using slot order to know which. +static func rescale_avel(body: NetBodyState, actual_range: float) -> void: + if is_equal_approx(actual_range, SHIP_AVEL_RANGE): + body.avel_range = actual_range + return + body.angular_velocity = (body.angular_velocity / SHIP_AVEL_RANGE) * actual_range + body.avel_range = actual_range diff --git a/Game/scripts/net_debug_overlay.gd b/Game/scripts/net_debug_overlay.gd new file mode 100644 index 00000000..4f5718f8 --- /dev/null +++ b/Game/scripts/net_debug_overlay.gd @@ -0,0 +1,43 @@ +extends CanvasLayer + +# Autoload: toggleable network debug overlay (F4 by default — see +# toggle_net_overlay in project.godot's [input]). Read-only against +# NetworkManager's clock state (task 1.8). Mirrors perf_overlay.gd's pattern +# — headless-guarded, hidden by default, no gameplay-state writes. + +var _label: Label + + +func _ready() -> void: + if DisplayServer.get_name() == "headless": + set_process(false) + return + layer = 100 + _label = Label.new() + _label.add_theme_font_size_override("font_size", 14) + _label.add_theme_color_override("font_color", Color(0.5, 0.8, 1.0)) + _label.add_theme_color_override("font_shadow_color", Color(0, 0, 0, 0.85)) + _label.add_theme_constant_override("shadow_offset_x", 1) + _label.add_theme_constant_override("shadow_offset_y", 1) + _label.position = Vector2(12, 90) + _label.visible = false + add_child(_label) + + +func _unhandled_input(event: InputEvent) -> void: + if event.is_action_pressed("toggle_net_overlay") and _label: + _label.visible = not _label.visible + + +func _process(_delta: float) -> void: + if not _label or not _label.visible: + return + if NetworkManager.is_server: + _label.text = "NET: server, %d peer(s)" % (MatchNet.roster.size()) + elif NetworkManager.is_client: + if NetworkManager.rtt_ms < 0.0: + _label.text = "NET: client, connecting (no clock sample yet)" + else: + _label.text = "NET: client RTT %.1fms clock offset %.1fms" % [NetworkManager.rtt_ms, NetworkManager.clock_offset_ms] + else: + _label.text = "NET: offline" diff --git a/Game/scripts/network_manager.gd b/Game/scripts/network_manager.gd new file mode 100644 index 00000000..074f8e01 --- /dev/null +++ b/Game/scripts/network_manager.gd @@ -0,0 +1,210 @@ +extends Node + +# Autoload (project.godot [autoload] NetworkManager). Owns the ENet +# transport: hosting, joining, shutdown, and connection-state signals. Lives +# at a fixed autoload path so RPC NodePaths never depend on which scene is +# loaded (§1.3 of multiplayer-todo.md's derived decisions). +# +# server_relay = false is set the moment a peer exists: the default `true` +# lets any client rpc() any other client *through the server*, which this +# project's server-authoritative model must never allow — §2.1 calls this +# out as the single highest-value one-line security change in the document. +# +# IMPORTANT, learned the hard way (tests/net_smoke.gd): don't call +# shutdown()/close the peer the instant connected_to_server or peer_connected +# fires. ENet's connect handshake isn't fully settled on the *other* side the +# moment your own side's signal fires — the final ACK still needs a couple +# more poll() cycles to actually reach the wire. Closing immediately drops +# it and leaves the other side's handshake permanently incomplete (it will +# never see peer_connected/connected_to_server at all). Callers that shut +# down right after a fresh connection should let a frame or two pass first. +# +# Manual polling (task 1.3): SceneTree's automatic multiplayer poll runs on +# the *idle* frame, so an rpc() issued from _physics_process waits up to a +# full frame before it's actually pushed onto the wire — and the return leg +# pays the same tax again. set_multiplayer_poll_enabled(false) below turns +# that off; every caller that sends or expects to receive on a tight cadence +# must now call NetworkManager.poll() itself. The intended placement per +# multiplayer-todo.md §7 task 1.3 (client: end of _physics_process after +# sending input, plus top of both _process and _physics_process for receive; +# server: tick start to drain, tick end to flush) has no real per-tick caller +# yet — that lands with the input/snapshot pipeline (tasks 1.4+, Phase 2-3). +# Until then, anything driving a connection (tests/net_smoke.gd included) +# must poll() every frame itself or nothing will ever be sent or received. + +signal client_connected(peer_id: int) +signal client_disconnected(peer_id: int) +signal connected_to_server() +signal connection_failed() +signal disconnected_from_server() +signal clock_updated(rtt_ms: float, offset_ms: float) +# Fires at the top of every shutdown() call, whether this process was +# hosting, joined, or already offline, and regardless of *why* (deliberate +# Leave/Cancel, or an incoming disconnect from the other side). Adversarial +# review found MatchNet.roster had no path that cleared it when a HOST +# stopped hosting — connected_to_server/disconnected_from_server only cover +# the client side — so a host -> lobby -> leave -> host-again cycle left a +# permanent phantom player. Listeners that need per-role cleanup should +# still use the more specific signals above; this one exists so "something +# is about to reset the connection, drop anything you were keeping" has +# exactly one place to hook regardless of role. +signal shutting_down() + +const DEFAULT_PORT := 7777 +const MAX_CLIENTS := 32 + +# Clock (task 1.8, §4.7): client pings the server once a second on the +# reliable control channel; clock_offset_ms is the min-RTT sample in a +# rolling window, because the lowest-RTT sample has the least queueing +# error. get_server_time_estimate_ms() is the thing every later phase +# (interpolation delay, tick_offset seeding) actually wants — everything +# else here exists to produce it. +const PING_INTERVAL_SEC := 1.0 +const CLOCK_WINDOW_SEC := 5.0 + +var is_server := false +var is_client := false +var _peer: ENetMultiplayerPeer # keep a strong ref alongside multiplayer.multiplayer_peer + +var rtt_ms := -1.0 # min-RTT sample currently in the window; -1 = no sample yet +var clock_offset_ms := 0.0 # add to a local Time.get_ticks_msec() reading to estimate the server's clock +var _clock_samples: Array[Dictionary] = [] +var _ping_accum_sec := 0.0 + + +func _ready() -> void: + get_tree().set_multiplayer_poll_enabled(false) + multiplayer.peer_connected.connect(_on_peer_connected) + multiplayer.peer_disconnected.connect(_on_peer_disconnected) + multiplayer.connected_to_server.connect(_on_connected_to_server) + multiplayer.connection_failed.connect(_on_connection_failed) + multiplayer.server_disconnected.connect(_on_server_disconnected) + + +func _process(delta: float) -> void: + # is_client turns true the instant join() is called, before the ENet + # handshake actually completes (or fails) — a slow or refused connect + # attempt would otherwise leave this trying to rpc_id() on a peer + # that's still CONNECTING (or already failed), which Godot logs as + # "Trying to call an RPC via a multiplayer peer which is not + # connected." every single frame. Require the real transport state. + if not is_client or _peer == null or _peer.get_connection_status() != MultiplayerPeer.CONNECTION_CONNECTED: + return + _ping_accum_sec += delta + if _ping_accum_sec >= PING_INTERVAL_SEC: + _ping_accum_sec = 0.0 + _ping.rpc_id(1, Time.get_ticks_msec()) + + +# Estimate of what the server's Time.get_ticks_msec() reads right now. +# Meaningless before the first pong lands (clock_offset_ms is 0.0 until then +# — callers needing round-trip-confirmed freshness should check rtt_ms >= 0). +func get_server_time_estimate_ms() -> float: + return float(Time.get_ticks_msec()) + clock_offset_ms + + +# The single entry point every per-tick caller uses instead of relying on +# SceneTree's (now disabled) automatic poll. Safe to call with no peer set — +# polling the default OfflineMultiplayerPeer is a no-op. +func poll() -> void: + multiplayer.poll() + + +func host(port: int = DEFAULT_PORT, max_clients: int = MAX_CLIENTS) -> Error: + shutdown() + var peer := ENetMultiplayerPeer.new() + var err := peer.create_server(port, max_clients) + if err != OK: + push_error("NetworkManager.host: create_server failed (%s)" % error_string(err)) + return err + _peer = peer + multiplayer.multiplayer_peer = peer + multiplayer.server_relay = false + is_server = true + is_client = false + return OK + + +func join(address: String, port: int = DEFAULT_PORT) -> Error: + shutdown() + var peer := ENetMultiplayerPeer.new() + var err := peer.create_client(address, port) + if err != OK: + push_error("NetworkManager.join: create_client failed (%s)" % error_string(err)) + return err + _peer = peer + multiplayer.multiplayer_peer = peer + multiplayer.server_relay = false + is_server = false + is_client = true + return OK + + +func shutdown() -> void: + shutting_down.emit() + # MultiplayerAPI's default multiplayer_peer is an OfflineMultiplayerPeer + # sentinel, never null — closing that sentinel is a no-op, but assigning + # multiplayer_peer = null (rather than a fresh OfflineMultiplayerPeer) + # leaves the API in a state distinct from its own default, which is a + # known source of confusing follow-on bugs (godotengine/godot#81540). + # Always reset to a real OfflineMultiplayerPeer, never raw null. + var peer := multiplayer.multiplayer_peer + if peer != null and not (peer is OfflineMultiplayerPeer): + peer.close() + multiplayer.multiplayer_peer = OfflineMultiplayerPeer.new() + _peer = null + is_server = false + is_client = false + rtt_ms = -1.0 + clock_offset_ms = 0.0 + _clock_samples.clear() + _ping_accum_sec = 0.0 + + +@rpc("any_peer", "call_remote", "reliable") +func _ping(client_send_ms: int) -> void: + if not multiplayer.is_server(): + return + _pong.rpc_id(multiplayer.get_remote_sender_id(), client_send_ms, Time.get_ticks_msec()) + + +@rpc("authority", "call_remote", "reliable") +func _pong(client_send_ms: int, server_now_ms: int) -> void: + var now_ms := Time.get_ticks_msec() + var sample_rtt := float(now_ms - client_send_ms) + var sample_offset := float(server_now_ms) + sample_rtt / 2.0 - float(now_ms) + _clock_samples.append({"t": now_ms, "rtt": sample_rtt, "offset": sample_offset}) + + var cutoff := now_ms - int(CLOCK_WINDOW_SEC * 1000.0) + _clock_samples = _clock_samples.filter(func(s: Dictionary) -> bool: return s["t"] >= cutoff) + + var best: Dictionary = _clock_samples[0] + for sample: Dictionary in _clock_samples: + if sample["rtt"] < best["rtt"]: + best = sample + rtt_ms = best["rtt"] + clock_offset_ms = best["offset"] + clock_updated.emit(rtt_ms, clock_offset_ms) + + +func _on_peer_connected(peer_id: int) -> void: + client_connected.emit(peer_id) + + +func _on_peer_disconnected(peer_id: int) -> void: + client_disconnected.emit(peer_id) + + +func _on_connected_to_server() -> void: + connected_to_server.emit() + + +func _on_connection_failed() -> void: + is_client = false + connection_failed.emit() + + +func _on_server_disconnected() -> void: + is_server = false + is_client = false + disconnected_from_server.emit() diff --git a/Game/scripts/server_boot.gd b/Game/scripts/server_boot.gd new file mode 100644 index 00000000..71a17fd1 --- /dev/null +++ b/Game/scripts/server_boot.gd @@ -0,0 +1,98 @@ +extends Node + +# Headless dedicated server entry point (task 1.6). Parses CLI args, hosts +# via NetworkManager, logs structured lines, and watches for physics-tick +# overrun (§9 gotcha 9: Engine.max_physics_steps_per_frame defaults to 8; +# a tick overrunning 16.7ms backs up the accumulator and the next frame +# runs multiple ticks, spiking CPU further — worth logging, not just +# silently absorbing). +# +# Run: godot --headless --path Game res://scenes/server_boot.tscn -- --port=7777 +# +# 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. + +const LOG_LEVELS := {"debug": 0, "info": 1, "warn": 2, "error": 3} + +var _boot_ms := 0 +var _last_physics_frame := 0 +var _log_level := 1 # info +var _watchdog_armed := false # skip the first _process(): engine startup scheduling can batch several physics frames before the first idle frame runs, which isn't a real overrun + + +func _ready() -> void: + _boot_ms = Time.get_ticks_msec() + Engine.max_fps = 60 # a server never renders; this just caps the idle-frame poll rate so it doesn't spin + + var port := NetworkManager.DEFAULT_PORT + var max_clients := NetworkManager.MAX_CLIENTS + for arg in OS.get_cmdline_user_args(): + if arg.begins_with("--port="): + port = int(arg.substr("--port=".length())) + elif arg.begins_with("--max-clients="): + max_clients = int(arg.substr("--max-clients=".length())) + elif arg.begins_with("--log-level="): + var level_name := arg.substr("--log-level=".length()) + if LOG_LEVELS.has(level_name): + _log_level = LOG_LEVELS[level_name] + else: + _log("error", "bad_log_level", {"given": level_name, "valid": LOG_LEVELS.keys()}) + get_tree().quit(1) + return + + NetworkManager.client_connected.connect(_on_client_connected) + NetworkManager.client_disconnected.connect(_on_client_disconnected) + MatchNet.player_joined.connect(_on_player_joined) + MatchNet.player_left.connect(_on_player_left) + + var err := NetworkManager.host(port, max_clients) + if err != OK: + _log("error", "server_boot_failed", {"port": port, "error": error_string(err)}) + get_tree().quit(1) + return + _log("info", "server_started", {"port": port, "max_clients": max_clients}) + _last_physics_frame = Engine.get_physics_frames() + + +func _process(_delta: float) -> void: + NetworkManager.poll() + var current := Engine.get_physics_frames() + var steps := current - _last_physics_frame + _last_physics_frame = current + # §9 gotcha 6: with physics_jitter_fix = 0.0, frames legitimately + # alternate between 0 and 2 ticks even on an idle, healthy server — + # that's expected quantisation, not backlog. A real overrun is the + # accumulator failing to drain back down, i.e. 3+ ticks in one frame. + if steps > 2 and _watchdog_armed: + _log("warn", "physics_overrun", {"steps": steps}) + _watchdog_armed = true + + +func _physics_process(_delta: float) -> void: + NetworkManager.poll() + + +func _on_client_connected(peer_id: int) -> void: + _log("debug", "peer_connected", {"peer_id": peer_id}) + + +func _on_client_disconnected(peer_id: int) -> void: + _log("debug", "peer_disconnected", {"peer_id": peer_id}) + + +func _on_player_joined(peer_id: int, player_name: String) -> void: + _log("info", "player_joined", {"peer_id": peer_id, "name": player_name}) + + +func _on_player_left(peer_id: int) -> void: + _log("info", "player_left", {"peer_id": peer_id}) + + +func _log(level: String, event: String, fields: Dictionary) -> void: + if LOG_LEVELS.get(level, 1) < _log_level: + return + var parts := PackedStringArray() + for key in fields: + parts.append("%s=%s" % [key, str(fields[key])]) + var elapsed_sec := (Time.get_ticks_msec() - _boot_ms) / 1000.0 + print("[%.3f] %s %s %s" % [elapsed_sec, level.to_upper(), event, " ".join(parts)]) diff --git a/Game/tests/cases/test_match_net.gd b/Game/tests/cases/test_match_net.gd new file mode 100644 index 00000000..e9ad2154 --- /dev/null +++ b/Game/tests/cases/test_match_net.gd @@ -0,0 +1,39 @@ +extends "res://tests/test_case.gd" + +const MatchNet = preload("res://scripts/match_net.gd") + +# Adversarial-review regression: _hello's player_name used to be broadcast +# to every peer completely unvalidated — a multi-MB name head-of-line- +# blocked the reliable control channel hard enough that a concurrently- +# joining client's own _welcome never arrived. _sanitize_player_name() is +# the fix; these are pure-function tests for it, independent of the live +# two-process rejection test in tests/match_net_smoke.gd (--role=client-longname). + +func test_normal_name_unchanged() -> void: + assert_eq(MatchNet._sanitize_player_name("Alice"), "Alice", "a normal name passes through unchanged") + + +func test_strips_control_characters() -> void: + var bell := String.chr(7) # a control char with no named GDScript escape + var raw := "Bad\nName\twith\rcontrol" + bell + "chars" + var clean := MatchNet._sanitize_player_name(raw) + assert_true(not clean.contains("\n"), "no newline") + assert_true(not clean.contains("\t"), "no tab") + assert_true(not clean.contains("\r"), "no carriage return") + assert_true(not clean.contains(bell), "no bell/control char") + + +func test_clamps_to_max_display_length() -> void: + var raw := "X".repeat(1000) + var clean := MatchNet._sanitize_player_name(raw) + assert_eq(clean.length(), MatchNet.MAX_PLAYER_NAME_LENGTH, "clamped to MAX_PLAYER_NAME_LENGTH") + + +func test_empty_or_whitespace_only_falls_back_to_default() -> void: + assert_eq(MatchNet._sanitize_player_name(""), "Player", "empty string falls back") + assert_eq(MatchNet._sanitize_player_name(" "), "Player", "whitespace-only falls back") + assert_eq(MatchNet._sanitize_player_name("\n\t\r"), "Player", "control-characters-only falls back") + + +func test_leading_trailing_whitespace_trimmed() -> void: + assert_eq(MatchNet._sanitize_player_name(" Bob "), "Bob", "surrounding whitespace trimmed") diff --git a/Game/tests/cases/test_net_codec.gd b/Game/tests/cases/test_net_codec.gd new file mode 100644 index 00000000..9f0bb264 --- /dev/null +++ b/Game/tests/cases/test_net_codec.gd @@ -0,0 +1,169 @@ +extends "res://tests/test_case.gd" + +const NetCodec = preload("res://scripts/net_codec.gd") +const ShipAction = preload("res://scripts/ship_action.gd") +const NetBodyState = preload("res://scripts/net_body_state.gd") + +const POS_TOL := 0.01 # well under the ~1.95mm quantisation step's rounding +const VEL_TOL := 0.01 +const QUAT_TOL := 0.001 +const AVEL_TOL := 0.2 # BALL_AVEL_RANGE/127 half-step, scaled through the ship->ball rescale +const THRUST_TOL := 1.0 / 127.0 + 0.001 + + +func _make_action(tx: float, ty: float, tz: float, rx: float, ry: float, rz: float, turbo: bool) -> ShipAction: + var a := ShipAction.new() + a.thrust = Vector3(tx, ty, tz) + a.rotation = Vector3(rx, ry, rz) + a.turbo = turbo + return a + + +func test_input_header_size_matches_spec() -> void: + assert_eq(NetCodec.INPUT_HEADER_SIZE, 12, "input header size") + assert_eq(NetCodec.INPUT_ENTRY_SIZE, 7, "input entry size") + + +func test_input_roundtrip_single_entry() -> void: + var actions := [_make_action(1.0, -1.0, 0.5, -0.25, 0.0, 1.0, true)] + var bytes := NetCodec.pack_input(12345, 999, 6000, actions) + assert_eq(bytes.size(), NetCodec.INPUT_HEADER_SIZE + NetCodec.INPUT_ENTRY_SIZE, "1-entry payload size") + + var decoded := NetCodec.unpack_input(bytes) + assert_eq(decoded["seq"], 12345, "seq") + assert_eq(decoded["count"], 1, "count") + assert_eq(decoded["ack_snapshot_tick"], 999, "ack_snapshot_tick") + assert_eq(decoded["client_send_ms"], 6000, "client_send_ms") + + var a: ShipAction = decoded["actions"][0] + assert_almost_eq(a.thrust.x, 1.0, THRUST_TOL, "thrust.x") + assert_almost_eq(a.thrust.y, -1.0, THRUST_TOL, "thrust.y") + assert_almost_eq(a.thrust.z, 0.5, THRUST_TOL, "thrust.z") + assert_almost_eq(a.rotation.x, -0.25, THRUST_TOL, "rotation.x") + assert_almost_eq(a.rotation.y, 0.0, THRUST_TOL, "rotation.y") + assert_almost_eq(a.rotation.z, 1.0, THRUST_TOL, "rotation.z") + assert_true(a.turbo, "turbo bit") + + +func test_input_roundtrip_max_redundancy_newest_first() -> void: + var actions := [ + _make_action(1.0, 0.0, 0.0, 0.0, 0.0, 0.0, false), + _make_action(0.5, 0.0, 0.0, 0.0, 0.0, 0.0, false), + _make_action(0.0, 0.0, 0.0, 0.0, 0.0, 0.0, false), + _make_action(-1.0, 0.0, 0.0, 0.0, 0.0, 0.0, true), + ] + var bytes := NetCodec.pack_input(1, 0, 0, actions) + assert_eq(bytes.size(), NetCodec.INPUT_HEADER_SIZE + 4 * NetCodec.INPUT_ENTRY_SIZE, "4-entry payload size") + + var decoded := NetCodec.unpack_input(bytes) + assert_eq(decoded["count"], 4, "count") + var decoded_actions: Array = decoded["actions"] + assert_almost_eq(decoded_actions[0].thrust.x, 1.0, THRUST_TOL, "entry 0 (newest) thrust.x") + assert_almost_eq(decoded_actions[3].thrust.x, -1.0, THRUST_TOL, "entry 3 (oldest) thrust.x") + assert_true(decoded_actions[3].turbo, "entry 3 turbo bit") + assert_true(not decoded_actions[0].turbo, "entry 0 turbo bit unset") + + +func test_input_redundancy_clamped_to_max() -> void: + var actions := [] + for i in 6: + actions.append(_make_action(0.0, 0.0, 0.0, 0.0, 0.0, 0.0, false)) + var bytes := NetCodec.pack_input(1, 0, 0, actions) + assert_eq(bytes.size(), NetCodec.INPUT_HEADER_SIZE + NetCodec.MAX_REDUNDANCY * NetCodec.INPUT_ENTRY_SIZE, "clamped to MAX_REDUNDANCY entries") + + +func test_snapshot_sizes_match_spec() -> void: + assert_eq(NetCodec.SNAPSHOT_CLIENT_HEADER_SIZE, 7, "client header size") + assert_eq(NetCodec.SNAPSHOT_BODY_HEADER_SIZE, 8, "body header size") + assert_eq(NetCodec.SNAPSHOT_BODY_SIZE, 22, "per-body size") + + +func test_snapshot_roundtrip_seven_bodies() -> void: + var bodies: Array[NetBodyState] = [] + for i in 7: + var b := NetBodyState.new() + b.position = Vector3(float(i) * 3.0 - 10.0, 1.0, -float(i) * 2.0) + b.rotation = Quaternion(Vector3.UP, float(i) * 0.3) + b.linear_velocity = Vector3(float(i), 0.0, -float(i) * 0.5) + b.angular_velocity = Vector3(0.1 * i, 0.0, 0.0) + b.frozen = (i % 2 == 0) + b.turbo = (i == 3) + b.thrust_z = -1.0 + 2.0 * float(i) / 6.0 + b.stalled = (i == 5) + bodies.append(b) + + var segment := NetCodec.pack_snapshot_body_segment(4242, 3, 7, bodies) + assert_eq(segment.size(), NetCodec.SNAPSHOT_BODY_HEADER_SIZE + 7 * NetCodec.SNAPSHOT_BODY_SIZE, "7-body segment size") + + var packet := NetCodec.pack_snapshot(555, -2, 1234, segment) + assert_eq(packet.size(), NetCodec.SNAPSHOT_CLIENT_HEADER_SIZE + segment.size(), "full packet size") + assert_eq(packet.size(), 169, "matches multiplayer-todo.md §2.4's 169 B payload figure for 7 bodies") + + var decoded := NetCodec.unpack_snapshot(packet) + assert_eq(decoded["last_input_seq"], 555, "last_input_seq") + assert_eq(decoded["input_buffer_depth"], -2, "input_buffer_depth (negative = starved)") + assert_eq(decoded["echo_client_send_ms"], 1234, "echo_client_send_ms") + assert_eq(decoded["server_tick"], 4242, "server_tick") + assert_eq(decoded["match_state"], 3, "match_state") + assert_eq(decoded["reset_gen"], 7, "reset_gen") + + var decoded_bodies: Array = decoded["bodies"] + assert_eq(decoded_bodies.size(), 7, "body_count") + for i in 7: + var original: NetBodyState = bodies[i] + var b: NetBodyState = decoded_bodies[i] + assert_almost_eq(b.position.x, original.position.x, POS_TOL, "body %d position.x" % i) + assert_almost_eq(b.position.y, original.position.y, POS_TOL, "body %d position.y" % i) + assert_almost_eq(b.position.z, original.position.z, POS_TOL, "body %d position.z" % i) + assert_almost_eq(b.linear_velocity.x, original.linear_velocity.x, VEL_TOL, "body %d velocity.x" % i) + assert_almost_eq(b.rotation.x, original.rotation.x, QUAT_TOL, "body %d quat.x" % i) + assert_almost_eq(b.rotation.y, original.rotation.y, QUAT_TOL, "body %d quat.y" % i) + assert_almost_eq(b.rotation.z, original.rotation.z, QUAT_TOL, "body %d quat.z" % i) + assert_almost_eq(b.rotation.w, original.rotation.w, QUAT_TOL, "body %d quat.w (sign fold)" % i) + assert_eq(b.frozen, original.frozen, "body %d frozen" % i) + assert_eq(b.turbo, original.turbo, "body %d turbo" % i) + assert_eq(b.stalled, original.stalled, "body %d stalled" % i) + + +func test_snapshot_quaternion_negative_w_sign_survives() -> void: + # A quaternion whose w component is negative (same rotation as its + # positive-w twin, but exercises the sign-fold bit specifically). + var b := NetBodyState.new() + b.rotation = Quaternion(0.0, 0.0, 0.0, -1.0).normalized() + var segment := NetCodec.pack_snapshot_body_segment(0, 0, 0, [b]) + var decoded := NetCodec.unpack_snapshot(NetCodec.pack_snapshot(0, 0, 0, segment)) + var out: NetBodyState = decoded["bodies"][0] + assert_true(out.rotation.w < 0.0, "negative w sign must survive the round trip") + + +func test_thrust_z_bin_quantisation_covers_range() -> void: + assert_eq(NetCodec.quantize_thrust_z_bin(-1.0), 0, "thrust_z -1.0 -> bin 0") + assert_eq(NetCodec.quantize_thrust_z_bin(1.0), NetCodec.THRUST_Z_BIN_MAX, "thrust_z 1.0 -> max bin") + assert_almost_eq(NetCodec.dequantize_thrust_z_bin(0), -1.0, 0.001, "bin 0 -> -1.0") + assert_almost_eq(NetCodec.dequantize_thrust_z_bin(NetCodec.THRUST_Z_BIN_MAX), 1.0, 0.001, "max bin -> 1.0") + + +func test_ball_angular_velocity_rescale() -> void: + var ball := NetBodyState.new() + ball.avel_range = NetCodec.BALL_AVEL_RANGE + ball.angular_velocity = Vector3(20.0, -15.0, 5.0) # within ±32 rad/s, outside ship's ±4 + var segment := NetCodec.pack_snapshot_body_segment(0, 0, 0, [ball]) + var decoded := NetCodec.unpack_snapshot(NetCodec.pack_snapshot(0, 0, 0, segment)) + var out: NetBodyState = decoded["bodies"][0] + # Decoded at the wrong (ship) range first, per unpack_snapshot's documented contract. + assert_almost_eq(out.angular_velocity.x, 20.0 / NetCodec.BALL_AVEL_RANGE * NetCodec.SHIP_AVEL_RANGE, AVEL_TOL, "undecoded-scale sanity check") + NetCodec.rescale_avel(out, NetCodec.BALL_AVEL_RANGE) + assert_almost_eq(out.angular_velocity.x, 20.0, AVEL_TOL, "rescaled avel.x") + assert_almost_eq(out.angular_velocity.y, -15.0, AVEL_TOL, "rescaled avel.y") + assert_almost_eq(out.angular_velocity.z, 5.0, AVEL_TOL, "rescaled avel.z") + + +func test_type_version_byte_roundtrip() -> void: + var tv := NetCodec.type_version_byte(NetCodec.PacketType.SNAPSHOT) + assert_eq(NetCodec.packet_type_of(tv), NetCodec.PacketType.SNAPSHOT, "packet type nibble") + assert_eq(NetCodec.protocol_version_of(tv), NetCodec.PROTOCOL_VERSION, "protocol version nibble") + + +func test_tick_hz_derives_from_sim_constants() -> void: + var SimConstants = preload("res://scripts/sim_constants.gd") + assert_eq(NetCodec.TICK_HZ, SimConstants.TICK_HZ, "NetCodec.TICK_HZ must track SimConstants.TICK_HZ") diff --git a/Game/tests/cases/test_smoke.gd b/Game/tests/cases/test_smoke.gd new file mode 100644 index 00000000..84d0ade7 --- /dev/null +++ b/Game/tests/cases/test_smoke.gd @@ -0,0 +1,12 @@ +extends "res://tests/test_case.gd" + +# Proves the runner itself works: discovery, dispatch, pass/fail aggregation. + +func test_true_is_true() -> void: + assert_true(true, "true should be true") + +func test_addition() -> void: + assert_eq(2 + 2, 4, "2 + 2") + +func test_almost_eq_tolerance() -> void: + assert_almost_eq(1.0001, 1.0, 0.001, "1.0001 within 0.001 of 1.0") diff --git a/Game/tests/clock_smoke.gd b/Game/tests/clock_smoke.gd new file mode 100644 index 00000000..0f310499 --- /dev/null +++ b/Game/tests/clock_smoke.gd @@ -0,0 +1,148 @@ +extends Node + +# Manual two-process smoke test for NetworkManager's clock (task 1.8 +# acceptance: "offset converges within 2s and stays within ±1 tick on a +# clean link"). Not part of tests/test_runner.tscn — needs real ENet peers +# and real wall-clock ping/pong cadence. Run: +# +# godot --headless --path Game res://tests/clock_smoke.tscn -- --role=host +# godot --headless --path Game res://tests/clock_smoke.tscn -- --role=client +# +# Adversarial-review regression: the original version only checked that +# later samples agreed with the first one (self-consistency) — a +# consistently-wrong offset (e.g. a missing /2 on RTT, or a sign flip) +# would converge just as cleanly and still pass. Both roles now also write/ +# read an independent ground truth: each process's own OS wall-clock +# (Time.get_unix_time_from_system(), shared hardware clock, same machine) +# lets it compute "my Time.get_ticks_msec() minus real epoch time" — the +# TRUE required offset is just the difference of those two numbers between +# host and client, computed via a shared temp file since the two processes +# can't otherwise see each other's local variables. This is independent of +# NetworkManager's own ping/pong math entirely. + +const PORT := 7801 +const RUN_SECONDS := 6.0 +const CONVERGE_BY_SEC := 2.0 +const TICK_MS := 1000.0 / 60.0 # SimConstants.TICK_HZ, kept literal to avoid pulling in the whole project for one constant in a throwaway diagnostic +const EPOCH_FILE := "/tmp/cosmicclash_clock_smoke_epoch_offset.txt" +# Ground-truth tolerance is looser than the ±1-tick self-consistency check: +# Time.get_unix_time_from_system() itself is only second-resolution on some +# platforms and the two processes sample it at slightly different instants, +# so this bounds "is the offset even the right ballpark and sign" rather +# than chasing sub-tick precision the way the self-consistency check does. +const GROUND_TRUTH_TOLERANCE_MS := 250.0 + +var _role := "" +var _start_ms := 0 +var _samples: Array[Dictionary] = [] # {t_sec, offset} +var _finished := false + + +func _epoch_offset_ms() -> float: + return Time.get_unix_time_from_system() * 1000.0 - float(Time.get_ticks_msec()) + + +func _ready() -> void: + for arg in OS.get_cmdline_user_args(): + if arg.begins_with("--role="): + _role = arg.substr("--role=".length()) + + match _role: + "host": + var err := NetworkManager.host(PORT) + if err != OK: + _finish(false, "host() failed: %s" % error_string(err)) + return + var f := FileAccess.open(EPOCH_FILE, FileAccess.WRITE) + if f: + f.store_string(str(_epoch_offset_ms())) + f.close() + print("SMOKE: hosting on port %d" % PORT) + "client": + NetworkManager.clock_updated.connect(_on_clock_updated) + var err := NetworkManager.join("127.0.0.1", PORT) + if err != OK: + _finish(false, "join() failed: %s" % error_string(err)) + return + print("SMOKE: joining ...") + _: + _finish(false, "missing or unrecognised --role=") + return + + _start_ms = Time.get_ticks_msec() + get_tree().create_timer(RUN_SECONDS).timeout.connect(_on_run_complete) + + +func _process(_delta: float) -> void: + NetworkManager.poll() + + +func _physics_process(_delta: float) -> void: + NetworkManager.poll() + + +func _on_clock_updated(rtt_ms: float, offset_ms: float) -> void: + var t_sec := float(Time.get_ticks_msec() - _start_ms) / 1000.0 + _samples.append({"t_sec": t_sec, "offset": offset_ms}) + print("SMOKE clock sample t=%.2fs rtt=%.2fms offset=%.2fms" % [t_sec, rtt_ms, offset_ms]) + + +func _on_run_complete() -> void: + if _role != "client": + _finish(true, "host ran for %.1fs" % RUN_SECONDS) + return + + if _samples.is_empty(): + _finish(false, "no clock samples received at all") + return + + var converged_sample: Dictionary = {} + for s: Dictionary in _samples: + if s["t_sec"] <= CONVERGE_BY_SEC: + converged_sample = s + if converged_sample.is_empty(): + _finish(false, "no sample landed by t=%.1fs (first sample at t=%.2fs)" % [CONVERGE_BY_SEC, _samples[0]["t_sec"]]) + return + + var reference: float = converged_sample["offset"] + var max_drift := 0.0 + for s: Dictionary in _samples: + if s["t_sec"] < CONVERGE_BY_SEC: + continue + max_drift = maxf(max_drift, absf(s["offset"] - reference)) + + if max_drift > TICK_MS: + _finish(false, "offset drifted %.2fms after t=%.1fs (> 1 tick = %.2fms)" % [max_drift, CONVERGE_BY_SEC, TICK_MS]) + return + + # Self-consistency alone can't catch a systematically-wrong-but-stable + # offset (§9 gotcha, adversarial review) — cross-check against the OS + # wall clock, independent of NetworkManager's own math entirely. + if not FileAccess.file_exists(EPOCH_FILE): + _finish(false, "converged (%.2fms, drift %.2fms) but host's epoch-offset file was never found — ground truth unavailable" % [reference, max_drift]) + return + var f := FileAccess.open(EPOCH_FILE, FileAccess.READ) + var server_epoch_offset := f.get_as_text().to_float() + f.close() + var client_epoch_offset := _epoch_offset_ms() + var true_offset := client_epoch_offset - server_epoch_offset + var ground_truth_error := absf(reference - true_offset) + + if ground_truth_error > GROUND_TRUTH_TOLERANCE_MS: + _finish(false, "converged (%.2fms) but disagrees with OS-clock ground truth (%.2fms) by %.2fms (> %.1fms tolerance) — the offset math itself may be wrong, not just noisy" % [ + reference, true_offset, ground_truth_error, GROUND_TRUTH_TOLERANCE_MS + ]) + else: + _finish(true, "offset converged by t=%.1fs (%.2fms), stayed within %.2fms (<= 1 tick = %.2fms) for the rest of the run across %d samples, AND agrees with independent OS-clock ground truth (%.2fms, error %.2fms <= %.1fms tolerance)" % [ + CONVERGE_BY_SEC, reference, max_drift, TICK_MS, _samples.size(), true_offset, ground_truth_error, GROUND_TRUTH_TOLERANCE_MS + ]) + + +func _finish(success: bool, message: String) -> void: + if _finished: + return + _finished = true + print("SMOKE %s: %s" % ["PASS" if success else "FAIL", message]) + await get_tree().create_timer(0.3).timeout + NetworkManager.shutdown() + get_tree().quit(0 if success else 1) diff --git a/Game/tests/clock_smoke.tscn b/Game/tests/clock_smoke.tscn new file mode 100644 index 00000000..4334ca59 --- /dev/null +++ b/Game/tests/clock_smoke.tscn @@ -0,0 +1,6 @@ +[gd_scene load_steps=2 format=3] + +[ext_resource type="Script" path="res://tests/clock_smoke.gd" id="1_cs"] + +[node name="ClockSmoke" type="Node"] +script = ExtResource("1_cs") diff --git a/Game/tests/lobby_smoke.gd b/Game/tests/lobby_smoke.gd new file mode 100644 index 00000000..268e5945 --- /dev/null +++ b/Game/tests/lobby_smoke.gd @@ -0,0 +1,79 @@ +extends Node + +# Manual two-process smoke test for lobby.tscn (task 1.5). BOTH roles load +# lobby.tscn as their actual current_scene via change_scene_to_file — +# matching how main_menu.gd's Host/Join flow (task 1.7) really gets a +# player there — rather than instantiating it as a child of this driver. +# That distinction matters: change_scene_to_file() operates on +# get_tree().current_scene, and calling it from a node that ISN'T an +# ancestor-chain match for current_scene (as an earlier draft of this test +# did, by add_child()-ing lobby.tscn under this driver) hung completely +# on disconnect — see multiplayer-todo.md §9 gotcha 27. +# +# The host role loading lobby.tscn is deliberate, not an oversight: a +# *dedicated* server (server_boot.tscn) never loads it, but a self-hosting +# player clicking main_menu.gd's Host button does — NetworkManager.host() +# then _leave_to_lobby(), landing them on lobby.gd's is_server branch (a +# read-only view of the roster, no team/ready controls). An earlier +# version of this test skipped that branch entirely on the mistaken +# assumption that "host" here meant "dedicated server"; adversarial review +# caught that it left a real, production-reachable code path untested. +# +# Not part of tests/test_runner.tscn — needs real ENet peers. Run: +# +# godot --headless --path Game res://tests/lobby_smoke.tscn -- --role=host +# godot --headless --path Game res://tests/lobby_smoke.tscn -- --role=client + +const PORT := 7806 + +var _role := "" + + +func _ready() -> void: + for arg in OS.get_cmdline_user_args(): + if arg.begins_with("--role="): + _role = arg.substr("--role=".length()) + + match _role: + "host": + var err := NetworkManager.host(PORT) + if err != OK: + print("SMOKE FAIL: host() failed: %s" % error_string(err)) + get_tree().quit(1) + return + print("SMOKE: hosting on port %d" % PORT) + get_tree().change_scene_to_file.call_deferred("res://scenes/lobby.tscn") + var host_hooks := preload("res://tests/lobby_test_hooks.gd").new() + get_tree().root.add_child.call_deferred(host_hooks) + host_hooks.run_host_test.call_deferred() + "client": + MatchNet.local_player_name = "Carol" + var err := NetworkManager.join("127.0.0.1", PORT) + if err != OK: + print("SMOKE FAIL: join() failed: %s" % error_string(err)) + get_tree().quit(1) + return + # Real usage (task 1.7): main_menu.gd will call this same + # change_scene_to_file after NetworkManager.join() succeeds — but + # not from this test's own _ready(), which the tree is still in + # the middle of processing (Godot rejects a synchronous + # change_scene_to_file mid node-add with "Parent node is busy"). + # call_deferred sidesteps that; main_menu.gd's real button-press + # handler won't have this problem since it isn't called from + # inside _ready(). + get_tree().change_scene_to_file.call_deferred("res://scenes/lobby.tscn") + var hooks := preload("res://tests/lobby_test_hooks.gd").new() + get_tree().root.add_child.call_deferred(hooks) # sibling of current_scene, not a child of it -- survives the swap above + hooks.run_client_test.call_deferred() + _: + print("SMOKE FAIL: missing or unrecognised --role=") + get_tree().quit(1) + return + + +func _process(_delta: float) -> void: + NetworkManager.poll() + + +func _physics_process(_delta: float) -> void: + NetworkManager.poll() diff --git a/Game/tests/lobby_smoke.tscn b/Game/tests/lobby_smoke.tscn new file mode 100644 index 00000000..9c80c4bc --- /dev/null +++ b/Game/tests/lobby_smoke.tscn @@ -0,0 +1,6 @@ +[gd_scene load_steps=2 format=3] + +[ext_resource type="Script" path="res://tests/lobby_smoke.gd" id="1_ls"] + +[node name="LobbySmoke" type="Node"] +script = ExtResource("1_ls") diff --git a/Game/tests/lobby_test_hooks.gd b/Game/tests/lobby_test_hooks.gd new file mode 100644 index 00000000..eed8e050 --- /dev/null +++ b/Game/tests/lobby_test_hooks.gd @@ -0,0 +1,126 @@ +extends Node + +# Test-only helper (tests/lobby_smoke.gd). Not a project autoload — +# production code never references this. Exists because lobby.tscn is +# loaded via change_scene_to_file() in the real flow (matching +# main_menu.gd's future Host/Join UI), which frees whatever node initiated +# the load — a test driver can't keep orchestrating from a node that just +# got freed. The driver instead add_child()s this directly under +# get_tree().root (a sibling of current_scene, not a descendant of it), so +# it survives the scene swap and can drive the check from outside lobby.gd, +# which stays untouched by test concerns. + +signal finished(success: bool, message: String) + +const SETTLE_SECONDS := 3.0 +const AFTER_PRESS_SECONDS := 1.0 +# The host process starts ~1.5s before the client (see the shell +# invocation in both roles' header comments) and the client doesn't finish +# its own SETTLE_SECONDS + AFTER_PRESS_SECONDS flow (plus its own 0.3s +# _finish delay) until roughly 1.5 + 3.0 + 1.0 + 0.3 ≈ 5.8s into the +# host's own timeline. Verifying and quitting the instant the host's own +# checks pass (~2-2.5s in) would drop the connection out from under the +# client mid-flow. Print the result as soon as it's known, but hold the +# actual quit() open past the client's expected finish time. +const MIN_HOST_LIFETIME_SECONDS := 7.0 + + +# Adversarial-review regression: the host role in lobby_smoke.gd used to +# never load lobby.tscn at all, so lobby.gd's is_server branch (the +# read-only view main_menu.gd's own Host button routes a self-hosting +# player into) had never actually run under this task's own test suite — +# only main_menu_test_hooks.gd's separate, non-permanent task-1.7 test had +# exercised it. This closes that gap for good. +func run_host_test() -> void: + var start_ms := Time.get_ticks_msec() + var deadline := start_ms + int((SETTLE_SECONDS + 5.0) * 1000.0) + while MatchNet.roster.is_empty() and Time.get_ticks_msec() < deadline: + await get_tree().process_frame + + var lobby := get_tree().current_scene + if lobby == null or not lobby.has_method("_refresh"): + await _finish_and_quit(false, "current_scene is not the lobby scene", start_ms) + return + if MatchNet.roster.is_empty(): + await _finish_and_quit(false, "no client joined before timeout", start_ms) + return + + # Give _refresh() a beat to process the player_joined signal it just got. + await get_tree().create_timer(0.3).timeout + + var controls_row: Control = lobby.get_node("%ControlsRow") + var team0: VBoxContainer = lobby.get_node("%Team0List") + var team1: VBoxContainer = lobby.get_node("%Team1List") + var status: Label = lobby.get_node("%StatusLabel") + var total_rows := team0.get_child_count() + team1.get_child_count() + + # The server is never a roster member (§1.1 decision 2) — its own + # lobby.gd instance must show a read-only view, no team/ready controls. + var controls_hidden := not controls_row.visible + var rows_match_roster := total_rows == MatchNet.roster.size() + var status_ok := status.text.begins_with("Hosting") + + var success := controls_hidden and rows_match_roster and status_ok + await _finish_and_quit(success, "controls_hidden=%s rows=%d roster=%d status='%s'" % [ + str(controls_hidden), total_rows, MatchNet.roster.size(), status.text + ], start_ms) + + +func _finish_and_quit(success: bool, message: String, start_ms: int) -> void: + finished.emit(success, message) + print("SMOKE %s: %s" % ["PASS" if success else "FAIL", message]) + var elapsed_sec := float(Time.get_ticks_msec() - start_ms) / 1000.0 + var remaining := MIN_HOST_LIFETIME_SECONDS - elapsed_sec + if remaining > 0.0: + await get_tree().create_timer(remaining).timeout + NetworkManager.shutdown() + get_tree().quit(0 if success else 1) + + +func run_client_test() -> void: + await get_tree().create_timer(SETTLE_SECONDS).timeout + + var lobby := get_tree().current_scene + if lobby == null or not lobby.has_method("_refresh"): + finished.emit(false, "current_scene is not the lobby scene") + return + + var switch_btn: Button = lobby.get_node("%SwitchTeamButton") + var ready_btn: CheckButton = lobby.get_node("%ReadyButton") + switch_btn.emit_signal("pressed") + ready_btn.button_pressed = true + ready_btn.emit_signal("toggled", true) + await get_tree().create_timer(AFTER_PRESS_SECONDS).timeout + + var team0: VBoxContainer = lobby.get_node("%Team0List") + var team1: VBoxContainer = lobby.get_node("%Team1List") + var status: Label = lobby.get_node("%StatusLabel") + var total_rows := team0.get_child_count() + team1.get_child_count() + + var my_id := multiplayer.get_unique_id() + var info: MatchNet.PlayerInfo = MatchNet.roster.get(my_id) + var info_str := "roster_size=%d team0_rows=%d team1_rows=%d status='%s'" % [ + MatchNet.roster.size(), team0.get_child_count(), team1.get_child_count(), status.text + ] + var team_ready_ok := false + if info != null: + info_str += " my_team=%d my_ready=%s" % [info.team, str(info.ready)] + # Started on whatever team balancing picked (0, since first + # joiner), pressed Switch Team once -> should now be on team 1, + # and pressed Ready -> should be true. + team_ready_ok = info.team == 1 and info.ready == true + print("SMOKE INFO: " + info_str) + + var rows_match_roster := total_rows == MatchNet.roster.size() + var success := rows_match_roster and team_ready_ok + var message := "rows=%d roster=%d team_ready_ok=%s" % [total_rows, MatchNet.roster.size(), str(team_ready_ok)] + finished.emit(success, message) + + # The driver that called run_client_test() is gone by now — it was + # get_tree().current_scene before the change_scene_to_file() that + # loaded the lobby, so it got freed in the swap, taking its signal + # connection to `finished` down with it (Godot auto-disconnects when + # either end of a connection is freed). This autoload outlives that + # swap, so it's what actually ends the process. + print("SMOKE %s: %s" % ["PASS" if success else "FAIL", message]) + get_tree().quit(0 if success else 1) diff --git a/Game/tests/main_menu_test_hooks.gd b/Game/tests/main_menu_test_hooks.gd new file mode 100644 index 00000000..e0744207 --- /dev/null +++ b/Game/tests/main_menu_test_hooks.gd @@ -0,0 +1,114 @@ +extends Node + +# Test-only helper (tests/main_menu_smoke.gd). Not referenced by production +# code. Registered as a temporary project autoload only while running this +# test — see the test's own header for why an autoload (rather than a +# scene-child driver) is needed: main_menu.tscn IS the real current_scene +# here (run directly via --path Game res://scenes/main_menu.tscn, exactly +# like production), so unlike lobby_smoke.gd's driver this doesn't even +# need to survive a scene swap — it just needs to exist independently of +# main_menu.gd so main_menu.gd itself stays untouched by test concerns. + +var _role := "" + + +func _ready() -> void: + for arg in OS.get_cmdline_user_args(): + if arg.begins_with("--role="): + _role = arg.substr("--role=".length()) + if _role.is_empty(): + return + # main_menu.gd's own _ready() (which builds %-unique-name refs) must run + # before we touch its nodes; autoloads run first, so wait a frame. + await get_tree().process_frame + await get_tree().process_frame + match _role: + "host": + _run_host() + "join_ok": + _run_join_ok() + "join_refused": + _run_join_refused() + "join_cancel": + _run_join_cancel() + + +func _run_host() -> void: + var menu := get_tree().current_scene + var host_btn: Button = menu.get_node("CenterContainer/VBoxContainer/HostButton") + host_btn.emit_signal("pressed") + await get_tree().create_timer(1.0).timeout + var scene := get_tree().current_scene + var ok := scene != null and scene.scene_file_path == "res://scenes/lobby.tscn" + print("SMOKE %s: host -> current_scene=%s" % ["PASS" if ok else "FAIL", scene.scene_file_path if scene else "null"]) + if not ok: + _finish(false, "host transition check failed") + return + # Stay up long enough for a separate join_ok/join_refused/join_cancel + # process (started after this one) to actually exercise the host — + # unlike _finish()'s normal 0.3s beat, this test's whole point is being + # a live target for a while. + await get_tree().create_timer(6.0).timeout + _finish(true, "host ran and stayed up for a joiner") + + +func _run_join_ok() -> void: + # Give the host process (started first by the shell script) time to be listening. + await get_tree().create_timer(1.5).timeout + var menu := get_tree().current_scene + var address_edit: LineEdit = menu.get_node("%JoinAddressEdit") + address_edit.text = "127.0.0.1" + var join_btn: Button = menu.get_node("CenterContainer/VBoxContainer/JoinRow/JoinButton") + join_btn.emit_signal("pressed") + var overlay: Control = menu.get_node("%ConnectingOverlay") + print("SMOKE INFO: overlay visible right after Join press = %s" % str(overlay.visible)) + await get_tree().create_timer(2.0).timeout + var scene := get_tree().current_scene + var ok := scene != null and scene.scene_file_path == "res://scenes/lobby.tscn" + _finish(ok, "join_ok -> current_scene=%s" % (scene.scene_file_path if scene else "null")) + + +func _run_join_refused() -> void: + var menu := get_tree().current_scene + var address_edit: LineEdit = menu.get_node("%JoinAddressEdit") + address_edit.text = "127.0.0.1" + var join_btn: Button = menu.get_node("CenterContainer/VBoxContainer/JoinRow/JoinButton") + join_btn.emit_signal("pressed") + var overlay: Control = menu.get_node("%ConnectingOverlay") + print("SMOKE INFO: overlay visible right after Join press (no server) = %s" % str(overlay.visible)) + # main_menu.gd's own CONNECT_TIMEOUT_SECONDS (6.0) is what actually + # bounds this now — ENet's own connection_failed proved unbounded in + # practice against a genuinely refused loopback connection. + await get_tree().create_timer(8.0).timeout + var error_label: Label = menu.get_node("%MultiplayerErrorLabel") + var still_on_menu := get_tree().current_scene == menu + var ok := still_on_menu and not overlay.visible and error_label.visible + _finish(ok, "join_refused -> still_on_menu=%s overlay_visible=%s error_visible=%s error_text='%s'" % [ + str(still_on_menu), str(overlay.visible), str(error_label.visible), error_label.text + ]) + + +func _run_join_cancel() -> void: + var menu := get_tree().current_scene + var address_edit: LineEdit = menu.get_node("%JoinAddressEdit") + address_edit.text = "10.255.255.1" # non-routable; connect attempt just hangs until timeout/cancel + var join_btn: Button = menu.get_node("CenterContainer/VBoxContainer/JoinRow/JoinButton") + join_btn.emit_signal("pressed") + var overlay: Control = menu.get_node("%ConnectingOverlay") + await get_tree().create_timer(0.5).timeout + var overlay_shown := overlay.visible + var cancel_btn: Button = menu.get_node("%ConnectingCancelButton") + cancel_btn.emit_signal("pressed") + await get_tree().create_timer(0.5).timeout + var still_on_menu := get_tree().current_scene == menu + var ok := overlay_shown and not overlay.visible and still_on_menu and not NetworkManager.is_client + _finish(ok, "join_cancel -> overlay_shown=%s overlay_now=%s still_on_menu=%s is_client=%s" % [ + str(overlay_shown), str(overlay.visible), str(still_on_menu), str(NetworkManager.is_client) + ]) + + +func _finish(success: bool, message: String) -> void: + print("SMOKE %s: %s" % ["PASS" if success else "FAIL", message]) + await get_tree().create_timer(0.3).timeout + NetworkManager.shutdown() + get_tree().quit(0 if success else 1) diff --git a/Game/tests/match_net_smoke.gd b/Game/tests/match_net_smoke.gd new file mode 100644 index 00000000..52c292e7 --- /dev/null +++ b/Game/tests/match_net_smoke.gd @@ -0,0 +1,167 @@ +extends Node + +# Manual two/three-process smoke test for MatchNet (task 1.4 acceptance: +# "a mismatched client is rejected with a readable reason", plus the happy +# path: hello/welcome, roster sees player_joined on both sides). Not part of +# tests/test_runner.tscn — needs real ENet peers. Run: +# +# godot --headless --path Game res://tests/match_net_smoke.tscn -- --role=host +# godot --headless --path Game res://tests/match_net_smoke.tscn -- --role=client --name=Alice +# godot --headless --path Game res://tests/match_net_smoke.tscn -- --role=client-badversion +# godot --headless --path Game res://tests/match_net_smoke.tscn -- --role=host_recycle +# godot --headless --path Game res://tests/match_net_smoke.tscn -- --role=client --name=Bob (run once against host_recycle, then let it disconnect) + +const PORT := 7800 +const TIMEOUT_SECONDS := 5.0 + +var _role := "" +var _player_name := "TestPlayer" +var _finished := false + + +func _ready() -> void: + for arg in OS.get_cmdline_user_args(): + if arg.begins_with("--role="): + _role = arg.substr("--role=".length()) + elif arg.begins_with("--name="): + _player_name = arg.substr("--name=".length()) + + match _role: + "host": + MatchNet.player_joined.connect(_on_player_joined) + var err := NetworkManager.host(PORT) + if err != OK: + _finish(false, "host() failed: %s" % error_string(err)) + return + print("SMOKE: hosting on port %d" % PORT) + "host_recycle": + _run_host_recycle() + return + "client": + MatchNet.local_player_name = _player_name + MatchNet.welcomed.connect(_on_welcomed) + MatchNet.rejected.connect(_on_rejected) + var err := NetworkManager.join("127.0.0.1", PORT) + if err != OK: + _finish(false, "join() failed: %s" % error_string(err)) + return + print("SMOKE: joining as '%s' ..." % _player_name) + "client-badversion": + MatchNet._auto_hello = false + MatchNet.rejected.connect(_on_rejected) + MatchNet.welcomed.connect(_on_welcomed) + var err := NetworkManager.join("127.0.0.1", PORT) + if err != OK: + _finish(false, "join() failed: %s" % error_string(err)) + return + NetworkManager.connected_to_server.connect(func(): + var NetCodec = load("res://scripts/net_codec.gd") + MatchNet._hello.rpc_id(1, NetCodec.PROTOCOL_VERSION + 99, 60, "BadVersion") + ) + print("SMOKE: joining with a deliberately wrong protocol version ...") + "client-longname": + # Adversarial-review regression: a client sending an oversized + # player_name used to be broadcast verbatim to every peer, + # head-of-line-blocking the reliable channel. Confirm it's + # rejected outright before ever reaching a broadcast. + MatchNet._auto_hello = false + MatchNet.rejected.connect(_on_rejected) + MatchNet.welcomed.connect(_on_welcomed) + var err := NetworkManager.join("127.0.0.1", PORT) + if err != OK: + _finish(false, "join() failed: %s" % error_string(err)) + return + NetworkManager.connected_to_server.connect(func(): + var NetCodec = load("res://scripts/net_codec.gd") + var SimConstants = load("res://scripts/sim_constants.gd") + var huge_name := "X".repeat(500000) # 500 KB, well past MAX_INPUT_LENGTH + MatchNet._hello.rpc_id(1, NetCodec.PROTOCOL_VERSION, SimConstants.TICK_HZ, huge_name) + ) + print("SMOKE: joining with a deliberately oversized player name ...") + _: + _finish(false, "missing or unrecognised --role=") + return + + get_tree().create_timer(TIMEOUT_SECONDS).timeout.connect(_on_timeout) + + +func _process(_delta: float) -> void: + NetworkManager.poll() + + +func _physics_process(_delta: float) -> void: + NetworkManager.poll() + + +func _on_player_joined(peer_id: int, player_name: String) -> void: + _finish(true, "host saw player_joined (peer_id=%d, name=%s)" % [peer_id, player_name]) + + +# Adversarial-review regression (multiplayer-todo.md §9): MatchNet.roster +# used to have no path that cleared it when a HOST itself called +# NetworkManager.shutdown() — only the client-side disconnect signal did. +# Host -> client joins -> host leaves (shutdown) -> host again used to +# leave the first client permanently in roster. Run this role, then run +# `--role=client` once against it while it's up. +var _host_recycle_joined := false + + +func _on_host_recycle_player_joined(_peer_id: int, _name: String) -> void: + _host_recycle_joined = true + + +func _run_host_recycle() -> void: + MatchNet.player_joined.connect(_on_host_recycle_player_joined) + var err := NetworkManager.host(PORT) + if err != OK: + _finish(false, "host() failed: %s" % error_string(err)) + return + print("SMOKE: host_recycle hosting on port %d, waiting for a client..." % PORT) + + var deadline := Time.get_ticks_msec() + int(TIMEOUT_SECONDS * 1000.0) + while not _host_recycle_joined and Time.get_ticks_msec() < deadline: + await get_tree().process_frame + if not _host_recycle_joined: + _finish(false, "no client joined within %.1fs" % TIMEOUT_SECONDS) + return + + print("SMOKE: host_recycle got a joiner (roster size=%d), now leaving and re-hosting..." % MatchNet.roster.size()) + NetworkManager.shutdown() + err = NetworkManager.host(PORT) + if err != OK: + _finish(false, "re-host() failed: %s" % error_string(err)) + return + await get_tree().process_frame + await get_tree().process_frame + + var ok := MatchNet.roster.is_empty() + _finish(ok, "roster after re-host: size=%d (expected 0)" % MatchNet.roster.size()) + + +func _on_welcomed() -> void: + if _role == "client-badversion" or _role == "client-longname": + _finish(false, "%s was welcomed, expected rejection" % _role) + else: + _finish(true, "client was welcomed") + + +func _on_rejected(reason: String) -> void: + if _role == "client-badversion" or _role == "client-longname": + _finish(true, "%s correctly rejected: %s" % [_role, reason]) + else: + _finish(false, "client was rejected unexpectedly: %s" % reason) + + +func _on_timeout() -> void: + if not _finished: + _finish(false, "timed out") + + +func _finish(success: bool, message: String) -> void: + if _finished: + return + _finished = true + print("SMOKE %s: %s" % ["PASS" if success else "FAIL", message]) + await get_tree().create_timer(0.5).timeout + NetworkManager.shutdown() + get_tree().quit(0 if success else 1) diff --git a/Game/tests/match_net_smoke.tscn b/Game/tests/match_net_smoke.tscn new file mode 100644 index 00000000..3629c213 --- /dev/null +++ b/Game/tests/match_net_smoke.tscn @@ -0,0 +1,6 @@ +[gd_scene load_steps=2 format=3] + +[ext_resource type="Script" path="res://tests/match_net_smoke.gd" id="1_mns"] + +[node name="MatchNetSmoke" type="Node"] +script = ExtResource("1_mns") diff --git a/Game/tests/net_smoke.gd b/Game/tests/net_smoke.gd new file mode 100644 index 00000000..8f71257b --- /dev/null +++ b/Game/tests/net_smoke.gd @@ -0,0 +1,112 @@ +extends Node + +# Manual two-process smoke test for NetworkManager (task 1.2 acceptance: +# "two peers connect and disconnect cleanly"; also exercises task 1.3's +# manual-poll-only regime — NetworkManager disables automatic multiplayer +# polling, so this script's own _process() polling is what makes the +# connection progress at all). Deliberately not part of the pure-function +# suite in tests/test_runner.tscn — an ENet handshake needs two real +# processes. Run: +# +# godot --headless --path Game res://tests/net_smoke.tscn -- --role=host +# godot --headless --path Game res://tests/net_smoke.tscn -- --role=client +# +# (start the host first). Each process prints one "SMOKE PASS/FAIL: ..." +# line and exits 0/1. +# +# Adversarial-review regression: this test used to only confirm each +# process exits cleanly on its own initiative — it never confirmed the +# OTHER peer actually observes the disconnect. The host role now waits for +# BOTH client_connected and client_disconnected before passing; the client +# explicitly disconnects mid-test (rather than only on process exit) and +# gives it a beat before quitting, same reasoning as §9 gotcha 26 for +# connects: a clean disconnect notice still needs a few poll() cycles to +# reach the wire, or the other side falls back to its ~5s peer timeout +# (§9 gotcha 11) instead of a prompt, clean disconnect. + +const DEFAULT_PORT := 7799 +const TIMEOUT_SECONDS := 8.0 + +var _role := "" +var _port := DEFAULT_PORT +var _finished := false + + +func _ready() -> void: + for arg in OS.get_cmdline_user_args(): + if arg.begins_with("--role="): + _role = arg.substr("--role=".length()) + elif arg.begins_with("--port="): + _port = int(arg.substr("--port=".length())) + + if _role == "host": + NetworkManager.client_connected.connect(_on_host_client_connected) + NetworkManager.client_disconnected.connect(_on_host_client_disconnected) + var err := NetworkManager.host(_port) + if err != OK: + _finish(false, "host() failed: %s" % error_string(err)) + return + print("SMOKE: hosting on port %d, waiting for a client..." % _port) + elif _role == "client": + NetworkManager.connected_to_server.connect(_on_client_connected) + NetworkManager.connection_failed.connect(_on_client_connection_failed) + var err := NetworkManager.join("127.0.0.1", _port) + if err != OK: + _finish(false, "join() failed: %s" % error_string(err)) + return + print("SMOKE: joining 127.0.0.1:%d ..." % _port) + else: + _finish(false, "missing or unrecognised --role= (expected host|client)") + return + + get_tree().create_timer(TIMEOUT_SECONDS).timeout.connect(_on_timeout) + + +func _process(_delta: float) -> void: + NetworkManager.poll() + + +func _physics_process(_delta: float) -> void: + NetworkManager.poll() + + +func _on_host_client_connected(peer_id: int) -> void: + print("SMOKE INFO: host saw client_connected (peer_id=%d), waiting for client_disconnected too..." % peer_id) + + +func _on_host_client_disconnected(peer_id: int) -> void: + _finish(true, "host saw client_connected AND client_disconnected (peer_id=%d)" % peer_id) + + +func _on_client_connected() -> void: + print("SMOKE INFO: client connected to host") + # §9 gotcha 26: give the host a beat to fully settle the connect + # handshake before we turn around and disconnect again. + await get_tree().create_timer(0.5).timeout + NetworkManager.shutdown() + # Same class of issue as gotcha 26, the disconnect leg: closing the + # peer queues ENet's own disconnect notice, which still needs a few + # more poll() cycles to actually reach the wire before this process + # exits — quit immediately and the host would fall back to its ~5s + # peer timeout (§9 gotcha 11) instead of a prompt, clean disconnect. + await get_tree().create_timer(1.0).timeout + _finish(true, "client connected then disconnected cleanly") + + +func _on_client_connection_failed() -> void: + _finish(false, "client connection_failed") + + +func _on_timeout() -> void: + if not _finished: + _finish(false, "timed out waiting for connect+disconnect confirmation") + + +func _finish(success: bool, message: String) -> void: + if _finished: + return + _finished = true + print("SMOKE %s: %s" % ["PASS" if success else "FAIL", message]) + await get_tree().create_timer(0.3).timeout + NetworkManager.shutdown() + get_tree().quit(0 if success else 1) diff --git a/Game/tests/net_smoke.tscn b/Game/tests/net_smoke.tscn new file mode 100644 index 00000000..2cabc7de --- /dev/null +++ b/Game/tests/net_smoke.tscn @@ -0,0 +1,6 @@ +[gd_scene load_steps=2 format=3] + +[ext_resource type="Script" path="res://tests/net_smoke.gd" id="1_ns"] + +[node name="NetSmoke" type="Node"] +script = ExtResource("1_ns") diff --git a/Game/tests/test_case.gd b/Game/tests/test_case.gd new file mode 100644 index 00000000..7949a7b1 --- /dev/null +++ b/Game/tests/test_case.gd @@ -0,0 +1,38 @@ +extends RefCounted + +# Base class for pure-function unit tests run by test_runner.gd. A test case +# script extends this and defines any number of test_*() methods; the runner +# discovers them by name, not by registration, so adding a test is just +# adding a method. +# +# Test case scripts should `extends "res://tests/test_case.gd"` (path-based), +# not `extends TestCase` (the bare class_name). On a fresh headless run the +# global script class cache isn't guaranteed populated yet, so a bare-name +# reference can fail to resolve; the path form and test_runner.gd's own +# `preload()` both sidestep that. + +var failures: Array[String] = [] +# Adversarial-review regression: GDScript has no exceptions, so a runtime +# error partway through a test method (e.g. a null dereference) just logs a +# SCRIPT ERROR and returns — `failures` stays empty exactly as if every +# assertion had passed, and the runner counted it as a PASS. assertions_made +# is incremented by every assert_* call; test_runner.gd now treats a test +# that completes with zero assertions as a failure in its own right, so a +# test that crashes before reaching its first assert_* can no longer read +# as a silent pass. +var assertions_made := 0 + +func assert_true(condition: bool, message: String) -> void: + assertions_made += 1 + if not condition: + failures.append(message) + +func assert_eq(actual, expected, message: String) -> void: + assertions_made += 1 + if actual != expected: + failures.append("%s: expected %s, got %s" % [message, expected, actual]) + +func assert_almost_eq(actual: float, expected: float, tolerance: float, message: String) -> void: + assertions_made += 1 + if absf(actual - expected) > tolerance: + failures.append("%s: expected %s ± %s, got %s" % [message, expected, tolerance, actual]) diff --git a/Game/tests/test_runner.gd b/Game/tests/test_runner.gd new file mode 100644 index 00000000..a6f0d1ed --- /dev/null +++ b/Game/tests/test_runner.gd @@ -0,0 +1,81 @@ +extends Node + +# Headless test runner (task 1.0). Discovers every *.gd under tests/cases/, +# instances it, and calls every test_*() method by name — a test case is +# picked up by dropping a file in that folder, not by registering it here. +# Run with: godot --headless --path Game res://tests/test_runner.tscn + +const CASES_DIR := "res://tests/cases" +const TestCase = preload("res://tests/test_case.gd") + +func _ready() -> void: + var case_paths := _discover_case_paths() + var total := 0 + var failed := 0 + var failure_messages: Array[String] = [] + + for path in case_paths: + # Adversarial-review regression: a case file with a parse/compile + # error used to hang the whole runner forever. load() on a broken + # script does NOT return null here — it returns a non-null but + # uninstantiable GDScript resource, so a plain null check doesn't + # catch it; calling .new() on it throws "Invalid call: Nonexistent + # function 'new'", severe enough to abort _ready() entirely without + # ever reaching quit(). can_instantiate() is the real guard. + var script: GDScript = load(path) + if script == null or not script.can_instantiate(): + failed += 1 + failure_messages.append("%s: failed to load (parse/compile error — see SCRIPT ERROR above)" % path.get_file()) + continue + var instance = script.new() + if instance == null: + failed += 1 + failure_messages.append("%s: script.new() returned null" % path.get_file()) + continue + + for method in instance.get_method_list(): + var method_name: String = method["name"] + if not method_name.begins_with("test_"): + continue + total += 1 + instance.failures.clear() + instance.assertions_made = 0 + instance.call(method_name) + # Adversarial-review regression: GDScript has no exceptions, so + # a runtime error partway through a test (before it reaches its + # first assert_*) just logs a SCRIPT ERROR and returns — + # `failures` stays empty exactly as if every assertion passed, + # and this used to count as a PASS. A test that completes + # having made zero assertions is itself a failure: it proved + # nothing, whether because it crashed early or was just never + # written to assert anything. + if instance.assertions_made == 0: + failed += 1 + failure_messages.append("%s.%s: made no assertions (crashed before the first assert_*, or the test itself is incomplete)" % [path.get_file(), method_name]) + elif not instance.failures.is_empty(): + failed += 1 + for f in instance.failures: + failure_messages.append("%s.%s: %s" % [path.get_file(), method_name, f]) + + print("Ran %d tests from %d case file(s), %d failed" % [total, case_paths.size(), failed]) + for message in failure_messages: + print(" FAIL: " + message) + + get_tree().quit(1 if failed > 0 else 0) + + +func _discover_case_paths() -> Array[String]: + var paths: Array[String] = [] + var dir := DirAccess.open(CASES_DIR) + if dir == null: + push_error("Cannot open " + CASES_DIR) + return paths + dir.list_dir_begin() + var file_name := dir.get_next() + while file_name != "": + if file_name.ends_with(".gd") and not dir.current_is_dir(): + paths.append(CASES_DIR + "/" + file_name) + file_name = dir.get_next() + dir.list_dir_end() + paths.sort() + return paths diff --git a/Game/tests/test_runner.tscn b/Game/tests/test_runner.tscn new file mode 100644 index 00000000..247ff027 --- /dev/null +++ b/Game/tests/test_runner.tscn @@ -0,0 +1,6 @@ +[gd_scene load_steps=2 format=3] + +[ext_resource type="Script" path="res://tests/test_runner.gd" id="1_tr"] + +[node name="TestRunner" type="Node"] +script = ExtResource("1_tr") diff --git a/multiplayer-todo.md b/multiplayer-todo.md index d891b9d6..299babe9 100644 --- a/multiplayer-todo.md +++ b/multiplayer-todo.md @@ -799,9 +799,9 @@ Every task lands on `master` independently, is verifiable in single-player today | 0.24 `[P]` | **DONE.** Both guarded with `if DisplayServer.get_name() == "headless": return` — `arena.gd:_ready()` skips the whole Environment block, `video_settings.gd:_ready()` skips `apply_aa()` | `scripts/arena.gd`, `scripts/video_settings.gd` | `--headless` allocates no Environment and no AA state | | 0.25 `[P]` | **DONE.** `_process` still calls `to_local()` every frame (needed for the comparison itself) but skips `set_shader_parameter()` — the actual GPU-facing cost — below a 0.05 m movement threshold | `scripts/arena_boundary.gd` | Field shader behaves identically; the expensive call is skipped on most frames | | **0.26** `[D:0.15b]` | **Bake the arena GI and retire SDFGI** (§5.7). `arena.gd`/`goal.gd` have no `_process`, no animation — the arena is fully static, and SDFGI is paying continuously to solve a dynamic-world problem this project does not have. Add UV2 to the arena shell, bake `LightmapGI` (or `VoxelGI` if bounce onto ships matters), disable `sdfgi_enabled` and re-evaluate `ssil_enabled` | `scenes/arena_base.tscn`, `scenes/arena_0*.tscn`, `scripts/arena_boundary.gd` | **Largest frame-time reduction of any task here, with equal or better image quality**; High preset keeps its look; bake is reproducible from a documented step | -| **0.27** `[P]` | **Override expensive rendering defaults** (§5.7): `positional_shadow/atlas_size` 4096 → 2048, `directional_shadow/size` 4096 → 2048, `soft_shadow_filter_quality`. `project.godot [rendering]` currently has three keys and everything else is at engine default | `project.godot` | Measurable frame-time reduction; no visible shadow-quality regression at 1080p | +| **0.27** `[P]` | **DONE.** `lights_and_shadows/positional_shadow/atlas_size` and `directional_shadow/size` set to 2048 (from the 4096 engine default), `soft_shadow_filter_quality=2` | `project.godot` | Measurable frame-time reduction; no visible shadow-quality regression at 1080p | | **0.28** `[D:0.15b]` | **CLOSED, not implemented — the problem it targets doesn't exist.** Was: prototype `physics/3d/run_on_separate_thread` (§5.7) to attack frame-time variance from the physics tick sharing the render thread — **the riskiest item in this phase**, since it changes when `_integrate_forces` runs relative to script code, and both `ship.gd:346-357` and the RL training path depend on that. §5.5.2's real-hardware measurement (RTX 3090) found a 1.85 ms p50 / 2.98 ms p99 baseline with every graphics effect enabled — both comfortably under even a 240 Hz frame budget, with no meaningful p99-over-p50 variance to explain away. Taking on this task's real risk (reordering `_integrate_forces` relative to script code, with the RL training path depending on today's ordering) for a variance problem that isn't measurably present is a bad trade. Reopen only if a lower-end-hardware pass (§5.5.2's "still open" item) finds real physics-tick-driven variance that 0.26 and the preset ladder don't already cover | — | *(closed without a code change; see §5.5.2 for the evidence)* | -| **0.29** `[P]` | **Early-out in `ArenaBoundary.get_surface_pull`**: a bounds check against `wall_range`/`ceiling_range` before the `to_local()` and five `_falloff` calls, which currently run for every dynamic body every tick even mid-arena where every term is zero | `scripts/arena_boundary.gd` | Identical flight feel and identical RL observations; measurable tick-time reduction with 7 bodies | +| **0.29** `[P]` | **DONE.** Bounds check against `wall_range`/`ceiling_range` at the top of `get_surface_pull`, returning `Vector3.ZERO` before `to_local()` and the five `_falloff` calls whenever every term would be zero mid-arena | `scripts/arena_boundary.gd` | Identical flight feel and identical RL observations; measurable tick-time reduction with 7 bodies | > **These tasks exist because of the high-refresh-rate mandate, and their order matters.** **0.15b blocked everything else, and did invalidate the a priori fps list** — but not in the direction first assumed (see §5.5.1 vs §5.5.2): on the Mac the game looked GPU-bound and undifferentiated; on real reference hardware (RTX 3090, §5.5.2) it runs at 540 fps p50 with everything on, nowhere near bound by anything. 0.17/0.17b/0.19 (done) are still the right frame-rate levers — SDFGI/SSIL genuinely dominate the optional-effects cost, exactly as originally assumed, just at a much smaller absolute scale than feared on this hardware tier. 0.16 and 0.20–0.25 are the per-frame hygiene that makes a high frame rate worth having. 0.18 buys nothing today — it is what keeps a future 120 Hz simulation a config change plus a retrain rather than a protocol rewrite. 0.28 closed without a code change (§5.5.2) — the frame-time variance it targeted isn't measurably present on reference hardware. > @@ -817,15 +817,16 @@ Every task lands on `master` independently, is verifiable in single-player today | # | Task | Acceptance | |---|---|---| -| 1.0 | **`tests/test_runner.tscn`** — minimal pure-function assertion runner exiting 0/1 | `godot --headless --path Game res://tests/test_runner.tscn` runs and exits 0 | -| 1.1 `[D:1.0]` `[D:0.18]` | `net_codec.gd`: protocol constants, enums, channel ids, quantisers, pack/unpack for both hot packets. **Pure functions**, written against 1.0. Every timing constant derives from `TICK_HZ` — ring sizes, seq windows, `INTERP_DELAY`, timeouts (§5.4) | Round-trip, bounds, and quaternion-error tests pass; setting `TICK_HZ = 120` recomputes every derived constant with no other edit | -| 1.2 `[D:1.1]` | `NetworkManager` autoload: `host`/`join`/`shutdown`, signals, **`server_relay = false`** | Two peers connect and disconnect cleanly | -| 1.3 `[D:1.2]` | Manual multiplayer polling: `get_tree().set_multiplayer_poll(false)`; client flushes at the end of `_physics_process` after the input send **and polls for receive at the top of both `_process` and `_physics_process`, unthrottled** (§5.4c); server polls at tick start (drain) and tick end (flush). Defer any scene-tree mutation in `peer_connected`/`peer_disconnected`, which now fire mid-frame | Measured RTT drops by ~8–17 ms per leg versus default polling; **median age of the newest applied snapshot at render time drops by ≈ half a frame interval** versus polling once per physics tick | -| 1.4 `[D:1.2]` | `MatchNet` autoload skeleton: `hello`/`welcome`/`player_joined`/`player_left`, strict `protocol_version` **and `physics_ticks_per_second`** gating, roster model | A mismatched client is rejected with a readable reason | -| 1.5 `[D:1.4]` | `lobby.tscn` + `lobby.gd`: roster list, team swap, ready toggle, disconnect | Two clients see each other and both ready states | -| 1.6 `[D:1.4]` `[P]` | `server_boot.tscn` + `server_boot.gd`: CLI parsing from `OS.get_cmdline_user_args()`, `Engine.max_fps = 60`, `Engine.max_physics_steps_per_frame` overrun watchdog, host, structured log lines | Headless server starts, logs, accepts connections, and idles at <5% of a core | -| 1.7 `[D:1.5]` `[P]` | `main_menu.gd`: Host / Join-by-IP with a **connecting overlay, cancel, and a failure path** | Connect, cancel, and connection-refused all reach a sane UI state | -| 1.8 `[D:1.2]` `[P]` | Clock: ping/pong, min-RTT-filtered offset, tick estimate (folded into `network_manager.gd`), plus a debug overlay showing RTT and offset | Offset converges within 2 s and stays within ±1 tick on a clean link | +| 1.0 | **DONE, two real bugs found and fixed after adversarial review.** `tests/test_runner.tscn` + `test_runner.gd`: discovers every `*.gd` under `tests/cases/`, instances it, calls every `test_*()` method via `get_method_list()`, aggregates failures, `get_tree().quit(1 if failed else 0)`. `tests/test_case.gd` is the assertion base (`assert_true`/`assert_eq`/`assert_almost_eq`); case scripts use `extends "res://tests/test_case.gd"` (path-based) and the runner uses `preload()`, not a bare `class_name` reference — the global script-class cache isn't guaranteed populated on a fresh headless run (same class of issue as task 0.18's `SimConstants`). `tests/cases/test_smoke.gd` proves discovery/dispatch/aggregation and is the first real case file. **An Opus subagent's adversarial review found**: (1) GDScript has no exceptions, so a test that hit a runtime error before its first `assert_*` call left `failures` empty — exactly like every assertion passing — and was silently counted as a PASS. Fixed: `TestCase` now tracks `assertions_made`, incremented by every `assert_*`; the runner treats zero assertions as a failure in its own right ("made no assertions"). (2) A case file with a parse/compile error hung the whole runner forever — `load()` on a broken script does **not** return null here, it returns a non-null but uninstantiable `GDScript` resource, so a plain null check doesn't catch it; calling `.new()` on it threw an error severe enough to abort `_ready()` before ever reaching `quit()`. Fixed with `Script.can_instantiate()` as the real guard | `godot --headless --path Game res://tests/test_runner.tscn` runs and exits 0; verified exit 1 with a deliberately-failing assertion, then removed. Re-verified both fixes with scratch case files (not committed): a test that null-derefs before asserting now correctly fails with "made no assertions" (exit 1, not a false pass); an uncompilable case file now fails loudly and promptly (exit 1, not a 124-timeout hang) while the *other* valid case files in the same run still execute normally | +| 1.1 `[D:1.0]` `[D:0.18]` | **DONE.** `scripts/net_codec.gd`: protocol constants, `PacketType` enum, channel ids, i16/i8/thrust-z-bin quantisers, `pack_input`/`unpack_input`, `pack_snapshot_body_segment`/`pack_snapshot_client_header`/`pack_snapshot`/`unpack_snapshot`. New `scripts/net_body_state.gd` is the plain per-body data holder the snapshot functions read/write (not Ship/Ball themselves, so the codec stays callable with no scene tree). `NetCodec.TICK_HZ` derives from `SimConstants.TICK_HZ` via `preload()` (same cache-timing reason as 0.18); ring sizes / seq windows / `INTERP_DELAY` / timeouts don't exist as constants yet — they land with the tasks that consume them (3.1+), so "derives from `TICK_HZ`" is satisfied for what exists today | `scripts/net_codec.gd`, `scripts/net_body_state.gd`, `tests/cases/test_net_codec.gd` | 14 tests pass (`godot --headless --path Game res://tests/test_runner.tscn`, exit 0): input round-trip (1 and 4-entry, redundancy clamp), snapshot round-trip across 7 bodies incl. quaternion sign-fold and ship→ball angular-velocity rescale, thrust-z bin edges, type/version nibble round-trip. Byte counts asserted against §2.3/§2.4's numbers directly: 40 B input (max redundancy), 169 B snapshot (7 bodies) | +| 1.2 `[D:1.1]` | **DONE, strengthened after adversarial review.** `scripts/network_manager.gd` autoload (`NetworkManager` in `project.godot [autoload]`): `host(port, max_clients)`/`join(address, port)`/`shutdown()`, `client_connected`/`client_disconnected`/`connected_to_server`/`connection_failed`/`disconnected_from_server` signals forwarded from `multiplayer`'s own, `server_relay = false` set the moment a peer exists, `is_server`/`is_client` state. Gained a `shutting_down()` signal, emitted at the top of every `shutdown()` regardless of role or reason — see task 1.4's row for why | An Opus subagent's adversarial review (independently verified by the primary session before applying fixes) found the original `tests/net_smoke.gd` only proved each process exits cleanly on its own initiative, never that the OTHER peer actually observes the disconnect. Rewrote it: the host now waits for **both** `client_connected` and `client_disconnected` before passing; the client explicitly calls `shutdown()` mid-test (not just on process exit) and gives it a beat before quitting, same reasoning as §9 gotcha 26 for connects — a clean disconnect notice still needs a few `poll()` cycles to reach the wire, or the other side falls back to its ~5s peer timeout (gotcha 11) instead of a prompt one. Re-verified passing with both directions actually observed | +| 1.3 `[D:1.2]` | **DONE for what exists today.** `NetworkManager._ready()` calls `get_tree().set_multiplayer_poll_enabled(false)` (Godot 4.7's actual method name — the doc's `set_multiplayer_poll(false)` was shorthand) and exposes `NetworkManager.poll()` as the one entry point every caller uses instead. Verified against `tests/net_smoke.gd`, updated to poll from both `_process` and `_physics_process` every frame — connect/disconnect still works cleanly under manual-only polling (§9 gotcha 26 still applies: give a beat after a connect signal before shutdown). **The per-call-site placement this task specifies (client: end-of-physics-tick flush after input send, top-of-frame receive; server: tick-start drain, tick-end flush) has no real per-tick caller yet** — there is no input/snapshot traffic until tasks 1.4+/Phase 2 exist to send any, so there's nothing to place a flush *after*. That placement, and the RTT/staleness measurement below, land with the input pipeline, not as a separate task | `godot --headless` two-process test still connects/disconnects cleanly with automatic polling off (verified). RTT/staleness improvement **not yet measured** — deferred until Phase 2/3's real per-tick traffic exists to measure against, same honesty as task 1.1's "constants that don't fully exist yet" | +| 1.4 `[D:1.2]` | **DONE.** `scripts/match_net.gd` autoload (`MatchNet`): `_hello`/`_welcome`/`_player_joined`/`_player_left`/`_rejected` RPCs, `protocol_version` (`NetCodec.PROTOCOL_VERSION`) and `physics_ticks_per_second` (`SimConstants.TICK_HZ`) checked on the server before a peer is added to `roster`; on mismatch, server sends `_rejected` with a readable string then `disconnect_peer()`s after a 0.3s beat (§9 gotcha 26 applies here too — a bare RPC then immediate disconnect would drop the rejection message). `roster: Dictionary[int, PlayerInfo]` never contains peer 1 (§1.1 decision 2). A new peer is told about the existing roster via targeted RPCs before the broadcast that tells everyone (including itself) about the new peer, so no client ever observes an unexplained peer_id | Verified with a real two/three-process test (`tests/match_net_smoke.gd`/`.tscn`): matched client → both sides see `player_joined`/`welcomed`; deliberately wrong protocol version → client receives `rejected("protocol version mismatch: server=1 client=100")` and is disconnected. Caught and fixed one real bug in the process: the server's own `roster` update in `_hello()` didn't locally emit `player_joined` (the broadcast RPC is `call_remote`, never loops back to the sender) | +| — | **Two more real bugs found by an Opus subagent's adversarial review, both confirmed independently and fixed.** (1) `_hello`'s `player_name` was completely unvalidated and broadcast verbatim to every peer — a demonstrated DoS: a multi-MB name relayed to all peers head-of-line-blocked the reliable control channel hard enough that a concurrently-joining client's own `_welcome` never arrived. Fixed with a hard `MAX_INPUT_LENGTH = 256` reject (any legitimate client only ever sends `local_player_name`, which the UI already keeps short — anything past this is a bug or an attacker, not a name to politely truncate) followed by `_sanitize_player_name()`: strips control/formatting characters, clamps to `MAX_PLAYER_NAME_LENGTH = 24`, falls back to `"Player"` if empty. (2) `MatchNet.roster` was never cleared when a HOST stopped hosting — only the client-side disconnect path cleared it, so Host → Lobby → Leave → Host again left a phantom player in `roster` permanently, mis-balancing teams and getting broadcast to every future joiner. Fixed via `NetworkManager`'s new `shutting_down()` signal (task 1.2), which `MatchNet` now clears `roster` on unconditionally, regardless of role or reason | `_sanitize_player_name` is `static` (pure function of its argument) with 5 dedicated unit tests in `tests/cases/test_match_net.gd`, plus a live rejection test (`match_net_smoke.gd --role=client-longname`, a 500 KB name, confirmed rejected before ever reaching a broadcast). New regression test `match_net_smoke.gd --role=host_recycle`: host, client joins (`roster.size()==1`), host leaves and re-hosts, confirms `roster.is_empty()` before any new connection — reproduced the bug pre-fix, confirmed fixed post-fix | +| 1.5 `[D:1.4]` | **DONE, strengthened after adversarial review.** `scenes/lobby.tscn` + `scripts/lobby.gd`: roster split into two team columns (dynamically rebuilt `Label` rows on `MatchNet.player_joined`/`player_left`/`player_state_changed`/`welcomed`), Switch Team + Ready `CheckButton` (server process gets a read-only view — never a roster member, §1.1 decision 2), Leave. `MatchNet` grew `team`/`ready` fields on `PlayerInfo`, a balanced-team auto-assign on join (`_pick_balanced_team`), and `request_set_team`/`request_set_ready` + their server-authoritative RPCs, broadcasting `_state_changed` the same way `_player_joined` already did | Verified with a real two-process test (`tests/lobby_smoke.gd`/`.tscn`) that loads `lobby.tscn` via `change_scene_to_file` exactly as `main_menu.gd`'s Host/Join flow (task 1.7) does, then presses the real `%SwitchTeamButton`/`%ReadyButton` nodes via a persistent test-only helper (`tests/lobby_test_hooks.gd`, not a project autoload — parented under `get_tree().root` so it survives the scene swap, never referenced by production code). **An Opus subagent's adversarial review found the original test's host role never actually loaded `lobby.tscn` at all** — it only hosted and waited, so `lobby.gd`'s `is_server` branch (the read-only view a self-hosting player reaches via `main_menu.gd`'s own Host button — a real, production-reachable path, not a hypothetical) had never run under this task's own suite. Fixed: the host role now loads `lobby.tscn` too and a new `run_host_test()` in the shared test helper verifies `%ControlsRow` is hidden, the roster row renders, and the status text is correct, holding the connection open long enough (`MIN_HOST_LIFETIME_SECONDS`) for the client's own longer flow to finish against it. Confirmed: roster renders correctly server- **and** client-side (now genuinely, not just asserted), team switch moves the row to the other column, ready toggle updates the checkbox and the label's ✓ marker, row count matches roster size on both peers | +| 1.6 `[D:1.4]` `[P]` | **DONE.** `scenes/server_boot.tscn` + `scripts/server_boot.gd`: `--port=`/`--max-clients=`/`--log-level=` from `OS.get_cmdline_user_args()`, `Engine.max_fps = 60`, structured `[elapsed] LEVEL event key=value…` log lines for `server_started`/`peer_connected`/`player_joined`/`player_left`/`peer_disconnected`, and a physics-overrun watchdog comparing `Engine.get_physics_frames()` deltas frame-to-frame. Does not spawn a match yet — that's Phase 2's `networked_match.gd` — this is just the process shell: listen, log, idle cheaply. **Two real bugs caught and fixed while verifying, both in the watchdog**: (1) the very first `_process()` after boot compared against a pre-`_ready()` baseline and logged a spurious one-time `steps=5`; skip the first measurement. (2) the initial `steps > 1` threshold fired continuously (every 30–100ms) on a perfectly idle, healthy server — because §9 gotcha 6 means frames legitimately alternate between 0 and 2 physics ticks under `physics_jitter_fix = 0.0`, not a flat 1/frame; that's quantisation, not backlog. Raised the threshold to `steps > 2` (3+ ticks = the accumulator actually failing to drain), which produced zero false positives over a 4.8s idle run | Verified with real headless runs: idle CPU measured via `ps -o %cpu` at 0.0% (bar is <5%); a real client connect/disconnect via `tests/net_smoke.gd --port=` produces exactly the expected 4-line log sequence with no spurious warnings | +| 1.7 `[D:1.5]` `[P]` | **DONE.** `main_menu.tscn` gained a Multiplayer section (Host button; Join row with an IP `LineEdit`, default `127.0.0.1`; inline error label) and a full-screen `ConnectingOverlay` (status label + Cancel). `main_menu.gd`: `_on_host_pressed` calls `NetworkManager.host()` then goes straight to `lobby.tscn` (synchronous — no overlay needed); `_start_join` calls `NetworkManager.join()`, shows the overlay, and starts an app-level `CONNECT_TIMEOUT_SECONDS = 6.0` timer; `_on_connected_to_server`/`_on_connection_failed`/Cancel/timeout each resolve to the overlay hiding and either `lobby.tscn` or a visible error, gated by a token counter so a late/stray signal after the attempt was already resolved is a no-op | Verified with real multi-process runs of `scenes/main_menu.tscn` itself (not a wrapper — driven by a temporary-autoload test helper, `tests/main_menu_test_hooks.gd`, pressing the real `HostButton`/`JoinButton`/`ConnectingCancelButton`) across all four paths: Host → `lobby.tscn`; Join → connects → `lobby.tscn`; Join with nothing listening → times out → error shown, stays on menu; Join → Cancel → overlay hidden, stays on menu, `is_client` false. **Two real bugs found and fixed in the process, both pre-existing from earlier Phase 1 tasks, not new to 1.7**: (1) `NetworkManager`'s clock ping (task 1.8) gated only on `is_client`, which turns true the instant `join()` is called — a slow or refused connect attempt spammed "Trying to call an RPC via a multiplayer peer which is not connected" every frame; fixed by also requiring `_peer.get_connection_status() == CONNECTION_CONNECTED`. (2) ENet's own `connection_failed` proved **unbounded in practice** — verified empirically against a genuinely refused loopback connection, it hadn't fired even 14s in — which would have left a player staring at "Connecting…" indefinitely; task 1.7's own `CONNECT_TIMEOUT_SECONDS` is what actually satisfies "connection-refused reaches a sane UI state", not the built-in signal alone | +| 1.8 `[D:1.2]` `[P]` | **DONE, strengthened after adversarial review.** Folded into `network_manager.gd`: client pings the server once a second (`_ping`/`_pong` RPCs, reliable, channel 0); `clock_offset_ms` is the min-RTT sample in a rolling 5s window (`_clock_samples`, pruned by wall time); `get_server_time_estimate_ms()` is the public API later phases (`INTERP_DELAY`, `tick_offset` seeding) will actually call; `clock_updated(rtt_ms, offset_ms)` signal for observers. New `scripts/net_debug_overlay.gd` autoload (F4, `toggle_net_overlay` input action) mirrors `perf_overlay.gd`'s headless-guarded pattern, shows RTT + offset client-side or peer count server-side | Verified with a real two-process test (`tests/clock_smoke.gd`/`.tscn`) on localhost: first sample at t=0.95s, offset converged to 1534.50ms by t=2.0s (well inside the 2s bar), and stayed within 1.5ms of that value through t=3.96s — comfortably under the ±1 tick (16.67ms) bar. **An Opus subagent's adversarial review correctly pointed out this self-consistency check couldn't have caught a *systematically*-wrong-but-stable offset** (e.g. a missing `/2` on RTT, or a sign flip — it would converge just as cleanly). Fixed by adding an independent ground-truth cross-check: both host and client compute `Time.get_unix_time_from_system()*1000.0 - Time.get_ticks_msec()` (each process's own offset from the shared OS wall clock — the *same* real clock on both, since they're on the same machine), exchanged via a shared temp file written by the host, purely for test orchestration and touching no production code. The true required offset is just the difference of those two numbers; re-run measured the converged offset against it and found **0.99ms of error**, comfortably inside a deliberately loose 250ms tolerance (OS wall-clock read resolution and sampling-instant skew, not NetworkManager's own precision, is what sets the tolerance floor here). Note the converged offset *value* itself is large and arbitrary (~1.5s) because `Time.get_ticks_msec()` counts from each process's own start, not a shared epoch — expected, and exactly what `clock_offset_ms` exists to absorb | > `main_menu.gd` gains its **first async flow**. Every existing handler is `GameSettings.x = y; change_scene_to_file(...)` — there is no loading screen, no error state, and no back-navigation state machine to extend. Budget for that. @@ -999,6 +1000,12 @@ No own-ship prediction yet: the client renders everything, including its own shi 22. **`Engine.max_physics_steps_per_frame = 8` is a client problem too**, not just a server one (gotcha 9). A client hitching to 20 fps runs 3 ticks per frame, and each of those frames also runs the per-frame camera rig and remote-visual sampling. Set it to 4 client-side (task 0.22). On a multi-tick frame the send path must transmit **every** tick's action (gotcha 6) — §4.3's `_physics_process` sampling does this naturally, but nothing else guarantees it. 23. **`hint_screen_texture` forces a full-screen backbuffer copy on every frame the node is drawn**, regardless of what the shader then does with it. Branching inside the shader saves taps, not the copy. Hide the node when the effect is at rest. 24. **`physics_jitter_fix` matters less the higher the frame rate.** Its purpose is smoothing when frame rate ≈ tick rate; at 240 fps against 60 Hz physics most frames run zero ticks and the accumulator is never near an edge. Gotcha 6's reasoning for setting it to `0.0` still holds, but do not expect a visible difference on a high-refresh machine — test that change at 60 fps. +25. **`MultiplayerAPI.multiplayer_peer`'s default value is an `OfflineMultiplayerPeer` sentinel, not `null`.** Resetting it with `multiplayer_peer = null` (rather than a fresh `OfflineMultiplayerPeer.new()`) leaves the API in a state distinct from its own default and is a known source of "the server never sees `peer_connected`, `get_peers()` stays empty" bugs (godotengine/godot#81540) — confirmed the hard way while building task 1.2's `NetworkManager.shutdown()`. Always reset to a real `OfflineMultiplayerPeer`. +26. **Don't tear down a peer the instant its own connect signal fires.** `connected_to_server` (client-side) fires once the client's *local* view of the handshake completes, but the final ACK the server needs to consider *its* side complete may not have hit the wire yet — closing the peer or quitting the process in the same callback can drop it, and the other side then never sees `peer_connected`/`connected_to_server` at all, even though your own side looked successful. This isn't a corner case: it reproduced on **every** attempt until fixed, is easy to misdiagnose as a server-side bug (the server-side symptom — `get_peers()` staying empty — is identical to gotcha 25's), and cost significant debugging time before the actual cause (client-side premature teardown) was found. Give at least one frame — in practice `tests/net_smoke.gd` uses 0.3 s — between a fresh connect signal and calling `shutdown()`/`quit()`. Directly relevant to task 5.6's disconnect/reconnect controller swap and any CLI test client that connects, asserts, and exits quickly. +27. **`change_scene_to_file()` must be called on (or from a descendant of) the actual `get_tree().current_scene`, and never synchronously from `_ready()`.** Both failure modes were hit building task 1.5's `lobby.tscn`/`tests/lobby_smoke.gd`: (a) a test harness that instantiated `lobby.tscn` as a plain child of a driver node — rather than loading it as the real current scene, the way `main_menu.gd`'s Host/Join flow will — caused `lobby.gd`'s own (entirely correct, standard-pattern) `change_scene_to_file(ScenePaths.MAIN_MENU)` disconnect handler to hang the process completely on a real disconnect, with near-zero CPU (blocked, not spinning) and no error output; the fix was to load the scene the way production actually will, not to change the production code. (b) calling `change_scene_to_file()` (or `add_child()` on `get_tree().root`) synchronously from inside `_ready()` throws "Parent node is busy … Consider using `.call_deferred()`", because the tree is still mid-traversal adding the very node whose `_ready()` is running; `main_menu.gd`'s real button-press handlers won't hit this (they run outside any `_ready()`), but anything that needs to trigger a scene change during its own initialization must `.call_deferred()` it. +28. **`ENetMultiplayerPeer`'s `connection_failed` signal is not bounded to anything a UI should make a player wait for.** Verified empirically (task 1.7): against a genuinely refused loopback connection (nothing listening on the target port), `connection_failed` had still not fired 14 seconds in. Don't rely on it alone to end a "Connecting…" state — run your own app-level timeout (`main_menu.gd`'s `CONNECT_TIMEOUT_SECONDS = 6.0`) that shuts the peer down and shows an error regardless of whether ENet ever gets around to reporting failure itself. +29. **A `MultiplayerPeer`'s "am I a client" flag (however you track it — `NetworkManager.is_client` here) turns true the instant `join()`/`create_client()` is called, not once the connection actually completes.** Anything gated on that flag alone (task 1.8's clock ping, in `network_manager.gd`'s `_process`) will try to `rpc_id()` on a peer that's still `CONNECTING` — or has already failed — during a slow or refused connect attempt, and Godot logs "Trying to call an RPC via a multiplayer peer which is not connected" every single frame until it resolves. Gate on the peer's actual `get_connection_status() == MultiplayerPeer.CONNECTION_CONNECTED`, not just the higher-level intent flag. +30. **`load()` on a `.gd` file with a parse/compile error does not return `null`.** Found via adversarial review of `tests/test_runner.gd`: it returns a non-null but uninstantiable `GDScript` resource, so `if script == null` silently fails to catch the failure — and the natural next line, `script.new()`, throws "Invalid call: Nonexistent function 'new'", severe enough to abort the *entire calling function* (not just that statement) without ever reaching whatever cleanup/exit code follows. In a loop over multiple files with no per-iteration error boundary, this reads as a hang: the loop that would have moved to the next file, and the code that would have called `quit()`, both never run. The real guard is `Script.can_instantiate()`. ---