diff --git a/Game/scripts/match_sim.gd b/Game/scripts/match_sim.gd index 8f99ccc2..6e1f2a0d 100644 --- a/Game/scripts/match_sim.gd +++ b/Game/scripts/match_sim.gd @@ -23,6 +23,7 @@ signal match_config_received(arena_path: String, peer_ids: PackedInt32Array, tea signal input_received(peer_id: int, decoded: Dictionary) # decoded: see NetCodec.unpack_input signal snapshot_received(decoded: Dictionary) # decoded: see NetCodec.unpack_snapshot signal score_update_received(score: Dictionary) +signal state_change_received(state: int, at_tick: int) # §6.1 MatchState.State # Input validation (multiplayer-todo.md §3.1 steps 2-3, task 3.4). Deliberately # lives here rather than in NetworkedMatch: framing/rate abuse is a protocol- @@ -164,6 +165,18 @@ func send_score_update(score: Dictionary) -> void: _score_update.rpc(score) +# §6.1 task 5.1. Reliable channel 0, and it carries the ABSOLUTE tick the +# transition happened on rather than a duration — §6.2's closing note: on a +# lossy link ENet's RTO can stretch a lifecycle burst to ~600ms, and a +# duration would then be applied from whenever it happened to arrive. +# The same state also rides every snapshot's match_state byte, so a client +# that misses this entirely still converges (see NetworkedMatch's own +# _on_snapshot_received) — this RPC exists to make the transition PROMPT and +# to carry `at_tick`, not to be the sole channel. +func send_state_change(state: int, at_tick: int) -> void: + _state_change.rpc(state, at_tick) + + @rpc("authority", "call_remote", "reliable", 0) func _match_config(arena_path: String, peer_ids: PackedInt32Array, teams: PackedInt32Array, spawn_indices: PackedInt32Array) -> void: match_config_received.emit(arena_path, peer_ids, teams, spawn_indices) @@ -250,6 +263,19 @@ func _snapshot(bytes: PackedByteArray) -> void: snapshot_received.emit(decoded) +@rpc("authority", "call_remote", "reliable", 0) +func _state_change(state: int, at_tick: int) -> void: + # "authority" already means a forging client is rejected by Godot itself + # (verified for _match_config/_score_update/_snapshot during Phase 2), but + # an authoritative server sending a state this build doesn't know about is + # a real forward-compatibility case — drop it rather than driving the + # client into an undefined state. + if not MatchState.is_valid(state): + push_warning("MatchSim: ignoring unknown match_state %d from server" % state) + return + state_change_received.emit(state, at_tick) + + @rpc("authority", "call_remote", "reliable", 0) func _score_update(score: Dictionary) -> void: score_update_received.emit(score) diff --git a/Game/scripts/match_state.gd b/Game/scripts/match_state.gd new file mode 100644 index 00000000..f8b390e6 --- /dev/null +++ b/Game/scripts/match_state.gd @@ -0,0 +1,88 @@ +class_name MatchState + +# Match lifecycle states (multiplayer-todo.md §6.1, task 5.1). +# +# Pure data + a transition table, deliberately with no scene, RPC or +# NetworkedMatch dependency — same reason net_codec.gd and +# input_jitter_buffer.gd are standalone: the table can then be exhaustively +# unit-tested without a live match. +# +# The integer values ARE the wire format. `match_state` has been a u8 in the +# snapshot header since §2.4 (net_codec.gd's pack_snapshot_body_segment), so +# these numbers are protocol, not an implementation detail: never renumber an +# existing state, only append. LOBBY is 0 so a zeroed/placeholder snapshot +# body decodes to a state that is obviously "not in a match" rather than to +# something mid-play. + +enum State { + LOBBY = 0, + LOADING = 1, + WARMUP = 2, + PLAYING = 3, + GOAL_PAUSE = 4, + FULL_TIME = 5, + OVERTIME_WARMUP = 6, + OVERTIME = 7, + RESULTS = 8, +} + +# Legal successors, straight from §6.1's diagram. Enforced rather than +# documented: an illegal transition is a server logic bug, and the failure it +# otherwise produces (clients following the server into a state its own code +# never expected to broadcast) is exactly the kind that shows up as an +# unreproducible field report three phases later. +# +# LOBBY is reachable from ANY state and is handled separately in +# can_transition() rather than being listed nine times — §6.4's "if the last +# human leaves, abort to LOBBY" can fire at any point, including mid-goal. +const _SUCCESSORS := { + State.LOBBY: [State.LOADING], + State.LOADING: [State.WARMUP], + State.WARMUP: [State.PLAYING], + # A goal, or the clock running out. FULL_TIME is entered on the clock even + # if a goal is in flight — §6.2 step 9's clock is authoritative. + State.PLAYING: [State.GOAL_PAUSE, State.FULL_TIME], + # Back to a kickoff, or straight to results when the goal that caused the + # pause also ended the match (golden goal in overtime, or a goal on the + # final tick). + State.GOAL_PAUSE: [State.WARMUP, State.OVERTIME_WARMUP, State.RESULTS], + State.FULL_TIME: [State.OVERTIME_WARMUP, State.RESULTS], + State.OVERTIME_WARMUP: [State.OVERTIME], + State.OVERTIME: [State.GOAL_PAUSE, State.RESULTS], + State.RESULTS: [State.LOBBY], +} + +# States in which the simulation is live and inputs drive ships. Everything +# else freezes bodies (§6.2 steps 6 and 8). Kept as a set here rather than as +# an `if state == PLAYING or state == OVERTIME` scattered through +# NetworkedMatch, so adding a future live state can't miss a site. +const _LIVE := [State.PLAYING, State.OVERTIME] + + +static func is_valid(state: int) -> bool: + return state in State.values() + + +static func is_live(state: int) -> bool: + return state in _LIVE + + +# True when the match is over and the clock should not advance. Distinct from +# `not is_live()`: a WARMUP is not live but the match is very much ongoing. +static func is_terminal(state: int) -> bool: + return state == State.RESULTS or state == State.LOBBY + + +static func can_transition(from_state: int, to_state: int) -> bool: + if not is_valid(from_state) or not is_valid(to_state): + return false + if to_state == State.LOBBY: + return from_state != State.LOBBY # §6.4 abort, from anywhere + return to_state in _SUCCESSORS.get(from_state, []) + + +static func to_name(state: int) -> String: + for key in State.keys(): + if State[key] == state: + return key + return "UNKNOWN(%d)" % state diff --git a/Game/scripts/match_state.gd.uid b/Game/scripts/match_state.gd.uid new file mode 100644 index 00000000..59fd5b07 --- /dev/null +++ b/Game/scripts/match_state.gd.uid @@ -0,0 +1 @@ +uid://b1etnxbdelq1p diff --git a/Game/scripts/networked_match.gd b/Game/scripts/networked_match.gd index ae2b9f15..598547f9 100644 --- a/Game/scripts/networked_match.gd +++ b/Game/scripts/networked_match.gd @@ -20,6 +20,10 @@ extends GameMode # but-dead signal shows a permanently frozen timer rather than correctly # hiding it the way free_play.gd's total absence of the signal does. signal score_changed(score: Dictionary) +# §6.1 task 5.1. Emitted on BOTH peers — server-side when it drives a +# transition, client-side when it follows one — so HUD/camera work can bind to +# one signal regardless of which process it runs in. +signal match_state_changed(state: int, at_tick: int) const NetCodec = preload("res://scripts/net_codec.gd") const NetBodyState = preload("res://scripts/net_body_state.gd") @@ -213,6 +217,27 @@ var _reset_gen := 0 # server only: bumped on every kickoff var _pending_reset_gen_bump := false var _pending_reset_gen_bump_tick := -1 +# §6.1 task 5.1. Authoritative on the server; on a client this mirrors what +# the server last told us, via state_change (prompt, carries at_tick) or the +# snapshot's match_state byte (the catch-up path — see _apply_match_state). +var match_state := MatchState.State.LOADING +var match_state_since_tick := 0 +# Client only: the match_state byte of the most recently decoded snapshot. +# Distinct from `match_state` on purpose — it is what the WIRE said, so a test +# can prove the byte is genuinely populated rather than passing on the +# reliable state_change RPC alone. +var _last_snapshot_match_state := -1 +# Server only: the tick the current state's own timer expires on, or -1 when +# the state has no timer (PLAYING ends on a goal or the clock, not a deadline). +var _state_deadline_tick := -1 +# Placeholder durations. Task 5.3 replaces the WARMUP one with the real +# broadcast kickoff (reset transforms + a countdown derived from server_tick), +# and 5.4 replaces the GOAL_PAUSE one with _goal_pause_seconds() and the +# client-cinematic split. They exist here only so 5.1 drives REAL transitions +# to verify against, rather than a state machine nothing ever moves. +const WARMUP_TICKS := 90 # 1.5s +const GOAL_PAUSE_TICKS := 120 # 2s + func _ready() -> void: add_to_group("game") @@ -238,6 +263,7 @@ func _ready() -> void: MatchSim.match_config_received.connect(_on_match_config_received) MatchSim.snapshot_received.connect(_on_snapshot_received) MatchSim.score_update_received.connect(_on_score_update_received) + MatchSim.state_change_received.connect(_on_state_change_received) _request_match_config_until_received() @@ -301,6 +327,14 @@ func _start_server() -> void: MatchSim.send_match_config(arena_path, peer_ids, teams, spawn_indices) MatchSim.input_received.connect(_on_input_received) + # §6.1: the arena, ball and every slot's ship now exist and match_config is + # out, so LOADING is genuinely over. Task 5.3 gates this on the clients' + # own scene_ready (with a 10s timeout) instead of leaving immediately — + # there is no scene_ready message yet, and inventing half of one here + # would be worse than the honest placeholder. + _apply_match_state(MatchState.State.LOADING, Engine.get_physics_frames()) + _set_match_state(MatchState.State.WARMUP) + func _on_input_received(peer_id: int, decoded: Dictionary) -> void: for slot in _slots: @@ -386,6 +420,72 @@ func _on_input_received(peer_id: int, decoded: Dictionary) -> void: _unknown_sender_input_count += 1 +# --- §6.1 match state machine (task 5.1) ----------------------------------- +# +# Deliberately does NOT gate physics, freezing or input this task. Tasks 5.3 +# and 5.4 own freeze/unfreeze at kickoff and goal, and doing it here would +# both duplicate that work and silently change the conditions every Phase 4 +# prediction gate was measured under. 5.1's job is the machine, the broadcast +# and the client following it. +func _set_match_state(new_state: int) -> void: + if not multiplayer.is_server(): + push_error("NetworkedMatch: only the server may drive match state") + return + if new_state == match_state: + return + if not MatchState.can_transition(match_state, new_state): + # Loud, not silent: this is a server logic error, and the symptom it + # produces otherwise (clients faithfully following into a state the + # server's own code never meant to reach) is near-impossible to + # diagnose from a field report. + push_error("NetworkedMatch: illegal match state transition %s -> %s" % [ + MatchState.to_name(match_state), MatchState.to_name(new_state) + ]) + return + var at_tick := Engine.get_physics_frames() + _apply_match_state(new_state, at_tick) + MatchSim.send_state_change(new_state, at_tick) + + +# The one place either peer's state actually changes, so the signal and the +# bookkeeping cannot drift apart between the server and client paths. +func _apply_match_state(new_state: int, at_tick: int) -> void: + if new_state == match_state: + return + match_state = new_state + match_state_since_tick = at_tick + _state_deadline_tick = -1 + if multiplayer.is_server(): + match new_state: + MatchState.State.WARMUP, MatchState.State.OVERTIME_WARMUP: + _state_deadline_tick = at_tick + WARMUP_TICKS + MatchState.State.GOAL_PAUSE: + _state_deadline_tick = at_tick + GOAL_PAUSE_TICKS + match_state_changed.emit(new_state, at_tick) + + +# Server only, once per physics tick. Advances the states that end on their +# own timer; goal- and clock-driven exits are pushed in from their own events. +func _update_match_state() -> void: + if _state_deadline_tick < 0 or Engine.get_physics_frames() < _state_deadline_tick: + return + match match_state: + MatchState.State.WARMUP: + _set_match_state(MatchState.State.PLAYING) + MatchState.State.OVERTIME_WARMUP: + _set_match_state(MatchState.State.OVERTIME) + MatchState.State.GOAL_PAUSE: + # Task 5.5 decides RESULTS-vs-another-kickoff here once full time + # and overtime exist; until then a goal always leads to a kickoff. + _set_match_state(MatchState.State.WARMUP) + + +func _on_state_change_received(state: int, at_tick: int) -> void: + # Client path. MatchSim already rejected an unknown state value, and the + # server is the only peer allowed to send this (rpc "authority"). + _apply_match_state(state, at_tick) + + func _on_goal_registered(conceding_team: int) -> void: _record_goal(1 - conceding_team) MatchSim.send_score_update(score.duplicate()) @@ -396,6 +496,11 @@ func _on_goal_scored(_conceding_team: int) -> void: reset_ships() _pending_reset_gen_bump = true _pending_reset_gen_bump_tick = Engine.get_physics_frames() + # Only from a live state: GameMode debounces the sensor, but a second goal + # landing while already in GOAL_PAUSE would otherwise be an illegal + # transition and get push_error'd for something that is not a bug. + if multiplayer.is_server() and MatchState.is_live(match_state): + _set_match_state(MatchState.State.GOAL_PAUSE) func _broadcast_snapshot() -> void: @@ -412,7 +517,7 @@ func _broadcast_snapshot() -> void: bodies.append(_ship_to_net_body_state(slot.ship, slot.jitter_buffer.stalled) if is_instance_valid(slot.ship) else NetBodyState.new()) if is_instance_valid(ball): bodies.append(_ball_to_net_body_state(ball)) - var segment := NetCodec.pack_snapshot_body_segment(server_tick, 0, _reset_gen, bodies) + var segment := NetCodec.pack_snapshot_body_segment(server_tick, match_state, _reset_gen, bodies) # Building the shared body segment once and reusing it per peer (rather # than re-encoding per client) is the whole reason §2.4 splits the wire # format into a per-client header + a shared body segment in the first @@ -647,6 +752,27 @@ func _on_snapshot_received(decoded: Dictionary) -> void: _expected_next_snapshot_tick = server_tick + 1 _last_received_snapshot_tick = server_tick _last_snapshot_wall_ms = Time.get_ticks_msec() + # match_state catch-up (§6.1). state_change is reliable, so this is not a + # loss-recovery path — it covers the cases reliability cannot: a client + # that joined mid-match and has not been sent a transition yet, and the + # window between scene load and the first state_change arriving. Snapshots + # carry no at_tick for the transition, so attribute it to this snapshot's + # own server_tick, which is the tightest bound available and is never + # later than the true transition tick. + var snapshot_state: int = decoded["match_state"] + _last_snapshot_match_state = snapshot_state + # The tick guard is load-bearing, not defensive padding. state_change is + # reliable on channel 0 while snapshots are unreliable_ordered on channel + # 2, and ordering is only guaranteed WITHIN a channel — so a state_change + # for tick N routinely arrives before a snapshot that was sent at tick + # N-2 and is still in flight. Without this the client would apply the new + # state, then be dragged straight back by the older snapshot's byte, and + # oscillate on every single transition. Observed exactly that while + # testing a deliberately-broken byte: LOADING -> WARMUP -> LOBBY -> + # PLAYING -> LOBBY -> ... Only accept a byte at least as new as whatever + # told us the current state. + if snapshot_state != match_state and MatchState.is_valid(snapshot_state) and server_tick >= match_state_since_tick: + _apply_match_state(snapshot_state, server_tick) # Per-client header (§2.4): unlike the shared body segment, this is # genuinely this recipient's own — input_buffer_depth is THIS client's # own slot's server-side InputJitterBuffer.depth() at send time, which @@ -931,6 +1057,8 @@ func get_net_debug_stats() -> Dictionary: if _last_local_prediction_comparison.get("authoritative_state", null) != null: server_stalled = (_last_local_prediction_comparison["authoritative_state"] as NetBodyState).stalled return { + "match_state": match_state, + "snapshot_match_state": _last_snapshot_match_state, "input_buffer_depth": _last_known_input_buffer_depth, "input_lead": _input_lead_controller.lead, "input_target_depth": _current_input_target_depth(), @@ -1001,6 +1129,9 @@ func _physics_process(_delta: float) -> void: if _owns_world_simulation(): _respawn_escaped_bodies() if multiplayer.is_server(): + # Before the broadcast, so a transition taken this tick ships in this + # tick's own match_state byte rather than trailing it by one. + _update_match_state() # _physics_process runs after this frame's _integrate_forces. Snapshot # FIRST: the body state therefore still describes the sequence consumed # on the prior callback. Sending after consume mislabeled that old state diff --git a/Game/tests/cases/test_match_state.gd b/Game/tests/cases/test_match_state.gd new file mode 100644 index 00000000..1df06a26 --- /dev/null +++ b/Game/tests/cases/test_match_state.gd @@ -0,0 +1,115 @@ +extends "res://tests/test_case.gd" + +# §6.1 match lifecycle state machine (task 5.1). The table is pure data, so +# it can be checked exhaustively rather than by example — which is the point +# of keeping it out of NetworkedMatch. + +const MatchStateScript = preload("res://scripts/match_state.gd") + + +func test_wire_values_are_stable() -> void: + # These integers ARE the snapshot's match_state byte. Renumbering an + # existing state silently reinterprets every packet from an older peer, + # so pin them: this test failing means a protocol break, not a typo. + assert_eq(MatchStateScript.State.LOBBY, 0, "LOBBY") + assert_eq(MatchStateScript.State.LOADING, 1, "LOADING") + assert_eq(MatchStateScript.State.WARMUP, 2, "WARMUP") + assert_eq(MatchStateScript.State.PLAYING, 3, "PLAYING") + assert_eq(MatchStateScript.State.GOAL_PAUSE, 4, "GOAL_PAUSE") + assert_eq(MatchStateScript.State.FULL_TIME, 5, "FULL_TIME") + assert_eq(MatchStateScript.State.OVERTIME_WARMUP, 6, "OVERTIME_WARMUP") + assert_eq(MatchStateScript.State.OVERTIME, 7, "OVERTIME") + assert_eq(MatchStateScript.State.RESULTS, 8, "RESULTS") + + +func test_every_state_fits_in_the_wire_byte() -> void: + for value in MatchStateScript.State.values(): + assert_true(value >= 0 and value <= 255, "state %d must fit a u8" % value) + + +func test_the_documented_happy_path_is_walkable() -> void: + # §6.1's own diagram, start to finish, including the goal loop. + var path := [ + MatchStateScript.State.LOBBY, MatchStateScript.State.LOADING, + MatchStateScript.State.WARMUP, MatchStateScript.State.PLAYING, + MatchStateScript.State.GOAL_PAUSE, MatchStateScript.State.WARMUP, + MatchStateScript.State.PLAYING, MatchStateScript.State.FULL_TIME, + MatchStateScript.State.OVERTIME_WARMUP, MatchStateScript.State.OVERTIME, + MatchStateScript.State.RESULTS, MatchStateScript.State.LOBBY, + ] + for i in path.size() - 1: + assert_true( + MatchStateScript.can_transition(path[i], path[i + 1]), + "%s -> %s must be legal" % [MatchStateScript.to_name(path[i]), MatchStateScript.to_name(path[i + 1])] + ) + + +func test_illegal_shortcuts_are_rejected() -> void: + var illegal := [ + [MatchStateScript.State.LOBBY, MatchStateScript.State.PLAYING], # must load first + [MatchStateScript.State.LOADING, MatchStateScript.State.PLAYING], # must warm up first + [MatchStateScript.State.WARMUP, MatchStateScript.State.GOAL_PAUSE], # cannot score before play + [MatchStateScript.State.PLAYING, MatchStateScript.State.RESULTS], # must pass full time + [MatchStateScript.State.RESULTS, MatchStateScript.State.PLAYING], # match is over + [MatchStateScript.State.FULL_TIME, MatchStateScript.State.PLAYING], # regulation cannot resume + ] + for pair in illegal: + assert_true( + not MatchStateScript.can_transition(pair[0], pair[1]), + "%s -> %s must be rejected" % [MatchStateScript.to_name(pair[0]), MatchStateScript.to_name(pair[1])] + ) + + +func test_abort_to_lobby_is_reachable_from_anywhere_but_lobby() -> void: + # §6.4: "if the last human leaves, abort to LOBBY" can fire at any point. + for state in MatchStateScript.State.values(): + if state == MatchStateScript.State.LOBBY: + assert_true(not MatchStateScript.can_transition(state, state), "LOBBY -> LOBBY is not a transition") + continue + assert_true( + MatchStateScript.can_transition(state, MatchStateScript.State.LOBBY), + "%s must be able to abort to LOBBY" % MatchStateScript.to_name(state) + ) + + +func test_no_state_transitions_to_itself() -> void: + for state in MatchStateScript.State.values(): + assert_true(not MatchStateScript.can_transition(state, state), "%s -> itself" % MatchStateScript.to_name(state)) + + +func test_every_state_is_reachable_and_can_make_progress() -> void: + # Guards against a state being added to the enum and forgotten in the + # table — an orphan would be broadcastable but a dead end, or unreachable + # but present on the wire. + for state in MatchStateScript.State.values(): + var has_exit := false + var has_entry := false + for other in MatchStateScript.State.values(): + if other != state and MatchStateScript.can_transition(state, other): + has_exit = true + if other != state and MatchStateScript.can_transition(other, state): + has_entry = true + assert_true(has_exit, "%s has no legal exit" % MatchStateScript.to_name(state)) + assert_true(has_entry, "%s is unreachable" % MatchStateScript.to_name(state)) + + +func test_only_playing_and_overtime_are_live() -> void: + # is_live() gates simulation in tasks 5.3/5.4; a warmup or a goal pause + # must never read as live. + assert_true(MatchStateScript.is_live(MatchStateScript.State.PLAYING), "PLAYING is live") + assert_true(MatchStateScript.is_live(MatchStateScript.State.OVERTIME), "OVERTIME is live") + for state in [ + MatchStateScript.State.LOBBY, MatchStateScript.State.LOADING, MatchStateScript.State.WARMUP, + MatchStateScript.State.GOAL_PAUSE, MatchStateScript.State.FULL_TIME, + MatchStateScript.State.OVERTIME_WARMUP, MatchStateScript.State.RESULTS, + ]: + assert_true(not MatchStateScript.is_live(state), "%s must not be live" % MatchStateScript.to_name(state)) + + +func test_unknown_values_are_rejected_rather_than_coerced() -> void: + # A newer server can legitimately send a state this build has never heard + # of; it must be refused, not clamped into a neighbouring valid state. + for bogus in [-1, 9, 42, 255]: + assert_true(not MatchStateScript.is_valid(bogus), "%d is not a valid state" % bogus) + assert_true(not MatchStateScript.can_transition(MatchStateScript.State.PLAYING, bogus), "cannot enter %d" % bogus) + assert_true(not MatchStateScript.can_transition(bogus, MatchStateScript.State.PLAYING), "cannot leave %d" % bogus) diff --git a/Game/tests/cases/test_match_state.gd.uid b/Game/tests/cases/test_match_state.gd.uid new file mode 100644 index 00000000..aedb0288 --- /dev/null +++ b/Game/tests/cases/test_match_state.gd.uid @@ -0,0 +1 @@ +uid://b062q6v6jmur2 diff --git a/Game/tests/networked_match_smoke.gd b/Game/tests/networked_match_smoke.gd index 51a4ea8c..dd2dc8f7 100644 --- a/Game/tests/networked_match_smoke.gd +++ b/Game/tests/networked_match_smoke.gd @@ -18,6 +18,7 @@ var _drive_seconds := DEFAULT_DRIVE_SECONDS var _exercise_ball_contact := false var _exercise_free_flight := false var _exercise_input_transitions := false +var _exercise_match_state := false var _warmup_seconds := 0.0 @@ -35,6 +36,8 @@ func _ready() -> void: _exercise_free_flight = true elif arg == "--exercise-input-transitions": _exercise_input_transitions = true + elif arg == "--exercise-match-state": + _exercise_match_state = true elif arg.begins_with("--warmup-seconds="): _warmup_seconds = maxf(0.0, arg.get_slice("=", 1).to_float()) @@ -94,7 +97,7 @@ func _on_host_player_joined(_peer_id: int, _name: String) -> void: get_tree().root.add_child.call_deferred(hooks) # The host must outlive client settle + drive, plus connection/shutdown # slack. This keeps --drive-seconds useful for sustained prediction QA. - hooks.run_host_check.call_deferred(_settle_seconds + _warmup_seconds + _drive_seconds + 4.0) + hooks.run_host_check.call_deferred(_settle_seconds + _warmup_seconds + _drive_seconds + 4.0, _exercise_match_state) func _on_client_welcomed() -> void: @@ -103,7 +106,7 @@ func _on_client_welcomed() -> void: get_tree().change_scene_to_file.call_deferred("res://scenes/networked_match.tscn") var hooks := preload("res://tests/networked_match_test_hooks.gd").new() get_tree().root.add_child.call_deferred(hooks) - hooks.run_client_check.call_deferred(_settle_seconds, _drive_seconds, _exercise_ball_contact, _exercise_free_flight, _warmup_seconds, _exercise_input_transitions) + hooks.run_client_check.call_deferred(_settle_seconds, _drive_seconds, _exercise_ball_contact, _exercise_free_flight, _warmup_seconds, _exercise_input_transitions, _exercise_match_state) func _on_abuser_welcomed() -> void: diff --git a/Game/tests/networked_match_test_hooks.gd b/Game/tests/networked_match_test_hooks.gd index 2ef73f0e..99cfa770 100644 --- a/Game/tests/networked_match_test_hooks.gd +++ b/Game/tests/networked_match_test_hooks.gd @@ -22,7 +22,7 @@ func _is_networked_match(node: Node) -> bool: return node != null and node.get_script() == NetworkedMatchScript -func run_host_check(lifetime_seconds: float) -> void: +func run_host_check(lifetime_seconds: float, force_goal: bool = false) -> void: await get_tree().create_timer(lifetime_seconds * 0.4).timeout var match_scene := get_tree().current_scene var ok := _is_networked_match(match_scene) @@ -40,7 +40,21 @@ func run_host_check(lifetime_seconds: float) -> void: var success := ok and ship_count == 1 and ball_ok print("SMOKE %s: host spawn check (ship_count=%d, ball_ok=%s)" % ["PASS" if success else "FAIL", ship_count, str(ball_ok)]) + if force_goal and ok: + # Drive a real PLAYING -> GOAL_PAUSE -> WARMUP -> PLAYING cycle so the + # client has a goal transition to follow. Teleporting the ball into a + # goal is the same deterministic trick the CI driver and Phase 2's + # goal-reset-ordering fix both use — two low-skill peers scoring + # naturally inside a short run is not reliable enough to gate on. + var goals: Array = match_scene.arena.get_goals() if match_scene.arena else [] + if is_instance_valid(match_scene.ball) and not goals.is_empty(): + match_scene.ball.linear_velocity = Vector3.ZERO + match_scene.ball.global_position = goals[0].global_position + print("SMOKE INFO: host forced a goal to exercise the GOAL_PAUSE transition") + await get_tree().create_timer(lifetime_seconds * 0.6).timeout + if force_goal and _is_networked_match(match_scene): + print("SMOKE INFO: host final match_state=%s" % MatchState.to_name(match_scene.match_state)) if _is_networked_match(match_scene) and not match_scene.ships.is_empty(): var ship: Ship = match_scene.ships[0] print("SMOKE INFO: host ship final position=%s action=%s (spawned, driven by client input if any arrived)" % [str(ship.global_position), str(ship.get_current_action_copy().thrust)]) @@ -48,7 +62,35 @@ func run_host_check(lifetime_seconds: float) -> void: get_tree().quit(0 if success else 1) -func run_client_check(settle_seconds: float, drive_seconds: float, exercise_ball_contact: bool = false, exercise_free_flight: bool = false, warmup_seconds: float = 0.0, exercise_input_transitions: bool = false) -> void: +func run_client_check(settle_seconds: float, drive_seconds: float, exercise_ball_contact: bool = false, exercise_free_flight: bool = false, warmup_seconds: float = 0.0, exercise_input_transitions: bool = false, exercise_match_state: bool = false) -> void: + # Subscribed BEFORE the settle wait, not after: the server leaves LOADING + # and enters WARMUP as soon as _start_server() finishes, and PLAYING 90 + # ticks later — both would already be history by the time a post-settle + # listener attached, and the test would silently observe nothing. + var observed_states: Array[int] = [] + var observed_ticks: Array[int] = [] + if exercise_match_state: + # change_scene_to_file is deferred, and so is this call — current_scene + # is still the smoke driver for the first few frames, so connecting + # immediately silently observes nothing at all (it did: empty list). + # Poll until the real scene exists, bounded so a genuine failure to + # load reports as an empty observation rather than hanging. + var deadline := Time.get_ticks_msec() + int(settle_seconds * 1000.0) + while Time.get_ticks_msec() < deadline and not _is_networked_match(get_tree().current_scene): + await get_tree().process_frame + var state_scene := get_tree().current_scene + if _is_networked_match(state_scene): + # Seed with whatever the client has already converged to. The + # server may legitimately have reached PLAYING before this client + # finished loading — that is the snapshot-byte catch-up path doing + # its job, not a missed transition. + observed_states.append(state_scene.match_state) + observed_ticks.append(state_scene.match_state_since_tick) + state_scene.match_state_changed.connect(func(s: int, at_tick: int) -> void: + observed_states.append(s) + observed_ticks.append(at_tick) + ) + await get_tree().create_timer(settle_seconds).timeout var match_scene := get_tree().current_scene @@ -261,7 +303,57 @@ func run_client_check(settle_seconds: float, drive_seconds: float, exercise_ball and int(net_stats.get("ball_blend_complete_count", 0)) > 0 \ and int(net_stats.get("ball_blend_max_duration_ms", BALL_BLEND_ACCEPTANCE_MS)) <= BALL_BLEND_ACCEPTANCE_MS \ and proxy_motion_ok) - var success := verification_movement > 1.0 and local_prediction_ok and prediction_quality_ok and ball_contact_ok + # §6.1 task 5.1: the client must FOLLOW the server's machine, not run its + # own. Assert three separable things — that transitions arrived at all, + # that every consecutive pair is legal per the shared table (so the client + # never lands somewhere the server could not have sent it), and that the + # specific documented sequence for this scenario was observed. + var match_state_ok := true + if exercise_match_state: + var names: Array[String] = [] + for s in observed_states: + names.append(MatchState.to_name(s)) + for i in observed_states.size() - 1: + if not MatchState.can_transition(observed_states[i], observed_states[i + 1]): + print("SMOKE FAIL: client observed an illegal transition %s -> %s" % [names[i], names[i + 1]]) + match_state_ok = false + # Ticks are absolute and monotonic; a transition attributed to an + # earlier tick than its predecessor means the at_tick plumbing is wrong. + for i in observed_ticks.size() - 1: + if observed_ticks[i + 1] < observed_ticks[i]: + print("SMOKE FAIL: transition ticks went backwards: %s" % str(observed_ticks)) + match_state_ok = false + var reached_playing := MatchState.State.PLAYING in observed_states + var saw_goal_pause := MatchState.State.GOAL_PAUSE in observed_states + # A goal must lead back to a kickoff, not leave the match parked. + var resumed_after_goal := false + for i in observed_states.size() - 1: + if observed_states[i] == MatchState.State.GOAL_PAUSE and observed_states[i + 1] == MatchState.State.WARMUP: + resumed_after_goal = true + if not (reached_playing and saw_goal_pause and resumed_after_goal): + match_state_ok = false + # The snapshot's match_state byte must carry the real state too, not a + # hardcoded 0. Everything above is driven by the reliable state_change + # RPC and would pass identically with a dead byte — which is exactly + # how Phase 4's mislabelled prediction history survived every gate. + # The byte is the only channel a late joiner or a client that missed a + # transition has (§6.3), so assert it independently. + var wire_state := int(net_stats.get("snapshot_match_state", -1)) + var live_state := int(net_stats.get("match_state", -1)) + if wire_state != live_state or not MatchState.is_valid(wire_state): + print("SMOKE FAIL: snapshot match_state byte is %s but the client is in %s" % [ + MatchState.to_name(wire_state), MatchState.to_name(live_state) + ]) + match_state_ok = false + if wire_state == MatchState.State.LOBBY: + print("SMOKE FAIL: snapshot match_state byte reads LOBBY (0) mid-match — likely never populated") + match_state_ok = false + print("SMOKE %s: client followed the server's match state (%s; reached_playing=%s goal_pause=%s resumed=%s ticks=%s)" % [ + "PASS" if match_state_ok else "FAIL", " -> ".join(names), + str(reached_playing), str(saw_goal_pause), str(resumed_after_goal), str(observed_ticks), + ]) + + var success := verification_movement > 1.0 and local_prediction_ok and prediction_quality_ok and ball_contact_ok and match_state_ok print("SMOKE %s: client locally predicted %.2fm horizontal, local_prediction_ok=%s prediction_quality_ok=%s" % [ "PASS" if success else "FAIL", moved_horizontal, str(local_prediction_ok), str(prediction_quality_ok) ]) diff --git a/multiplayer-todo.md b/multiplayer-todo.md index e6bc34c9..eb92ae7a 100644 --- a/multiplayer-todo.md +++ b/multiplayer-todo.md @@ -964,7 +964,7 @@ Note the free-flight p99 **improved** (0.170/0.176/0.184 → 0.141/0.168/0.154) | # | Task | Acceptance | |---|---|---| -| 5.1 `[D:2.1]` | Server state machine, `state_change` broadcast, `match_state` snapshot byte | Clients follow every transition | +| 5.1 `[D:2.1]` | **DONE.** `scripts/match_state.gd` (enum + validated transition table, pure/unit-testable), server-driven machine in `NetworkedMatch`, `state_change` RPC on reliable channel 0 carrying an absolute `at_tick`, and the snapshot `match_state` byte populated for real | Client observed `LOADING -> WARMUP -> PLAYING -> GOAL_PAUSE -> WARMUP` with monotonic ticks in a real two-process run; every consecutive pair legal; wire byte asserted independently of the RPC | | 5.2 `[D:5.1]` | Tick-derived clock replacing the `Timer` + `_process` polling; `clock_state` RPC; goal-time freeze as `end_tick += (resume_tick - goal_tick)` | Clocks agree across peers to within a tick; no float drift across 10 goals | | 5.3 `[D:5.1]` | `kickoff` RPC with broadcast transforms, freeze/unfreeze, `reset_gen`, countdown derived from `server_tick`, **and the specified late-arrival behaviour** | A `kickoff` delayed past `resume_tick` applies immediately without a negative countdown | | 5.4 `[D:5.1]` | `goal_scored` RPC; server-side pause window via `_goal_pause_seconds()` and `_set_frozen()` (**never `Engine.time_scale`**); client cinematic split from timing | Server reset no longer fires while clients are mid-celebration | @@ -979,6 +979,27 @@ Note the free-flight p99 **improved** (0.170/0.176/0.184 → 0.141/0.168/0.154) > Task 5.10 is the highest-value debuggability investment here. The packets are already flat bytes, so it is ~50 lines. Without it, "my ship snapped" is permanently unreproducible from a field report — the CI gate catches regressions, but it cannot debug a player's bad night. +#### Task 5.1 notes + +`scripts/match_state.gd` holds the enum and the §6.1 transition table as pure data with no scene/RPC dependency — the same reason `net_codec.gd` and `input_jitter_buffer.gd` are standalone — so the table is checked exhaustively (every state reachable, every state has an exit, no self-transitions, abort-to-LOBBY from anywhere, illegal shortcuts rejected) rather than by example. **The enum's integer values are the wire format**, pinned by a test: `match_state` has been a `u8` in the snapshot header since §2.4, so renumbering an existing state silently reinterprets packets from an older peer. Only append. + +The server validates every transition and `push_error`s an illegal one rather than following it, because the symptom otherwise — clients faithfully following into a state the server's own code never meant to reach — is near-impossible to diagnose from a field report. + +**Two channels carry the state, deliberately.** `state_change` (reliable, channel 0) is prompt and carries the absolute `at_tick`; the snapshot's `match_state` byte is the catch-up path for a client that has not been sent a transition yet — a late joiner (§6.3), or the window between scene load and the first RPC. **The byte needs a tick guard**: snapshots are `unreliable_ordered` on channel 2 and ordering holds only *within* a channel, so a `state_change` for tick N routinely arrives before an in-flight snapshot from tick N-2. Without the guard the client applies the new state and is immediately dragged back by the older byte, oscillating on every transition — observed directly (`LOADING -> WARMUP -> LOBBY -> PLAYING -> LOBBY -> ...`) while running a deliberately-broken-byte control. Only a byte at least as new as `match_state_since_tick` is accepted. + +The client deliberately does **not** enforce the transition table — authoritative state must be accepted, and a late joiner legitimately jumps straight to `PLAYING`. The table is a server-side invariant. The smoke test asserts legality of what the client *observes*, seeding its first sample from whatever state the client converged to rather than counting that as a transition, so late-loading clients (seen seeding at `WARMUP` rather than `LOADING`) still pass. + +**5.1 does not gate physics, freezing or input on state.** Tasks 5.3 and 5.4 own freeze/unfreeze at kickoff and goal; doing it here would both duplicate that work and change the conditions every Phase 4 prediction gate was measured under. `MatchState.is_live()` exists for them to use. `WARMUP_TICKS`/`GOAL_PAUSE_TICKS` are honest placeholders so 5.1 drives *real* transitions to verify against — 5.3 replaces the first with the broadcast kickoff (reset transforms + countdown from `server_tick`), 5.4 the second with `_goal_pause_seconds()` and the client-cinematic split. The server also leaves `LOADING` immediately rather than waiting for `scene_ready`, which does not exist yet (5.3). + +New smoke flag `--exercise-match-state` (pass to **both** roles — the host forces a goal to drive a `GOAL_PAUSE` cycle, the client records and validates the sequence): + +``` +godot --headless --path Game res://tests/networked_match_smoke.tscn -- --role=host --drive-seconds=6 --exercise-match-state +godot --headless --path Game res://tests/networked_match_smoke.tscn -- --role=client --drive-seconds=6 --exercise-match-state +``` + +Verified against a control: hardcoding the snapshot byte back to `0` fails both the byte assertion and the transition-legality assertion. That control is why the gate asserts the wire byte separately from the RPC at all — everything else in the check is RPC-driven and would pass identically with a dead byte, which is exactly how Phase 4's mislabelled history survived every gate (gotcha 47). + **Phase gate:** a full 3v3 start-to-finish including a mid-match disconnect and a late joiner. ### Phase 6 — Dedicated server productionisation