PDA

View Full Version : Best effiecent scripting


Jack_Leslie
31/07/2012, 03:27 AM
Hi there,

I'm currently working on a custom script, from complete scratch, even color definitions, I'm only currently at 1,261 lines but before I go any further I'd like to get some opinions on the best way to script efficiently, so in a years time the script is still scriptable (unlike some messy godfather edits).

I'm using y_ini and y_commands, with Dini for some other commands that require certain things that y_ini can't do. I'm then using foreach, sscanf2 and streamer. So in my opinion, that's pretty efficient so far.

How ever it's the actual style/method of coding I'm worried about that's going to make it inefficient. One thing is, for dialogs, I use dialog defines so that I can keep track of what dialog numbers I'm at.
Example:
#define DIALOG_LOGIN 1

Then, for player related variables (not sure of the technical name), I use a define for them.
Example:

#define MAX_PLAYERS MP

new gIsPlayerLoggedIn[MP];


I then use this style of coding when I create a command:

CMD:connected(playerid, params[])
{
if(IsPlayerConnected(playerid)) return SendClientMessage(playerid, -1, "You are connected.");
else return SendClientMessage(playerid, -1, "For some reason, you are not connected.");
}


Instead of:

CMD:connected(playerid, params[])
{
if(IsPlayerConnected(playerid))
{
SendClientMessage(playerid, -1, "You are connected.");
return 1;
}
else
{
SendClientMessage(playerid, -1, "For some reason, you are not connected.");
}
return 1;
}



So, how do you think I'm going with efficiency so far? Should I keep going the way I am or change things? Input your opinions below, and voice your opinion on anything else about scripting efficiently :)

dannyk0ed
31/07/2012, 03:33 AM
Welcome to ZCMD

ReneG
31/07/2012, 03:53 AM
How ever it's the actual style/method of coding I'm worried about that's going to make it inefficient. One thing is, for dialogs, I use dialog defines so that I can keep track of what dialog numbers I'm at.
Example:
#define DIALOG_LOGIN 1

That's fine.

Then, for player related variables (not sure of the technical name), I use a define for them.
Example:

#define MAX_PLAYERS MP

new gIsPlayerLoggedIn[MP];


#define MP MAX_PLAYERS
Is the correct way, you had it backwards.


I then use this style of coding when I create a command:

CMD:connected(playerid, params[])
{
if(IsPlayerConnected(playerid)) return SendClientMessage(playerid, -1, "You are connected.");
else return SendClientMessage(playerid, -1, "For some reason, you are not connected.");
}


Instead of:

CMD:connected(playerid, params[])
{
if(IsPlayerConnected(playerid))
{
SendClientMessage(playerid, -1, "You are connected.");
return 1;
}
else
{
SendClientMessage(playerid, -1, "For some reason, you are not connected.");
}
return 1;
}


There is literally no difference in efficiency between the two, it just makes you read it differently. Personal choice really.

Jack_Leslie
31/07/2012, 03:56 AM
That's fine.

#define MP MAX_PLAYERS
Is the correct way, you had it backwards.


There is literally no difference in efficiency between the two, it just makes you read it differently. Personal choice really.


Thanks, that's helpful! :)

coole210
31/07/2012, 03:56 AM
I think you're confusing efficiency with making the code shorter.

Jack_Leslie
31/07/2012, 04:01 AM
I think you're confusing efficiency with making the code shorter.

Yeah but let's say someone has a command that's 50 lines long, when it could easily be 10 lines long, the scripts not going to be very efficient in the long run.

coole210
31/07/2012, 04:22 AM
Yeah but let's say someone has a command that's 50 lines long, when it could easily be 10 lines long, the scripts not going to be very efficient in the long run.

You mean to say that the one that's 50 lines long is harder to read and understand? I think the one that tries to fit 50 lines worth of code into 10 is a bit harder to understand.

JoBullet
31/07/2012, 05:00 AM
Lookup Slice's project called PAWN-Boilerplate, though unreleased yet, I find it very mature and it serves exactly to make building a new gamemode easier. Why start from scratch, when you can build on others' people work, this will also mean you will able to work on other features unique to your gamemode(i.e. not common ones, like group system, user system etc.)

CaHbKo
31/07/2012, 08:40 AM
Judging code by amount of lines is like judging a plane by it's weight.

Sniper Kitty
31/07/2012, 09:14 AM
A plane's weight is important... too much and it cannot fly.

CaHbKo
31/07/2012, 09:16 AM
A plane's weight is important... too much and it cannot fly.

But both 2-seater and Boeing 747 serve their purpose.

Sinner
31/07/2012, 09:31 AM
Judging code by amount of lines is like judging a plane by it's weight.

I think you mean the famous quote:

“Measuring programming progress by lines of code is like measuring aircraft building progress by weight.”

