Hey nice to see you release things, i'm gonna drop some comments here to help you improve,
1- the variable mechjob[MAX_PLAYERS]; accepts only values 1 and 0 so bool:mechjob[MAX_PLAYERS]; and true/false with if(mechjob[playerid]) || if(!mechjob[playerid]) for 0 and 1 whereas 0 is false and 1 is true would've been better. The same deal applies to the variable 'Repairing' and 'request'
2- For a faster loop instead of doing:
Код:
for(new i=0 ; i<MAX_PLAYERS ; i++)
to avoid the original MAX_PLAYERS number and looping through players not connected you could:
Код:
for(new i, j = GetPlayerPoolSize(); i <= j; i++)
{
if(!IsPlayerConnected(i)) continue; //no need to loop through players not connected
3- There's absolutely no need for Getname and GetPlayerNameEx only one of them would work perfectly for all situations.
4- You could've destroyed the pickup and cars at OnFilterScriptExit (suggestion and optional)
5- No need at all to reset variables at BOTH onplayerconnect & disconnect, one would be enough
6- Why use this:
Код:
new pname[MAX_PLAYER_NAME], string[120];
GetPlayerName(playerid, pname, sizeof(pname));
format(string, sizeof(string), "%s is now one of our mechanics in San Andreas!", pname);
When you already made a function to get the player's name? it would've been better to put it to use
7- Instead of this:
Код:
if(GetPlayerState(playerid) == PLAYER_STATE_DRIVER) return SendClientMessage(playerid, COLOR_MECHANIC, "You have to be on foot to use this command");
if(GetPlayerState(playerid) == PLAYER_STATE_PASSENGER) return SendClientMessage(playerid, COLOR_MECHANIC, "You have to be on foot to use this command");
You could've done this:
Код:
if(GetPlayerState(playerid) != PLAYER_STATE_ONFOOT ) return SendClientMessage(playerid, COLOR_MECHANIC, "You have to be on foot to use this command");
8- And if you already used this:
Код:
if(mechjob[playerid] == 0) return SendClientMessage(playerid, COLOR_MECHANIC, "You're not a mechanic!");
You didn't have to put this afterwards:
Код:
if(mechjob[playerid] == 1)
{//code
9- Here :
Код:
else if(GetPlayerState(id) == PLAYER_STATE_ONFOOT) return SendClientMessage(playerid,COLOR_MECHANIC, "That player is not in a vehicle!");
else if(GetPlayerState(id) == PLAYER_STATE_PASSENGER) return SendClientMessage(playerid, COLOR_MECHANIC, "That player is not driving a vehicle!");
else if(GetPlayerState(playerid) == PLAYER_STATE_DRIVER) return SendClientMessage(playerid, COLOR_MECHANIC, "You have to be on foot to use this command!");
else if(GetPlayerState(playerid) == PLAYER_STATE_PASSENGER) return SendClientMessage(playerid, COLOR_MECHANIC, "You have to be on foot to use this command!");
Could've been summed up using the way i mentioned on #7
10- This:
Код:
if(GetPlayerState(id) == PLAYER_STATE_DRIVER)
{
Same as #8
11- Instead of this:
Код:
if(vhealth == 1000.0) return SendClientMessage(playerid, COLOR_MECHANIC, "That player's vehicle has enough health already!");
if(vhealth > 1000.0) return SendClientMessage(playerid, COLOR_MECHANIC, "That player's vehicle has enough health already!");
You could've done this:
Код:
if(vhealth >= 1000.0) return SendClientMessage(playerid, COLOR_MECHANIC, "That player's vehicle has enough health already!");// >= for bigger than or equal to
12- Here:
Код:
GetPlayerName(id, pName, sizeof(pName));
RepairVehicle(GetPlayerVehicleID(id));
format(string, sizeof(string), "You've successfully repaired %s's vehicle (ID: %d)! +$4200", pName, id);
Same as #6 repeated 2x times at the same spot also using string then formatting it and then defining another string called 'stringx' would've been better to re-format the older string after using sendclientmessage like this:
Код:
format(string, sizeof(string), "You've successfully repaired %s's vehicle (ID: %d)! +$4200", pName, id);
SendClientMessage(playerid, COLOR_MECHANIC, string);
format(string, sizeof(string), "[MECHANIC]: %s has fixed %s's vehicle!", pname, client);
SendClientMessageToAll(COLOR_MECHANIC, string);
13- This:
Код:
new IsMechOnline = 0;
Is not needed to be equaled to 0 on definition as the variable is already 0 the moment you define it,
Also the loop right underneath could've been better same as #2
14- At line 263-264 same deal as #8 and #10
The rest is just repeated as the numbers above so take a look at them and try to edit these parts but overall good job +rep.
EDIT: I also recommend pastebin/github it would've made it easier to look over the code.
