Fix multiplayer standalone crash: client stats race + host accept code (#7319)

* Fix multiplayer standalone crash: client stats race + host accept code

Fixes a crash in multiplayer PXO/tracker games hosted on a standalone
server where clients crash at debrief close after scoring a kill:

  pilotfile.cpp:295: Ship kills of '<ship class>' not found,
  should have been added by pilotfile::update_stats

Two independent root causes were found, both of which must be fixed for
correct behavior:

--- Root cause 1: race condition in multi_sexp_end_mission() (sexp.cpp) ---

The previous code called send_debrief_event() before
multi_handle_end_mission_request() as a "hack" to skip the warp-out
sequence when an end-mission SEXP fires. On standalone servers this is
a race condition: scoring_eval_kill only runs on the master, and the
master hasn't broadcast m_okKills to clients yet at the point
multi_sexp_end_mission() is called. Entering debrief this early causes
scoring_level_close() to run with zeroed m_okKills, so
Pilot.update_stats() creates no kill entries for that session.
When debrief_close() later calls scoring_backout_accept() it cannot
find those entries and hits the UNREACHABLE.

Fix: Remove the send_debrief_event() call entirely. The normal flow
via multi_handle_end_mission_request() has the server broadcast stats
first and then send MISSION_END, which causes the client to enter
debrief with correct kill data already populated. The master enters
debrief through sexp_end_mission() -> send_debrief_event() as before.

The behavioral side-effect of removing this hack: clients that go
through the normal warp-out sequence will no longer skip it on
end-mission. Clients in a state where they cannot warp still receive
immediate debrief via multi_handle_sudden_mission_end().

--- Root cause 2: host accept code never set on PXO standalone (multiui.cpp) ---

In multi_debrief_accept_hit() and multi_debrief_esc_hit(), when the
game is a PXO/tracker game and the local player is the game host
(NETINFO_FLAG_GAME_HOST), the code calls send_store_stats_packet() to
tell clients to record their stats. It then sets
Multi_debrief_stats_accept_code = 1 via the per-client response path
inside the NETINFO_FLAG_AM_MASTER block.

On a standalone server the game host is NOT the master
(NETINFO_FLAG_AM_MASTER is false for the client-side host). The
NETINFO_FLAG_AM_MASTER block is skipped entirely, and
send_store_stats_packet() uses multi_io_send_to_all_reliable() which
skips the local player — so the host never receives its own packet.
Multi_debrief_stats_accept_code stays at -1 (unset).

When debrief_close() checks this value and sees it is not 1, it
assumes stats were tossed or the window was closed without accepting,
and runs scoring_backout_accept() unconditionally. With fix 1 in place
this no longer crashes (entries exist), but silently strips all session
kill credit from the host's pilot record.

Fix: After the send_store_stats_packet() block, call
multi_debrief_stats_accept() unconditionally for the game host so the
accept code is set locally regardless of whether the host is also the
standalone master.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Preserve end-mission warp-out skip via deferred debrief entry

The previous fix removed send_debrief_event() from
multi_sexp_end_mission() entirely, which was correct for the stats
race condition but had a side effect: clients would no longer skip the
warp-out sequence when an end-mission SEXP fires.  The warp-out skip
was an intentional feature — process_endgame_packet() checks whether
the client is already in debrief state and, if so, bypasses
multi_warpout_all_players().

Restore the original behavior by deferring the send_debrief_event()
call to process_endgame_packet() rather than eliminating it.
multi_sexp_end_mission() now sets Multi_sexp_end_mission_pending=true
before calling multi_handle_end_mission_request().  When the client
receives the MISSION_END packet, process_endgame_packet() checks the
flag, calls send_debrief_event() there, then falls through to the
debrief-state check which skips the warp-out exactly as before.

This is safe because send_endgame_packet() calls multi_broadcast_stats()
before sending MISSION_END.  Both use reliable delivery, so stats are
guaranteed to be received and applied before MISSION_END is processed,
meaning scoring_level_close() now sees correct kill data when
send_debrief_event() fires.

Alt-J-initiated mission ends are unaffected: the flag is never set,
so the warp-out proceeds normally for those paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Revert multiui.cpp accept code change — tracker master owns stats

The previous change added multi_debrief_stats_accept() calls for the
game host in multi_debrief_accept_hit() and multi_debrief_esc_hit() to
work around Multi_debrief_stats_accept_code staying -1 on a standalone
server.  This was based on incorrect analysis.

In PXO tracker games the standalone master is intentionally the sole
authority for stat storage (via multi_fs_std_tracker_store_stats() in
multi_standalone_postgame_close()).  The host and clients must not save
stats to their local pilot files independently.  Multi_debrief_stats_
accept_code remaining -1 and triggering scoring_backout_accept() on the
host is correct behavior: kills are backed out of the local pilot file
because the tracker holds the authoritative record, not the local file.

The race condition fix (deferred send_debrief_event()) is sufficient to
prevent the crash: Pilot.update_stats() now creates kill entries with
correct m_okKills data, so scoring_backout_accept() can find and remove
them cleanly rather than hitting the UNREACHABLE.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Move Multi_sexp_end_mission_pending from multiutil to multi and reset in multi_vars_init()

Addresses review feedback: the flag has no relation to multiutil, and
resetting it in multi_vars_init() ensures it is always cleared between
missions regardless of which code path was taken.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: chief1983

code/network/multi.cpp

@@ -161,6 +161,8 @@ char Multi_tracker_id_string[255];
 ushort Multi_current_file_checksum = 0;
 int Multi_current_file_length = -1;
 
+bool Multi_sexp_end_mission_pending = false;
+
 
 // -------------------------------------------------------------------------------------------------
 //	multi_init() is called only once, at game start-up.  Get player address + port, initialize the
@@ -248,6 +250,9 @@ void multi_vars_init()
 	Multi_current_file_checksum = 0;
 	Multi_current_file_length = -1;
 
+	// sexp end-mission deferred debrief flag
+	Multi_sexp_end_mission_pending = false;
+
 	Active_games.clear();
 	Game_server_head = NULL;
 

code/network/multi.h

@@ -920,6 +920,12 @@ extern char Multi_tracker_id_string[255];
 extern ushort Multi_current_file_checksum;
 extern int Multi_current_file_length;
 
+// Set by multi_sexp_end_mission() so that process_endgame_packet() knows to call
+// send_debrief_event() immediately upon receiving MISSION_END, skipping warp-out.
+// Must be checked after stats have been broadcast by the server (which happens
+// in send_endgame_packet() before the MISSION_END packet is sent).
+extern bool Multi_sexp_end_mission_pending;
+
 // ip address list vars
 #define IP_STRING_LEN 60
 #define MAX_IP_ADDRS 100

code/network/multimsgs.cpp

@@ -4046,9 +4046,17 @@ void process_endgame_packet(ubyte * /*data*/, header *hinfo)
 		// do any special processing for being in a state other than the gameplay state
 		multi_handle_state_special();
 
+		// If end-mission fired via SEXP, enter debrief immediately (skipping warp-out).
+		// Stats have already been received via multi_broadcast_stats() which send_endgame_packet()
+		// calls before sending this packet, so scoring_level_close() will see correct kill data.
+		if (Multi_sexp_end_mission_pending) {
+			Multi_sexp_end_mission_pending = false;
+			send_debrief_event();
+		}
+
 		// make sure we're not already in the debrief state
 		if((gameseq_get_state() != GS_STATE_DEBRIEF) && (gameseq_get_state() != GS_STATE_MULTI_DOGFIGHT_DEBRIEF)){
-			multi_warpout_all_players();			
+			multi_warpout_all_players();
 		}
 	}	
 }

