The task works good like the way it should, but can I optimize a code a bit?
Allowing better readability and less code executed under 0.5 sec(repeat time)?
i think i overpowered it...
Too poorly coded..
Use get_players without any flag and then loop and check for whatever you need, don't create 2 loops (especially don't loop through players without using get_players).
Second, don't create variables inside loops.
Don't check if_user_connected if you're already checking is_user_alive.
- if this is a permanent task then you are better off using a thinking entity
- ffs don't declare variables and in no way ARRAYS inside a loop. you are freeing and reallocating the memory at every iteration
- you can use "static arr[]" instead of "new arr[]" to statically allocate memory. This means those bytes will be used forever from when the server starts until it shuts down, BUT it will not be freed / reallocated every time. So the memory is only allocated once not every time the task runs. For everything that happens pretty often it's a good idea to use static variables as CPU is a bit more important than RAM these days
- things like get_user_points(id) should be cached locally inside a variable if you are using it multiple times in the same function
at a first glance without actually trying to understand what the code does
no need to be super crazy about optimization but some of the things such as declaring things in loops is horrible...
__________________
Meanwhile, in 2050:
Quote:
Originally Posted by aron9forever
useless small optimizations
Quote:
Originally Posted by Black Rose
On a map that is 512x512x128 units you end up with 3,355,443,200,000 different "positions". To store each one of those positions individually in the variable "user_or" you need 12 terabytes of memory.
Too poorly coded..
Use get_players without any flag and then loop and check for whatever you need, don't create 2 loops (especially don't loop through players without using get_players).
Second, don't create variables inside loops.
Don't check if_user_connected if you're already checking is_user_alive.
Why do I actually have to get players without validating any flags? Your opinion just inceases the amount of loops that code does.
I'm just looping inside a loop which I currently ran out of ideas on how to prevent doing this.
is_user_conected() doesn't seem to do anything compared to is_user_alive(), done.
Quote:
Originally Posted by aron9forever
- if this is a permanent task then you are better off using a thinking entity
- ffs don't declare variables and in no way ARRAYS inside a loop. you are freeing and reallocating the memory at every iteration
- you can use "static arr[]" instead of "new arr[]" to statically allocate memory. This means those bytes will be used forever from when the server starts until it shuts down, BUT it will not be freed / reallocated every time. So the memory is only allocated once not every time the task runs. For everything that happens pretty often it's a good idea to use static variables as CPU is a bit more important than RAM these days
- things like get_user_points(id) should be cached locally inside a variable if you are using it multiple times in the same function
at a first glance without actually trying to understand what the code does
no need to be super crazy about optimization but some of the things such as declaring things in loops is horrible...
Being honest aron I don't see any significant changes to the code by using prethink or even entity think. Set task seems fair enough to deal with this brain cracker.
Even tho memory is a very important thing, I'll take care about the suggestion that you gave.
get_user_points() is more accessible and manageable using localized integer, done.
I won't create variables on loops anymore.
He did not say PreThink. DO NOT use prethink for such things. I can agree with you on one thing, in this case a task seems fine. Tho, I would increase the time from 0.5 to 1 second.
I don't see an issue with using set_task() either. From what I've been told, it is the creation of a task that is expensive, not one that repeats itself. I don't know if I agree with the 1 second interval though, it will be too much of a delay from a player perspective, IMO.
PHP Code:
public execute_set_task() { static id;
//You should loop using get_players(). This will load an array with all connected player id's which will //reduce both the loop iterations and native calls needed to accomplish your task. for(id = 1; id <= get_maxplayers(); id++) { //These natives are eliminated when using get_players(). Also, you never need to use is_user_connected() in combination //with is_user_alive() since is_user_alive() has a connected check built-in. if(is_user_alive(id) && is_user_connected(id)) { //Do not declare variables within a loop. Also, is 63 characters needed for this? new Message[64]; formatex(Message, charsmax(Message),"P: %d", get_user_points(id));
//Try to avoid using magic numbers, in this case 361. When others are reviewing your code it is difficult to //understand what your code is doing using offset values.. few know offhand what offset 361 is for. Define constants for this. set_pdata_int(id, 361, get_pdata_int(id, 361) | (1<<5)); }
new Players[32], Num, Spectator;
get_players(Players, Num, "bch"); for(new index = 0; index < Num; ++index) { Spectator = Players[index]; if(pev(Spectator, pev_iuser2) == id) { //You already called natives to check if connected and alive for 'id', you should store the result in a variable //so you can re-check a variable value instead of re-calling the natives. You should also put non-native calls first //in your if-statements/conditionals. If 'id != spectator' fails, you will avoid 3 natives from being called because //no other conditions will be checked once one fails. You also do not need to check if specators are connected since you //retrieved them using get_players() which only gives you connected players. if(is_user_connected(id) && is_user_connected(Spectator) && is_user_alive(id) && id != Spectator) { //You do not need to use a new variable and re-format this message since it is the same //one shown to the alive player. Just re-use Message from above. //The same rule applies for calling get_user_points(), you should re-use the value by storing it in a variable. new Message[64]; formatex(Message, charsmax(Message),"P: %d", get_user_points(id));
Good point bugsy. I suggested 1.0 because IMO a value lower than that is too frequent.
OP, you could try 1.0 and see if you are satisfied. You may feel the delay as bugsy said.
Okay so I choosed Bugsy's way, thanks for your very indeed explanation.
Also do you also suggest me using ONLY static variables when pernamently repeating a task?
Increased the repeat time to 1.0.
Okay so I choosed Bugsy's way, thanks for your very indeed explanation.
Also do you also suggest me using ONLY static variables when pernamently repeating a task?
Increased the repeat time to 1.0.
It's debatable whether static should be used in this case since the variables are small. Had you used a large array, then static should definitely be used.
I don't see an issue with using set_task() either. From what I've been told, it is the creation of a task that is expensive, not one that repeats itself. I don't know if I agree with the 1 second interval though, it will be too much of a delay from a player perspective, IMO.
I'm not sure about this one either, but from my understanding the task system in amxmodx is pretty bad in general and will check whether the task should fire at every server frame. By using a thinking entity you defer this time management to the engine where (I hope) it's done in a more optimal way, otherwise the game couldn't handle a large amount of entities which it does fairly decently.
@OP I worded my post a bit poorly, everything except the declaring stuff inside loops is merely a suggestion, in your scope of a single repeating task it makes virtually no difference, but it's good to do it properly all the time so it comes naturally in scenarios where it matters, such as time intensive functions like prethink, preframe, addtofullpack etc
__________________
Meanwhile, in 2050:
Quote:
Originally Posted by aron9forever
useless small optimizations
Quote:
Originally Posted by Black Rose
On a map that is 512x512x128 units you end up with 3,355,443,200,000 different "positions". To store each one of those positions individually in the variable "user_or" you need 12 terabytes of memory.
Linear Mode
