[StrikerZ]

(Removed)

[Jelly23]

Why 2 timers? you could use 1 for both bunny hop and cheat detection..

Also this:

PHP Code:

new string[120],name[24];
GetPlayerName(i,name,sizeof(name));
format(string,sizeof(string),"[ANTI-CHEAT]:  %s is speed hacking.",name); 


Why 120 cells? There is only 58 (i counted name aswell which is 24) ...

[Logic_]

More cleaner version can be done.

PHP Code:

new string[70]; // 70 is fine.
format(string, sizeof string, "[Anti-Cheat]: {FFFFFF}%s (%d) is speed hacking.", name); 


Run one timer, use KeyStateChanges for anti-bunny hop.

Add a configurable panel.

[RyderX]

Good job

[maximthepain]

Code:

GetPlayerHealth(i,health);
                    GetPlayerArmour(i,armour);
                    if(health >= 100)

This is by far the simplest code you can make, You check if players health did not go 100 or above? How about health hacks who put 33 hp at a time or only 10hp at a time or even easy-adjustable hacks who can auto 99 hp.
my point: If you want a realiable health anti-cheat, you should make server-side health and check every second if player health is changed not by the server-side health.

Can't call this anti-cheat. Fine job for newbies anyway.

[Yashas]

Code:

#define GivePlayerMoney                     GivePlayerCash

Terrible way to hook a function.

ALS Hook Method

Code:

public SendToAdmin(color,Message[])
{
    for(new i;i<MAX_PLAYERS;i++)
    {
        if(IsPlayerConnected(i))
        {
            if(i != INVALID_PLAYER_ID && IsPlayerAdmin(i))
            {
                SendClientMessage(i,color,Message);
            }
        }
    }
    return 1;
}

