Quote:
Originally Posted by VincentDunn
This isn't the Script Request thread.
|
Well, might as well give him a few tips!
For starters, I have a topic that I created as a tutorial many months ago -
Converting strcmp+strtok commands to zcmd+sscanf.
A huge profit from using zcmd is that it is faster than running string comparisons (and sometimes a large amount of strcmp) in OnPlayerCommandText. I suppose my tutorial gives a proper overview of what's different, but lets have a look at your command...
First start off by including zcmd in your script. Either just #include <zcmd> with zcmd.inc in your includes folder or implement a similar system on your own.
pawn Код:
COMMAND:warn(playerid, params[])
{
return 1;
}
Now I see you've done something quite awkward for beginning, your code works like this:
1. Get player's name
2. Check if parameters are valid
3. Check if player is logged
4. Check if player can use the command at all
5. Use the player's name.
Key to what I'm trying to get to here is that do not get the player's name unless you're 100% sure you're going to use it. This is a quite irrelevant case for such coding practices, but such approach can become useful for larger pieces of code.
pawn Код:
COMMAND:warn(playerid, params[])
{
// Check if the player can use this command at all!
if(logged[playerid] == 0)
{
SendClientMessage(playerid, COLOR_RED, "You Must be logged in to use this command!");
return 1;
}
if(adminlevel[playerid] == 0)
{
SendClientMessage(playerid, COLOR_RED, "You do not have permission to use that command!");
return 1;
}
// Our player is an admin and has the rights to use this command, so parse the parameters
new giveplayerid, reason[32];
if(sscanf(params, "us[128]", giveplayerid, reason))
{
SendClientMessage(playerid, COLOR_DBLUE, "Correct Usage: /warn [PlayerID] [Reason]");
return 1;
}
// Check if the given player ID/name is connected/found
if(giveplayerid == INVALID_PLAYER_ID)
{
SendClientMessage(playerid, COLOR_RED, "This player is not connected!");
return 1;
}
GetPlayerName(giveplayerid, giveplayer, sizeof(giveplayer));
// Rest of the code here
return 1;
}
Now you'll see that I have implemented
sscanf. But why? Because you're expecting more than one parameter - player and a reason. sscanf's "u" specifier improves player finding even more - it allows player to enter a name as parameter and finds the according player. Why not strtok? It is slower and I already listed a good reason or two above.
Another suggestion that I have for you is that you have a lot of code that looks like this:
pawn Код:
if(WarnedTimes[giveplayerid] == 0)
{
WarnedTimes[giveplayerid]+=1;
GetPlayerName(giveplayerid, giveplayer, sizeof(giveplayer));
GetPlayerName(playerid, sendername, sizeof (sendername));
format(str, sizeof (str), "AdmCMD: %s was warned by %s for: %s [1/3 Warnings]", giveplayer, sendername, cmdtext[8]);
SendClientMessageToAll(COLOR_BLUE, str);
print(str);
return 1;
}
if(WarnedTimes[giveplayerid] == 1)
{
WarnedTimes[giveplayerid]+=1;
GetPlayerName(giveplayerid, giveplayer, sizeof(giveplayer));
GetPlayerName(playerid, sendername, sizeof (sendername));
format(str, sizeof (str), "AdmCMD: %s was warned by %s for: %s [2/3 Warnings]", giveplayer, sendername, cmdtext[8]);
SendClientMessageToAll(COLOR_BLUE, str);
print(str);
return 1;
}
else if(WarnedTimes[giveplayerid] == 2)
{
GetPlayerName(giveplayerid, giveplayer, sizeof(giveplayer));
GetPlayerName(playerid, sendername, sizeof (sendername));
format(str, sizeof (str), "AdmCMD: %s was kicked by %s for: %s [3/3 Warnings]", giveplayer, sendername, cmdtext[8]);
SendClientMessageToAll(COLOR_BLUE, str);
Kick(giveplayerid);
return 1;
}
How can this be simplified, you might ask, well... this is simple. One thing that you know is that you always need the player's name and the admin's name. Also you send a message and in most cases, increment the value of WarnedTimes[giveplayerid].
pawn Код:
GetPlayerName(playerid, sendername, sizeof(sendername));
GetPlayerName(giveplayerid, giveplayer, sizeof(giveplayer));
WarnedTimes[giveplayerid] ++; // WarnedTimes[giveplayerid] += 1;
if(WarnedTimes[giveplayerid] == 3)
{
format(str, sizeof(str), "AdmCMD: %s was kicked by %s for: %s [3/3 warnings]", giveplayer, sendername, reason);
}
else
{
format(str, sizeof(str), "AdmCMD: %s was warned by %s for: %s [%d/3 warnings]", giveplayer, sendername, reason, WarnedTimes[giveplayerid]);
Kick(giveplayerid);
}
SendClientMessageToAll(COLOR_BLUE, str);
There! Some simplification to your code. I hope you can learn at least something from it so you don't run redundant code!
