server: reworked gamestate retransmission/acknowledgment

server: bring back serverId update during map_restart for compatibility reasons

code/client/cl_main.c

@@ -1093,7 +1093,7 @@ void CL_MapLoading( void ) {
 		Com_Memset( cls.updateInfoString, 0, sizeof( cls.updateInfoString ) );
 		Com_Memset( clc.serverMessage, 0, sizeof( clc.serverMessage ) );
 		Com_Memset( &cl.gameState, 0, sizeof( cl.gameState ) );
-		clc.lastPacketSentTime = -9999;
+		clc.lastPacketSentTime = cls.realtime - 9999;  // send packet immediately
 		cls.framecount++;
 		SCR_UpdateScreen();
 	} else {
@@ -2757,7 +2757,7 @@ static qboolean CL_ConnectionlessPacket( const netadr_t *from, msg_t *msg ) {
 		Netchan_Setup( NS_CLIENT, &clc.netchan, from, Cvar_VariableIntegerValue( "net_qport" ), clc.challenge, clc.compat );
 
 		cls.state = CA_CONNECTED;
-		clc.lastPacketSentTime = -9999;		// send first packet immediately
+		clc.lastPacketSentTime = cls.realtime - 9999; // send first packet immediately
 		return qtrue;
 	}
 

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;	// last time \map_restart was issued
 	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)
@@ -150,7 +149,6 @@ struct leakyBucket_s {
 	leakyBucket_t *prev, *next;
 };
 
