[joaquimandrade]

I offer you a little code donation:

PHP Code:

enum Rank
{
    ADMIN_SLOT = 0,
    ADMIN_OFFICER,
    ADMIN_MODERATOR,
    ADMIN_ADMINISTRATOR,
    ADMIN_OWNER
}

new g_Flags[Rank];

new g_cvars_rank[Rank];

public plugin_init()
{
    register_plugin( "Admin Hierarchy" , "0.1" , "bugsy" );
    register_cvar( "admin_hierarchy", "0.1", FCVAR_SERVER);
    
    g_pMode = register_cvar( "ah_mode" , "0" );
    g_pUser = register_cvar( "ah_user" , "z" );
    
    g_cvars_rank[ADMIN_OWNER] = register_cvar( "ah_owner" , "abcdefghijklmnopqrstu" );
    g_cvars_rank[ADMIN_ADMINISTRATOR] = register_cvar( "ah_administrator" , "bcdefijmnopqrstu" );
    g_cvars_rank[ADMIN_MODERATOR] = register_cvar( "ah_moderator" , "bcdefiju" );
    g_cvars_rank[ADMIN_OFFICER] = register_cvar( "ah_officer", "bcefij" );
    g_cvars_rank[ADMIN_SLOT] = register_cvar( "ah_slot", "b" );
}

public plugin_cfg()
{
    new szFlags[23];

    for ( new Rank:i = ADMIN_SLOT; i < Rank ; i++ )
    {
        get_pcvar_string (g_cvars_rank[i], szFlags , 22);
        g_Flags[i] = read_flags( szFlags );
    }
}

(...)

for ( new Rank:i = ADMIN_SLOT ; i < Rank ; i++ )
{
    if ( g_AdminFlags[id] == g_Flags[i] )
    {
        g_AdminLevel[id] = _:i;
        g_CurrentLevel[id] = _:i;
        break;
    }
} 


Also:

I see that you are using get_players with a flag. As you can see here it is not recommend. I also don't recommend, at least cause of the code readability.

Also:

After taking a look to your algorithm i think that:

PHP Code:

public client_disconnect(id)
{
    //If an admin is disconnecting we clear out his powers so they will not be recognized in the 
    //SetAdmins function.
    if ( g_AdminFlags[id] )
    {
        g_AdminFlags[id] = 0;
        g_AdminLevel[id] = 0;
        g_CurrentLevel[id] = 0;

        SetAdmins(0);
    }
} 


can be:

PHP Code:

public client_disconnect(id)
{
    //If an admin is disconnecting we clear out his powers so they will not be recognized in the 
    //SetAdmins function.
    if ( g_AdminFlags[id] )
    {
        g_AdminFlags[id] = 0;
        g_AdminLevel[id] = 0;
        
        if(g_CurrentLevel[id])
        {
            g_CurrentLevel[id] = 0;
            SetAdmins(0);    
        }
    }
} 


[Bugsy]

@joaquimandrade

I don't see the benefit of switching to an array of cvar pointers for a plugin with only a handful of cvars; it only reduces readability in my opinion.

As for get_players, it functions consistently fine in every plugin I've used it in. I do see that bailopan does not recommend using it with flags but being it functions fine consistently, I choose to use it. If\when it is done away with in future releases of AMX-X, I will then do away with using it.

This portion I agree with and I had already corrected.

PHP Code:

public client_disconnect(id)
{
    //If an admin is disconnecting we clear out his powers so they will not be recognized in the 
    //SetAdmins function.
    if ( g_AdminFlags[id] )
    {
        g_AdminFlags[id] = 0;
        g_AdminLevel[id] = 0;
        
        if(g_CurrentLevel[id])
        {
            g_CurrentLevel[id] = 0;
            SetAdmins(0);    
        }
    }
} 


[joaquimandrade]

The array of cvar pointers is self documenting and improves readability and the code itself . Looking at the code you will see that they are cvars related to the ranks, when you iterate you see that you are iterating over a Rank, and if you want to add a rank it would be easier to do it.

By using it you avoid code like:

PHP Code:

    for ( new i = 0; i <= 4 ; i++ )
    {
        switch ( i )
        {
            case ADMIN_OWNER: get_pcvar_string ( g_pOwner , szFlags , 22);
            case ADMIN_ADMINISTRATOR: get_pcvar_string ( g_pAdministrator , szFlags , 22 );
            case ADMIN_MODERATOR: get_pcvar_string ( g_pModerator , szFlags , 22 );
            case ADMIN_OFFICER: get_pcvar_string ( g_pOfficer , szFlags , 22 );
            case ADMIN_SLOT: get_pcvar_string ( g_pSlot , szFlags , 22 );
        }

        g_Flags[i] = read_flags( szFlags );
    } 


The getplayers question:

