AlliedModders - GetEntityClassname() and connecting players

AlliedModders (https://forums.alliedmods.net/index.php)

- Scripting (https://forums.alliedmods.net/forumdisplay.php?f=107)

- - GetEntityClassname() and connecting players (https://forums.alliedmods.net/showthread.php?t=338324)

GetEntityClassname() and connecting players

I asked about this in the IRC server, but no response.

So I can notice sometimes in the server error logs that some plugins, in this case Engineer pads, are throwing invalid entity errors.

PHP Code:


		
			
L 06/24/2022 - 20:28:17: [SM] Exception reported: Entity 21 (21) is invalid
L 06/24/2022 - 20:28:17: [SM] Blaming: Gameplay\Buildings\engipads.smx
L 06/24/2022 - 20:28:17: [SM] Call stack trace:
L 06/24/2022 - 20:28:17: [SM]   [0] GetEntPropString
L 06/24/2022 - 20:28:17: [SM]   [1] Line 758, C:\Users\User\Dropbox\compilator\include\entity.inc::GetEntityClassname
L 06/24/2022 - 20:28:17: [SM]   [2] Line 404, engipads.sp::HookSound

Ok, I checked this offending plugin and I can see that:

PHP Code:


		
			
public Action HookSound(int clients[MAXPLAYERS], int &numClients, char sample[PLATFORM_MAX_PATH],
        int &entity, int &channel, float &volume, int &level, int &pitch, int &flags,
        char soundEntry[PLATFORM_MAX_PATH], int &seed)
{
    if (IsValidEntity(entity))
    {
        char className[64];
        GetEntityClassname(entity, className, sizeof(className)); // Line 404
    
        if (StrEqual(className, "obj_attachment_sapper") && TF2_GetObjectType(entity) == TFObject_Sapper && channel == SNDCHAN_STATIC)
        {
            //...
        }
    }
        
    return Plugin_Continue;
}

So there's entity validity check before calling GetEntityClassname(). So I dig deeper. GetEntityClassname() calls GetEntPropString(). All these Get/SetEntProp[Int/String/Float] functions call IndexToAThings() function. If it returns false this error will be printed. Inside this function i see some special casing for player entities:

PHP Code:


		
			
    int index = g_HL2.ReferenceToIndex(num);
    if (index > 0 && index <= g_Players.GetMaxClients())
    {
        CPlayer *pPlayer = g_Players.GetPlayerByIndex(index);
        if (!pPlayer || !pPlayer->IsConnected())
        {
            return false;
        }
    }

There's a IsConnected() check inside. So we can't get entity classname(or use any of the prop getting/setting functions) if it's a player that's unconnected. Why? Is this an oversight(SM bug) or something plugins should check before using these functions? What should be done? I need opinion on this.

Re: GetEntityClassname() and connecting players

Maybe call "IsValidEdict" and use "GetEdictClassname".

Also you should check "channel == SNDCHAN_STATIC" before all the other checks since that consumes the least processing power while the others require more and if the channel is different then you just wasted a bunch of CPU time. I would personally also init the variable as "static char className" so it's not constantly created over and over, sound hooks fire very often.

Re: GetEntityClassname() and connecting players

Quote:

Originally Posted by Silvers (Post 2782502)

The topic is about IsValidEntity() not being enough of a check in order to use some of the other functions, but you started it and I will say that stack allocation of arrays should be dirt cheap, no? If it's static, then the plugin will take more runtime space, no?

Edit: Ok, i see this in the sourcepawn transition syntax docs: Also note, there is no equivalent of decl in the new declarator syntax. decl is considered to be dangerous and unnecessary. If an array's zero initialization is too costly, consider making it static or global. .
Maybe decl should have been kept in some way.

Re: GetEntityClassname() and connecting players

Some people were using decl wrong and not filling the variable before reading it, so they got unexpected results as it was sometimes filled with garbage. Static does use more memory but its so small its not going to impact. So long as the variable is set before reading using static in intensive functions is fine.

If I remember correctly an edict can be valid while an entity can be invalid, so that's why I recommend the edict check and getting edict classname.

Re: GetEntityClassname() and connecting players

Quote:

Originally Posted by Silvers (Post 2782507)

Thanks for the response. I'm going to test if that's the case soon. Still, the error message is confusing at least.