feat(multiplayer): Phase 5 tasks 5.6-5.10 - disconnects, spectators, replay log

Completes Phase 5's implementation. Every task is verified at 1v1; the 3v3 phase gate itself has not been run and remains outstanding. 5.6/5.7 disconnects: a ship is never despawned. The slot keeps it and swaps the controller (--fill-bots gives it a bot, the default leaves it inert per §1.4), sets `stalled` immediately so the nameplate greys out rather than waiting ~500ms for the abandoned jitter buffer to starve, and reserves the slot for 30s keyed by player name so a reconnect gets the same ship back. 5.7 was a real bug, found by the test rather than by review: SlotInfo.controller was declared RLShipController, but the takeover swaps in an AIShipController or the base controller - the narrower type makes that assignment fail its type check, leaving the field pointing at the controller set_controller() just queue_free()d. It surfaced as controller_valid=false on the first run. The per-tick action write is now also gated on `is RLShipController`, since a disconnected slot's bot drives itself and overwriting it from a starving buffer would pin it to the departed player's last input. §6.4's two rules conflict: reserve for 30s, but abort when the last human leaves. Applied naively the abort wins instantly in a 1v1 and the reservation can never be redeemed, making reconnect unreachable exactly when it matters. Abort now waits for no connections AND no outstanding reservations. 5.8 spectators: a slotless peer spawns no ship and receives the same snapshot broadcast. HUDController.spectator_mode keeps the clock, score and goal celebration and hides only the ship instrument cluster - it previously push_error'd and bailed, leaving a spectator with a dead HUD. Camera cycles ships in slot order then the ball. --max-spectators caps it, counted from the live peer list so a dropped spectator cannot leak a unit of the cap. 5.9 escape respawn: 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 are unaffected - the base is a no-op. 5.10 replay log: scripts/replay_log.gd, --replay-log=<path>, storing the wire bytes verbatim in both directions rather than re-serialising - a re-encode would launder away precisely the malformed payload being chased. A live 6s match recorded 1115 records (557 inputs / 558 snapshots) and a stored snapshot decodes back to server_tick=100 match_state=WARMUP bodies=2. Note for future work: --check-only --script is the only thing that catches a parse error in networked_match.gd, because the unit runner never loads it. Two separate breakages passed the full unit suite while breaking every two-process run. A new class_name also needs --import before it resolves. Test surface: --role=host-disconnect (three-process 5.6/5.7 scenario), --match-length=<s>, --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, which 5.3 broke by adding KICKOFF_YAW_JITTER; thrusting while turning took it from 2/3 to 5/5. Regression: 87 unit tests; free-flight LAN p99 0.094m with 0 hard snaps; transition gate 0.00%; ball contact 5/5; lifecycle goal cycle and full match to RESULTS/LOBBY; disconnect+reconnect; two-bot CI.
2026-09-11 18:43:43 +00:00 · 2026-08-21 10:25:15 +01:00
parent 3d6906b981
commit a5cbc977b5
11 changed files with 708 additions and 14 deletions
@@ -107,7 +107,13 @@ class SlotInfo:
 	var team: int
 	var spawn_index: int
 	var ship: Ship
-	var controller: RLShipController      # server only
+	# Base type, NOT RLShipController: §6.4's takeover swaps in either an
+	# AIShipController (--fill-bots) or the inert base controller, and a
+	# narrower declared type makes that assignment fail its type check — which
+	# leaves this field pointing at the controller set_controller() just
+	# queue_free()d. Exactly task 5.7's dangling reference, and it showed up as
+	# controller_valid=false the first time the disconnect test ran.
+	var controller: ShipController        # server only
 	var jitter_buffer := InputJitterBuffer.new()   # server only (§3.2)
 	# Server only. Consecutive packets rejected by the seq-range guard, reset by
 	# any accepted one. The guard's bound is derived from a value only an
@@ -115,6 +121,12 @@ class SlotInfo:
 	# permanently — see the guard's own comment in _on_input_received.
 	var consecutive_seq_rejects := 0
 	var last_client_send_ms := 0          # server only: echoed back per-peer next snapshot (§2.4)
