06.04.2016, 20:18
There are a few things that I would alter. I'll sum them up below.
If you don't want to read everything below, here's a summary:
> To avoid ID collision and save time, use an enum to store your dialogs in.
> You are best to put player-dependent global variable in your enum containing the player variables.
> Name your variables, enums and defines according to their purpose.
> You either use the stock keyword or you don't. Alternating between the use of it looks messy.
> Putting too many spaces in your statements and functions makes the code harder to read rather than enhance it.
> Instead of a bunch of if-statements to check constant values, use a switch-statement. It improves performance and readability.
> Use escape sequences where they are due such as: \t - this is like a tab in your IDE. It produces a horizontal tab aka indents the text.
Defining dialog ids was something I used to do myself and upon taking a closer look at the ShowPlayerDialog wiki page, I noticed that I could save time and avoid ID collisions by simply putting them in an enum. I highly recommend everyone to do so! You can keep the same names, so it's literally only the defines that you remove. So why not?:
I made a habit of putting every player-dependent variable in my enum. So these:
Would be put into the enum holding all the player variables:
The enum's name is also something we could argue about. It is conventional to name them according to their purpose. A few examples:
Same goes for naming your variables. An example from your gamemode:
I would go for something more clear like:
Or just 'Player' is also sufficient:
I've got nothing against people that use the stock keyword where it's not needed. I encourage correct usage of keywords and such, but I tend to just ignore it when I see it. On the other hand, alternating use of the keyword is something that bothers me. You either use it or you don't. It even makes the code look messy.
Putting too many spaces is also not conventional. And the funny part is that you do this occasionally making the script look even messier. A few examples from your gamemode:
Use a switch-statement when checking a a lot of constant values instead of a bunch of if-statements. It improves performance and readability. I won't copy paste examples from your gamemode as I'm addressing your whole OnDialogResponse callback.
Use escape sequences where they are due. In your case, an example would be '\t'. It's the escape sequence to indent code like your IDE would do with code. So use the '\t' character instead of having text with a bunch of spaces like below:
I haven't check every single line and I won't. I don't want to look like a douchebag by criticizing your code this much, but do take it as constructive criticism and try to improve where possible.
If you don't want to read everything below, here's a summary:
> To avoid ID collision and save time, use an enum to store your dialogs in.
> You are best to put player-dependent global variable in your enum containing the player variables.
> Name your variables, enums and defines according to their purpose.
> You either use the stock keyword or you don't. Alternating between the use of it looks messy.
> Putting too many spaces in your statements and functions makes the code harder to read rather than enhance it.
> Instead of a bunch of if-statements to check constant values, use a switch-statement. It improves performance and readability.
> Use escape sequences where they are due such as: \t - this is like a tab in your IDE. It produces a horizontal tab aka indents the text.
Defining dialog ids was something I used to do myself and upon taking a closer look at the ShowPlayerDialog wiki page, I noticed that I could save time and avoid ID collisions by simply putting them in an enum. I highly recommend everyone to do so! You can keep the same names, so it's literally only the defines that you remove. So why not?:
PHP Code:
enum {
DIALOG_REGISTER,
DIALOG_LOGIN,
DIALOG_CHANGENAME,
DIALOG_PASS,
DIALOG_SETTINGS,
DIALOG_ACCOUNT,
DIALOGID,
DIALOG_VEHICLES,
VSETTINGS,
PSETTINGS,
WELCOME,
DIALOG_DM,
DIALOG_BAN,
DIALOG_TEMPBAN,
DIALOG_ADMINS,
DIALOG_COLOR,
DIALOG_RADIO,
DIALOG_CMDS,
DIALOG_CMDS_ADMIN,
DIALOG_CMDS_VEHICLE,
DIALOG_CMDS_ACCOUNT,
DIALOG_CMDS_VIP,
DIALOG_CMDS_PLAYER,
DIALOG_CMDS_HOUSE,
DIALOG_TELES,
ADMIN1,
ADMIN2,
ADMIN3,
ADMIN4,
ADMIN5,
DIALOG_CS,
DIALOG_ATTACH_INDEX,
DIALOG_ATTACH_INDEX_SELECTION,
DIALOG_ATTACH_EDITREPLACE,
DIALOG_ATTACH_MODEL_SELECTION,
DIALOG_ATTACH_BONE_SELECTION,
DIALOG_ATTACH_OBJECT_SELECTION,
DIALOG_ATTACH_OBJECT2_SELECTION,
DIALOG_STATS
};
PHP Code:
new Float:SpecX[MAX_PLAYERS], Float:SpecY[MAX_PLAYERS], Float:SpecZ[MAX_PLAYERS], vWorld[MAX_PLAYERS], Inter[MAX_PLAYERS];
new IsSpecing[MAX_PLAYERS], IsBeingSpeced[MAX_PLAYERS],spectatorid[MAX_PLAYERS];
PHP Code:
enum Data
{
Float:SpecX,
Float:SpecY,
Float:SpecZ,
vWorld,
Inter,
IsSpecing,
IsBeingSpeced,
spectatorid
};
PHP Code:
enum E_PLAYER_DATA // This is usually the format I go with: E_*_DATA
PHP Code:
enum PlayerInfo
PHP Code:
enum PlayerData
PHP Code:
new Info[MAX_PLAYERS][Data];
PHP Code:
new PlayerData[MAX_PLAYERS][ENUM_NAME];
PHP Code:
new Player[MAX_PLAYERS][ENUM_NAME];
Putting too many spaces is also not conventional. And the funny part is that you do this occasionally making the script look even messier. A few examples from your gamemode:
PHP Code:
switch (Info[playerid][inDMZone])
{
case 1:
{
new Random = random(sizeof(RandomSpawnsDE));
createdm(playerid,RandomSpawnsDE[Random][0], RandomSpawnsDE[Random][1], RandomSpawnsDE[Random][2], RandomSpawnsDE[Random][3],3,1,1,24,25,100,"");
}
case 2:
{
new Random = random(sizeof(RandomSpawnsRW));
createdm(playerid,RandomSpawnsRW[Random][0], RandomSpawnsRW[Random][1], RandomSpawnsRW[Random][2], RandomSpawnsRW[Random][3],1,2,2,26,28,100,"");
return 1;
}
case 3:
{
new Random = random(sizeof(RandomSpawnsSOS));
createdm(playerid,RandomSpawnsRW[Random][0], RandomSpawnsSOS[Random][1], RandomSpawnsSOS[Random][2], RandomSpawnsSOS[Random][3],10,2,3,26,32,100,"");
}
case 4:
{
new Random = random(sizeof(RandomSpawnsSNIPE));
createdm(playerid,RandomSpawnsSNIPE[Random][0], RandomSpawnsSNIPE[Random][1], RandomSpawnsSNIPE[Random][2], RandomSpawnsSNIPE[Random][3],3,4,4,25,34,100,"");
}
case 5:
{
new Random = random(sizeof(RandomSpawnsCS));
createdm(playerid,RandomSpawnsCS[Random][0], RandomSpawnsCS[Random][1], RandomSpawnsCS[Random][2], RandomSpawnsCS[Random][3],0,77,5,31,16,100,"");
}
}
PHP Code:
if ( sscanf( params, "d", skinid )) return SendClientMessage( playerid, 0x6FFF00FF, "{F07F1D}USAGE: {BBFF00}/skin <ID>" );
PHP Code:
CMD:changename ( playerid )
Use escape sequences where they are due. In your case, an example would be '\t'. It's the escape sequence to indent code like your IDE would do with code. So use the '\t' character instead of having text with a bunch of spaces like below:
PHP Code:
strcat(bigstring, ""ORANGE" •"YELLOW" /ban "PINK" - "CYAN"Ban a Rule breaker\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /spec "PINK" - "CYAN"Spectate a player\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /specoff "PINK" - "CYAN"Stop spectating\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /warn "PINK" - "CYAN"Warn a Rule breaker\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /tempban "PINK" - "CYAN"Ban a Rule breaker temporary\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /jail "PINK" - "CYAN"Jail the Rule Breaker\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /unjail "PINK" - "CYAN"Unjail the jailed one\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /mute "PINK" - "CYAN"Shut the mouth of Rule breaker \n");
strcat(bigstring, ""ORANGE" •"YELLOW" /unmute "PINK" - "CYAN"Un mute the muted one\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /gethere "PINK" - "CYAN"Get a plaer to Admin's location\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /announce "PINK" - "CYAN"Make an Announcement\n");
strcat(bigstring, ""ORANGE" •"YELLOW" /kick "PINK" - "CYAN"Kick a Rule breaker\n");