- Bill Gates

Sniper Kitty
31/07/2012, 09:32 AM
I think you mean the famous quote:

“Measuring programming progress by lines of code is like measuring aircraft building progress by weight.”

- Bill Gates

Which would make alot more sense.

Vince
31/07/2012, 09:49 AM
Dini sucks. Please that me what it is that dini can do that any other (and better) ini writer can't do. Furthermore, use enumerators to keep track of dialogs. Much easier to maintain. Also you can undefine MAX_PLAYERS and set it to your own.

#if defined MAX_PLAYERS
#undef MAX_PLAYERS
#endif
#define MAX_PLAYERS (100)

Jack_Leslie
31/07/2012, 11:07 AM
Dini sucks. Please that me what it is that dini can do that any other (and better) ini writer can't do. Furthermore, use enumerators to keep track of dialogs. Much easier to maintain. Also you can undefine MAX_PLAYERS and set it to your own.

#if defined MAX_PLAYERS
#undef MAX_PLAYERS
#endif
#define MAX_PLAYERS (100)



I defined MP as MAX_PLAYERS so when I create player related variables I only need to do "[MP]", not "[MAX_PLAYERS]", just shortens scripting time.

I'm using Dini for an unbanned command, and I'll use it for other commands to. The reason I'm using that instead of y_ini (I still use y_ini for the the main things, like loading and saving players data) is because y_ini can't get a single value from a file, without needing the load the whole file and parse it (from my knowledge). The only function I use with Dini is dini_Get.

Slice
31/07/2012, 11:10 AM
I defined MP as MAX_PLAYERS so when I create player related variables I only need to do "[MP]", not "[MAX_PLAYERS]", just shortens scripting time.


That's just silly..

Jack_Leslie
31/07/2012, 11:12 AM
That's just silly..

That's why I made this thread, so I can see what I'm doing wrong and if I can improve :)

IstuntmanI
31/07/2012, 11:13 AM
I think
CMD:connected( playerid, params[ ] )
{
if( IsPlayerConnected( playerid ) )
return SendClientMessage( playerid, -1, "You are connected." );

return SendClientMessage( playerid, -1, "For some reason, you are not connected." );
}
is better (at least shorter) than
CMD:connected(playerid, params[])
{
if(IsPlayerConnected(playerid))
{
SendClientMessage(playerid, -1, "You are connected.");
return 1;
}
else
{
SendClientMessage(playerid, -1, "For some reason, you are not connected.");
}
return 1;
}

Slice
31/07/2012, 11:26 AM
Personally, I really don't like the way people use "return SendClientMessage". The return value of SendClientMessage has nothing to do with the return value of a command - it just happens to be the value you want to return anyway.

I'd say something like this is better:

CMD:connected(playerid, params[]) {
if (!IsPlayerConnected(playerid))
SendClientMessage(playerid, -1, "For some reason, you are not connected.");
else
SendClientMessage(playerid, -1, "You are connected.");

return 1;
}

CMD:kick(playerid, params[]) {
new otherid = strval(params);

if (!HasPermission(playerid))
SendClientMessage(playerid, -1, "You don't have permission.");
else if (!IsPlayerConnected(otherid))
SendClientMessage(playerid, -1, "Invalid player id given.");
else if (IsPlayerAdmin(otherid))
SendClientMessage(playerid, -1, "Other player is admin.");
else {
Kick(otherid);

SendClientMessage(playerid, -1, "KDONE");
}

return 1;
}

Jack_Leslie
31/07/2012, 11:46 AM
Personally, I really don't like the way people use "return SendClientMessage". The return value of SendClientMessage has nothing to do with the return value of a command - it just happens to be the value you want to return anyway.

I'd say something like this is better:

CMD:connected(playerid, params[]) {
if (!IsPlayerConnected(playerid))
SendClientMessage(playerid, -1, "For some reason, you are not connected.");
else
SendClientMessage(playerid, -1, "You are connected.");

return 1;
}

CMD:kick(playerid, params[]) {
new otherid = strval(params);

if (!HasPermission(playerid))
SendClientMessage(playerid, -1, "You don't have permission.");
else if (!IsPlayerConnected(otherid))
SendClientMessage(playerid, -1, "Invalid player id given.");
else if (IsPlayerAdmin(otherid))
SendClientMessage(playerid, -1, "Other player is admin.");
else {
Kick(otherid);

SendClientMessage(playerid, -1, "KDONE");
}

return 1;
}

I never thought of it that way, I'll most likely script similar to that, seems more efficient, which is what I was aiming to get from posting this thread, thank you!

Vince
31/07/2012, 11:52 AM
I don't see what's wrong with return SendClientMessage. You can always still add a 1 to the end of the statement, but what's the point? It's not like the return value of SendClientMessage is going to change any time soon. I'm not too keen on large if-elseif statements. It makes code less readable.