+	# §6.4 (tasks 5.6/5.7). A ship is NEVER despawned on disconnect — the slot
+	# keeps its ship and swaps the controller, so body order (and therefore
+	# every snapshot index) stays stable for the whole match.
+	var player_name := ""                 # identity key for reconnect; peer_id changes across a reconnect
+	var disconnected := false
+	var reserved_until_tick := -1         # server only: slot held for this player until here
 	var interpolator := NetInterpolator.new()   # client only
 	var visual_smoother_reset := true
 	var visual_position_offset := Vector3.ZERO
@@ -255,6 +267,14 @@ var _clock_running := false
 var _last_emitted_second := -1
@export var match_length_seconds := 150.0

+# §6.4's --fill-bots takeover controller. Mirrors match_mode.gd's exports so a
+# server operator configures the replacement bot exactly as a single-player
+# match configures its opponent, rather than through a second parallel scheme.
+@export_group("Disconnect fill bot")
+@export_file("*.json") var bot_model_path: String = ""
+@export_range(1, 60) var bot_reaction_ticks: int = 8
+@export_range(0.0, 1.0) var bot_action_noise: float = 0.0
+
 # §6.2 step 6. The tick play resumes on — the countdown's own end. Both peers
 # derive the displayed count from this and their own server-tick estimate, so
 # nothing depends on a local Timer staying in step.
@@ -265,6 +285,18 @@ var _pending_kickoff := {}
 # Freeze is applied on a strictly later tick than the kickoff teleport that
 # precedes it — see _apply_kickoff. -1 when nothing is pending.
 var _pending_freeze_tick := -1
+# §1.4: public servers default to leaving an abandoned ship inert rather than
+# handing it to a bot, so a disconnect cannot change the competitive balance
+# of a match in progress. --fill-bots opts in.
+var _fill_bots := false
+# Task 5.10, server only. null unless --replay-log= was passed.
+var _replay_log: ReplayLog = null
+# §6.3 (task 5.8), client only.
+var _is_spectator := false
+var _spectator_target_index := 0
+# §6.3's "cap with --max-spectators". Server only; 0 disables spectating
+# entirely, negative means unlimited.
+var _max_spectators := -1
 var _last_emitted_countdown := -1
 var _in_overtime := false
 var _match_over := false
@@ -277,7 +309,24 @@ func _ready() -> void:
 		_kickoff_rng.randomize()
 	if multiplayer.is_server():
 		for arg: String in OS.get_cmdline_user_args():
-			if arg.begins_with("--match-length="):
+			if arg == "--fill-bots":
+				_fill_bots = true
+			elif arg == "--no-fill-bots":
+				_fill_bots = false
+			elif arg.begins_with("--max-spectators="):
+				_max_spectators = maxi(0, arg.get_slice("=", 1).to_int())
+			elif arg.begins_with("--replay-log="):
+				# Task 5.10. Diagnostic only: a log that cannot be opened must
+				# never stop the server serving the match.
+				var replay_path := arg.get_slice("=", 1)
+				_replay_log = ReplayLog.new()
+				var replay_err := _replay_log.open_for_write(replay_path)
+				if replay_err != OK:
+					push_warning("NetworkedMatch: could not open replay log %s (%s)" % [replay_path, error_string(replay_err)])
+					_replay_log = null
+				else:
+					print("NetworkedMatch: recording replay log to %s" % replay_path)
+			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
 				# only — a client cannot shorten anyone's match.
@@ -357,6 +406,7 @@ func _start_server() -> void:
 		slot.peer_id = peer_id
 		slot.team = info.team
 		slot.spawn_index = spawn_index
+		slot.player_name = info.player_name
 		slot.controller = RLShipController.new()
 		slot.ship = spawn_ship(info.team, spawn_index, slot.controller)
 		_slots.append(slot)
@@ -366,6 +416,8 @@ func _start_server() -> void:

 	MatchSim.send_match_config(arena_path, peer_ids, teams, spawn_indices)
 	MatchSim.input_received.connect(_on_input_received)
+	NetworkManager.client_disconnected.connect(_on_client_disconnected)
+	MatchNet.player_joined.connect(_on_player_joined_midmatch)

 	# §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'
@@ -454,6 +506,8 @@ func _on_input_received(peer_id: int, decoded: Dictionary) -> void:
 				# Fall through and accept: this is the escape hatch, not a
 				# missing `return`.
 			slot.consecutive_seq_rejects = 0