code/network/multiui.cpp

@@ -8837,6 +8837,7 @@ void multi_debrief_esc_hit()
 						multi_sw_report(stats_saved);
 					}
 				}
+
 			}
 
 			multi_quit_game(PROMPT_HOST);

code/parse/sexp.cpp

@@ -17217,10 +17217,18 @@ void sexp_end_mission(int n)
 
 void multi_sexp_end_mission()
 {
-	// This is a bit of hack, but when in a debrief state clients will skip the
-	// warp out sequence when the endgame packet is processed.
-	send_debrief_event();
-	// Standard way to end mission (equivalent to Alt-J)
+	// Signal process_endgame_packet() to skip the warp-out sequence by entering
+	// debrief immediately when the MISSION_END packet arrives.  We cannot call
+	// send_debrief_event() here directly: on standalone servers the client has not
+	// yet received mission stats (m_okKills etc.) at this point, so
+	// scoring_level_close() would run with zeroed kill counts and
+	// Pilot.update_stats() would create no kill entries.  debrief_close() would
+	// then try to back out entries that were never created and hit an UNREACHABLE.
+	//
+	// send_endgame_packet() broadcasts stats to clients *before* sending MISSION_END,
+	// so by the time process_endgame_packet() fires, the stats are already present
+	// and it is safe to enter debrief there.
+	Multi_sexp_end_mission_pending = true;
 	multi_handle_end_mission_request();
 }