Universal
31/07/2012, 12:17 PM
Which way is more efficient?
for(new i=0; i<MAX_PLAYERS; i++) {
if(IsPlayerConnected(i)) {
// code goes here
}
}
OR
for(new i=0; i<MAX_PLAYERS; i++) {
if(!IsPlayerConnected(i)) continue;
// code goes here
}

Jack_Leslie
31/07/2012, 12:20 PM
Which way is more efficient?
for(new i=0; i<MAX_PLAYERS; i++) {
if(IsPlayerConnected(i)) {
// code goes here
}
}
OR
for(new i=0; i<MAX_PLAYERS; i++) {
if(!IsPlayerConnected(i)) continue;
// code goes here
}

I'd personally say the second one.

Slice
31/07/2012, 12:21 PM
I don't see what's wrong with return SendClientMessage. You can always still add a 1 to the end of the statement, but what's the point? It's not like the return value of SendClientMessage is going to change any time soon. I'm not too keen on large if-elseif statements. It makes code less readable.
It's not about whether it works. It just isn't right.

Which way is more efficient?
for(new i=0; i<MAX_PLAYERS; i++) {
if(IsPlayerConnected(i)) {
// code goes here
}
}
OR
for(new i=0; i<MAX_PLAYERS; i++) {
if(!IsPlayerConnected(i)) continue;
// code goes here
}

There's no significant difference. The former has fewer AMX instructions (maybe 3).

SuperViper
31/07/2012, 04:26 PM
In some cases, returning a function in a command can be useful for debugging because if it returns 0 you'll see unknown command, which usually means a function failed.

The__
31/07/2012, 06:08 PM
Which way is more efficient?
for(new i=0; i<MAX_PLAYERS; i++) {
if(IsPlayerConnected(i)) {
// code goes here
}
}
OR
for(new i=0; i<MAX_PLAYERS; i++) {
if(!IsPlayerConnected(i)) continue;
// code goes here
}
I'd sayforeach(new i : Player)

SuperViper
31/07/2012, 07:05 PM
I'd sayforeach(new i : Player)

Foreach isn't a magical tool that speeds up all player loops. In some cases, regular loops are faster than foreach.

Finn
31/07/2012, 07:15 PM
Foreach isn't a magical tool that speeds up all player loops. In some cases, regular loops are faster than foreach.
Interesting, what's that case?

The__
31/07/2012, 07:24 PM
Foreach isn't a magical tool that speeds up all player loops. In some cases, regular loops are faster than foreach.
I remember someone saying the foreach loops between online players, and not max player capacity, which is better IMO.

Universal
31/07/2012, 07:36 PM
I'd sayforeach(new i : Player)
I dont remember asking about foreach.

OT. Thanks for answers, I was just curious.

The__
31/07/2012, 07:45 PM
I dont remember asking about foreach.

OT. Thanks for answers, I was just curious.
I'm just advertising a better loop :).

SuperViper
31/07/2012, 11:12 PM
Interesting, what's that case?

for(new i = 0; i < MAX_PLAYERS; i++)
{
if(IsPlayerAdmin(i))
{
// Code
}
}

Foreach will use an online check which isn't needed because offline players will never be RCON admin. The same goes for any default SA:MP functions which are

IsPlayer*****.

FUNExtreme
31/07/2012, 11:38 PM
for(new i = 0; i < MAX_PLAYERS; i++)
{
if(IsPlayerAdmin(i))
{
// Code
}
}

Foreach will use an online check which isn't needed because offline players will never be RCON admin. The same goes for any default SA:MP functions which are

IsPlayer*****.

So.. What you are saying is that it is faster to loop through all player slots (even the empty ones) then to loop through all the used ones?

Real example: 5 players online, 500 slots avaible
=> Normal loop will go through all 500 slots
=> Foreach will go through 5

Whatever your argument might be, it is invalid.

Xentiarox
31/07/2012, 11:43 PM
One thing is, for dialogs, I use dialog defines so that I can keep track of what dialog numbers I'm at.
Example:
#define DIALOG_LOGIN 1

[/pawn]



Well I hope you are using FDLG by Gamer_Z which makes your dialogs very fast, and he also made a version which supports dialogs with names instead of numbers, it's called Named Dialogs (NDLG).

Examples are here:
http://forum.sa-mp.com/showthread.php?t=281980


for loops I suggest grabbing a vector implementation plugin for pawn, push back online players, and create an extra vector, for example, admins, and if a player is admin push his ID into the vector, in ondisconnect delete the player from the vectors. Looping will be very fast and efficient.

SuperViper
01/08/2012, 12:14 AM
So.. What you are saying is that it is faster to loop through all player slots (even the empty ones) then to loop through all the used ones?

