Commit: fda0458f443afe5e8cc906df39e97b49f44aeb9a - zandronum-tspg (hg)

This is a fork of Zandronum used on servers hosted by The Sentinels Playground (TSPG).

Commit MetaInfo

• Revisión: fda0458f443afe5e8cc906df39e97b49f44aeb9a (tree)
• Tiempo: 2021-10-19 12:30:04
• Autor: Adam Kaminski <kaminskiadam9@gmai...>
• Commiter: Adam Kaminski

Log Message

Added packet loss mitigation, which the client can control using the CVar "cl_backupcommands".

Cambiar Resumen

• modified: docs/zandronum-history.txt
• modified: src/cl_commands.cpp
• modified: src/cl_commands.h
• modified: src/cl_main.cpp
• modified: src/cl_main.h
• modified: src/network_enums.h
• modified: src/p_tick.cpp
• modified: src/sv_main.cpp
• modified: src/sv_main.h
• modified: wadsrc/static/menudef.za

Diferencia incremental

diff -r 629a821a3980 -r fda0458f443a docs/zandronum-history.txt
--- a/docs/zandronum-history.txt	Mon Oct 18 23:27:43 2021 -0400
+++ b/docs/zandronum-history.txt	Mon Oct 18 23:30:04 2021 -0400
@@ -59,6 +59,7 @@
 +	- Added the EVENT script type: GAMEEVENT_PLAYERCONNECT, indicating when a client or bot joins the server. [Kaminsky]
 +	- Added the EVENT script type GAMEEVENT_ACTOR_SPAWNED and GAMEVENT_ACTOR_DAMAGED, which are triggered just before an actor's first tic and when an actor takes damage. Note that for performance reasons, these events are disabled by default so modders have to enable them by themselves. [Kaminsky]
 +	- Added DMFlags: "sv_shootthroughallies" and "sv_dontpushallies", so a player's attacks can pass through and not push their allies. [Kaminsky]
++	- Added packet loss mitigation, which the client can control using the CVar "cl_backupcommands". [Kaminsky]
 -	- Fixed: Bots tries to jump to reach item when sv_nojump is true. [sleep]
 -	- Fixed: ACS function SetSkyScrollSpeed didn't work online. [Edward-san]
 -	- Fixed: color codes in callvote reasons weren't terminated properly. [Dusk]

diff -r 629a821a3980 -r fda0458f443a src/cl_commands.cpp
--- a/src/cl_commands.cpp	Mon Oct 18 23:27:43 2021 -0400
+++ b/src/cl_commands.cpp	Mon Oct 18 23:30:04 2021 -0400
@@ -76,6 +76,9 @@
 static	bool g_bIgnoreWeaponSelect = false;
 SDWORD g_sdwCheckCmd = 0;
 
+// [AK] Backups of the last few movement commands we sent to the server.
+static RingBuffer<CLIENT_MOVE_COMMAND_s, MAX_BACKUP_COMMANDS> g_BackupMoveCMDs;
+
 //*****************************************************************************
 //	FUNCTIONS
 
@@ -90,6 +93,13 @@
 
 //*****************************************************************************
 //
+void CLIENT_ClearBackupCommands( void )
+{
+	g_BackupMoveCMDs.clear( );
+}
+
+//*****************************************************************************
+//
 void CLIENT_IgnoreWeaponSelect( bool bIgnore )
 {
 	g_bIgnoreWeaponSelect = bIgnore;
@@ -415,8 +425,42 @@
 	// [AK] Create the movement command for the current tic.
 	CLIENT_MOVE_COMMAND_s moveCMD = clientcommand_CreateMoveCommand( );
 
-	CLIENT_GetLocalBuffer( )->ByteStream.WriteByte( CLC_CLIENTMOVE );
-	clientcommand_WriteMoveCommandToBuffer( moveCMD );
+	// [AK] If we don't want to send backup commands, send only this one and that's it.
+	if ( cl_backupcommands == 0 )
+	{
+		CLIENT_GetLocalBuffer( )->ByteStream.WriteByte( CLC_CLIENTMOVE );
+		clientcommand_WriteMoveCommandToBuffer( moveCMD );
+	}
+	else
+	{
+		// [AK] Save the movement command from this tic for future use.
+		g_BackupMoveCMDs.put( moveCMD );
+
+		ULONG ulNumSavedCMDs = 0;
+		ULONG ulNumExpectedCMDs = cl_backupcommands + 1;
+
+		// [AK] Determine how many movement commands we now have saved in the buffer.
+		for ( unsigned int i = 0; i < MAX_BACKUP_COMMANDS; i++ )
+		{
+			if ( g_BackupMoveCMDs.getOldestEntry( i ).ulGametic != 0 )
+				ulNumSavedCMDs++;
+		}
+
+		ULONG ulNumCMDsToSend = MIN( ulNumSavedCMDs, ulNumExpectedCMDs );
+		CLIENT_GetLocalBuffer( )->ByteStream.WriteByte( CLC_CLIENTMOVEBACKUP );
+
+		// [AK] We need to tell the server the number of movement commands we sent, and
+		// up to how many movment commands we actually want to send.
+		CLIENT_GetLocalBuffer( )->ByteStream.WriteShortByte( ulNumCMDsToSend, 4 );
+		CLIENT_GetLocalBuffer( )->ByteStream.WriteShortByte( ulNumExpectedCMDs, 4 );
+
+		// [AK] Older movement commands must be written to the buffer before newer ones.
+		for ( int i = ulNumCMDsToSend; i >= 1; i-- )
+		{
+			moveCMD = g_BackupMoveCMDs.getOldestEntry( MAX_BACKUP_COMMANDS - i );
+			clientcommand_WriteMoveCommandToBuffer( moveCMD );
+		}
+	}
 }
 
 //*****************************************************************************