+			if _replay_log != null:
+				_replay_log.record_input(Engine.get_physics_frames(), peer_id, decoded.get("raw", PackedByteArray()))
 			jb.ingest(seq, decoded["actions"])
 			slot.last_client_send_ms = decoded["client_send_ms"]
 			return
@@ -876,6 +930,152 @@ func _on_goal_registered(conceding_team: int) -> void:
 	_broadcast_clock_state()


+# Task 5.9. Server-only by construction: _respawn_escaped_bodies() is gated on
+# _owns_world_simulation(). The bump uses Phase 2's deferred path because the
+# respawn only QUEUES a teleport — bumping now would broadcast the new
+# generation alongside the still-escaped position, which is exactly the 27m
+# slide that fix exists to prevent.
+# --- §6.4 disconnects and reconnects (tasks 5.6/5.7) -----------------------
+
+const SLOT_RESERVATION_SECONDS := 30.0
+
+
+func _on_client_disconnected(peer_id: int) -> void:
+	if not multiplayer.is_server():
+		return
+	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)
+		_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
+		])
+		break
+	_abort_if_abandoned()
+
+
+# §6.4 has two rules that pull against each other: reserve a departed player's
+# slot for 30s, and abort to the lobby once the last human leaves. Applied
+# naively the abort wins instantly in a 1v1 — the moment the only player drops,
+# the match is torn down and their reservation can never be redeemed, which
+# makes the reconnect path unreachable exactly when it matters most (a single
+# player whose connection blipped). The reservation therefore takes precedence:
+# abort only once nobody is connected AND nobody is still expected back.
+func _abort_if_abandoned() -> void:
+	if MatchState.is_terminal(match_state):
+		return
+	var now := Engine.get_physics_frames()
+	for slot in _slots:
+		if not slot.disconnected:
+			return   # somebody is still playing
+		if slot.reserved_until_tick >= 0 and now <= slot.reserved_until_tick:
+			return   # somebody may still come back
+	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)
+
+
+# Task 5.7. Ship.set_controller() calls queue_free() on the OUTGOING
+# controller, so slot.controller is a dangling reference the instant the swap
+# happens — and _physics_process writes slot.controller.action every single
+# tick. Rebinding must therefore happen in the same transaction as the swap,
+# never as a follow-up statement that an early return or an await could skip.
+func _swap_slot_controller(slot: SlotInfo, replacement: ShipController) -> void:
+	if not is_instance_valid(slot.ship):
+		slot.controller = null
+		return
+	slot.ship.set_controller(replacement)
+	slot.controller = replacement
+
+
+func _build_takeover_controller() -> ShipController:
+	if _fill_bots:
+		return _build_opponent(bot_model_path, bot_reaction_ticks, bot_action_noise, "NetworkedMatch")
+	# §6.4's default for public servers: inert but still simulated, exactly the
+	# placeholder GameMode already uses for an unfilled slot. An abandoned ship
+	# that keeps flying on its last input would be worse than one that coasts.
+	return ShipController.new()
+
+
+# Called when a peer joins while this match is already running. Returns true if
+# it reclaimed a reserved slot (§6.4's 30s identity-keyed reservation).
+func _try_reclaim_slot(peer_id: int, player_name: String) -> bool:
+	if not multiplayer.is_server():
+		return false
+	var now := Engine.get_physics_frames()
+	for slot in _slots:
+		if not slot.disconnected or slot.player_name == "" or slot.player_name != player_name:
+			continue
+		if slot.reserved_until_tick >= 0 and now > slot.reserved_until_tick:
+			continue   # reservation lapsed; this is a fresh joiner, not a return
+		slot.peer_id = peer_id
+		slot.disconnected = false
+		slot.reserved_until_tick = -1
+		# Reset the input pipeline: the returning client starts its sequence
+		# numbering from scratch, and the old buffer's cursor belongs to a
+		# different epoch entirely (input_jitter_buffer.gd's seeding comment).
+		slot.jitter_buffer = InputJitterBuffer.new()
+		slot.consecutive_seq_rejects = 0
+		_swap_slot_controller(slot, RLShipController.new())
+		print("NetworkedMatch: peer %d reclaimed %s's reserved slot" % [peer_id, player_name])
+		return true
+	return false
+
+
+# §6.3/§6.4. A peer joining while this match runs is either a returning player
+# claiming their reserved slot, or a late joiner — who spectates until the next
+# kickoff, because swapping a controller at a kickoff boundary is free and
+# mid-play it is not.
+func _on_player_joined_midmatch(peer_id: int, player_name: String) -> void:
+	if not multiplayer.is_server() or _slots.is_empty():
+		return
+	if _try_reclaim_slot(peer_id, player_name):
+		return
+	if _max_spectators >= 0 and _spectator_count() > _max_spectators:
+		print("NetworkedMatch: spectator cap (%d) reached, disconnecting peer %d" % [_max_spectators, peer_id])
+		# Same call the abuse paths use (match_sim.gd:285, match_net.gd:207) —
+		# default force=false, so ENet flushes cleanly rather than leaving the
+		# server's own peer bookkeeping inconsistent (§9 gotcha on force=true).
+		multiplayer.multiplayer_peer.disconnect_peer(peer_id)
+		return
+	print("NetworkedMatch: peer %d (%s) joined mid-match; spectating until the next kickoff" % [peer_id, player_name])
+
+
+# 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.
+func _spectator_count() -> int:
+	var slotted := {}
+	for slot in _slots:
+		if not slot.disconnected:
+			slotted[slot.peer_id] = true
+	var count := 0
+	for peer_id in multiplayer.get_peers():
+		if not slotted.has(peer_id):
+			count += 1
+	return count
+
+
+func _expire_slot_reservations() -> void:
+	var now := Engine.get_physics_frames()
+	for slot in _slots:
+		if slot.disconnected and slot.reserved_until_tick >= 0 and now > slot.reserved_until_tick:
+			slot.reserved_until_tick = -1
+			print("NetworkedMatch: %s's slot reservation lapsed" % slot.player_name)
+			# The abort was deferred while this reservation was live; now that
+			# it has lapsed, re-check whether anyone is left at all.
+			_abort_if_abandoned()
+
+
+func _on_bodies_respawned() -> void:
+	if not multiplayer.is_server():
+		return
+	_pending_reset_gen_bump = true
+	_pending_reset_gen_bump_tick = Engine.get_physics_frames()
+
+
 func _on_goal_scored(_conceding_team: int) -> void:
 	# Deliberately empty. Before task 5.4 this reset the world the instant the
 	# sensor fired, which is precisely the "server reset fires while clients