Real example: 5 players online, 500 slots avaible
=> Normal loop will go through all 500 slots
=> Foreach will go through 5

Whatever your argument might be, it is invalid.

If foreach actually only loops through the connected players, then my arguement is invalid.

SuperViper
01/08/2012, 12:16 AM
Well I hope you are using FDLG by Gamer_Z which makes your dialogs very fast, and he also made a version which supports dialogs with names instead of numbers, it's called Named Dialogs (NDLG).

Examples are here:
http://forum.sa-mp.com/showthread.php?t=281980


for loops I suggest grabbing a vector implementation plugin for pawn, push back online players, and create an extra vector, for example, admins, and if a player is admin push his ID into the vector, in ondisconnect delete the player from the vectors. Looping will be very fast and efficient.

It's impossible to make dialogs faster.

AndreT
01/08/2012, 01:16 AM
for loops I suggest grabbing a vector implementation plugin for pawn, push back online players, and create an extra vector, for example, admins, and if a player is admin push his ID into the vector, in ondisconnect delete the player from the vectors. Looping will be very fast and efficient.
This sounds fairly interesting as I've ran tests in the past to see how fast vector functions from Teprey's plugin are compared to GVars and foreach in different situations. Foreach outdoes vectors (which makes perfect sense by the way) but there are situations where one might not want to create a lot of arrays statically even though their largest index is known.

At least stick regular looping to foreach, it is not very costly.

