Raised This Month: $ Target: $400
 0% 

Please review my plugin, Any room for improvements?


Post New Thread Reply   
 
Thread Tools Display Modes
Author Message
devWaleed
Member
Join Date: Apr 2013
Location: Karachi, Pakistan
Old 07-22-2013 , 18:17   Please review my plugin, Any room for improvements?
Reply With Quote #1

Hey, I am just learning amxx and pawn language. I play in HsCorp community, I made them a plugin for afk_6killer server (me manager there). It is a death match server. So, if you join any team you were not able to join spectator mode for spectating cheaters. Also, If your're in spectator mode, You can't join any team. So I made a /team function for it. Also an "/afk" function so if you're afk you can goto spec mode and you don't loose score or deaths. If you type in again /afk in spec mode. You join your previous team. I also made a team-balancer for that server. Since I am learning, I made everything myself. It is working right now and installed on that server. I just want to know, what useless code or mistakes I have made in it. Is there a better way of doing it? All the criticism is welcome! Just point any mistake you see or anything which can be done in a better way.

Spoiler


EDIT: Updated code 8/13/2013
Spoiler

Thank You!

Last edited by devWaleed; 08-12-2013 at 18:01. Reason: Added improved code for afk thingy.
devWaleed is offline
Send a message via Skype™ to devWaleed
fysiks
Veteran Member
Join Date: Sep 2007
Location: Flatland, USA
Old 07-22-2013 , 23:09   Re: Please review my plugin, Any room for improvements?
Reply With Quote #2

The whitespace and brace placement could be improved (for better readability).
__________________
fysiks is offline
devWaleed
Member
Join Date: Apr 2013
Location: Karachi, Pakistan
Old 07-23-2013 , 05:43   Re: Please review my plugin, Any room for improvements?
Reply With Quote #3

ok. I found using spaces and gaps easy for me to read. Is there any improvement possible? For Example: I used <fun> and <cstrike> only for setting user's team and frags. Is it possible to do this with base amx? or with Hamsandwich?

Last edited by devWaleed; 07-23-2013 at 05:44.
devWaleed is offline
Send a message via Skype™ to devWaleed
Shooting King
RAAASENGAN
Join Date: Mar 2012
Location: India
Old 07-23-2013 , 11:55   Re: Please review my plugin, Any room for improvements?
Reply With Quote #4

1. Any variable created in pawn will be automatically zeroed. No need for "new afk = 0,...etc".
2. What if cl_cmd("kill") is blocked by other plugin ?? use user_kill(id).
3. What happens when two players of different team used /afk ??
PHP Code:
//Set previous team
        
cs_set_user_team(idteam); 
Where did you store previous team ?? Create an new array szAfkTeams[33] and store their team ids in this array. So when they come back from afk you can assign them to their correct team. Also same goes for afk( new afk[33] ).
4. Remove task at round end.
5.
PHP Code:
get_players ctplayersnum,"ae""CT");
    