@@ -897,7 +1097,10 @@ func _broadcast_snapshot() -> void:
 	# total-garbage failure mode the moment that stops being true, and the
 	# fix costs nothing.
 	for slot in _slots:
-		bodies.append(_ship_to_net_body_state(slot.ship, slot.jitter_buffer.stalled) if is_instance_valid(slot.ship) else NetBodyState.new())
+		# §6.4: `stalled` is what greys out the nameplate, so a disconnected
+		# player must set it immediately rather than waiting the ~500ms it
+		# takes their abandoned jitter buffer to starve into the same state.
+		bodies.append(_ship_to_net_body_state(slot.ship, slot.jitter_buffer.stalled or slot.disconnected) 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, match_state, _reset_gen, bodies)
@@ -924,6 +1127,8 @@ func _broadcast_snapshot() -> void:
 			# genuine sustained server starvation event.
 			var advertised_depth := -2 if slot.jitter_buffer.starved_ticks >= STARVATION_ADVERTISEMENT_TICKS else slot.jitter_buffer.depth()
 			var bytes := NetCodec.pack_snapshot(last_input_seq, advertised_depth, slot.last_client_send_ms, segment)
+			if _replay_log != null:
+				_replay_log.record_snapshot(server_tick, bytes)
 			MatchSim.send_snapshot(slot.peer_id, bytes)


