AlliedModders

AlliedModders (https://forums.alliedmods.net/index.php)
-   Scripting Help (https://forums.alliedmods.net/forumdisplay.php?f=11)
-   -   Is this execution correct? (https://forums.alliedmods.net/showthread.php?t=338691)

Krystek. 07-21-2022 03:44

Is this execution correct?
 
Is this execution correct?

PHP Code:

    new damage_better_items random_num(312);
    new 
damage_items         random_num(16);

    new 
perks_id[] = { 1234713 };

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) == perks_id[i] ) {
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_better_items)? damage_better_itemsdurability_perks[vid];
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } 
    }

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) != perks_id[i] ) {
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_items)? damage_itemsdurability_perks[vid];
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } 
    } 

The code is pretty loose so don't worry about log_amx and the like.

damage220 07-21-2022 09:07

Re: Is this execution correct?
 
What do you mean by correct? It depends. Just compile and run it.
1. You should never use sizeof( in a loop condition. Create a variable and store the size in it.
2. To speed up execution, move non-dynamic arrays out to global scope (precache), make them constant.
3. Try to avoid magic numbers, use constants instead.

fysiks 07-22-2022 00:29

Re: Is this execution correct?
 
Quote:

Originally Posted by damage220 (Post 2784361)
1. You should never use sizeof( in a loop condition. Create a variable and store the size in it.

That's the first time I've ever heard this in the 15 years I've been around here. I was under the impression that sizeof was evaluated at compile-time (except in specific use cases). I've only ever heard people say that you shouldn't use a function (sizeof is not a function, it's an operator) in a for or while loop's definition if the value will not change during the loop.

Quote:

Originally Posted by damage220 (Post 2784361)
2. To speed up execution, move static arrays out to global scope, precache them.

This is not good advice. If something is only used in that function and has no usefulness to other areas of code, it should not be global (making a local variable/constant global just pollutes the global namespace).

If it's relatively small and the function is not called very often then it's generally negligible to declare it as "new". If it never changes then it should be declared as a constant ("const"). If it's a large variable (e.g. an array with a lot of cells) and it's not constant, it should be declared as a static variable ("static").

HamletEagle 07-22-2022 07:05

Re: Is this execution correct?
 
1. fysiks is correct. sizeof is an operator and its result is known at compile time. The compilr replaces it with the actual value in the generated binary.
There is no runtime overhead.

Things are different with something like ArraySize, which is a function. It should be cached before the loop to avoid the runtime overhead of calling the function(pushing, popping the stack, jumping in a different code location) with each loop iteration.

2. I again must agree with fysiks. He explained it well so I will not reiterate. I just want to say that static is not meant to be used as an optimization technique. It can be used as such in very specific scenarios, but this is a side effect of how static works and not its main purpose.

damage220 07-22-2022 15:14

Re: Is this execution correct?
 
I am sorry for the first advice. Though I knew that sizeof in C is calculated at compile time, I had no luck to find any confirmation regarding PAWN.

"If it's relatively small and the function is not called very often"
Why did you decide that the function is not called very often? In fact I do not see any function at all, just the code. There is no function signature or event hook declared.

"This is not good advice. If something is only used in that function and has no usefulness to other areas of code, it should not be global (making a local variable/constant global just pollutes the global namespace)."
If a variable is used only in one function, I agree it should be declared in that function.
If a constant is never changes, like PI, it can be declared in a function.
But when a constant is subject to change I would prefer to have it global, just for convenience for future edits. I was kind of thinking that perks_id is subject to change, and so I would move it out of the function to simplify future edits and to reduce the number of memory allocations.

fysiks 07-23-2022 00:04

Re: Is this execution correct?
 
Quote:

Originally Posted by Krystek. (Post 2784340)
Is this execution correct?

PHP Code:

    new damage_better_items random_num(312);
    new 
damage_items         random_num(16);

    new 
perks_id[] = { 1234713 };

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) == perks_id[i] ) {
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_better_items)? damage_better_itemsdurability_perks[vid];
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } 
    }

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) != perks_id[i] ) {
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_items)? damage_itemsdurability_perks[vid];
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } 
    } 

The code is pretty loose so don't worry about log_amx and the like.

The thing that stands out to me about this is that you have code execution for all every cell in perks_id[] (because the if statements are opposite so one or the other code blocks will get executed) but you're being inefficient about it by using two loops.

You can do this in a single loop by just looping through all cells of the array and just using an if-then-else to determine which code you execute on each iteration.

P.S. While I understand that it's only part of your debugging, it's probably a good thing to point out so that you get better at seeing your coding mistakes in general: your log_amx() functions in your second loop should be throwing errors because you pass two arguments but the format string only has one.