If foreach actually only loops through the connected players, then my arguement is invalid.
Open up the source, spend a good half hour trying to understand it, close it in frustration because you do not understand it entirely and just know that its implementation of a loop is complex (or well, might be so it just looks complex, but I don't know why on Earth would Y_Less do that) and also is pretty quick at looping through players.

It's impossible to make dialogs faster.
It depends on what you mean by making dialogs faster. As far as I know, the script aforementioned acts similarly to ZCMD (CallLocalFunction). This does not only provide speed, it also allows the scripter to have dialog responses spread in various locations thorough the script with an ease. (Probably have to admit though, since no string comparisons are in place anyways with dialogs, the speed difference might not be large.) There's also a downside since people are used to doing this:
#define AWESOME_DIALOG 9977
// ...
ShowPlayerDialog(playerid, AWESOME_DIALOG, ...);
Then this:
DIALOG(AWESOME_DIALOG)
... will not compile to what people hope it compiles to. In fact someone released a modified compiler for this purpose (haven't tried it). I myself revered to something like this:
DIALOG(9977) // AWESOME_DIALOG
Means I can still use search to find the dialog easily and also use AWESOME_DIALOG in ShowPlayerDialog.

linuxthefish
01/08/2012, 01:26 AM
Don't bother to much, just make sure you can read it. Making your code super efficient might just waste time.

SuperViper
01/08/2012, 02:59 AM
This sounds fairly interesting as I've ran tests in the past to see how fast vector functions from Teprey's plugin are compared to GVars and foreach in different situations. Foreach outdoes vectors (which makes perfect sense by the way) but there are situations where one might not want to create a lot of arrays statically even though their largest index is known.

At least stick regular looping to foreach, it is not very costly.


Open up the source, spend a good half hour trying to understand it, close it in frustration because you do not understand it entirely and just know that its implementation of a loop is complex (or well, might be so it just looks complex, but I don't know why on Earth would Y_Less do that) and also is pretty quick at looping through players.


It depends on what you mean by making dialogs faster. As far as I know, the script aforementioned acts similarly to ZCMD (CallLocalFunction). This does not only provide speed, it also allows the scripter to have dialog responses spread in various locations thorough the script with an ease. (Probably have to admit though, since no string comparisons are in place anyways with dialogs, the speed difference might not be large.) There's also a downside since people are used to doing this:
#define AWESOME_DIALOG 9977
// ...
ShowPlayerDialog(playerid, AWESOME_DIALOG, ...);
Then this:
DIALOG(AWESOME_DIALOG)
... will not compile to what people hope it compiles to. In fact someone released a modified compiler for this purpose (haven't tried it). I myself revered to something like this:
DIALOG(9977) // AWESOME_DIALOG
Means I can still use search to find the dialog easily and also use AWESOME_DIALOG in ShowPlayerDialog.

I haven't bothered to look at the source code of foreach and putting dialogs in a function will not make it faster. Do you have any clue why commands are faster when put into a function? That's because it's faster than comparing a string. For dialogs you only need to compare an integer which is basically instant.

Steven82
01/08/2012, 03:43 AM
Don't bother to much, just make sure you can read it. Making your code super efficient might just waste time.

Nothing wrong with making code efficient. As your server(player base) and script get larger and it isn't efficient at all, likely you might run into problems later on.

I for one, like to make my script work and find out ways to make it more efficient. And when I REALLY get bored, is when I find ways to make it either more efficient or recode it to look nicer ;)

Xentiarox
01/08/2012, 04:44 AM
I haven't bothered to look at the source code of foreach and putting dialogs in a function will not make it faster. Do you have any clue why commands are faster when put into a function? That's because it's faster than comparing a string. For dialogs you only need to compare an integer which is basically instant.


So you say that doing 10000 times


if(dialogid == something)else if(dialogid == something2)


is faster than using just one call:


CallLocalFunction


?


even integer comparision take time, especially if/else. Switch is much faster. But with great amount of dialogs then CallLocalFunction will beat if/else and switch.

iggy1
01/08/2012, 08:26 AM
So you say that doing 10000 times.

But who has ten thousand dialogs?

Even if you check 10000 dialogs that will only take a couple of MS. In most cases the checking for an int is faster. There is additional overhead using CallLocalFuntion (putting the name of the func in a var, the actual function call ect).

Personally i don't see the point in turning dialogs into functions.

Sniper Kitty
01/08/2012, 08:41 AM
I find the switch statement to be the most effective way of sorting out a variable such as dialogid, certainly simpler and more 'clean'.

Slice
01/08/2012, 08:44 AM
As for declaring dialog IDs and avoiding collisions, simply do this:
enum {
DIALOG_SOMETHING = 1000, // id 1000
DIALOG_HELLO, // id 1001
DIALOG_WORLD // id 1002
// etc.
}

Sniper Kitty
01/08/2012, 08:46 AM
Don't you have to assign the enum to an array?

sampreader
01/08/2012, 09:34 AM
efficient

MadeMan
01/08/2012, 09:40 AM
#define MP MAX_PLAYERS

new gIsPlayerLoggedIn[MP];


If another scripter reads the code (or you a year later), "MAX_PLAYERS" makes sense what it means, "MP" does not. So in order to know what "MP" means, you need to find the definition first. Not very efficient IMO.

I defined MP as MAX_PLAYERS so when I create player related variables I only need to do "[MP]", not "[MAX_PLAYERS]", just shortens scripting time.

I'm using Dini for an unbanned command, and I'll use it for other commands to. The reason I'm using that instead of y_ini (I still use y_ini for the the main things, like loading and saving players data) is because y_ini can't get a single value from a file, without needing the load the whole file and parse it (from my knowledge). The only function I use with Dini is dini_Get.

To shorten scripting time, use a scripting editor that supports auto-complete, for example Notepad++ (http://forum.sa-mp.com/showthread.php?t=174046).

Dini can be used only if you need to get just a single value at a time (your case). Loading/saving more than 1 value at a time is not recommended (dini_Set(Int/Float) for example reads and writes the WHOLE file 2 times every time you call it).

Personally, I really don't like the way people use "return SendClientMessage". The return value of SendClientMessage has nothing to do with the return value of a command - it just happens to be the value you want to return anyway.

I disagree. The success/failure of the command depends on SendClientMessage. If the message was sent, the command is handled. If not, the command is not handled. Also what SuperViper said:

In some cases, returning a function in a command can be useful for debugging because if it returns 0 you'll see unknown command, which usually means a function failed.

Don't you have to assign the enum to an array?

No.

http://wiki.sa-mp.com/wiki/Keywords:Initialisers#enum

Slice
01/08/2012, 10:02 AM
I disagree. The success/failure of the command depends on SendClientMessage. If the message was sent, the command is handled. If not, the command is not handled. Also what SuperViper said:


How does whether a command exists depend on the return value of SendClientMessage? Getting a message saying "Unknown command" when SendClientMessage fails (which it can't anyway) still doesn't make any sense!

MadeMan
01/08/2012, 10:24 AM
How does whether a command exists depend on the return value of SendClientMessage? Getting a message saying "Unknown command" when SendClientMessage fails (which it can't anyway) still doesn't make any sense!

Well yes, it depends how you interpret the return value of OnPlayerCommandText. Does it show
1) if a command exists?
2) if a response was sent to the player?

A question of preference I guess. I like the second one better.

AndreT
01/08/2012, 01:10 PM
I haven't bothered to look at the source code of foreach and putting dialogs in a function will not make it faster. Do you have any clue why commands are faster when put into a function? That's because it's faster than comparing a string. For dialogs you only need to compare an integer which is basically instant.
Too bad. If you don't know how it works, clearly say so in your post as well the next time.

Yes, and no reason to be rude. And a perfect reason to actually read my post wholly is that I happen to know a thing or two about this.

public OnDialogResponse(...)
{
if(dialogid == 0) {}
else if(dialogid == 1) {}
// ...
else if(dialogid == 99) {}
}
Executing this a million times took 3.2 seconds.

public OnDialogResponse(...)
{
switch(dialogid)
{
case 0: {}
// ...
case 499: {}
}
}
Executing this a million times took 0.9 seconds.

public OnDialogResponse(...)
{
new string[7];
format(string, sizeof(string), "DLG_%d", dialogid);
CallLocalFunction(string, "iiis", playerid, response, listitem, (isnull(inputtext) ? ("\1") : (inputtext)))
}
Executing this a million times took 1.1 seconds.

So why is this useful again, and the major reason I use it anyways? I can, with ease, move dialog response functions across my gamemode. Calling another function from a switch() or if else filled OnDialogResponse, IMO, would be even worse.
With many dialogs, it is possible to achieve that using CallLocalFunction actually becomes faster than switch statement. About being faster than a bunch of if and if-else statements, it does not take many dialogs to prove that. Perhaps 40 or so.

Also, next time, please read my post.

JoBullet
01/08/2012, 02:53 PM
Actually switch in current virtual machine implementation is pretty much same as if/else block

iggy1
01/08/2012, 04:18 PM
I'm starting to change my mind a little. I think i will create a script using dialogs as functions. Because i think it will make the code more managable.

Not because of the speed because from AndreT' post that wont be a problem, the slowest method in his tests executes 1 time per 0.0000032 seconds, seeing that OnDialogResponse shouldn't be called in large loops, speed wont ever really be a problem for dialogs.

AndreT
01/08/2012, 06:12 PM
Not because of the speed because from AndreT' post that wont be a problem, the slowest method in his tests executes 1 time per 0.0000032 seconds, seeing that OnDialogResponse shouldn't be called in large loops, speed wont ever really be a problem for dialogs.That's awesome thinking. I mean, I discovered myself in a similar position the other day when I was looking at assembly output and running some speed tests on another subject. Then I seriously realized that if it makes almost no difference in speed (for example going from 0.006ms to 0.003ms in executing a routine) and makes the code readability worse, it is not worth taking into consideration.

But changes like this, specifically talking dialog responses called using CallLocalFunction, are not only faster at a large quantity of dialogs, but also improve readability (I love to have my gamemode laid out in several files). When it is a slightly better practice, I suppose it is worth keeping in mind!

Y_Less
01/08/2012, 08:06 PM
I don't think you know what "from scratch" means! If you are using multiple libraries and plugins you are using other people's code and thus are not working from scratch. However, this its a much better way of doing it.

Ok, I have read the rest of this topic now and have a few points:

1) Do not try and optimise at this stage.

2) DO NOT TRY AND OPTIMISE AT THIS STAGE!

I said it twice because it is so important! As you develop your choose things will change often and if you wrote it as compactly and as specialized as possible the first time round, then modifying it will be nigh on impossible. Use braces after if statements, use explicit returns, comment everything. You asked the wrong questions to start with - it isn't about raw performance with new scripts, its about development effort. You mentioned you are even making your own colour defines - why? That's thousands of lines of code that I and others have already written - green is green, who cares who wrote that line? Save yourself some time!

Size of code makes no difference after compilation so don't sacrifice readability for short coffee since you gain nothing.

Finally, and this is the MOST important part - you don't know what code is slow. You can spend days writing the most beautifully crafted and highly efficient vehicle system, but if all your players do is DM, then you wasted that time! Use as much of other peoples' code as you can (free scripters for your team) and get the mode out the door, ugly as it may be. Once people start playing, only THEN can you find out which bits of code require attention by profiling it constantly. What's more, since you wrote your code so neatly and documented everything, modifying it for speed at this point is easy.

Sniper Kitty
01/08/2012, 09:42 PM
However, this its a much better way of doing it.

I don't find it better.
Custom code takes more time, yes, but there's a benefit!
The benefit is you learn more along the way.
Although I still do relay on a_samp and the MySQL plugin, I don't use any other plugins or includes in my scripts.

SuperViper
01/08/2012, 09:52 PM
Too bad. If you don't know how it works, clearly say so in your post as well the next time.

Yes, and no reason to be rude. And a perfect reason to actually read my post wholly is that I happen to know a thing or two about this.

public OnDialogResponse(...)
{
if(dialogid == 0) {}
else if(dialogid == 1) {}
// ...
else if(dialogid == 99) {}
}
Executing this a million times took 3.2 seconds.

public OnDialogResponse(...)
{
switch(dialogid)
{
case 0: {}
// ...
case 499: {}
}
}
Executing this a million times took 0.9 seconds.

public OnDialogResponse(...)
{
new string[7];
format(string, sizeof(string), "DLG_%d", dialogid);
CallLocalFunction(string, "iiis", playerid, response, listitem, (isnull(inputtext) ? ("\1") : (inputtext)))
}
Executing this a million times took 1.1 seconds.

So why is this useful again, and the major reason I use it anyways? I can, with ease, move dialog response functions across my gamemode. Calling another function from a switch() or if else filled OnDialogResponse, IMO, would be even worse.
With many dialogs, it is possible to achieve that using CallLocalFunction actually becomes faster than switch statement. About being faster than a bunch of if and if-else statements, it does not take many dialogs to prove that. Perhaps 40 or so.

Also, next time, please read my post.

1) Did you actually create the function?
2) You're checking for 3 seperate dialogs and 2 in the top two examples.

