Quote:
Originally Posted by pawnoholic
Not everyone uses y_iterate (foreach)
My not recursive version:
PHP Code:
ReturnRandomPlayer()
{
new
lastPlayer = GetPlayerPoolSize();
for (new i = random(lastPlayer + 1); i < lastPlayer; i++)
{
if (IsPlayerConnected(i))
{
return i;
}
}
return -1;
}
|
Quote:
Originally Posted by [HLF]Southclaw
Stop doing meaningless iterations! Build a list of connected players first then simply randomly choose from that list. Iterating through players then doing a `continue;` if the player isn't connected is possibly the worst way to do it.
|
Again, same mistake: If player ID 990 is online and no one else is then you could actually hang the server for a meaningful amount of time. (Yes I know this is an extreme example but you should always keep these kinds of scenarios in mind and evaluate whether or not they truly affect your program at runtime!)
The main thing to learn from this is: predictability. I can not effectively predict the performance of the function because there's no way to predict how many times that for loop could run. The solution below is predictable and is simply a linear function of the highest possible player ID (MAX_PLAYERS).
Here's the best way to do it without foreach:
pawn Code:
stock GetRandomPlayer() {
new
online[MAX_PLAYERS],
total;
for(new i, j = GetPlayerPoolSize(); i < j; ++i) {
if(IsPlayerConnected(i)) {
online[total++] = i;
}
}
if(total == 0) {
return INVALID_PLAYER_ID;
}
return online[random(total)];
}
The obvious evolution of this method would be to simply store a global list of online players and only update it when a player connects or disconnects - that's effectively how the foreach Player iterator works. This way you don't need to calculate the entire list every time because it already exists.
But since that would be more of a snippet than a pure function, this is the quick and dirty alternative - which doesn't really matter if both usage frequency of the function and the player count is low.
If your server is highly populated AND you are calling this function a LOT then I'd advise looking into either foreach or simply storing a up-to-date global list of players. But for all other use-cases, this is fine.