[austin070]

I created a cell phone system that has three different types of hanging up.. EndCallForPlayer (for the caller if he wants the call to stop), EndCallForBoth (for both clients), and EndCallFromRing. The timer calls EndCallFromRing, but it appears to make the server lag near the timer's expiration. Is it somehow coded inefficiently?

pawn Код:

forward EndCallFromRing(playerid, recipient);
new ringtimer[MAX_PLAYERS];

public OnPlayerText(playerid, text[])
{
    if(Calling[playerid] == 1)
    {
        if(IsNumeric(text))
        {
            new recip, str[128];
            recip = GetIDFromCell(strval(text));
            if(recip == -1) return SendClientMessage(playerid, COLOR_RED, "(ERROR) {FFFFFF}Invalid phone number.");
            
            format(str, sizeof(str), "(CELL) Receiving call from %d...", PlayerInfo[playerid][CellNumber]);
            SendClientMessage(recip, COLOR_WHITE, str);
            format(str, sizeof(str), "(CELL) You are calling %d...", PlayerInfo[recip][CellNumber]);
            SendClientMessage(playerid, COLOR_WHITE, str);
            ReceivingCall[recip] = PlayerInfo[playerid][CellNumber];
            SendingCall[playerid] = PlayerInfo[recip][CellNumber];
            ringtimer[playerid] = SetTimerEx("EndCallFromRing", 30000, 0, "ii", playerid, recip);
            Calling[playerid] = 0;
            
            return 0;
        }
        else
        {
            SendClientMessage(playerid, COLOR_RED, "(ERROR) {FFFFFF}Invalid phone number.");
            Calling[playerid] = 0;
            return 0;
        }
    }
   return 1;
}


public EndCallFromRing(playerid, recipient)
{
    KillTimer(ringtimer[playerid]);
    SendClientMessage(playerid, COLOR_WHITE, "(CALL) The call has ended.");
    SendClientMessage(recipient, COLOR_WHITE, "(CELL) You have a missed call.");
    SendingCall[playerid] = 0;
    ReceivingCall[recipient] = 0;

    return 1;
}


stock GetIDFromCell(number)
{
    new id = -1;
    foreach(Player, i)
    {
        if(PlayerInfo[i][CellNumber] == number)
        {
            id = i;
        }
    }

    return id;
}

My gut tells me it is from GetIDFromCell.

[Snipa]

It's very inefficient.

Recode it.

[austin070]

Quote:

Originally Posted by Snipa It's very inefficient. Recode it.

Any advice on what is inefficient specifically?

[Vince]

I don't see anything inherently wrong with it. It isn't a looping timer so you don't need the variable, nor do you need to kill the timer. But that shouldn't really affect anything.

The GetIDFromCell function can be slightly optimized by immediately returning the playerid once found, rather than continuing the loop.

Pro tip: use the predefined constant INVALID_PLAYER_ID when handling an invalid playerid, rather than -1, 9999 or some other arbitrary value. Seems straightforward, but yet it's very underused.

[austin070]

Quote:

Originally Posted by Vince I don't see anything inherently wrong with it. It isn't a looping timer so you don't need the variable, nor do you need to kill the timer. But that shouldn't really affect anything. The GetIDFromCell function can be slightly optimized by immediately returning the playerid once found, rather than continuing the loop. Pro tip: use the predefined constant INVALID_PLAYER_ID when handling an invalid playerid, rather than -1, 9999 or some other arbitrary value. Seems straightforward, but yet it's very underused.

I do need the variable in order to end the ring elsewhere. Otherwise, thanks for the tip.