Bottom line is, putting dialogs in functions won't make things faster. There's no point in arguing anyway because the difference is so small that it won't be noticed at all.

Sniper Kitty
01/08/2012, 09:56 PM
the difference is so small that it won't be noticed at all.

Actually in his example the differences are near seconds which actually is a noticeable difference.
Though in your main argument you're saying that all the methods are basically the same result, I agree that it's a matter of personal choice.

SuperViper
01/08/2012, 10:12 PM
Actually in his example the differences are near seconds which actually is a noticeable difference.
Though in your main argument you're saying that all the methods are basically the same result, I agree that it's a matter of personal choice.

In his example he did it a million times.

Sniper Kitty
01/08/2012, 10:12 PM
It does seem unlikely.

AndreT
01/08/2012, 10:24 PM
1) Did you actually create the function?
2) You're checking for 3 seperate dialogs and 2 in the top two examples.

Bottom line is, putting dialogs in functions won't make things faster. There's no point in arguing anyway because the difference is so small that it won't be noticed at all.Perhaps I should of have been clearer about the examples I brought in.

The first example uses a bunch of "if" and "if else" statements. More precisely one if-statement and 99 if else statements. Running the callback with a hundred (100) if-checks a million times (1000000). For reference, the dialog ID passed on to OnPlayerDialog in the loop (which iterated a million times, as I said already) was 99 - the HIGHEST, so it will have the most checking to do.

