View Single Post
HamletEagle
AMX Mod X Plugin Approver
Join Date: Sep 2013
Location: Romania
Old 09-18-2018 , 12:22   Re: Knife Bounty + Secure (v1.3)
Reply With Quote #22

For approval:

1.Even if the style is subjective, I would recommend you to use braces even when the block has only one instruction:
PHP Code:
for ( new isizeof g_sCommandListi++ )
    
register_clcmdg_sCommandList], "SecurePlayer" ) ; 
For example in my text editor(NP ++) this looks unindented(register_clcmd and for have the same level of indentation).

2.Instead of:
PHP Code:
#define is_valid_connected(%0) ( 1 <= %0 <= g_iMaxPlayers ) 
is_user_connected can be used directly.

3."if( VictimMoney == EMPTY )" EMPTY is badly named. You are checking a variable(not some kind of container), which can't be empty. Probably a better name would be NO_MONEY or ZERO_MONEY.

4.In fw_HamSpawnPost there's a potential logic error.
PHP Code:
if( ! ( get_pcvar_numg_iCvars] ) || get_pcvar_numg_iCvars] ) ) ) 
This translated to !get_pcvar_num(g_iCvars[0]) && !get_pcvar_num(g_iCvars[1]). What this means is that you stop the code from being executed only if the plugin is disabled AND players can't secure their money.
In the case the plugin is enabled but knife_bounty_secure is 0 or the plugin is disabled but knife_bount_secure is 1 then the code will still execute.
You probably meant to do an OR instead of AND.
__________________

Last edited by HamletEagle; 09-18-2018 at 12:24.
HamletEagle is offline