From ff725e1ffab2645469f4069f6d8ffefbbd60f5c5 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:47:35 +0100 Subject: [PATCH] =?UTF-8?q?feat(multiplayer):=20=C2=A76.3=20late=20joiners?= =?UTF-8?q?=20take=20a=20vacated=20slot=20at=20the=20next=20kickoff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Spectate now, take the slot at the next kickoff" was a print statement. The server logged it and never acted; on the client, _is_spectator was assigned once in _on_match_config_received and never revisited - and that handler returns early whenever _slots is non-empty, so no rebroadcast could promote an in-match spectator. The reconnect path only worked because a returning player is a fresh process. Server: late joiners are queued in arrival order and the queue is drained from _begin_kickoff, before the reset transforms are read, so a promoted player's ship is placed by that same kickoff and the controller swap lands on an already-frozen body. A slot is available only once its player has gone AND their 30s reservation has lapsed - §6.4 outranks §6.3, since taking a reserved slot would quietly break the reconnect promise. _abort_if_abandoned now counts a waiting spectator as somebody present, or the one person queued for the slot that just opened is dumped to the lobby at the moment they were about to get it. Client: new broadcast slot_assigned (reliable, channel 0). Broadcast because every client holds its own slot list and one naming the wrong peer keeps flying somebody else's ship as a remote body; reliable because no per-snapshot field would re-converge a client that missed it. The promoted client undoes what made the body remote - fresh interpolator, physics interpolation back on, offsets cleared - and deliberately does not unfreeze, clearing _local_prediction_ready so the next snapshot teleports it to a real authoritative pose first. The controller-attach block moved to _take_local_ownership rather than being copied. New --role=host-latejoin/--role=client-latejoin and --slot-reservation-seconds=. Verified 4/4 both sides: queued, NOT promoted merely because the reservation lapsed, takes the slot at the kickoff, same ship instance, and both peers independently measure ~45.7m under its input. Control with a 90s reservation: kickoff fires, nothing is promoted, the slot still reads the departed player's name. --- Game/scripts/match_sim.gd | 17 ++ Game/scripts/networked_match.gd | 207 ++++++++++++++++++++--- Game/tests/networked_match_smoke.gd | 39 +++++ Game/tests/networked_match_test_hooks.gd | 200 ++++++++++++++++++++++ multiplayer-todo.md | 14 +- 5 files changed, 447 insertions(+), 30 deletions(-) diff --git a/Game/scripts/match_sim.gd b/Game/scripts/match_sim.gd index 0372b294..3f3d19af 100644 --- a/Game/scripts/match_sim.gd +++ b/Game/scripts/match_sim.gd @@ -38,6 +38,8 @@ signal kickoff_received(positions: PackedVector3Array, rotations: PackedFloat32A signal goal_scored_received(scoring_team: int, score: Dictionary, goal_tick: int, resume_tick: int) signal clock_state_received(running: bool, end_tick: int, remaining_ticks: int, at_tick: int) signal match_bootstrap_received(state: int, at_tick: int, score: Dictionary, end_tick: int, clock_running: bool, reset_gen: int, remaining_ticks: int) +# §6.3 task 5.8: a spectator has been given a vacated slot at a kickoff. +signal slot_assigned_received(peer_id: int, slot_index: int) # 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- @@ -314,11 +316,26 @@ func send_match_bootstrap(peer_id: int, state: int, at_tick: int, score: Diction _match_bootstrap.rpc_id(peer_id, state, at_tick, score, end_tick, clock_running, reset_gen, remaining_ticks) +# §6.3's late-joiner promotion. BROADCAST, not addressed to the new owner +# alone: every client holds its own copy of the slot list, and a peer_id that +# only the promoted client learns about leaves everyone else's copy naming a +# player who is no longer in that seat. Reliable channel 0 — a client that +# misses this keeps flying somebody else's ship as a remote body forever, and +# unlike match_state there is no per-snapshot field that would re-converge it. +func send_slot_assigned(peer_id: int, slot_index: int) -> void: + _slot_assigned.rpc(peer_id, slot_index) + + @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) +@rpc("authority", "call_remote", "reliable", 0) +func _slot_assigned(peer_id: int, slot_index: int) -> void: + slot_assigned_received.emit(peer_id, slot_index) + + @rpc("any_peer", "call_remote", "reliable", 0) func _request_match_config() -> void: if not multiplayer.is_server() or _last_match_config.is_empty(): diff --git a/Game/scripts/networked_match.gd b/Game/scripts/networked_match.gd index a82691a6..22fbf8f6 100644 --- a/Game/scripts/networked_match.gd +++ b/Game/scripts/networked_match.gd @@ -303,6 +303,9 @@ var _client_goal_resume_tick := -1 # §6.3 (task 5.8), client only. var _is_spectator := false var _spectator_target_index := 0 +# §6.3, server only. Peers that joined mid-match with no slot to reclaim, in +# arrival order, waiting for the next kickoff to hand them a vacated slot. +var _late_joiners: Array[Dictionary] = [] # §6.3's "cap with --max-spectators". Server only; 0 disables spectating # entirely, negative means unlimited. var _max_spectators := -1 @@ -335,6 +338,8 @@ func _ready() -> void: _replay_log = null else: print("NetworkedMatch: recording replay log to %s" % replay_path) + elif arg.begins_with("--slot-reservation-seconds="): + _slot_reservation_seconds = maxf(0.0, arg.get_slice("=", 1).to_float()) elif arg.begins_with("--match-length="): # Regulation is 150s; a smoke test cannot wait that long to see # FULL_TIME/OVERTIME/RESULTS, so allow an override. Server-side @@ -363,6 +368,7 @@ func _ready() -> void: MatchSim.goal_scored_received.connect(_on_goal_scored_received) MatchSim.clock_state_received.connect(_on_clock_state_received) MatchSim.match_bootstrap_received.connect(_on_match_bootstrap_received) + MatchSim.slot_assigned_received.connect(_on_slot_assigned) # lobby.gd does this; the match scene never did. Without it a client # whose host exits stays in a dead match forever, emitting thousands of # "multiplayer instance isn't currently active" / "RPC via a peer which @@ -676,6 +682,9 @@ func _apply_match_state(new_state: int, at_tick: int) -> void: # needs both sides to consume the stream in identical order forever and the # first randf() anyone adds to the reset path desyncs kickoff silently. func _begin_kickoff() -> void: + # §6.3: before the reset, so a promoted player's ship is placed by this very + # kickoff rather than left wherever its previous owner abandoned it. + _promote_late_joiners() reset_ball() reset_ships() # Bump before the broadcast so the kickoff and the reset_gen it announces @@ -1077,19 +1086,28 @@ func _on_goal_registered(conceding_team: int) -> void: # --- §6.4 disconnects and reconnects (tasks 5.6/5.7) ----------------------- const SLOT_RESERVATION_SECONDS := 30.0 +# Server only, --slot-reservation-seconds=. §6.3's promotion can only happen at +# a kickoff AFTER the departed player's reservation lapses, so a smoke test of +# it would otherwise have to run for over half a minute before the interesting +# moment. Same rationale and same shape as --match-length: a server-side +# override, never something a client can shorten for anyone. +var _slot_reservation_seconds := SLOT_RESERVATION_SECONDS func _on_client_disconnected(peer_id: int) -> void: if not multiplayer.is_server(): return + # A spectator waiting for a slot can leave too, and a queue entry for a + # departed peer would hand the next free slot to nobody. + _forget_late_joiner(peer_id) for slot in _slots: if slot.peer_id != peer_id or slot.disconnected: continue slot.disconnected = true - slot.reserved_until_tick = Engine.get_physics_frames() + int(SLOT_RESERVATION_SECONDS * SimConstants.TICK_HZ) + slot.reserved_until_tick = Engine.get_physics_frames() + int(_slot_reservation_seconds * SimConstants.TICK_HZ) _swap_slot_controller(slot, _build_takeover_controller()) print("NetworkedMatch: peer %d (%s) disconnected; ship kept, slot reserved for %.0fs" % [ - peer_id, slot.player_name, SLOT_RESERVATION_SECONDS + peer_id, slot.player_name, _slot_reservation_seconds ]) break _abort_if_abandoned() @@ -1111,6 +1129,14 @@ func _abort_if_abandoned() -> void: return # somebody is still playing if slot.reserved_until_tick >= 0 and now <= slot.reserved_until_tick: return # somebody may still come back + # §6.3's queue counts as "somebody is still here" for the same reason the + # reservation does. Without this, a spectator waiting for the slot that just + # opened up is dumped back to the lobby at the exact moment they were about + # to get it — and they are a connected human watching a live match, which is + # not what "abandoned" means. + for entry in _late_joiners: + if int(entry["peer_id"]) in multiplayer.get_peers(): + return print("NetworkedMatch: no players left and no reservations outstanding, aborting to lobby") _set_match_state(MatchState.State.LOBBY) get_tree().change_scene_to_file.call_deferred(ScenePaths.LOBBY) @@ -1192,9 +1218,75 @@ func _on_player_joined_midmatch(peer_id: int, player_name: String) -> void: # §6.3: a spectator/late joiner reconstructs from this, since match_config # carries arena and roster only — no score, clock or match state. _send_match_bootstrap(peer_id) + # "Spectate now, take the slot at the next kickoff" — queued here, acted on + # in _promote_late_joiners(). Queued in arrival order and consumed from the + # front, so waiting is first-come-first-served rather than whichever slot + # index happens to free up first. + _late_joiners.append({"peer_id": peer_id, "player_name": player_name}) print("NetworkedMatch: peer %d (%s) joined mid-match; spectating until the next kickoff" % [peer_id, player_name]) +# §6.3's "free slot mid-match → spectate now, take the slot at the next +# kickoff". Called from _begin_kickoff BEFORE the reset transforms are read, so +# a promoted player's ship is placed by the same kickoff everyone else gets and +# the controller swap lands on an already-frozen body — which is the whole +# reason the spec puts it at a kickoff boundary rather than mid-play. +# +# A slot is available when its player has gone AND their 30s reservation has +# lapsed (§6.4). Taking a still-reserved slot would quietly break the reconnect +# promise, so the reservation always outranks the queue. +func _promote_late_joiners() -> void: + if not multiplayer.is_server() or _late_joiners.is_empty(): + return + var connected := multiplayer.get_peers() + # A queued joiner may have left again while waiting. Drop them here rather + # than handing a slot to a peer that no longer exists — which would look + # exactly like an occupied slot nobody is playing. + var waiting: Array[Dictionary] = [] + for entry in _late_joiners: + if int(entry["peer_id"]) in connected: + waiting.append(entry) + _late_joiners = waiting + + var now := Engine.get_physics_frames() + var promoted := false + for index in _slots.size(): + if _late_joiners.is_empty(): + break + var slot := _slots[index] + if not slot.disconnected: + continue + if slot.reserved_until_tick >= 0 and now <= slot.reserved_until_tick: + continue + var joiner: Dictionary = _late_joiners.pop_front() + var joiner_peer := int(joiner["peer_id"]) + slot.peer_id = joiner_peer + slot.player_name = String(joiner["player_name"]) + slot.disconnected = false + slot.reserved_until_tick = -1 + # Same reasoning as the reclaim path: the arriving client numbers its + # input sequence from scratch, and the old cursor belongs to a different + # epoch entirely (see input_jitter_buffer.gd's seeding comment). + slot.jitter_buffer = InputJitterBuffer.new() + slot.consecutive_seq_rejects = 0 + _swap_slot_controller(slot, RLShipController.new()) + MatchSim.send_slot_assigned(joiner_peer, index) + promoted = true + print("NetworkedMatch: peer %d (%s) took slot %d at the kickoff" % [joiner_peer, slot.player_name, index]) + if promoted: + # Same cache hazard the reclaim path documents: MatchSim replays the + # last match_config to anyone who asks, and it now names the wrong peer + # for this slot. + _rebroadcast_match_config() + + +func _forget_late_joiner(peer_id: int) -> void: + for i in _late_joiners.size(): + if int(_late_joiners[i]["peer_id"]) == peer_id: + _late_joiners.remove_at(i) + return + + # Connected peers that hold no slot. Counted from the live peer list rather # than tracked incrementally, so a spectator that drops cannot leak a unit of # the cap permanently. @@ -1437,37 +1529,96 @@ func _on_match_config_received(arena_path: String, peer_ids: PackedInt32Array, t print("NetworkedMatch: no slot for this peer — spectating (%d ship(s) + ball)" % _slots.size()) if is_instance_valid(_my_slot) and is_instance_valid(_my_slot.ship): spawn_camera_rig(_my_slot.ship) - _my_slot.ship.ball_contact.connect(_on_local_ball_contact) - # Headless training ships intentionally do not install Ship's render-side - # body_entered signal. Attach this client-only callback only to the - # locally predicted match ship so contact QA sees the same event without - # changing training instances. - if DisplayServer.get_name() == "headless": - _my_slot.ship.body_entered.connect(_on_local_ship_body_entered) - if not _test_bot_model_path.is_empty(): - # --test-bot (task 3.6): attach a real - # AIShipController. Unlike PlayerShipController, this one needs - # real scene context (get_parent() as Ship for itself, plus - # ball/teammate/opponent discovery via groups) — Ship.set_controller() - # parents it correctly, satisfying that. Known limitation: this - # local bot controller to the genuinely simulated local ship. - var bot := AIShipController.new() - bot.model_path = _test_bot_model_path - _my_slot.ship.add_child(bot) - _local_input_timeline = LocalInputTimeline.new() - _local_net_controller = LocalNetShipController.new(bot, _local_input_timeline) - _my_slot.ship.set_controller(_local_net_controller) - else: - var player := PlayerShipController.new() - _local_input_timeline = LocalInputTimeline.new() - _local_net_controller = LocalNetShipController.new(player, _local_input_timeline) - _local_net_controller.add_child(player) - _my_slot.ship.set_controller(_local_net_controller) + _take_local_ownership(_my_slot) # The roster now exists, so a kickoff that raced ahead of match_config can # finally be placed against the right bodies. _apply_pending_kickoff() +# Client only. Everything that makes one of the spawned ships THIS peer's own: +# contact hooks and the local input controller. Factored out of +# _on_match_config_received because §6.3's late-joiner promotion needs the +# identical setup at a completely different moment, and a second copy of it +# would be a copy that silently drifts. +func _take_local_ownership(slot: SlotInfo) -> void: + slot.ship.ball_contact.connect(_on_local_ball_contact) + # Headless training ships intentionally do not install Ship's render-side + # body_entered signal. Attach this client-only callback only to the + # locally predicted match ship so contact QA sees the same event without + # changing training instances. + if DisplayServer.get_name() == "headless": + slot.ship.body_entered.connect(_on_local_ship_body_entered) + if not _test_bot_model_path.is_empty(): + # --test-bot (task 3.6): attach a real + # AIShipController. Unlike PlayerShipController, this one needs + # real scene context (get_parent() as Ship for itself, plus + # ball/teammate/opponent discovery via groups) — Ship.set_controller() + # parents it correctly, satisfying that. Known limitation: this + # local bot controller to the genuinely simulated local ship. + var bot := AIShipController.new() + bot.model_path = _test_bot_model_path + slot.ship.add_child(bot) + _local_input_timeline = LocalInputTimeline.new() + _local_net_controller = LocalNetShipController.new(bot, _local_input_timeline) + slot.ship.set_controller(_local_net_controller) + else: + var player := PlayerShipController.new() + _local_input_timeline = LocalInputTimeline.new() + _local_net_controller = LocalNetShipController.new(player, _local_input_timeline) + _local_net_controller.add_child(player) + slot.ship.set_controller(_local_net_controller) + + +# §6.3 (task 5.8), client only: the server has handed this peer a vacated slot +# at a kickoff. Broadcast, so every client runs the first half — their own copy +# of the slot list must name the new owner — and only the promoted peer runs +# the second. +func _on_slot_assigned(peer_id: int, slot_index: int) -> void: + if multiplayer.is_server() or slot_index < 0 or slot_index >= _slots.size(): + return + var slot := _slots[slot_index] + slot.peer_id = peer_id + if peer_id != multiplayer.get_unique_id() or not _is_spectator: + return + + # This body has been a REMOTE one until now: driven by transform writes from + # the interpolator, with Godot's own physics interpolation switched off so + # those writes could not fight it (§4.6). Both have to be undone, and the + # interpolator emptied — its buffered samples describe the previous owner's + # flight and would otherwise be smoothed into the first predicted frames. + _my_slot = slot + _is_spectator = false + slot.interpolator = NetInterpolator.new() + slot.visual_smoother_reset = true + slot.visual_position_offset = Vector3.ZERO + slot.visual_rotation_offset = Quaternion.IDENTITY + if is_instance_valid(slot.ship) and is_instance_valid(slot.ship.visual): + slot.ship.visual.physics_interpolation_mode = Node.PHYSICS_INTERPOLATION_MODE_INHERIT + # Stay frozen until the first authoritative pose arrives, exactly as a fresh + # client does — _on_snapshot_received teleports to it, unfreezes, and starts + # prediction. Unfreezing here instead would predict from whatever pose the + # interpolator last wrote, which is a render-side approximation. + _local_prediction_ready = false + _input_seq = 0 + # Same call the reset path uses: everything recorded so far belongs to a + # peer that was not simulating anything. + _local_prediction_history.begin_epoch() + _pending_local_reconciliation = {} + _take_local_ownership(slot) + # The HUD was built in spectator mode, which hides the ship instruments and + # wires nothing to a ship. It reads spectator_mode once, a frame after + # _ready, so flipping the flag on the live instance does nothing — rebuild. + if is_instance_valid(hud): + hud.queue_free() + _spawn_hud() + if is_instance_valid(_camera_rig): + _camera_rig.target = slot.ship + hud.ship = slot.ship + else: + spawn_camera_rig(slot.ship) + print("NetworkedMatch: promoted from spectator to player in slot %d" % slot_index) + + func _spawn_hud() -> void: hud = HUD_SCENE.instantiate() # BEFORE add_child: HUDController reads this in _initialize_hud(), which diff --git a/Game/tests/networked_match_smoke.gd b/Game/tests/networked_match_smoke.gd index b47df320..0b3aa6b7 100644 --- a/Game/tests/networked_match_smoke.gd +++ b/Game/tests/networked_match_smoke.gd @@ -50,6 +50,17 @@ func _ready() -> void: return print("SMOKE: hosting (disconnect/reconnect scenario) on port %d ..." % PORT) MatchNet.player_joined.connect(_on_disconnect_host_player_joined) + "host-latejoin": + # §6.3: a spectator takes a vacated slot at the next kickoff. Run + # with --slot-reservation-seconds= small, a plain `client` that + # leaves, and a `client-latejoin` watching. + var lerr := NetworkManager.host(PORT) + if lerr != OK: + print("SMOKE FAIL: host() failed: %s" % error_string(lerr)) + get_tree().quit(1) + return + print("SMOKE: hosting (late-joiner promotion scenario) on port %d ..." % PORT) + MatchNet.player_joined.connect(_on_latejoin_host_player_joined) "host": var err := NetworkManager.host(PORT) if err != OK: @@ -79,6 +90,17 @@ func _ready() -> void: return print("SMOKE: rejoining to reclaim a reserved slot ...") MatchNet.welcomed.connect(_on_reconnect_welcomed) + "client-latejoin": + # A name nobody reserved, so it starts as a spectator and can only + # become a player via §6.3's kickoff promotion. + MatchNet.local_player_name = "LateComer" + var jerr := NetworkManager.join("127.0.0.1", PORT) + if jerr != OK: + print("SMOKE FAIL: join() failed: %s" % error_string(jerr)) + get_tree().quit(1) + return + print("SMOKE: joining late, expecting to spectate then be promoted ...") + MatchNet.welcomed.connect(_on_latejoin_welcomed) "client-spectator": # A name nobody reserved, so the server has no slot for it. MatchNet.local_player_name = "Watcher" @@ -148,6 +170,23 @@ func _on_disconnect_host_player_joined(_peer_id: int, _name: String) -> void: hooks.run_disconnect_host_check.call_deferred(_drive_seconds) +func _on_latejoin_host_player_joined(_peer_id: int, _name: String) -> void: + MatchNet.player_joined.disconnect(_on_latejoin_host_player_joined) + print("SMOKE: host loading networked_match.tscn (late-joiner scenario) ...") + 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_late_joiner_host_check.call_deferred(_drive_seconds) + + +func _on_latejoin_welcomed() -> void: + MatchNet.welcomed.disconnect(_on_latejoin_welcomed) + 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_late_joiner_client_check.call_deferred(_drive_seconds) + + func _on_reconnect_welcomed() -> void: MatchNet.welcomed.disconnect(_on_reconnect_welcomed) print("SMOKE: reconnecting client loading networked_match.tscn ...") diff --git a/Game/tests/networked_match_test_hooks.gd b/Game/tests/networked_match_test_hooks.gd index 8b533ba9..48e88d22 100644 --- a/Game/tests/networked_match_test_hooks.gd +++ b/Game/tests/networked_match_test_hooks.gd @@ -797,6 +797,206 @@ func run_spectator_check(run_seconds: float) -> void: get_tree().quit(0 if success else 1) +# §6.3's "free slot mid-match → spectate now, take the slot at the next +# kickoff", server side. The sequence this drives: a player leaves, their §6.4 +# reservation lapses (run with --slot-reservation-seconds= small, or this waits +# 30 real seconds for the interesting moment), a goal is forced to produce a +# kickoff, and the waiting spectator must be holding the slot afterwards. +# +# The forced goal is the same deterministic trick the CI driver and the +# match-state check use — waiting for two peers to score naturally inside a +# short run is not something to gate on. +func run_late_joiner_host_check(lifetime_seconds: float) -> void: + await get_tree().create_timer(2.0).timeout + var match_scene := get_tree().current_scene + if not _is_networked_match(match_scene): + print("SMOKE FAIL: host scene is not NetworkedMatch") + get_tree().quit(1) + return + if match_scene._slots.is_empty(): + print("SMOKE FAIL: host has no slots — the first client never joined") + NetworkManager.shutdown() + get_tree().quit(1) + return + var original_peer: int = match_scene._slots[0].peer_id + var original_name: String = match_scene._slots[0].player_name + var ship_before = match_scene._slots[0].ship + + # Wait for the seated player to drop. + var drop_deadline := Time.get_ticks_msec() + int(lifetime_seconds * 1000.0) + while Time.get_ticks_msec() < drop_deadline and _is_networked_match(match_scene) and not match_scene._slots[0].disconnected: + await get_tree().physics_frame + if not _is_networked_match(match_scene) or not match_scene._slots[0].disconnected: + print("SMOKE FAIL: the seated player never dropped") + NetworkManager.shutdown() + get_tree().quit(1) + return + # A spectator must be queued by now, or the rest of this proves nothing. + var queued: int = match_scene._late_joiners.size() + + # Then for the reservation to lapse. Until it does, the slot belongs to the + # player who left — §6.4 outranks §6.3, and taking it early would quietly + # break the reconnect promise. + var lapse_deadline := Time.get_ticks_msec() + int(lifetime_seconds * 1000.0) + 32000 + while Time.get_ticks_msec() < lapse_deadline and _is_networked_match(match_scene) \ + and match_scene._slots[0].reserved_until_tick >= 0 \ + and Engine.get_physics_frames() <= match_scene._slots[0].reserved_until_tick: + await get_tree().physics_frame + if not _is_networked_match(match_scene): + print("SMOKE FAIL: match aborted while the spectator was waiting for the slot") + NetworkManager.shutdown() + get_tree().quit(1) + return + # Nothing may have promoted yet: the reservation lapsing is not a kickoff. + var promoted_before_kickoff: bool = not match_scene._slots[0].disconnected + 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 produce a kickoff") + + var promote_deadline := Time.get_ticks_msec() + 10000 + while Time.get_ticks_msec() < promote_deadline and _is_networked_match(match_scene) and match_scene._slots[0].disconnected: + await get_tree().physics_frame + if not _is_networked_match(match_scene): + print("SMOKE FAIL: match aborted before the kickoff could promote anyone") + NetworkManager.shutdown() + get_tree().quit(1) + return + + var slot = match_scene._slots[0] + var took_slot: bool = not slot.disconnected and slot.peer_id != original_peer + var renamed: bool = slot.player_name != original_name and slot.player_name != "" + var same_ship: bool = is_instance_valid(slot.ship) and slot.ship == ship_before + var controller_valid: bool = is_instance_valid(slot.controller) + # Not load-bearing on its own: the queue also empties when a waiting peer + # gives up and leaves, which is exactly what a control run with a long + # reservation showed. took_slot plus the name change is the real evidence. + var queue_drained: bool = match_scene._late_joiners.is_empty() + print("SMOKE INFO: late joiner queued=%d promoted_before_kickoff=%s took_slot=%s new_name=%s same_ship=%s queue_drained=%s" % [ + queued, str(promoted_before_kickoff), str(took_slot), slot.player_name, str(same_ship), str(queue_drained) + ]) + + # It must be a real seat, not just a relabelled one: hold on and require + # the new owner's input to move the ship the server owns, sampled while + # they are still connected. + var start_position: Vector3 = slot.ship.global_position if is_instance_valid(slot.ship) else Vector3.ZERO + var last_connected_position := start_position + var saw_connected := false + var hold_deadline := Time.get_ticks_msec() + 8000 + while Time.get_ticks_msec() < hold_deadline and _is_networked_match(match_scene): + if slot.peer_id in multiplayer.get_peers(): + saw_connected = true + if is_instance_valid(slot.ship): + last_connected_position = slot.ship.global_position + await get_tree().physics_frame + var moved := Vector2(last_connected_position.x - start_position.x, last_connected_position.z - start_position.z).length() + var drove: bool = saw_connected and moved > 1.0 + + var success := queued > 0 and not promoted_before_kickoff and took_slot and renamed \ + and same_ship and controller_valid and queue_drained and drove + print("SMOKE %s: late joiner took the vacated slot at the kickoff (queued=%d waited_for_kickoff=%s took_slot=%s same_ship=%s drove=%.2fm)" % [ + "PASS" if success else "FAIL", queued, str(not promoted_before_kickoff), str(took_slot), str(same_ship), moved + ]) + NetworkManager.shutdown() + get_tree().quit(0 if success else 1) + + +# The same promotion from the SPECTATOR's side. It must start with no slot, +# gain one without reloading the scene, and be able to fly it — the client's +# _is_spectator was assigned once at match_config time and never revisited, +# so "the server promoted me" and "I can actually play" are separate claims. +func run_late_joiner_client_check(lifetime_seconds: float) -> void: + var load_deadline := Time.get_ticks_msec() + 10000 + while Time.get_ticks_msec() < load_deadline and not _is_networked_match(get_tree().current_scene): + await get_tree().process_frame + var match_scene := get_tree().current_scene + if not _is_networked_match(match_scene): + print("SMOKE FAIL: late joiner never loaded the match scene") + get_tree().quit(1) + return + await get_tree().create_timer(1.0).timeout + var started_spectating: bool = match_scene._my_slot == null and match_scene._is_spectator + if not started_spectating: + print("SMOKE FAIL: late joiner was given a slot immediately — it should spectate until a kickoff (my_slot=%s is_spectator=%s)" % [ + str(match_scene._my_slot != null), str(match_scene._is_spectator) + ]) + NetworkManager.shutdown() + get_tree().quit(1) + return + + var promote_deadline := Time.get_ticks_msec() + int(lifetime_seconds * 1000.0) + 42000 + while Time.get_ticks_msec() < promote_deadline and _is_networked_match(match_scene) and match_scene._my_slot == null: + await get_tree().physics_frame + if not _is_networked_match(match_scene): + print("SMOKE FAIL: match scene torn down before the late joiner was promoted") + get_tree().quit(1) + return + var promoted: bool = match_scene._my_slot != null and not match_scene._is_spectator + if not promoted: + print("SMOKE FAIL: late joiner never got a slot (my_slot=%s is_spectator=%s)" % [ + str(match_scene._my_slot != null), str(match_scene._is_spectator) + ]) + NetworkManager.shutdown() + get_tree().quit(1) + return + + # Wait for live play — a promotion lands at a kickoff, so the very next + # thing is a countdown with every body frozen. + var live_deadline := Time.get_ticks_msec() + 15000 + while Time.get_ticks_msec() < live_deadline and _is_networked_match(match_scene) and not MatchState.is_live(match_scene.match_state): + await get_tree().physics_frame + var my_slot = match_scene._my_slot + var owns_slot: bool = my_slot != null and my_slot.peer_id == multiplayer.get_unique_id() + var ship_ok: bool = my_slot != null and is_instance_valid(my_slot.ship) + # The promoted ship was a REMOTE body a moment ago: frozen kinematic and fed + # by the interpolator. Promotion deliberately does NOT unfreeze it on the + # spot — it waits for the first authoritative pose, exactly as a fresh + # client does — so this WAITS for prediction to start rather than sampling + # at whichever frame the state happened to go live. Sampling immediately is + # a race the run loses about half the time, reporting predicting=false on a + # client that then flew 45m perfectly well. + # Poll the whole condition, not _local_prediction_ready alone. Unfreezing is + # QUEUED and applied on the body's own next _integrate_forces (task 0.15), + # so there is a real window where the state is PLAYING and the flag is set + # but ship.freeze has not flipped yet — sampling on that frame reported + # predicting=false for a client that then flew 45m, twice in five runs. + var predicting := false + var predict_deadline := Time.get_ticks_msec() + 5000 + while Time.get_ticks_msec() < predict_deadline and _is_networked_match(match_scene): + predicting = ship_ok and match_scene._local_prediction_ready \ + and not my_slot.ship.freeze and not my_slot.interpolator.has_samples() + if predicting: + break + await get_tree().physics_frame + var controller_ok: bool = ship_ok and my_slot.ship.controller != null and my_slot.ship.controller.get_parent() == my_slot.ship + + var start_position: Vector3 = my_slot.ship.global_position if ship_ok else Vector3.ZERO + Input.action_press("move_forward") + await get_tree().create_timer(3.0).timeout + Input.action_release("move_forward") + if not _is_networked_match(match_scene) or not (ship_ok and is_instance_valid(my_slot.ship)): + print("SMOKE FAIL: promoted client lost its ship or scene mid-drive") + get_tree().quit(1) + return + var end_position: Vector3 = my_slot.ship.global_position + var moved := Vector2(end_position.x - start_position.x, end_position.z - start_position.z).length() + var moved_ok := moved > 1.0 + + print("SMOKE INFO: promotion spectated_first=%s owns_slot=%s ship_ok=%s predicting=%s (ready=%s frozen=%s interp_samples=%s state=%s) controller_ok=%s moved=%.2fm" % [ + str(started_spectating), str(owns_slot), str(ship_ok), str(predicting), + str(match_scene._local_prediction_ready), str(ship_ok and my_slot.ship.freeze), + str(ship_ok and my_slot.interpolator.has_samples()), MatchState.to_name(match_scene.match_state), + str(controller_ok), moved + ]) + var success := started_spectating and promoted and owns_slot and ship_ok and predicting and controller_ok and moved_ok + print("SMOKE %s: spectator was promoted to player and can fly the slot it inherited (moved=%.2fm)" % [ + "PASS" if success else "FAIL", moved + ]) + NetworkManager.shutdown() + get_tree().quit(0 if success else 1) + + # §6.4's reconnect, graded from the RECONNECTING PLAYER's side. The # host-disconnect scenario already asserts the server's bookkeeping — slot # reserved, ship kept, reclaimed by name — but every one of those assertions diff --git a/multiplayer-todo.md b/multiplayer-todo.md index 182134d0..ba303488 100644 --- a/multiplayer-todo.md +++ b/multiplayer-todo.md @@ -971,7 +971,7 @@ Note the free-flight p99 **improved** (0.170/0.176/0.184 → 0.141/0.168/0.154) | 5.5 `[D:5.1]` `[P]` | **DONE.** Clock expiry -> `FULL_TIME` -> sudden death on a draw or `RESULTS`, golden goal in overtime, then `LOBBY` on both peers. `get_tree().paused` is never used in the networked path | Full run observed end to end: `LOADING -> WARMUP -> PLAYING -> FULL_TIME -> OVERTIME_WARMUP -> OVERTIME -> GOAL_PAUSE -> RESULTS -> LOBBY`, both peers returning to the lobby scene | | 5.6 `[D:5.1]` `[P]` | **DONE.** Controller swap on disconnect (ship never despawned), 30 s identity-keyed reservation, reclaim on reconnect, `--fill-bots`/`--no-fill-bots`, `stalled` set immediately for the nameplate | Real 3-process run: ship survived, controller valid, slot reserved, reclaimed by name, same ship instance | | 5.7 `[D:5.6]` | **DONE.** `_swap_slot_controller()` rebinds in the same transaction; `slot.controller` retyped to the base `ShipController`; every use `is_instance_valid`-guarded | The disconnect test caught the real bug: the narrower `RLShipController` type made the swap assignment fail, leaving a freed reference | -| 5.8 `[D:5.1]` `[P]` | **DONE.** A slotless peer spectates (no ship spawned, same snapshot stream), `HUDController.spectator_mode` keeps clock/score/celebration and hides only the ship cluster, camera cycles ships then ball, `--max-spectators` cap | Spectator path exercised by the mid-match joiner; HUD no longer `push_error`s and bails with a dead HUD | +| 5.8 `[D:5.1]` `[P]` | **DONE.** A slotless peer spectates (no ship spawned, same snapshot stream), `HUDController.spectator_mode` keeps clock/score/celebration and hides only the ship cluster, camera cycles ships then ball, `--max-spectators` cap. **§6.3's "take the slot at the next kickoff" is now implemented, not just printed** — the line claiming it was there from the start while `_is_spectator` was assigned once and never revisited (see the Phase 5 note below) | Spectator path exercised by the mid-match joiner; HUD no longer `push_error`s and bails with a dead HUD; promotion verified 4/4 from both sides, with a control proving §6.4's reservation outranks the queue | | 5.9 `[D:5.3]` `[P]` | **DONE.** New `GameMode._on_bodies_respawned()` virtual; `NetworkedMatch` bumps `reset_gen` through Phase 2's deferred path so the bump and the respawned pose land in the same broadcast | Single-player modes unaffected (base is a no-op) | | 5.10 `[D:5.1]` `[P]` | **DONE.** `scripts/replay_log.gd`, `--replay-log=`, storing wire bytes verbatim in both directions | Live 6 s match recorded 1115 records (557 inputs / 558 snapshots); a stored snapshot decodes back to `server_tick=100 match_state=WARMUP bodies=2`; 6 unit tests incl. truncation and foreign-file rejection | @@ -1032,13 +1032,23 @@ Fixed by not policing a backlog the server caused: `MatchSim._physics_process` w Abuse detection is unweakened and this was checked rather than argued: all three abuse roles still disconnect, and **no flood induced a server stall in any run**, so the grace cannot be farmed by flooding. An attacker who *can* induce server stalls to earn budget already has a strictly worse capability than sending extra input packets. +**§6.3's "spectate now, take the slot at the next kickoff" was a print statement, not a feature.** The server logged *"joined mid-match; spectating until the next kickoff"* and then never did anything about it; on the client, `_is_spectator` was assigned once during `_on_match_config_received` and never revisited — and that handler returns early whenever `_slots` is non-empty, so no rebroadcast could ever promote an in-match spectator. The reconnect path only worked because a returning player is a *fresh process* that runs `_on_match_config_received` from scratch. + +Implemented on both sides. The server queues late joiners in arrival order and drains the queue from `_begin_kickoff()` — before the reset transforms are read, so a promoted player's ship is placed by that same kickoff instead of being left wherever its previous owner abandoned it, and the controller swap lands on an already-frozen body, which is the entire reason §6.3 puts this at a kickoff boundary. A slot only becomes available once its player has gone **and** their 30s reservation has lapsed: §6.4 outranks §6.3, because taking a still-reserved slot would quietly break the reconnect promise. `_abort_if_abandoned` now counts a waiting spectator as somebody still present, for the same reason it already counts an outstanding reservation — otherwise the one person queued for the slot that just opened gets dumped to the lobby at the exact moment they were about to receive it. + +The client gets a new broadcast `slot_assigned` (reliable, channel 0). Broadcast rather than addressed to the new owner, because every client holds its own slot list and one that names the wrong peer keeps flying somebody else's ship as a remote body; reliable, because unlike `match_state` there is no per-snapshot field that would re-converge a client that missed it. The promoted client undoes everything that made that body remote — fresh interpolator (its buffered samples describe the *previous owner's* flight), Godot's own physics interpolation switched back on, visual offsets cleared — and then deliberately does **not** unfreeze: it clears `_local_prediction_ready` so the next snapshot teleports it to a genuine authoritative pose and starts prediction there, exactly as a fresh client does. The controller-attach block was factored out of `_on_match_config_received` into `_take_local_ownership()` rather than copied, since a copy is a copy that drifts. + +New `--role=host-latejoin` / `--role=client-latejoin` and `--slot-reservation-seconds=` (a server-side override in the same shape as `--match-length`, because the interesting moment is otherwise 30 real seconds away). Verified 4/4 from both sides: the joiner is queued, is **not** promoted merely because the reservation lapsed, takes the slot at the forced goal's kickoff, keeps the same ship instance, and both peers independently measure ~45.7m of movement under its input — the client's own number and the server's agree, so the promoted seat is real rather than relabelled. Control with a 90s reservation: the kickoff fires and nothing is promoted, the slot still reads the departed player's name, and the joiner stays a spectator. The existing spectator test is a second control — a spectator with no free slot is never promoted. + +Two test-side races were fixed while getting there, both worth remembering because they produced confident false failures: sampling `predicting` at an arbitrary frame reported `false` for a client that then flew 45m, because unfreezing is *queued* and applied on the body's next `_integrate_forces` (task 0.15), so there is a real window where the state is PLAYING and `_local_prediction_ready` is set but `ship.freeze` has not flipped yet. Poll the whole condition with a deadline, never a proxy signal, and never one instant. + **§6.4's reconnect was only ever graded from the server's side, and the client's side was failing the whole time.** `run_disconnect_host_check` ticked 60 physics frames (1.0s) past the reclaim and then shut the server down — so the reconnecting client, whose wiring check waits a 2.0s settle before it looks at anything, had its peer torn out from under it every single run and reported `current_scene is not NetworkedMatch after 2.0s`. The host printed PASS throughout, and the host was the side anyone read. The hold is now a real window (default 8s), and the host additionally asserts that the reconnected player's input reaches the server and moves the ship the server owns — every other assertion there is slot bookkeeping that would hold identically for a client whose input pipeline came back dead, which is the exact failure the reservation exists to prevent. Both the position and the connection state are sampled *while the peer is still connected*, not once at the end of the hold: the client leaves on its own schedule, and an end-of-hold sample reported `still_connected=false` for a perfectly good run — the same mis-timed sampling a Phase 3 review caught in the CI gate. New `--role=client-reconnect` grades the returning player: not a spectator, owns a slot whose `peer_id` is its own, has a real ship, rejoined a live match with the clock already known (`_end_tick >= 0` — §6.2 step 2's bootstrap, since a player who must wait for the next goal to learn the score has not really rejoined), and its input still moves its ship. That set is chosen because a stale `_last_match_config` once made a reconnecting player a spectator, and *that bug was visible in this scenario's own logs while it reported PASS*. Verified 3/3 both sides, with a control that rejoins while the slot is still occupied and correctly fails on `is_player=false`. The first version of that control failed with the generic "lost its ship mid-drive", so the spectator case is now reported before the drive rather than after. `tools/replay_dump.gd` reads a log back — record counts by kind, plus how much of the input sequence stream actually reached the server once redundancy is counted. It is committed rather than left in a scratch directory because it is what turned "the server dropped some input" into the numbers above, and a log nobody can read is half a feature. -**New/changed test surface:** `--exercise-match-state` (both roles; host forces a goal, client validates the whole observed sequence and the wire byte), `--role=host-disconnect` for the 5.6/5.7 three-process scenario (paired with `--role=client-reconnect`, which grades the returning player), `--match-length=` to reach `FULL_TIME` in a short run, `--replay-log=`, `--fill-bots`/`--no-fill-bots`, `--max-spectators=`. The ball-contact scenario now **steers at the ball with closed-loop real input** instead of a hand-tuned fixed-heading burst, which 5.3 broke by adding `KICKOFF_YAW_JITTER` (0 contacts in 3/3 runs); it thrusts while turning rather than hovering to aim, which took it from 2/3 to 5/5. +**New/changed test surface:** `--exercise-match-state` (both roles; host forces a goal, client validates the whole observed sequence and the wire byte), `--role=host-disconnect` for the 5.6/5.7 three-process scenario (paired with `--role=client-reconnect`, which grades the returning player), `--role=host-latejoin`/`--role=client-latejoin` plus `--slot-reservation-seconds=` for §6.3's kickoff promotion, `--match-length=` to reach `FULL_TIME` in a short run, `--replay-log=`, `--fill-bots`/`--no-fill-bots`, `--max-spectators=`. The ball-contact scenario now **steers at the ball with closed-loop real input** instead of a hand-tuned fixed-heading burst, which 5.3 broke by adding `KICKOFF_YAW_JITTER` (0 contacts in 3/3 runs); it thrusts while turning rather than hovering to aim, which took it from 2/3 to 5/5. **Phase gate:** a full 3v3 start-to-finish including a mid-match disconnect and a late joiner. **Not yet run** — every scenario above was verified at 1v1 (plus a two-bot CI match). The 3v3 gate needs a real multi-client session and is the outstanding item for this phase, alongside Phase 4's own un-run human playtest.