The second example uses a switch() statement with 500 cases, from 0 to 499. The dialog ID passed on to OnPlayerDialog in the loop was 499 - the HIGHEST once again. PAWN has its interesting way of switch() behavior and as JoBullet mentioned, apparently quite alike if statements.

The third example is the version I quickly made up. I believe this could be tuned to be even a bit faster. The ID passed on to that is 99, in fact it does not even make a difference.

So for the results: as soon as you have more than about 30 dialogs, using the third method with CallLocalFunction will be faster than a bunch of if-checks. If you are clever and use switch(), and I assume most people do, it will take about 500 dialogs in total until the CallLocalFunction will get faster.

Yes, not all servers have 500 dialogs (mine has about 90), but the benefit from using the third method is that you can also move the dialog responses thorough your gamemode files. If you were to call a function from OnDialogResponse's if-or-switch statement(s), you'd be left with a far slower version. Furthermore, your own argumentation does not make sense in the last post you made. We are talking about what's actually faster and some speed can of course be traded in for readability or scripter preferences.

Have a good evening!

Y_Less
01/08/2012, 10:27 PM
I don't find it better.
Custom code takes more time, yes, but there's a benefit!
The benefit is you learn more along the way.
Although I still do relay on a_samp and the MySQL plugin, I don't use any other plugins or includes in my scripts.

That doesn't make any sense at all... Let's say you write 1000 lines of code and learn 2 things on the way - that's great but it will take you the same amount of time regardless of what those lines do. So you can either spend that time learning and re-writing code done before, or spend that time learning and writing something original. Either way you code and learn, so why waste it?

1) Did you actually create the function?
2) You're checking for 3 seperate dialogs and 2 in the top two examples.

Bottom line is, putting dialogs in functions won't make things faster. There's no point in arguing anyway because the difference is so small that it won't be noticed at all.

That's not a bottom line because you have no evidence to support your claim that thousands of integer comparisons and branches are somehow faster than one function call, despite evidence (though admittedly weak) to the contrary! You can't just claim something for no good reason. Anyway, I personally would say use y_dialogs, not for the speed but for the simplicity (and if you want to argue with that, make sure you understand my last post first).

AndreT
01/08/2012, 11:11 PM
You are right about the evidence I presented being rather weak. From the first moment on, I was intrigued how weird it would be to clock a hundred if-checks against calls to format and CallLocalFunction. But eventually they would be designed to do the same thing IF I was to make the code call a function in every if statement scope. However since it is quite late, I cannot even be bothered to write something that'll generate such code for me, I only did I partially.