diff -r 629a821a3980 -r fda0458f443a src/cl_commands.h
--- a/src/cl_commands.h	Mon Oct 18 23:27:43 2021 -0400
+++ b/src/cl_commands.h	Mon Oct 18 23:30:04 2021 -0400
@@ -70,6 +70,7 @@
 //	PROTOTYPES
 
 void	CLIENT_ResetFloodTimers( void );
+void	CLIENT_ClearBackupCommands( void );
 void	CLIENT_IgnoreWeaponSelect( bool bIgnore );
 bool	CLIENT_GetIgnoreWeaponSelect( void );
 bool	CLIENT_AllowSVCheatMessage( void );

diff -r 629a821a3980 -r fda0458f443a src/cl_main.cpp
--- a/src/cl_main.cpp	Mon Oct 18 23:27:43 2021 -0400
+++ b/src/cl_main.cpp	Mon Oct 18 23:30:04 2021 -0400
@@ -169,6 +169,16 @@
 // [JS] Always makes us ready when we are in intermission.
 CVAR( Bool, cl_autoready, false, CVAR_ARCHIVE )
 
+// [AK] Let the user send backup copies of old commands, in case of packet loss.
+CUSTOM_CVAR( Int, cl_backupcommands, 0, CVAR_ARCHIVE )
+{
+	if ( self < 0 )
+		self = 0;
+
+	if ( self > MAX_BACKUP_COMMANDS - 1 )
+		self = MAX_BACKUP_COMMANDS - 1;
+}
+
 //*****************************************************************************
 //	PROTOTYPES
 
@@ -3407,6 +3417,8 @@
 			( priorState == PST_REBORN ) || ( priorState == PST_REBORNNOINVENTORY ))
 		{
 			g_ulFirstSpawnedTic = gametic;
+			// [AK] Any backup commands we have saved are now invalid, so remove them.
+			CLIENT_ClearBackupCommands( );
 		}
 	}
 	else

diff -r 629a821a3980 -r fda0458f443a src/cl_main.h
--- a/src/cl_main.h	Mon Oct 18 23:27:43 2021 -0400
+++ b/src/cl_main.h	Mon Oct 18 23:30:04 2021 -0400
@@ -61,6 +61,9 @@
 #define	CONNECTION_RESEND_TIME		( 3 * TICRATE )
 #define	GAMESTATE_RESEND_TIME		( 3 * TICRATE )
 