@@ -1026,7 +1231,15 @@ func _on_match_config_received(arena_path: String, peer_ids: PackedInt32Array, t
 		if is_local:
 			_my_slot = slot

+	# §6.3 (task 5.8): a peer with no slot is a spectator. It receives the
+	# identical snapshot broadcast (zero extra server work), spawns no ship of
+	# its own, and points a camera rig at somebody else's.
+	_is_spectator = _my_slot == null
 	_spawn_hud()
+	if _is_spectator:
+		_spectator_target_index = 0
+		_point_spectator_camera()
+		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)
@@ -1062,9 +1275,70 @@ func _on_match_config_received(arena_path: String, peer_ids: PackedInt32Array, t

 func _spawn_hud() -> void:
 	hud = HUD_SCENE.instantiate()
+	# BEFORE add_child: HUDController reads this in _initialize_hud(), which
+	# runs one process frame after _ready(). Setting it afterwards would be a
+	# race against that frame, and losing it means a spectator's HUD
+	# push_error()s about a missing ship and wires up nothing at all.
+	hud.spectator_mode = _is_spectator
 	add_child(hud)


+# §6.3's "points a camera rig at a chosen ship or the ball", plus the target
+# cycling. Targets are every ship in slot order, then the ball.
+# §6.3's "cycle targets". Bound to the existing `reset_ball` action, which is
+# already a mode-level key and is meaningless to a spectator (it only fires in
+# Free Play), rather than adding a new binding to project.godot for one mode.
+func _unhandled_input(event: InputEvent) -> void:
+	if not _is_spectator:
+		return
+	if event.is_action_pressed("reset_ball"):
+		cycle_spectator_target(1)
+		get_viewport().set_input_as_handled()
+
+
+func _spectator_target_count() -> int:
+	return _slots.size() + (1 if is_instance_valid(ball) else 0)
+
+
+func _point_spectator_camera() -> void:
+	var count := _spectator_target_count()
+	if count == 0:
+		return
+	_spectator_target_index = posmod(_spectator_target_index, count)
+	var target: Node3D = null
+	if _spectator_target_index < _slots.size():
+		target = _slots[_spectator_target_index].ship
+	else:
+		target = ball
+	if not is_instance_valid(target):
+		return
+	if not is_instance_valid(_camera_rig):
+		# spawn_camera_rig types its parameter as Ship, so the ball can only
+		# ever be a LATER target, never the one the rig is created with.
+		var first_ship: Ship = null
+		for slot in _slots:
+			if is_instance_valid(slot.ship):
+				first_ship = slot.ship
+				break
+		if first_ship == null:
+			return
+		spawn_camera_rig(first_ship)
+	if is_instance_valid(_camera_rig):
+		_camera_rig.target = target
+		if is_instance_valid(hud):
+			hud.ship = target if target is Ship else null
+
+
+func cycle_spectator_target(step: int = 1) -> void:
+	if not _is_spectator:
+		return
+	var count := _spectator_target_count()
+	if count == 0:
+		return
+	_spectator_target_index = posmod(_spectator_target_index + step, count)
+	_point_spectator_camera()
+
+
 func _send_local_input(record_prediction: bool = true) -> void:
 	if _slots.is_empty():
 		return   # match_config hasn't arrived yet
@@ -1527,6 +1801,7 @@ func _physics_process(_delta: float) -> void:
 		# Also 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()
+		_expire_slot_reservations()
 	# Countdown and clock are derived from absolute ticks on both peers, so
 	# these run on the client too.
 	_apply_pending_freeze()
@@ -1543,7 +1818,17 @@ func _physics_process(_delta: float) -> void:
 		# integration. This preserves the existing one-tick server input delay
 		# while keeping snapshot.last_input_seq truthfully coupled to its body.
 		for slot in _slots:
-			slot.controller.action = slot.jitter_buffer.consume()
+			# is_instance_valid, not a null check: set_controller() queue_free()s
+			# the outgoing controller on every disconnect swap, and a freed
+			# object is non-null right up until the frame it is collected.
+			var consumed := slot.jitter_buffer.consume()
+			# Only a live player's slot is driven by the wire. A slot whose
+			# player disconnected now holds a bot or the inert base controller
+			# (§6.4), which drives itself — overwriting its action every tick
+			# from a permanently-starving jitter buffer would pin it to the
+			# departed player's last input forever.
+			if is_instance_valid(slot.controller) and slot.controller is RLShipController:
+				(slot.controller as RLShipController).action = consumed
 		if _pending_reset_gen_bump and Engine.get_physics_frames() > _pending_reset_gen_bump_tick:
 			_reset_gen = (_reset_gen + 1) % 256
 			_pending_reset_gen_bump = false