# HG changeset patch # User Adam Kaminski # Date 1634614204 14400 # Mon Oct 18 23:30:04 2021 -0400 # Node ID 91eca24b9b6996e774fd8232b0187f5f0d9d5490 # Parent 0878f44e527a4fc647976d237fa49835753cd617 Added packet loss mitigation, which the client can control using the CVar "cl_backupcommands". diff -r 0878f44e527a -r 91eca24b9b69 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 0878f44e527a -r 91eca24b9b69 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 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 0878f44e527a -r 91eca24b9b69 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 0878f44e527a -r 91eca24b9b69 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 0878f44e527a -r 91eca24b9b69 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 0878f44e527a -r 91eca24b9b69 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 0878f44e527a -r 91eca24b9b69 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 @@ -333,6 +333,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 )) @@ -342,9 +345,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; @@ -353,8 +357,14 @@ client->LastMoveCMD = new ClientMoveCommand( *static_cast( 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 0878f44e527a -r 91eca24b9b69 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 @@ -142,7 +142,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 ); @@ -4561,7 +4561,7 @@ pszString = GetStringCLCC ( static_cast ( 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; @@ -4743,11 +4743,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++; @@ -5230,6 +5231,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 ); @@ -5393,6 +5428,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 ); } @@ -5651,7 +5688,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; @@ -5660,7 +5697,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 ( pByteStream ); + if ( !bSentBackup ) + { + g_aClients[g_lCurrentClient].ulNumSentCMDs = 1; + g_aClients[g_lCurrentClient].ulNumExpectedCMDs = 1; + + return server_ParseBufferedCommand( 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( 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 0878f44e527a -r 91eca24b9b69 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 @@ -458,6 +458,14 @@ // we need to know how much thrust we need to add back in case the backtrace succeeds. fixed_t backtraceThrust[3]; + // [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; @@ -618,6 +626,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 0878f44e527a -r 91eca24b9b69 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 } // =================================================================================================