_____________________________________________ _____


Quote:

Originally Posted by damage220 (Post 2784443)
I am sorry for the first advice. Though I knew that sizeof in C is calculated at compile time, I had no luck to find any confirmation regarding PAWN.

I too was looking in the Pawn Language Guide to see if I could find it stated as such but I could not. This is why I was less absolute about my words on this one.

Quote:

Originally Posted by damage220 (Post 2784443)
Why did you decide that the function is not called very often? In fact I do not see any function at all, just the code. There is no function signature or event hook declared.

My advice was not 100% based on the OP's code because I was primarily correcting what you said so that people don't accidentally use it.

You mentioned moving a variable to the global scope as an optimization. While that would potentially optimize it (depending on the specific code) it's not the only way to do it. The reason you said to move it to the global scope was so that it didn't have to dynamically re-allocate the memory each time the function is called. Well, you can do this same thing while keeping it in the function scope by declaring it as static.

Regarding if a function (or rather a block of code) is called often, that's because of the dynamic allocation functionality. If it's called often, it has to get re-allocated a lot. If it's big then this re-allocation becomes more expensive. So, making it static (or global) removes the re-allocation and thus reduce processor time required. For small variables or code that is rarely called, the processor time required stays relatively low so it's not really worth optimizing it (i.e. it's a negligible performance cost).

Quote:

Originally Posted by damage220 (Post 2784443)
If a variable is used only in one function, I agree it should be declared in that function.
If a constant is never changes, like PI, it can be declared in a function.
But when a constant is subject to change I would prefer to have it global, just for convenience for future edits. I was kind of thinking that perks_id is subject to change, and so I would move it out of the function to simplify future edits and to reduce the number of memory allocations.

Whether or not it's a constant (constants don't change) or a variable does not dictate in what scope it should be declared. You should declare the constant or variable in the proper scope for it's use case. If a constant or variable is only useful to the code within a specific function, it should be declared in the function.

Whether or not the perks_id array will change is not clear in the code posted by the OP. If it's within a function and is not used outside the function, it makes sense to declare it inside the function. If the OP decides to change how perks_id is used, the best scope for that variable can be re-evaluated.

Granted, you can declare everything globally if you really wanted to as long as you don't have naming conflicts with any other variable in the entire code. However, this can make the code unnecessarily complex. Scoping variables helps makes code simpler and more modular.

_____________________________________

We probably don't need to go any further with this part of the discussion. We don't want to hijack the thread any more :).

Krystek. 07-23-2022 02:42

Re: Is this execution correct?
 
Quote:

Originally Posted by fysiks (Post 2784472)
The thing that stands out to me about this is that you have code execution for all every cell in perks_id[] (because the if statements are opposite so one or the other code blocks will get executed) but you're being inefficient about it by using two loops.

You can do this in a single loop by just looping through all cells of the array and just using an if-then-else to determine which code you execute on each iteration.

P.S. While I understand that it's only part of your debugging, it's probably a good thing to point out so that you get better at seeing your coding mistakes in general: your log_amx() functions in your second loop should be throwing errors because you pass two arguments but the format string only has one.

PHP Code:

    new damage_better_items random_num(312);
    new 
damage_items         random_num(16);

    new 
perks_id[] = { 1234713 };

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) == perks_id[i] ) {
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_better_items)? damage_better_itemsdurability_perks[vid];
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } else {
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_items)? damage_itemsdurability_perks[vid];
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        }
    } 

By doing it this way, only the content after the else is executed.

fysiks 07-23-2022 03:41

Re: Is this execution correct?
 
You forgot to remove the return statements. Remove them both. Optionally, you can put it after the for loop (depending on what kind of function this is). I probably forgot to mention this part. Sorry about that.

Krystek. 07-23-2022 04:00

Re: Is this execution correct?
 
Quote:

Originally Posted by fysiks (Post 2784479)
You forgot to remove the return statements. Remove them both. Optionally, you can put it after the for loop (depending on what kind of function this is). I probably forgot to mention this part. Sorry about that.

Removing a return does something like this.
https://prnt.sc/I5tY7-cFzfht

fysiks 07-23-2022 04:03

Re: Is this execution correct?
 
Quote:

Originally Posted by Krystek. (Post 2784482)
Removing the return does not do anything.

Did you remove both of them? It also looks like you haven't fixed the amx_log arguments that I mentioned. I think that will probably cause an error and prevent the loop from working like it should.


All times are GMT -4. The time now is 15:38.

Powered by vBulletin®
Copyright ©2000 - 2024, vBulletin Solutions, Inc.