http://pastebin.com/eArP5Ahk
(and no, this isn't the test from before, although to compensate it only a little bit, I still pass the same amount of values to the DLG_%d function)

I also tested it so the dialog ID varies, going from 0 to 99. In such cases, the difference between the timings will be smaller (the second method is only ~1.7 times slower than the first one).

If the test is flawed, please let me know.

That's not a bottom line because you have no evidence to support your claim that thousands of integer comparisons and branches are somehow faster than one function call, despite evidence (though admittedly weak) to the contrary! You can't just claim something for no good reason. Anyway, I personally would say use y_dialogs, not for the speed but for the simplicity (and if you want to argue with that, make sure you understand my last post first).
I've never objected to using frameworks, libraries or whatever might make a scripter's job easier when creating a mode.
Mine has been running for a few years now and during the time, the time I've spent tweaking existing code according to tips from the SA-MP community, is very long. For example, I have also implemented the code from ZCMD.inc into my gamemode's main file. Why? So I can change it to my needs and integrate it with my command system a bit more thoroughly without losing efficiency there. Zeex's approach to scripters is very user-friendly by providing a few callbacks and the overhead from calling them can be reduced by implementing it in your mode.
In the position of someone who has an entirely unique idea and wants to put it into code without having to worry about "ugh, how do I most conveniently use dialogs", I would definitely use the most user-friendly library.
As for the post about when to optimize, I entirely agree!

[KHK]Khalid
02/08/2012, 03:12 AM
You've been given good methods for defining dialogs and they work efficiently, I'll give you one more method which I prefer though. Hmm what about writing an special include file for dialogs? Let me explain:

Open up notepad and put something like this:

// Dialogs IDs

#if defined _diags_included
#endinput
#endif
#define _diags_included
// Dialogs definations
#define Dialog 1 // Example
#define Dialogo Dialog+1 // another example actually Dialogo is set to 2 now (1 + Dialog = 1 + 1 = 2)
#define Dialogoo 3
// Other dialogs


Save as .inc (Include File) then put this file into your pawno include folder.

After that open up your a_samp include file then include the dialogs inc file there:


#include <core>
#include <float>
#include <string>
#include <file>
#include <time>
#include <datagram>
#include <a_players>
#include <a_vehicles>
#include <a_objects>
#include <a_sampdb>
#include <dialogs> // replace "dialogs" with your dialogs include file name.


So as soon as you include a_samp, the dialogs inc file will be included as well. So, whenever you want to add/remove a dialog, you'll just have to go to the dialogs inc file and define/un-define it there.

Now you can easily use your dialogs in any script that includes the a_samp we modified above:
ShowPlayerDialog(playerid, Dialogo, ...);

The same can be done with virtual worlds and such things.

Y_Less
02/08/2012, 07:07 AM
You shouldn't modify the original includes.

Ash.
02/08/2012, 11:29 AM
HellSphinX comes in with the left hook, but Y_Less takes him down with a right!

Sorry... anyway @HellSphinX It would be better if you put that in a new include. That way you don't 'lose' the defines when the server version updates, and it makes sure that you don't break original includes.

WackoX
02/08/2012, 12:25 PM
That's fine.

#define MP MAX_PLAYERS
Is the correct way, you had it backwards.


There is literally no difference in efficiency between the two, it just makes you read it differently. Personal choice really.

The .pwn and .amx would be smaller, plus it is much easier to find other things.
BTW changing MAX_PLAYERS to MP doesn't make any sence.

Y_Less
02/08/2012, 02:13 PM
The .pwn and .amx would be smaller, plus it is much easier to find other things.
BTW changing MAX_PLAYERS to MP doesn't make any sence.

It doesn't affect the .amx size at all, our only marginally affects the .pwn size while seriously affecting readability.

[KHK]Khalid
02/08/2012, 05:00 PM
You shouldn't modify the original includes.

Would you mind telling me what could happen (or what issue could it cause)?

It's done to make it simpler, so you just include a_samp (which is needed in any script except npcs) instead of including dialogs (or any special include file you ever made) in every script you want to use those dialogs in.


Edit:

@HellSphinX It would be better if you put that in a new include. That way you don't 'lose' the defines when the server version updates, and it makes sure that you don't break original includes.

It's been written in a new include file if you didn't notice. All what I did modify in the original include (a_samp) was putting one line to include my special include file so I won't have to include it in every script I wanna use diags on.


Well, waiting for Y_Less' reply to know if it's illegal or something.

Y_Less
02/08/2012, 06:44 PM
Its not illegal, its just not generally done. Those are provided for you to write your own scripts with and can change without warning, meaning you will loose all your custom code. Why not do it the other way round? Include a_samp from your include and then only ever include your include instead.

Also, if you ever release your mode it won't compile for other people as they won't modify their a_samp.

[KHK]Khalid
02/08/2012, 07:10 PM
Include a_samp from your include and then only ever include your include instead.

Sweet! This should be better when writing a script to be released (Even though you could give the special include file too when releasing your script).

I personally never had a problem with it being included from the a_samp include though.


Edit:

Or you can use #tryinclude (http://wiki.sa-mp.com/wiki/Keywords:Directives#.23tryinclude).