sourcemod icon indicating copy to clipboard operation
sourcemod copied to clipboard

Function int GetClientCount(bool inGameOnly) not accurate

Open GabenManPowered opened this issue 2 years ago • 14 comments

The internal function int GetClientCount(bool inGameOnly) is not accurate.

I wondered why sometimes I had a difference between the number of players (slots) used by the servers and those returned by the int GetClientCount(bool inGameOnly) function.

This is because the native sm_GetClientCount is not correct. Indeed in the case where we use the boolean inGameOnly at false, we make a loop on the players but SM has not yet recorded that the player was in the process of connection (i.e. state "connecting" in the engine).

playerhelpers (PlayerManager) is updated too late (when the client has reached the state connected => OnClientConnect()).

When player is in this state (connecting), it is not counted in SM whereas it must be, it use a slot!

To fix the case and have the correct count, we must use the function "GetClientCount() "from IServer (baseserver.h), then it will be correct (as it used in the engine ie (sv.GetClientCount()).

I have a fix (tested) wich work if we add iserver to sourcemod_api then use iserver->GetClientCount() (PR will come soon)

GabenManPowered avatar May 03 '22 00:05 GabenManPowered

when exactly was it inaccurate? when were you using the function?

JoinedSenses avatar May 03 '22 00:05 JoinedSenses

it is inacurrate in this case for example:

hostname: Bongo Gaben Land
version : 6630498/24 6630498 secure
udp/ip  : 124.56.69.21:27015  (public ip: 124.56.69.21)
map     : cs_office at: 0 x, 0 y, 0 z
tags    : startmoney
players : 2 humans, 19 bots (32 max)
edicts  : 595 used of 2048 max
# userid name                uniqueid            connected ping loss state  adr
#    728 "Dave"         BOT                                     active
#    732 "Mike"            BOT                                     active
#     45 "Pardoos�"         [U:1:*******]    00:09    14    0 connecting ***********
#    733 "Striker"           [U:1:******]       02:07      183    1 active **********

It cause problem too when we want :

  • to manage slots on server
  • to manage bots, because, if we want to create somes bots (make a own quota), those connecting players are not counted.
  • in local reporting or in database is not correct (ok its a fast case but it happened!)

Actually => GetClientCount(false) => 1 With my fix => GetClientCount(false) => 2

GabenManPowered avatar May 03 '22 01:05 GabenManPowered

InGame != connected. For your example above, there is only one human client that has passed ClientPutInServer.

psychonic avatar May 03 '22 01:05 psychonic

InGame != connected. For your example above, there is only one human client that has passed ClientPutInServer.

the connecting player must be counted

GabenManPowered avatar May 03 '22 01:05 GabenManPowered

I misread a bit. This is still a bit tricky. The behavior of this has always been the count of clients that are addressable by SourceMod. If the client isn't in a connected state, they are not able to be referenced in any meaningful way. Opting all users into changed behavior after it's been this way since the beginning can have unintended consequences.

psychonic avatar May 03 '22 01:05 psychonic

The fix i propose is less impact. You can have a look on https://github.com/GabenManPowered/sourcemod/tree/FixGetClientCount

here is a summary:

core\PlayerManager.cpp become:

int PlayerManager::GetNumPlayers(const bool inGameOnly)
{
	int count = 0;
	
	if (inGameOnly)
	{
		count = m_PlayerCount;
	}
	else
	{
		for ( int i = 0; i < iserver->GetClientCount(); i++ )
		{
		
			IClient *client = iserver->GetClient( i );
			
			if ( !client->IsConnected() )
				continue; // not connected yet, maybe challenging
			
			count++;
		}
	}
	
	return count;
}

core\logic\smn_players.cpp become

static cell_t sm_GetClientCount(IPluginContext *pCtx, const cell_t *params)
{
	return playerhelpers->GetNumPlayers(params[1]);
}

And in core\sourcemm_api.cpp we expose iserver to get GetClientCount() from engine

GabenManPowered avatar May 03 '22 01:05 GabenManPowered

i imagine it would be better to make a new function to serve this purpose rather than modifying an existing one and affecting those who use it. or make use of OnClientConnect to check if a client has initiated a connection

JoinedSenses avatar May 03 '22 01:05 JoinedSenses

i imagine it would be better to make a new function to serve this purpose rather than modifying an existing one and affecting those who use it. or make use of OnClientConnect to check if a client has initiated a connection

This, or a new parameter, or something. Bug or not, there are 15+ years of plugins potentially relying on the existing behavior.

psychonic avatar May 03 '22 01:05 psychonic

I see 3 possibilities:

  1. we use my fix by modifying GetNumPlayers()

  2. we add a parameter int GetClientCount(bool inGameOnly) => int GetClientCount(bool inGameOnly, bool FromEngine=false)

  3. Make a new function GetEngineClientCount()

I think you will like the 3rd :)

GabenManPowered avatar May 03 '22 01:05 GabenManPowered

or keep track of clients in your own plugin by utilizing OnClientConnect / OnClientDisconnected

JoinedSenses avatar May 03 '22 01:05 JoinedSenses

or keep track of clients in your own plugin by utilizing OnClientConnect / OnClientDisconnected

dont work, "connecting" clients are not tracked, OnClientConnect is called later

GabenManPowered avatar May 03 '22 01:05 GabenManPowered

Theres OnClientConnect and OnClientConnected, i assumed OnClientConnect was called when they were attempting to connect.

JoinedSenses avatar May 03 '22 01:05 JoinedSenses

Theres OnClientConnect and OnClientConnected, i assumed OnClientConnect was called when they were attempting to connect.

No, when players are in state "connecting" the OnClientConnect is not called, checked with a trace in

bool PlayerManager::OnClientConnect(edict_t *pEntity, const char *pszName, const char *pszAddress, char *reject, int maxrejectlen)
{
	int client = IndexOfEdict(pEntity);
	CPlayer *pPlayer = &m_Players[client];
	++m_PlayersSinceActive;
	
	printf("[DebugConnect] id=%d \"%s\" \"%s\" \n", client, pszName, pszAddress);
	
Client "Test" connected (**************).
Server waking up from hibernation
status
hostname: Bongo Gaben Land
version : 6630498/24 6630498 secure
udp/ip  : 124.56.69.21:27015  (public ip: 124.56.69.21)
map     : cs_office at: 0 x, 0 y, 0 z
tags    : startmoney
players : 1 humans, 0 bots (32 max)
edicts  : 669 used of 2048 max
# userid name                uniqueid            connected ping loss state  adr
#      2 "Test"   [U:1:**********]    00:14       15   0 connecting **************

As you can see, OnClientConnect is not called

GabenManPowered avatar May 03 '22 01:05 GabenManPowered

Hook Event "player_connect", "player_disconnect"

ambaca avatar May 03 '22 13:05 ambaca