+// [AK] The maximum number of move commands we're allowed to send per tic.
+#define MAX_BACKUP_COMMANDS			3
+
 //*****************************************************************************
 enum CONNECTIONSTATE_e
 {
@@ -211,6 +214,7 @@
 EXTERN_CVAR( String, cl_password )
 EXTERN_CVAR( String, cl_joinpassword )
 EXTERN_CVAR( Bool, cl_hitscandecalhack )
+EXTERN_CVAR( Int, cl_backupcommands ) // [AK]
 
 // Not in cl_main.cpp, but this seems like a good enough place for it.
 EXTERN_CVAR( Int, cl_skins )

diff -r 629a821a3980 -r fda0458f443a src/network_enums.h
--- a/src/network_enums.h	Mon Oct 18 23:27:43 2021 -0400
+++ b/src/network_enums.h	Mon Oct 18 23:30:04 2021 -0400
@@ -407,6 +407,7 @@
 	ENUM_ELEMENT( CLC_ENDCHAT ),
 	ENUM_ELEMENT( CLC_SAY ),
 	ENUM_ELEMENT( CLC_CLIENTMOVE ),
+	ENUM_ELEMENT( CLC_CLIENTMOVEBACKUP ),
 	ENUM_ELEMENT( CLC_MISSINGPACKET ),
 	ENUM_ELEMENT( CLC_PONG ),
 	ENUM_ELEMENT( CLC_WEAPONSELECT ),

diff -r 629a821a3980 -r fda0458f443a src/p_tick.cpp
--- a/src/p_tick.cpp	Mon Oct 18 23:27:43 2021 -0400
+++ b/src/p_tick.cpp	Mon Oct 18 23:30:04 2021 -0400
@@ -334,6 +334,9 @@
 						ulNumMoveCMDs++;
 				}
 
+				// [AK] Check if we should process this client's movement commands.
+				bool bProcessMoveCMD = SERVER_ShouldProcessMoveCommand( ulIdx, ulNumMoveCMDs );
+
 				// [AK] Handle the skip correction. If it explicity returns false, then we won't process two
 				// movement commands during this tic for the client.
 				if ((sv_smoothplayers) && (SERVER_HandleSkipCorrection(ulIdx, ulNumMoveCMDs) == false))
