[Spock]

Like this

[Bugsy]

It only uses steam-id. I personally do not use amx bans so I do not know the correct format. Someone posted the below line in my aimbot detector plugin. What is the proper format?

PHP Code:

server_cmd( "amx_ban ^"0^" ^"%s^" ^"VAC Ban Found^"" , g_szAuthID[idCheck]); 


[diamond-optic]

default amxx: amx_ban client time reason
default amxbans: amx_ban time client reason

and i believe getting the unknown_STEAMID on the amxbans name/ip happens when the ban is added before a client has connected or after they have disconnected

[|PJ| Shorty]

PHP Code:

server_cmd( "amx_ban 0 %s VAC Ban Found",g_szAuthID[idCheck]); 


should do it with amxbans.

btw, good plugin idea, will try it +karma

[crazyeffect]

Maybe a MultiLangual?

You got pretty much client_prints...

[Bugsy]

I can do multilingual. I will put together the english lang and post it so people can translate.

[Hawk552]

There are a few mistakes and optimizations that can be made, however it is acceptable.

If you would like any information regarding possible adjustments you could make or things you could do to make this better, please feel free to post here or PM me.

Approved.

[Bugsy]

Originally Posted by Hawk552 There are a few mistakes and optimizations that can be made, however it is acceptable. If you would like any information regarding possible adjustments you could make or things you could do to make this better, please feel free to post here or PM me. Approved.

Thanks for the approval. I always welcome criticism and advice.

[Hawk552]

I assume that means you're asking me for advice on what to change.

new szHUD[479];

Make this static, as it is called relatively frequently.

                switch( PUNISHMETHOD )
                {
                    case SHOOTBLANKS:
                    {
                        if( g_PutInServer[id] )
                            set_user_hitzones( id , 0 , 0 );
                
                        g_ShootsBlanks[id] = 1;
                    }
                }

This is strange and unless you're planning on adding more cases you should use a conditional (if) statement.

                new param[1];
                param[0] = id;
                set_task( 10.0 , "NotifyAdmin" , _ , param , 1 );

This is unnecessary redirection. Simply pass "id" into the task_id parameter.

g_PutInServer[id] = 1;

Although this is technically a mild optimization, it's probably better to just use is_user_connected() whenever you need it.

if( is_user_admin(id) && (get_user_flags(id) & ADMIN_BAN) )

This is redundant. If they have the ban flag, they must be an admin, so remove the is_user_admin() call.

public NotifyAdmin(param[1])

Unnecessary redirection. Just use something like this:

public NotifyAdmin(id)

Even if you choose not to take my earlier suggestion on the set_task() call, the memory layout is the same, so changing it to this would remove unnecessary redirection.

new szNames[513];

You might consider making a global "cache" variable which is 513 cells wide. You could then use this for the previous problem where I stated you should use the "static" operator.

    for( new i = 1; i <= g_MaxPlayers; i++ )
    {
        if( is_user_connected(i) && g_VACBanned[i] )
        {
            iFound++;
            get_user_name( i , szName , 32 );
            format( szNames , 512 , "%s%s^n" , szNames , szName );
        }
    }

Bad form. Use get_players().

    if( iFound )
    {
        set_hudmessage(255, 0, 0, 0.10, 0.55, 0, 6.0, 6.0, 0.5, 0.15, 3);
        show_hudmessage(id, szNames);
        client_cmd(id,"spk buttons/blip1.wav");
    }

Bad form and unnecessary memory usage. Simply move this into the condition inside the for loop, then run break/return at the end. You're also using iFound as bool and since tags are meaningless you may consider renaming it to bFound if you choose not to take this suggestion.

new szPacket[512];

Use the global cache variable.

        if( iRequestType  )
            console_print(id , "* [VAC Ban Status] Error loading socket");

Inconsistent styling. Use brackets or don't.

message_begin(MSG_ONE, 2 , {0,0,0}, target);

Don't use static variables for messages. Also, consider using MSG_ONE_UNRELIABLE. Also, you can replace {0,0,0} with _.

char_to_num(chNum)
{
    new iNum;
    switch(chNum)
    {
        case '0':        iNum = 0;
        case '1':        iNum = 1;
        case '2':        iNum = 2;
        case '3':        iNum = 3;
        case '4':        iNum = 4;
        case '5':        iNum = 5;
        case '6':        iNum = 6;
        case '7':        iNum = 7;
        case '8':        iNum = 8;
        case '9':        iNum = 9;
    }
    
    return iNum;
}

num_to_char(iNum)
{
    new chNum;
    switch(iNum)
    {
        case 0:        chNum = '0';
        case 1:        chNum = '1';
        case 2:        chNum = '2';
        case 3:        chNum = '3';
        case 4:        chNum = '4';
        case 5:        chNum = '5';
        case 6:        chNum = '6';
        case 7:        chNum = '7';
        case 8:        chNum = '8';
        case 9:        chNum = '9';
    }
    
    return chNum;
}

These can be optimized by using '0' and 0 as basepoints and simply adding/subtracting.

[Bugsy]

Thanks for the help

Originally Posted by Hawk552                 switch( PUNISHMETHOD )                 {                     case SHOOTBLANKS:                     {                         if( g_PutInServer[id] )                             set_user_hitzones( id , 0 , 0 );                                         g_ShootsBlanks[id] = 1;                     }                 } This is strange and unless you're planning on adding more cases you should use a conditional (if) statement.

I used a switch because I am using a const to define the selected punishment. If I use an if-statement I get a compiler warning.

Originally Posted by Hawk552                 new param[1];                 param[0] = id;                 set_task( 10.0 , "NotifyAdmin" , _ , param , 1 ); This is unnecessary redirection. Simply pass "id" into the task_id parameter.

I guess I don't understand set_task 100% then. I thought task_id always specifies the task_id used as a handle of the task (which can later be used for remove_task, task_exists later on). Are you saying if I pass a player_id to it the function passed will receive it? What if I want to specify a task_id as well?

Originally Posted by Hawk552     if( iFound )     {         set_hudmessage(255, 0, 0, 0.10, 0.55, 0, 6.0, 6.0, 0.5, 0.15, 3);         show_hudmessage(id, szNames);         client_cmd(id,"spk buttons/blip1.wav");     } Bad form and unnecessary memory usage. Simply move this into the condition inside the for loop, then run break/return at the end. You're also using iFound as bool and since tags are meaningless you may consider renaming it to bFound if you choose not to take this suggestion.

That code is making a list of banned players so it waits until all players are checked before displaying the HUD msg. If I put it inside the loop it will display the first name found only and be done, which is not what I want.

Originally Posted by Hawk552         if( iRequestType  )             console_print(id , "* [VAC Ban Status] Error loading socket"); Inconsistent styling. Use brackets or don't.

I do try to always use brackets if one portion of a statement uses them, like if I use them for an 'if', I will also in 'else.' When the statement is only 1 line, I tend to leave out the brackets. I've seen this done in many scripts I have looked at so I figured it was accepted.