get_players tplayersnum2,"ae""TERRORIST"); 
Are you trying to balance only alive players ?? if not use "e" instead of "ae".
6. Return PLUGIN_HANDLED in cl cmds.
7.
PHP Code:
if(is_user_alive(id) == 1
to
PHP Code:
if(is_user_alive(id)) 
and
PHP Code:
if(afk == 0
to
PHP Code:
if(!afk
8. What happenes when /balance is called by an admin in middle of the round. Its better to store a "bool:Balance = 1" when admin calls /balance and run function "ExecuteTeamBalancer" at round end by checking booleen. Same goes for /afk also.
__________________
As every time said, don't ever UNDERESTIMATE me.

Donate - Here
Shooting King is offline
devWaleed
Member
Join Date: Apr 2013
Location: Karachi, Pakistan
Old 07-23-2013 , 12:10   Re: Please review my plugin, Any room for improvements?
Reply With Quote #5

3. I didn't felt any issue. It is working normally, But I will check it.
4. Task is removed at round start, and restarted as new task.
5. It is for CSDM, so it shouldn't make any difference but yes sometimes It can, I will change it.
6. Didn't get you :\
7. No I think it is better to use if(is_user_alive(id) == 1) instead of if(is_user_alive(id)), Yours will return like if(0) which doesn't make sense :\ and about afk == 0, Ok.
8. As I said, It is for CSDM so there is no middle of round or in some cases round end too (You know that). So its fine.

Thank You for giving these suggestions.

Last edited by devWaleed; 07-23-2013 at 12:11.
devWaleed is offline
Send a message via Skype™ to devWaleed
fysiks
Veteran Member
Join Date: Sep 2007
Location: Flatland, USA
Old 07-23-2013 , 19:20   Re: Please review my plugin, Any room for improvements?
Reply With Quote #6

Quote:
Originally Posted by devWaleed View Post
ok. I found using spaces and gaps easy for me to read. Is there any improvement possible? For Example: I used <fun> and <cstrike> only for setting user's team and frags. Is it possible to do this with base amx? or with Hamsandwich?
You should use the simplest, most direct method that you can for a particular function regardless of the number of includes/modules used.
__________________
fysiks is offline
devWaleed
Member
Join Date: Apr 2013
Location: Karachi, Pakistan
Old 07-23-2013 , 22:07   Re: Please review my plugin, Any room for improvements?
Reply With Quote #7

I observed the /afk problem, It can only be used by 1 player at a time. I am new with arrays in amxx. So I need a little help.

I will have to do
PHP Code:
new teams[32]; 
Then save players id and team like this.
PHP Code:
teams[id] = get_user_team(id); 
and later, call it as required.
PHP Code:
cs_set_user_team(idteams[id]); 
and for making it useable for more than 1 player, I will have to tweak:

PHP Code:
new afk[32];
if( ........ && 
afk[id] == 0  && .....){
  
//make afk
  //Reset variable
  
afk[id] = ;

Will this work for afk thingy ?

Last edited by devWaleed; 07-23-2013 at 22:11.
devWaleed is offline
Send a message via Skype™ to devWaleed
MPD
Member
Join Date: May 2013
Location: Lithuania
Old 07-24-2013 , 04:26   Re: Please review my plugin, Any room for improvements?
Reply With Quote #8

Quote:
Originally Posted by devWaleed View Post
3. I didn't felt any issue. It is working normally, But I will check it.
4. Task is removed at round start, and restarted as new task.
5. It is for CSDM, so it shouldn't make any difference but yes sometimes It can, I will change it.
6. Didn't get you :\
7. No I think it is better to use if(is_user_alive(id) == 1) instead of if(is_user_alive(id)), Yours will return like if(0) which doesn't make sense :\ and about afk == 0, Ok.
8. As I said, It is for CSDM so there is no middle of round or in some cases round end too (You know that). So its fine.

Thank You for giving these suggestions.
6.
PHP Code:
public plugin_init() {
    
register_clcmd("sss""ToTeam");
}

public 
ToTeam(id) {
    
//blablabla
    
    
return PLUGIN_HANDLED;

7. if user is alive it will return true (1), if not alive it will return false (0). Everything makes sense.

Last edited by MPD; 07-24-2013 at 04:28.
MPD is offline
Send a message via Skype™ to MPD
devWaleed
Member
Join Date: Apr 2013
Location: Karachi, Pakistan
Old 07-24-2013 , 08:08   Re: Please review my plugin, Any room for improvements?
Reply With Quote #9

Quote:
Originally Posted by devWaleed View Post
I observed the /afk problem, It can only be used by 1 player at a time. I am new with arrays in amxx. So I need a little help.

I will have to do
PHP Code:
new teams[32]; 
Then save players id and team like this.
PHP Code:
teams[id] = get_user_team(id); 
and later, call it as required.
PHP Code:
cs_set_user_team(idteams[id]); 
and for making it useable for more than 1 player, I will have to tweak:

PHP Code:
new afk[32];
if( ........ && 
afk[id] == 0  && .....){
  
//make afk
  //Reset variable
  
afk[id] = ;

Will this work for afk thingy ?
Can anybody tell me, Am I correct or wrong? 1 thing, Do I need to empty the array if player leaves? So it doesn't get full.

Quote:
Originally Posted by MPD View Post
6.
PHP Code:
public plugin_init() {
    
register_clcmd("sss""ToTeam");
}

public 
ToTeam(id) {
    
//blablabla
    
    
return PLUGIN_HANDLED;

7. if user is alive it will return true (1), if not alive it will return false (0). Everything makes sense.
I mean that, is_user_alive(id) returns 1 then,
PHP Code:
  if(1){
  
//adasd
}else if(0){
  
//...
}
 
// if(1) is not a proper condition. 1 is always 1 and 0 is always zero, its not a condition. But
if(is_user_alive(id) == 1){
 
// makes more sense.
}
//Or
if(== 1)
{
 
//This makes more sense

}else if(== 1){
 
//Which is not equal, and makes more sense. 

devWaleed is offline
Send a message via Skype™ to devWaleed
YamiKaitou
Has a lovely bunch of coconuts
Join Date: Apr 2006
Location: Texas
Old 07-24-2013 , 08:15   Re: Please review my plugin, Any room for improvements?
Reply With Quote #10

Pawn takes any Non-Zero integer value to be true and 0 to be false. So, if(0) is the same as if(false)
__________________
ProjectYami Laboratories

I do not browse the forums regularly anymore. If you need me for anything (asking questions or anything else), then PM me (be descriptive in your PM, message containing only a link to a thread will be ignored).
YamiKaitou is offline
Reply



Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT -4. The time now is 06:28.


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