From c90d0540f9008763e3d7f0b0a8e3c0decfe711b8 Mon Sep 17 00:00:00 2001
From: Noah Metzger <chomenor@gmail.com>
Date: Sun, 26 May 2024 14:57:15 -0500
Subject: [PATCH] server: don't change serverid during map restart

Fixes client getting stuck since ed4af16763684a9b492c4c6737833531b43963e2 if map restart occurs while client is loading map. Also simplifies code and avoids the need for a systeminfo update during map restarts.
---
 code/server/server.h    |  4 ++--
 code/server/sv_ccmds.c  | 11 +++++------
 code/server/sv_client.c | 12 ++++++------
 code/server/sv_init.c   |  1 -
 4 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/code/server/server.h b/code/server/server.h
index 174423182..e3e8fb48f 100644
--- a/code/server/server.h
+++ b/code/server/server.h
@@ -67,7 +67,6 @@ typedef struct {
 	serverState_t	state;
 	qboolean		restarting;			// if true, send configstring changes during SS_LOADING
 	int				serverId;			// changes each server start
-	int				restartedServerId;	// serverId before a map_restart
 	int				checksumFeed;		// the feed key that we use to compute the pure checksum strings
 	// https://zerowing.idsoftware.com/bugzilla/show_bug.cgi?id=475
 	// the serverId associated with the current checksumFeed (always <= serverId)
@@ -87,7 +86,8 @@ typedef struct {
 	playerState_t	*gameClients;
 	int				gameClientSize;		// will be > sizeof(playerState_t) due to game private data
 
-	int				restartTime;
+	int				restartTime;		// sv.time of pending restart
+	int				restartedTime;		// sv.time of last restart
 	int				time;
 
 	byte			baselineUsed[ MAX_GENTITIES ];
diff --git a/code/server/sv_ccmds.c b/code/server/sv_ccmds.c
index ab8a6494d..9c288b459 100644
--- a/code/server/sv_ccmds.c
+++ b/code/server/sv_ccmds.c
@@ -240,11 +240,13 @@ static void SV_MapRestart_f( void ) {
 	const char		*denied;
 	qboolean	isBot;
 	int			delay;
+	static int	lastRestartFrame;
 
 	// make sure we aren't restarting twice in the same frame
-	if ( com_frameTime == sv.serverId ) {
+	if ( com_frameTime == lastRestartFrame ) {
 		return;
 	}
+	lastRestartFrame = com_frameTime;
 
 	// make sure server is running
 	if ( !com_sv_running->integer ) {
@@ -288,11 +290,6 @@ static void SV_MapRestart_f( void ) {
 	// map_restart has happened
 	svs.snapFlagServerBit ^= SNAPFLAG_SERVERCOUNT;
 
-	// generate a new serverid	
-	// TTimo - don't update restartedserverId there, otherwise we won't deal correctly with multiple map_restart
-	sv.serverId = com_frameTime;
-	Cvar_SetIntegerValue( "sv_serverid", sv.serverId );
-
 	// if a map_restart occurs while a client is changing maps, we need
 	// to give them the correct time so that when they finish loading
 	// they don't violate the backwards time check in cl_cgame.c
@@ -360,6 +357,8 @@ static void SV_MapRestart_f( void ) {
 	sv.time += 100;
 	VM_Call( gvm, 1, GAME_RUN_FRAME, sv.time );
 	svs.time += 100;
+
+	sv.restartedTime = sv.time;
 }
 
 
diff --git a/code/server/sv_client.c b/code/server/sv_client.c
index f3736b5a3..f9e5c4250 100644
--- a/code/server/sv_client.c
+++ b/code/server/sv_client.c
@@ -2156,6 +2156,12 @@ static void SV_UserMove( client_t *cl, msg_t *msg, qboolean delta ) {
 	// in the commands will cause them to be immediately discarded
 	for ( i = 0; i < cmdCount; i++ ) {
 		// if this is a cmd from before a map_restart ignore it
+		if ( cmds[i].serverTime - sv.restartedTime < 0 ) {
+			Com_DPrintf( "ignoring pre-restart usercmd: client(%i) cmdTime(%i) restartedTime(%i)\n",
+					cl - svs.clients, cmds[i].serverTime, sv.restartedTime );
+			continue;
+		}
+		// ignore command if future command time goes backwards (shouldn't happen...?)
 		if ( cmds[i].serverTime - cmds[cmdCount-1].serverTime > 0 ) {
 			continue;
 		}
@@ -2251,12 +2257,6 @@ void SV_ExecuteClientMessage( client_t *cl, msg_t *msg ) {
 	// but we still need to read the next message to move to next download or send gamestate
 	// I don't like this hack though, it must have been working fine at some point, suspecting the fix is somewhere else
 	if ( serverId != sv.serverId && !*cl->downloadName && !strstr(cl->lastClientCommandString, "nextdl") ) {
-		// TTimo - use a comparison here to catch multiple map_restart
-		if ( serverId - sv.restartedServerId >= 0 && serverId - sv.serverId < 0 ) {
-			// they just haven't caught the \map_restart yet
-			Com_DPrintf( "%s: ignoring pre map_restart / outdated client message\n", cl->name );
-			return;
-		}
 		// if we can tell that the client has dropped the last gamestate we sent them, resend it
 		if ( cl->state != CS_ACTIVE && cl->messageAcknowledge - cl->gamestateMessageNum > 0 ) {
 			if ( !SVC_RateLimit( &cl->gamestate_rate, 4, 1000 ) ) {
diff --git a/code/server/sv_init.c b/code/server/sv_init.c
index f786809d3..264b17509 100644
--- a/code/server/sv_init.c
+++ b/code/server/sv_init.c
@@ -511,7 +511,6 @@ void SV_SpawnServer( const char *mapname, qboolean killBots ) {
 
 	// serverid should be different each time
 	sv.serverId = com_frameTime;
-	sv.restartedServerId = sv.serverId; // I suppose the init here is just to be safe
 	sv.checksumFeedServerId = sv.serverId;
 	Cvar_Set( "sv_serverid", va( "%i", sv.serverId ) );