-
 typedef struct client_s {
 	clientState_t	state;
 	char			userinfo[MAX_INFO_STRING];		// name, etc
@@ -169,6 +167,9 @@ typedef struct client_s {
 	sharedEntity_t	*gentity;			// SV_GentityNum(clientnum)
 	char			name[MAX_NAME_LENGTH];			// extracted from userinfo, high bits masked
 
+	qboolean		gamestateAcked;		// set to qtrue when serverId = sv.serverId & messageAcknowledge = gamestateMessageNum
+	qboolean		downloading;		// set at "download", reset at gamestate retransmission
+
 	// downloading
 	char			downloadName[MAX_QPATH]; // if not empty string, we are downloading
 	fileHandle_t	download;			// file being downloaded

code/server/sv_ccmds.c

@@ -242,7 +242,7 @@ static void SV_MapRestart_f( void ) {
 	int			delay;
 
 	// make sure we aren't restarting twice in the same frame
-	if ( com_frameTime == sv.restartedServerId ) {
+	if ( com_frameTime == sv.serverId ) {
 		return;
 	}
 
@@ -288,6 +288,10 @@ static void SV_MapRestart_f( void ) {
 	// map_restart has happened
 	svs.snapFlagServerBit ^= SNAPFLAG_SERVERCOUNT;
 
+	// generate a new serverid
+	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
@@ -365,8 +369,6 @@ static void SV_MapRestart_f( void ) {
 			client->lastUsercmd.serverTime = sv.time - 1;
 		}
 	}
-
-	sv.restartedServerId = com_frameTime;
 }
 
 

code/server/sv_client.c

@@ -1028,6 +1028,9 @@ static void SV_SendClientGameState( client_t *client ) {
 
 	client->state = CS_PRIMED;
 
+	client->gamestateAcked = qfalse;
+	client->downloading = qfalse;
+
 	client->pureAuthentic = qfalse;
 	client->gotCP = qfalse;
 
@@ -1061,11 +1064,12 @@ static void SV_SendClientGameState( client_t *client ) {
 
 	// write the configstrings
 	for ( start = 0 ; start < MAX_CONFIGSTRINGS ; start++ ) {
-		if (sv.configstrings[start][0]) {
+		if ( *sv.configstrings[ start ] != '\0' ) {
 			MSG_WriteByte( &msg, svc_configstring );
 			MSG_WriteShort( &msg, start );
-			MSG_WriteBigString( &msg, sv.configstrings[start] );
+			MSG_WriteBigString( &msg, sv.configstrings[ start ] );
 		}
+		client->csUpdated[ start ] = qfalse;
 	}
 
 	// write the baselines
@@ -1117,6 +1121,8 @@ void SV_ClientEnterWorld( client_t *client ) {
 
 	client->state = CS_ACTIVE;
 
+	client->gamestateAcked = qtrue;
+
 	// resend all configstrings using the cs commands since these are
 	// no longer sent when the client is CS_PRIMED
 	SV_UpdateConfigstrings( client );
@@ -1251,6 +1257,12 @@ static void SV_BeginDownload_f( client_t *cl ) {
 	// cl->downloadName is non-zero now, SV_WriteDownloadToClient will see this and open
 	// the file itself
 	Q_strncpyz( cl->downloadName, Cmd_Argv(1), sizeof(cl->downloadName) );
+
+	SV_PrintClientStateChange( cl, CS_CONNECTED );
+	cl->state = CS_CONNECTED;
+	cl->gentity = NULL;
+
+	cl->downloading = qtrue;
 }
 
 
@@ -1376,10 +1388,6 @@ static int SV_WriteDownloadToClient( client_t *cl )
 
 		Com_Printf( "clientDownload: %d : beginning \"%s\"\n", (int) (cl - svs.clients), cl->downloadName );
 
-		// Init
-		SV_PrintClientStateChange( cl, CS_CONNECTED );
-		cl->state = CS_CONNECTED;
-
 		cl->downloadCurrentBlock = cl->downloadClientBlock = cl->downloadXmitBlock = 0;
 		cl->downloadCount = 0;
 		cl->downloadEOF = qfalse;
@@ -2125,7 +2133,7 @@ static void SV_UserMove( client_t *cl, msg_t *msg, qboolean delta ) {
 	if ( cl->state == CS_PRIMED ) {
 		if ( sv_pure->integer != 0 && !cl->gotCP ) {
 			// we didn't get a cp yet, don't assume anything and just send the gamestate all over again
-			if ( !SVC_RateLimit( &cl->gamestate_rate, 4, 1000 ) ) {
+			if ( !SVC_RateLimit( &cl->gamestate_rate, 2, 1000 ) ) {
 				Com_DPrintf( "%s: didn't get cp command, resending gamestate\n", cl->name );
 				SV_SendClientGameState( cl );
 			}
@@ -2249,25 +2257,29 @@ void SV_ExecuteClientMessage( client_t *cl, msg_t *msg ) {
 	// the gamestate changes.  After the download is finished, we'll
 	// notice and send it a new game state
 	//
-	// https://zerowing.idsoftware.com/bugzilla/show_bug.cgi?id=536
-	// don't drop as long as previous command was a nextdl, after a dl is done, downloadName is set back to ""
-	// 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") ) {
-		// 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 ) ) {
-				if ( cl->gentity )
-					Com_DPrintf( "%s: dropped gamestate, resending\n", cl->name );
+	if ( cl->state == CS_ACTIVE ) {
+		if ( serverId != sv.serverId ) {
+			Com_DPrintf( "%s: ignoring pre map_restart / outdated client message\n", cl->name );
+			return;
+		}
+	} else if ( cl->state == CS_CONNECTED ) {
+		if ( !cl->downloading ) {
+			// send initial gamestate, client may not acknowledge it in next command but start downloading after SV_ClientCommand()
+			if ( !SVC_RateLimit( &cl->gamestate_rate, 2, 1000 ) ) {
 				SV_SendClientGameState( cl );
 			}
+			return;
 		}
-		return;
 	}
+	// else if ( cl->state == CS_PRIMED ) {
+		// in case of download intention client replies with (messageAcknowledge - gamestateMessageNum) >= 0 and (serverId == sv.serverId)
+		// in case of lost gamestate client replies with (messageAcknowledge - gamestateMessageNum) > 0 and (serverId == sv.serverId)
+		// in case of disconnect/etc. client replies with any serverId
+	//}
 
 	// this client has acknowledged the new gamestate so it's
 	// safe to start sending it the real time again
-	if( cl->oldServerTime && serverId == sv.serverId ){
+	if ( cl->oldServerTime && serverId == sv.serverId ) {
 		Com_DPrintf( "%s acknowledged gamestate\n", cl->name );
 		cl->oldServerTime = 0;
 	}
@@ -2286,6 +2298,24 @@ void SV_ExecuteClientMessage( client_t *cl, msg_t *msg ) {
 		}
 	} while ( 1 );
 
+	if ( cl->state == CS_PRIMED && !cl->gamestateAcked ) {
+		// not downloading and gamestate is not yet acknowledged
+		const int gsDelta = cl->messageAcknowledge - cl->gamestateMessageNum;
+		if ( gsDelta == 0 && serverId == sv.serverId ) {
+			// exact match, acknowledge
+			cl->gamestateAcked = qtrue;
+		} else {
+			// dropped gamestate or invalid serverId
+			if ( gsDelta > 0 ) {
+				Com_DPrintf( "%s: dropped gamestate, resending\n", cl->name );
+				if ( !SVC_RateLimit( &cl->gamestate_rate, 2, 1000 ) ) {
+					SV_SendClientGameState( cl );
+				}
+			}
+			return;
+		}
+	}
+
 	// read the usercmd_t
 	if ( c == clc_move ) {
 		SV_UserMove( cl, msg, qtrue );

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 = com_frameTime;
 	sv.checksumFeedServerId = sv.serverId;
 	Cvar_Set( "sv_serverid", va( "%i", sv.serverId ) );
 
@@ -568,10 +567,14 @@ void SV_SpawnServer( const char *mapname, qboolean killBots ) {
 				// was connected before the level change
 				SV_DropClient( &svs.clients[i], denied );
 			} else {
-				if( !isBot ) {
+
+				svs.clients[i].gamestateAcked = qfalse;
+
+				if ( !isBot ) {
 					// when we get the next packet from a connected client,
 					// the new gamestate will be sent
 					svs.clients[i].state = CS_CONNECTED;
+					svs.clients[i].gentity = NULL;
 				}
 				else {
 					client_t		*client;

code/server/sv_snapshot.c

@@ -756,15 +756,18 @@ void SV_SendClientMessages( void )
 	svs.msgTime = Sys_Milliseconds();
 
 	// send a message to each connected client
-	for( i = 0; i < sv_maxclients->integer; i++ )
+	for ( i = 0; i < sv_maxclients->integer; i++ )
 	{
 		c = &svs.clients[ i ];
 
 		if ( c->state == CS_FREE )
 			continue;		// not connected
 
-		if ( *c->downloadName )
-			continue;		// Client is downloading, don't send snapshots
+		//if ( *c->downloadName )
+		//	continue;		// Client is downloading, don't send snapshots
+
+		if ( c->state == CS_CONNECTED )
+			continue;		// waiting usercmd/downloading
 
 		// 1. Local clients get snapshots every server frame
 		// 2. Remote clients get snapshots depending from rate and requested number of updates