1. Use GetPlayerPoolSize to reduce the number of iterations.
2. INVALID_PLAYER_ID is 65535. The counter variable 'i' never takes the value 65535. The if check is redundant.
3. IsPlayerAdmin already has an internal IsPlayerConnected. A ghost cannot be an admin (a player who isn't connected cannot be an admin)
4. There is no need to keep that function public

Code:

GetPlayerHealth(i,health);
                    GetPlayerArmour(i,armour);
                    if(health >= 100)
                    {
                        new string[120],name[24];
                        GetPlayerName(i,name,sizeof(name));
                        format(string,sizeof(string),"[ANTI-CHEAT]:  %s is health-hacking. (Server health limit: 99) (Player's health: %d)",name,health);
                        SendToAdmin(COLOR_RED,string);
                    }
                    if(armour >= 100)
                    {
                        new string[120],name[24];
                        GetPlayerName(i,name,sizeof(name));
                        format(string,sizeof(string),"[ANTI-CHEAT]:  %s is armour-hacking. (Server armour limit: 99) (Player's armour: %d)",name,armour);
                        SendToAdmin(COLOR_RED,string);
                    }

1. Only one dumb person among 1000 hackers will set his health to a value more than 100. So this anti health hack is not at all efficient.
2. Add .0 to real values to avoid a useless float function call. (Refer to 'Do not mix floats and integers in an expression' here)

Code:

if(GetPlayerSpecialAction(i) == SPECIAL_ACTION_USEJETPACK)
                    {
                        new string[120],name[24];
                        GetPlayerName(i,name,sizeof(name));
                        format(string,sizeof(string),"[ANTI-CHEAT]:  %s is hacking jetpack.",name);
                        SendToAdmin(COLOR_RED,string);
                    }

Almost every server makes use of jetpack. If not for players, there will be a command for the administrators. Your code doesn't account for jetpack legitimately given to players.

You need to hook GetPlayerSpecialAction and keep track of jetpacks given.

Code:

if(Cash[i] != GetPlayerMoney(i))
                    {
                        new string[120],name[24];
                        GetPlayerName(i,name,sizeof(name));
                        format(string,sizeof(string),"[ANTI-CHEAT]:  %s is hacking money.",name);
                        SendToAdmin(COLOR_RED,string);
                    }

1. You lose 100$ when you use a Pay 'n' Spray
2. You lose 100$ when you die (removed by GTA SA by default)
3. You lose 1$ when you buy a drink

Your anti cheat will trigger false warnings for all those cases.

Code:

if(GetVehicleSpeed(GetPlayerVehicleID(i)) > 1000)
                    {
                        new string[120],name[24];
                        GetPlayerName(i,name,sizeof(name));
                        format(string,sizeof(string),"[ANTI-CHEAT]:  %s is speed hacking.",name);
                        SendToAdmin(COLOR_RED,string);
                    }

1000? The warning will almost never be triggered.

Code:

public OnPlayerKeyStateChange(playerid, newkeys, oldkeys)
{
    if((newkeys & KEY_JUMP) && !(oldkeys & KEY_JUMP))
    {
        if(!IsPlayerInAnyVehicle(playerid))
        {
            BunnyHop[playerid] += 1;
            SetTimer("TimerBunnyHop", 2000, false);
            new string[120],name[24];
            GetPlayerName(playerid,name,sizeof(name));
            format(string,sizeof(string),"[ANTI-CHEAT]:  %s is bunny-hopping.",name);
            if(BunnyHop[playerid] >= 3)
            {
                SendToAdmin(COLOR_RED,string);
            }
        }
    }
    return 1;
}

Vehicle model check is missing... you cannot bunny hop with an infernus.

Code:

public OnPlayerDisconnect(playerid, reason)
{
    return 1;
}

What purpose does it serve?

And lastly, if a hacker (or maybe an innocent) triggers a warning, the administrator's chat will be flooded with warnings. Two warnings per second to be exact.

Please test the code before publishing.

[StrikerZ]

Thanks for telling me.It was my first ever anti cheat I'll try to improve it asap. New update with the fixes will come soon

[TwinkiDaBoss]

Why are you making an entire callback for SendToAdmin(color,Message[]); ?
Just create it as a function...

You dont need both

pawn Code:

if(IsPlayerConnected(i))

and

pawn Code:

i != INVALID_PLAYER_ID

Just use the != INVALID_PLAYER_ID


Also, since I see that you are using the basic method for GetPlayerHeath to detect if they have over 100 hp etc. Do something about this

pawn Code:

format(string,sizeof(string),"[ANTI-CHEAT]:  %s is health-hacking. (Server health limit: 99) (Player's health: %d)",name,health);
                        SendToAdmin(COLOR_RED,string);

Because if someone gets detected as a hacker, this will send like 4 messages a second since you dont have a check to see if he was already confirmed a hacker or not

You could just do something like

pawn Code:

new bool:isAHacker[MAX_PLAYERS];

public OnPlayerConnect(playerid) {
    isAHacker[playerid] = false;
}

And then when you want to check if he is a hacking or not, check if he was already marked as a hacker

pawn Code:

if(isAHacker[playerid] == false) 
//now check

and when you want to set him as a hacker?

pawn Code:

isAHacker[playerid] = true;

Also regarding the vehicle speed, get a jet, and see how fast you can go, also try falling and also see whats the max speed you get I think that 1000.00 km/h is too much, try doing some tests and see what is natural & what un-natural

Also instead of using a timer for anti b-hop use timestamps

Example code for anti b-hop (untested & not compiled, just as an example)

pawn Code:

#define MAX_JUMPS       3 //maximum 3 jumps before we report him as a bunnyhopper
#define JUMPS_EXPIRE    5 //jumps will expire after 5 seconds

new timePassed[MAX_PLAYERS],
    playerJumps[MAX_PLAYERS];

if((newkeys & KEY_JUMP) && !(oldkeys & KEY_JUMP)) {
    if(!IsPlayerInAnyVehicle(playerid)) {
        if(gettime() > timePassed[playerid]) {
            playerJumps[playerid] = 0; //reseting their jumps if 5 seconds passed
            timePassed[playerid] = gettime()+JUMPS_EXPIRE;
            playerJumps[playerid] += 1;
        }
        else { //if 5 seconds didnt pass
            playerJumps[playerid] += 1;
            if(playerJumps[playerid] >= MAX_JUMPS) { //if they jumpped more than 3 times in less than 5 seconds
                Kick(playerid); //kick them or something
            }
        }
    }
}

All in all, considering its your 4th time making a release, please be more prepared when you make releases. Also I'd highly recommend switch to github so others can contribute to your code & make it better.


EDIT: Also for the bunnyhop, check player animation. When I was creating my own anti bunnyhop system I had a lot of problems by just simply checking how many times they pressed jump button. Make sure you check their animation when they press enter since imagine you just press space few times fast (you would actually just jump once) it will detect you as a bunnyhopper

[Yashas]

Quote:

Originally Posted by TwinkiDaBoss Why are you making an entire callback for SendToAdmin(color,Message[]); ? Just create it as a function...

All public functions are NOT callbacks. In his current code, SendToAdmin is just a function which is accesssible outside the script.

Quote:

Originally Posted by TwinkiDaBoss You dont need both pawn Code: if(IsPlayerConnected(i)) and pawn Code: i != INVALID_PLAYER_ID Just use the != INVALID_PLAYER_ID

You will never reach that iteration. Read my previous reply.

Quote:

Originally Posted by TwinkiDaBoss Also, since I see that you are using the basic method for GetPlayerHeath to detect if they have over 100 hp etc. Do something about this pawn Code: format(string,sizeof(string),"[ANTI-CHEAT]:  %s is health-hacking. (Server health limit: 99) (Player's health: %d)",name,health);                         SendToAdmin(COLOR_RED,string);

I forgot to mention in my previous post that even if you set health to 99.0, it needn't be updated for various reasons (packet loss). The player by default gets 100.0 HP on spawn.

Quote:

Originally Posted by TwinkiDaBoss Because if someone gets detected as a hacker, this will send like 4 messages a second since you dont have a check to see if he was already confirmed a hacker or not You could just do something like pawn Code: new bool:isAHacker[MAX_PLAYERS]; public OnPlayerConnect(playerid) {     isAHacker[playerid] = false; } And then when you want to check if he is a hacking or not, check if he was already marked as a hacker pawn Code: if(isAHacker[playerid] == false) //now check and when you want to set him as a hacker? pawn Code: isAHacker[playerid] = true;

He must use variables to count warnings. For every N number of warnings, a warning should be sent to the administrators. False detections happen often.

[TwinkiDaBoss]

[QUOTE=Yashas;3804414]All public functions are NOT callbacks. In his current code, SendToAdmin is just a function which is accesssible outside the script. [quote]

Yes but its not needed at all. I dont see a reason someone would use a public for a filterscript function that will get used only inside the filterscript itself.

Quote:

He must use variables to count warnings. For every N number of warnings, a warning should be sent to the administrators. False detections happen often.

Agreed, but I am just pointing him out to basically add any sort of check since his code doesnt have any (or I didnt catch it).