09.04.2011, 13:37
I wrote this for new scripters, or old scripters who have bad scripting habits.
Most things are about keeping the code orginized, some to make the code more effective.
Not Using arrays or loops
Instead of using a lot of integers, it's shorter and more effective to use arrays and loops.
Bad example:
This code is very long, there's a shorter and more effective way to do it:
Using the same code on multiple places
This is the same piece of code, used in multiple callbacks:
Good example, here we use a stock, which makes it easier to use the code on multiple places, and to edit it:
Not using sscanf in an if statement
It's better to use sscanf in an if statement, so that you can give an error when the input isn't valid:
When using sscanf in an if statement:
Not using switch statements:
This is how most people do it:
When using a switch statement:
This is more effective, and easier to use in some cases, another example:
Not necessary brackets
Without unnecessary brackets:
You can use brackets if you want to, but when you have a big script with many useless brackets, it takes longer to scroll trough the code.
Bad indentation:
Always keep your indentation nice, this makes it way easier to see if you're missing a bracket, and it prevents bad indentation warnings:
Bad indentation:
Good indentation:
I hope that this tutorial was useful, and that new scripters will understand that some things can be better.
If anyone knows another bad scripting habit, just post it in this topic, and I'll add it when I see it.
Nothing in the code has been tested, so it can contain errors, but the codes are only examples to illustrate my points.
~Yoshi
Most things are about keeping the code orginized, some to make the code more effective.
Not Using arrays or loops
Instead of using a lot of integers, it's shorter and more effective to use arrays and loops.
Bad example:
pawn Code:
public OnPlayerConnect(playerid)
{
new Weapon0;
new Weapon1;
new Weapon2;
new Weapon3;
new Weapon4;
new Weapon5;
new Weapon6;
new Weapon7;
new Weapon8;
new Weapon9;
new Weapon10;
new Weapon11;
new Weapon12;
new Path[25];
format(Path, sizeof(Path), "\scriptfiles\Accounts\%s.ini", pName(playerid));
Weapon0 = dini_Int(Path, Weapon0);
Weapon1 = dini_Int(Path, Weapon1);
Weapon2 = dini_Int(Path, Weapon2);
Weapon3 = dini_Int(Path, Weapon3);
Weapon4 = dini_Int(Path, Weapon4);
Weapon5 = dini_Int(Path, Weapon5);
Weapon6 = dini_Int(Path, Weapon6);
Weapon7 = dini_Int(Path, Weapon7);
Weapon8 = dini_Int(Path, Weapon8);
Weapon9 = dini_Int(Path, Weapon9);
Weapon10 = dini_Int(Path, Weapon10);
Weapon11 = dini_Int(Path, Weapon11);
Weapon12 = dini_Int(Path, Weapon12);
GivePlayerWeapon(playerid, Weapon0, 5000000000);
GivePlayerWeapon(playerid, Weapon1, 5000000000);
GivePlayerWeapon(playerid, Weapon2, 5000000000);
GivePlayerWeapon(playerid, Weapon3, 5000000000);
GivePlayerWeapon(playerid, Weapon4, 5000000000);
GivePlayerWeapon(playerid, Weapon5, 5000000000);
GivePlayerWeapon(playerid, Weapon6, 5000000000);
GivePlayerWeapon(playerid, Weapon7, 5000000000);
GivePlayerWeapon(playerid, Weapon8, 5000000000);
GivePlayerWeapon(playerid, Weapon9, 5000000000);
GivePlayerWeapon(playerid, Weapon10, 5000000000);
GivePlayerWeapon(playerid, Weapon11, 5000000000);
GivePlayerWeapon(playerid, Weapon12, 5000000000);
return 1;
}
pawn Code:
public OnPlayerConnect(playerid)
{
new Weapon[13] /* One more than 12 since 0 wont be used */, Path[25];
format(Path, sizeof(Path), "\scriptfiles\Accounts\%s.ini", pName(playerid));
for(new i = 1; i < 13; i++) // Loop - As long as i is less than 13 it repeats the code
{
Weapon[i] = dini_Int(path, Weapon[i]); // i is counting +1, every time the code gets called
GivePlayerWeapon(playerid, Weapon[i], 5000000000); // Give the weapons
}
return 1;
}
This is the same piece of code, used in multiple callbacks:
pawn Code:
public OnPlayerSpawn(playerid)
{
if(IsPlayerAdmin(playerid))
{
SetPlayerPos(playerid, 1509, -5014, 16);
SetPlayerInterior(playerid, 3);
SetPlayerVirtualWorld(playerid, 1);
SendClientMessage(playerid, -1, "Welcome to the Admin Base");
}
return ;
}
CMD:base(playerid, params[])
{
if(IsPlayerAdmin(playerid))
{
SetPlayerPos(playerid, 1509, -5014, 16);
SetPlayerInterior(playerid, 3);
SetPlayerVirtualWorld(playerid, 1);
SendClientMessage(playerid, -1, "Welcome to the Admin Base");
}
else return SendClientMessage(playerid, -1, "This command is for RCON admins only");
return 1;
}
pawn Code:
public OnPlayerSpawn(playerid)
{
if(IsPlayerAdmin(playerid)) PutPlayerInBase(playerid);
return 1;
}
CMD:base(playerid, params[])
{
if(IsPlayerAdmin(playerid)) PutPlayerInBase(playerid);
else return SendClientMessage(playerid, -1, "This command is for RCON admins only");
return 1;
}
stock PutPlayerInBase(playerid) // This makes it easier to edit the script, instead of editing the same thing a few times, you only have to change it once.
{
SetPlayerPos(playerid, 1509, -5014, 16);
SetPlayerInterior(playerid, 3);
SetPlayerVirtualWorld(playerid, 1);
return SendClientMessage(playerid, -1, "Welcome to the Admin Base");
}
It's better to use sscanf in an if statement, so that you can give an error when the input isn't valid:
pawn Code:
CMD:kick(playerid, params[])
{
if(IsPlayerAdmin(playerid))
{
new id, reason[75], string[150];
sscanf(params, "us[75]", id, reason) // Instead of doing this, use an if statement to make sure if the input is valid, so you can send an error if it's not.
format(string, sizeof(string), "You've been kicked for %s", reason);
SendClientMessage(id, -1, string);
Kick(id);
return 1;
}
else return SendClientMessage(playerid, -1, "This command is for RCON admins only");
}
pawn Code:
CMD:kick(playerid, params[])
{
if(IsPlayerAdmin(playerid))
{
new id, reason[75], string[150];
if(sscanf(params, "us[75]", id, reason)) return SendClientMessage(playerid, -1, "Syntax: /kick (playerid/name) (reason)");
format(string, sizeof(string), "You've been kicked for %s", reason);
SendClientMessage(id, -1, string);
Kick(id);
return 1;
}
else return SendClientMessage(playerid, -1, "This command is for RCON admins only");
}
This is how most people do it:
pawn Code:
public OnDialogResponse(playerid, dialogid, response, listitem, inputtext[])
{
if(dialogid == 1) // if statement
{
SendClientMessage(playerid, -1, inputtext);
}
if(dialogid == 2) // if statement
{
if(listitem == 0) // if statement
{
SendClientMessage(playerid, -1, "Listitem 0");
}
if(listitem == 1) // if statement
{
SendClientMessage(playerid, -1, "Listitem 1");
}
}
return 1;
}
pawn Code:
public OnDialogResponse(playerid, dialogid, response, listitem, inputtext[])
{
switch(dialogid) // switch statement
{
case 1: SendClientMessage(playerid, -1, inputtext);
case 2:
{
switch(listitem) // switch statement
{
case 0: SendClientMessage(playerid, -1, "Listitem 0");
case 1: SendClientMessage(playerid, -1, "Listitem 1");
}
}
}
return 1;
}
pawn Code:
switch(integer)
{
case 0: SendClientMessage(playerid, -1, "Case 0");
case 1: SendClientMessage(playerid, -1, "Case 1");
case 2..7: SendClientMessage(playerid, -1, "case 2, 3, 4, 5, 6 and 7");
default: SendClientMessage(playerid, -1, "All Other cases");
}
pawn Code:
public OnPlayerDeath(playerid, killerid, reason)
{
if(killerid == INVALID_PLAYER_ID)
{
SendClientMessageToAll(-1, "This time, the player died because of suicide"); // Since we're only using one line of code, you don't need to use
brackets.
}
return 1;
}
pawn Code:
public OnPlayerDeath(playerid, killerid, reason)
{
if(killerid == INVALID_PLAYER_ID) SendClientMessageToAll(-1, "This time, the player died because of suicide"); // Since we're only using one line of code, you don't need to use brackets.
return 1;
}
Bad indentation:
Always keep your indentation nice, this makes it way easier to see if you're missing a bracket, and it prevents bad indentation warnings:
Bad indentation:
pawn Code:
public OnPlayerConnect(playerid)
{
if(IsPlayerAdmin(playerid)) {
SendClientMessageToAll(-1, "An Admin Connected, press F12!");
}
for(new i = 0; i < MAX_PLAYERS; i++)
{
new string[20];
format(string, sizeof(string), "Admins In-Game: ~r~%d", GetOnlineAdmins());
TextDrawSetString(AdminCount[i], string);
}
return 1;
}
pawn Code:
public OnPlayerConnect(playerid)
{
if(IsPlayerAdmin(playerid))
{
SendClientMessageToAll(-1, "An Admin Connected, press F12!");
}
for(new i = 0; i < MAX_PLAYERS; i++)
{
new string[20];
format(string, sizeof(string), "Admins In-Game: ~r~%d", GetOnlineAdmins());
TextDrawSetString(AdminCount[i], string);
}
return 1;
}
If anyone knows another bad scripting habit, just post it in this topic, and I'll add it when I see it.
Nothing in the code has been tested, so it can contain errors, but the codes are only examples to illustrate my points.
~Yoshi