@@ -343,9 +346,10 @@
 				{
 					// Process only one movement command.
 					const bool bMovement = client->MoveCMDs[0]->isMoveCmd( );
-					client->MoveCMDs[0]->process( ulIdx );
 
-					if ( bMovement )
+					// [AK] Only update the last movement command if we're supposed to be processing any
+					// movement commands in the buffer at this time.
+					if (( bProcessMoveCMD ) && ( bMovement ))
 					{
 						if ( client->LastMoveCMD != NULL )
 							delete client->LastMoveCMD;
@@ -354,8 +358,14 @@
 						client->LastMoveCMD = new ClientMoveCommand( *static_cast<ClientMoveCommand *>( client->MoveCMDs[0] ));
 					}
 
-					delete client->MoveCMDs[0];
-					client->MoveCMDs.Delete(0);
+					// [AK] Only process movement commands if we're allowed to at this time. On the other
+					// hand, we can still process other commands in the buffer.
+					if (( bProcessMoveCMD ) || ( bMovement == false ))
+					{
+						client->MoveCMDs[0]->process( ulIdx );
+						delete client->MoveCMDs[0];
+						client->MoveCMDs.Delete( 0 );
+					}
 
 					if ( bMovement == true )
 						break;

diff -r 629a821a3980 -r fda0458f443a src/sv_main.cpp
--- a/src/sv_main.cpp	Mon Oct 18 23:27:43 2021 -0400
+++ b/src/sv_main.cpp	Mon Oct 18 23:30:04 2021 -0400
@@ -145,7 +145,7 @@
 
 static	bool	server_Ignore( BYTESTREAM_s *pByteStream );
 static	bool	server_Say( BYTESTREAM_s *pByteStream );
-static	bool	server_ClientMove( BYTESTREAM_s *pByteStream );
+static	bool	server_ClientMove( BYTESTREAM_s *pByteStream, bool bSentBackup );
 static	bool	server_MissingPacket( BYTESTREAM_s *pByteStream );
 static	bool	server_UpdateClientPing( BYTESTREAM_s *pByteStream );
 static	bool	server_WeaponSelect( BYTESTREAM_s *pByteStream );
@@ -4644,7 +4644,7 @@
 		pszString = GetStringCLCC ( static_cast<CLCC> ( lCommand ) );
 	else
 	{
-		if (( sv_showcommands >= 2 ) && ( lCommand == CLC_CLIENTMOVE ))
+		if (( sv_showcommands >= 2 ) && ( lCommand == CLC_CLIENTMOVE || lCommand == CLC_CLIENTMOVEBACKUP ))
 			return;
 		if (( sv_showcommands >= 3 ) && ( lCommand == CLC_PONG ))
 			return;
@@ -4826,11 +4826,12 @@
 		// Client is talking.
 		return ( server_Say( pByteStream ));
 	case CLC_CLIENTMOVE:
+	case CLC_CLIENTMOVEBACKUP:
 		{
 			bool	bPlayerKicked;
 
 			// Client is sending movement information.
-			bPlayerKicked = server_ClientMove( pByteStream );
+			bPlayerKicked = server_ClientMove( pByteStream, lCommand == CLC_CLIENTMOVEBACKUP );
 
 			if ( g_aClients[g_lCurrentClient].lLastMoveTick == gametic )
 				g_aClients[g_lCurrentClient].lOverMovementLevel++;
@@ -5313,6 +5314,40 @@
 
 //*****************************************************************************
 //
+bool SERVER_ShouldProcessMoveCommand( ULONG ulClient, ULONG ulNumMoveCMDs )
+{
+	// [AK] If the client isn't sending us backup commands, then always process movement commands.
+	if ( g_aClients[ulClient].ulNumExpectedCMDs == 1 )
+		return true;
+
+	// [AK] If the client is sending us backup commands and their tic buffer is getting too full, we
+	// must always process their movement commands. This condition is true when the number of commands
+	// stored in the buffer exceeds the number of commands we expect them to send us per tic.
+	if ( ulNumMoveCMDs >= g_aClients[ulClient].ulNumExpectedCMDs )
+		return true;
+
+	// [AK] Don't execute this block if we already processed a movement commands on this tic.
+	if ( g_aClients[ulClient].lLastMoveTickProcess != gametic )
+	{
+		// [AK] We have received enough movement commands from this client to process them, but we can only
+		// do this once per tic. Otherwise, their tic buffer will become too empty for us to process any
+		// backup commands properly.
+		if ( g_aClients[ulClient].ulNumSentCMDs == g_aClients[ulClient].ulNumExpectedCMDs )
+			return true;
+
+		// [AK] Are we still waiting for the client to send us more movement commands than we expect? In case
+		// case they suffer from packet loss, we don't want to wait forever to receive all of them , so we'll
+		// just have process whatever we have. To do this, increment the counter once per tic, which will
+		// eventually satisfy the condition above this one.
+		if ( g_aClients[ulClient].lLastMoveTick != gametic )
+			g_aClients[ulClient].ulNumSentCMDs++;
+	}
+
+	return false;
+}
+
+//*****************************************************************************
+//
 CLIENT_PLAYER_DATA_s::CLIENT_PLAYER_DATA_s ( player_t *player )
 {
 	PositionData = MOVE_THING_DATA_s( player->mo );
@@ -5476,6 +5511,8 @@
  
 	// [AK] We want to reset this client's last backtrace tic only when we reset their tic buffer.
 	g_aClients[ulClient].lLastBacktraceTic = 0;
+	// [AK] Also reset the backup command counters.
+	g_aClients[ulClient].ulNumSentCMDs = g_aClients[ulClient].ulNumExpectedCMDs = 1;
 
 	SERVER_ResetClientExtrapolation( ulClient );
 }
@@ -5740,7 +5777,7 @@
 
 //*****************************************************************************
 //
-static bool server_ClientMove( BYTESTREAM_s *pByteStream )
+static bool server_ClientMove( BYTESTREAM_s *pByteStream, bool bSentBackup )
 {
 	// Don't timeout.
 	g_aClients[g_lCurrentClient].ulLastCommandTic = gametic;
@@ -5749,7 +5786,43 @@
 	// in a buffer. This way we can limit the amount of movement commands
 	// we process for a player in a given tic to prevent the player from
 	// seemingly teleporting in case too many movement commands arrive at once.
-	return server_ParseBufferedCommand<ClientMoveCommand> ( pByteStream );
+	if ( !bSentBackup )
+	{
+		g_aClients[g_lCurrentClient].ulNumSentCMDs = 1;
+		g_aClients[g_lCurrentClient].ulNumExpectedCMDs = 1;
+
+		return server_ParseBufferedCommand<ClientMoveCommand>( pByteStream );
+	}
+
+	// [AK] If we get to this point, that means the client also wanted to
+	// send us backup movement commands. Here, we'll determine how many commands
+	// they actually sent us (ulNumSentCMDs), and how many commands they're
+	// trying to send to us per tic (ulNumExpectedCMDs = cl_backupcommands + 1).
+	ULONG ulNumSentCMDs = pByteStream->ReadShortByte( 4 );
+	ULONG ulNumExpectedCMDs = pByteStream->ReadShortByte( 4 );
+
+	ULONG ulOldBufferSize = g_aClients[g_lCurrentClient].MoveCMDs.Size( );
+	bool result = false;
+
+	// [AK] Parse all of the movement commands, but also remember if the client
+	// needs to be kicked at all or not.
+	for ( unsigned int i = 1; i <= ulNumSentCMDs; i++ )
+	{
+		if ( server_ParseBufferedCommand<ClientMoveCommand>( pByteStream ))
+			result = true;
+	}
+
+	// [AK] We only want to update the number of sent/expected commands from
+	// the client if these movement commands weren't treated as late commands
+	// because of the skip correction. We can figure this out by checking if
+	// the size of the tic buffer changed at all.
+	if ( ulOldBufferSize != g_aClients[g_lCurrentClient].MoveCMDs.Size( ))
+	{
+		g_aClients[g_lCurrentClient].ulNumSentCMDs = ulNumSentCMDs;
+		g_aClients[g_lCurrentClient].ulNumExpectedCMDs = ulNumExpectedCMDs;
+	}
+
+	return result;
 }
 
 ClientMoveCommand::ClientMoveCommand ( BYTESTREAM_s *pByteStream )

diff -r 629a821a3980 -r fda0458f443a src/sv_main.h
--- a/src/sv_main.h	Mon Oct 18 23:27:43 2021 -0400
+++ b/src/sv_main.h	Mon Oct 18 23:30:04 2021 -0400
@@ -454,6 +454,14 @@
 	// [AK] The number of tics we extrapolated this player's movement.
 	ULONG			ulExtrapolatedTics;
 
+	// [AK] How many movement commands did this player send us in the last packet? If this is greater than
+	// one, that means they also sent us backups of older commands.
+	ULONG			ulNumSentCMDs;
+
+	// [AK] How many movement commands are we expecting from this player? If this is greater than one, that
+	// means we're expecting them to also send us backups of older commands.
+	ULONG			ulNumExpectedCMDs;
+
 	// [BB] Variables for the account system
 	FString username;
 	unsigned int clientSessionID;
@@ -614,6 +622,7 @@
 void		SERVER_KillCheat( const char* what );
 void STACK_ARGS SERVER_PrintWarning( const char* format, ... ) GCCPRINTF( 1, 2 );
 void		SERVER_FlagsetChanged( FIntCVar& flagset, int maxflags = 2 );
+bool		SERVER_ShouldProcessMoveCommand( ULONG ulClient, ULONG ulNumMoveCMDs );
 bool		SERVER_HandleSkipCorrection( ULONG ulClient, ULONG ulNumMoveCMDs );
 bool		SERVER_IsBacktracingPlayer( ULONG ulClient );
 void		SERVER_ResetClientTicBuffer( ULONG ulClient, bool bClearMoveCMDs = true );

diff -r 629a821a3980 -r fda0458f443a wadsrc/static/menudef.za
--- a/wadsrc/static/menudef.za	Mon Oct 18 23:27:43 2021 -0400
+++ b/wadsrc/static/menudef.za	Mon Oct 18 23:30:04 2021 -0400
@@ -93,6 +93,13 @@
 	1.0, "DSL"
 }
 
+OptionValue ZA_BackupRate
+{
+	0.0, "None"
+	1.0, "One per tick"
+	2.0, "Two per tick"
+}
+
 OptionMenu ZA_NetworkOptions
 {
 	Title "NETWORK OPTIONS"
@@ -101,9 +108,21 @@
 	Option		"Unlagged",					"cl_unlagged",				"OnOff"
 	Option		"Unlag Type",				"cl_ping_unlagged",			"ZA_UnlagType"
 	Option		"Update Rate",				"cl_ticsperupdate",			"ZA_UpdateRate"
+	Option		"Send backup commands",		"cl_backupcommands",		"ZA_BackupRate"
 	Option		"Hitscan decals",			"cl_hitscandecalhack",		"OnOff"
 	Option		"Clientside puffs",			"cl_clientsidepuffs",		"OnOff"
 	Option		"Display packet loss",			"cl_showpacketloss",		"YesNo"
+
+	// [AK] Sending backup commands comes with drawbacks, such as increased outbound
+	// net traffic and their commands being delayed on the server's end. If the client
+	// decides to enable this, they should be made aware of the consequences too.
+	StaticText	" "
+	StaticText	"--- WARNING ---"
+	StaticText	"Sending backup commands helps mitigate packet loss", 1
+	StaticText	"but increases outbound net traffic and delays your input!", 1
+	StaticText	" "
+	StaticText	"You should only enable them if you're actually", 1
+	StaticText	"suffering from severe packet loss!", 1
 }
 
 // =================================================================================================