When you look at "c", you don't what that means, or if you know, you can easily forget. That don't happens if you use the more readable !is_user_bot(i)

[Bugsy]

Originally Posted by joaquimandrade The array of cvar pointers is self documenting and improves readability and the code itself . Looking at the code you will see that they are cvars related to the ranks, when you iterate you see that you are iterating over a Rank, and if you want to add a rank it would be easier to do it.

I know what you are saying and understand it. The cvars are only used in plugin_init and plugin_cfg so they are easily accessible to edit\add anything. If the plugin was large with more cvars that were scattered throughout the code, I would agree with you 100%.

Originally Posted by joaquimandrade The getplayers question: When you look at "c", you don't what that means, or if you know, you can easily forget. That don't happens if you use the more readable !is_user_bot(i)

Using get_players() with no flag requires calling is_user_bot() which is just an additional unneeded native. If anyone reading the code for reference purposes is unsure of the flags, they can be easily referenced in the functions page. A comment can also be added above the function call showing the flags purpose.

[joaquimandrade]

Originally Posted by Bugsy Using get_players() with no flag requires calling is_user_bot() which is just an additional unneeded native. If anyone reading the code for reference purposes is unsure of the flags, they can be easily referenced in the functions page. A comment can also be added above the function call showing the flags purpose.

But by using the flag you are making get_players more expensive (it has to make more checks) that without using the flag, so what you gain by not calling is_user_bot you spend it in get_players. Anyway, even being get_players("c") (a little) more faster than is_user_bot, "making the code less readable" its not worthy. Anyway, i told you my opinion and btw i will test with ammxprofiler to see the performance difference.

[Bugsy]

Originally Posted by joaquimandrade But by using the flag you are making get_players more expensive (it has to make more checks) that without using the flag, so what you gain by not calling is_user_bot you spend it in get_players. Anyway, even being get_players("c") (a little) more faster than is_user_bot, "making the code less readable" its not worthy. Anyway, i told you my opinion and btw i will test with ammxprofiler to see the performance difference.

Every programmer has their own personal opinion\preference for the best way to do things.

I do not see much of an expense in passing a flag to get_players(). Let me know your findings.

PHP Code:

static cell AMX_NATIVE_CALL get_players(AMX *amx, cell *params) /* 4 param */
{
    int iNum = 0;
    int ilen;
    char* sptemp = get_amxstring(amx, params[3], 0, ilen);
    int flags = UTIL_ReadFlags(sptemp);

    cell *aPlayers = get_amxaddr(amx, params[1]);
    cell *iMax = get_amxaddr(amx, params[2]);

    int team = 0;

    if (flags & 48)
    {
        sptemp = get_amxstring(amx, params[4], 0, ilen);

        if (flags & 16)
        {
            if (flags & 64)
                team = g_teamsIds.findTeamId(sptemp);
            else
                team = g_teamsIds.findTeamIdCase(sptemp);
        }
    }

    for (int i = 1; i <= gpGlobals->maxClients; ++i)
    {
        CPlayer* pPlayer = GET_PLAYER_POINTER_I(i);
        if (pPlayer->ingame)
        {
            if (pPlayer->IsAlive() ? (flags & 2) : (flags & 1))
                continue;
            if (pPlayer->IsBot() ? (flags & 4) : (flags & 8))
                continue;
            if ((flags & 16) && (pPlayer->teamId != team))
                continue;
            if ((flags & 128) && (pPlayer->pEdict->v.flags & FL_PROXY))
                continue;
            if (flags & 32)
            {
                if (flags & 64)
                {
                    if (stristr(pPlayer->name.c_str(), sptemp) == NULL)
                        continue;
                }
                else if (strstr(pPlayer->name.c_str(), sptemp) == NULL)
                    continue;
            }
            aPlayers[iNum++] = i;
        }
    }

    *iMax = iNum;
    
    return 1;
} 


[joaquimandrade]

Here they go. As you will see, the flag improvement is negligible.

Edit: ups. I need to test again. I tested without bots.

Ok now. Here they go. The improvement of the flag use is 1 second in 30000 calls.

What i have to say?

I learned that calling is_user_bot is a little more expensive than i thought. So this is a trade-off. Expensiveness for readability but, no big deal whatever the choice.

[Glist3r]

Well, the plugin doesn't work as it should.

When I connect with owner access , the other admins have the same powers as they had before. They don't lose their powers.

[joaquimandrade]

Originally Posted by Glist3r Well, the plugin doesn't work as it should. When I connect with owner access , the other admins have the same powers as they had before. They don't lose their powers.

Tell your cvars configurations and your flags. And be sure that you are the most powerful admin.

[Glist3r]

ah_mode 0
ah_user "bi"


"Glist3r" "******" "abcdefghijklmnopqrstu" "a"