Please review my plugin, Any room for improvements?
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.
new afkers[33], teams[33], name[33], mode, csdm = 0;
public plugin_init() { register_logevent("RoundStart", 2, "1=Round_Start"); register_logevent("RoundEnd", 2, "1=Round_End"); register_plugin("Mercury", "2.0", "Waleed"); register_clcmd("say /afk", "makeafk"); register_clcmd("say /team", "showteammenu"); register_clcmd("say /balance", "BalanceTeam"); mode = register_cvar("amx_m_mode", "1") if(get_pcvar_num(mode) == 1){ csdm = 1; } } public RoundStart(id){ TeamBalancer(); set_task(30.0, "TeamBalancer", 13345, _, _, "b"); } public RoundEnd(id){ remove_task(13345); } public makeafk(id){ if(is_user_alive(id) && afkers[id] == 0){ get_user_name(id, name, charsmax(name)); client_print(0, print_chat, "[AMXX] %s is AFK!", name); client_print(id, print_chat, "[AMXX] Type /afk again to play."); teams[id] = get_user_team(id); cs_set_user_team(id, CS_TEAM_SPECTATOR); user_kill(id, 1); afkers[id] = 1; }else if(afkers[id] == 1){ get_user_name(id, name, charsmax(name)); client_print(0, print_chat, "[AMXX] %s is no longer AFK.", name); cs_set_user_team(id, teams[id]); if(csdm == 1){ ExecuteHamB(Ham_CS_RoundRespawn, id); client_cmd(id, "say /guns"); }else{ client_print(id, print_chat, "[AMXX] You will respawn in next round."); } afkers[id] = 0; } return PLUGIN_HANDLED; } public client_disconnect(id){ afkers[id] = 0; } public showteammenu(id){ if(afkers[id] == 1){ client_print(id, print_chat, "[AMXX] Type /afk again to play."); }else{ new menu = menu_create("Which team you want to switch to?", "menu_handler"); menu_additem( menu, "Switch to Terrorist!", "", 0); menu_additem( menu, "Switch to Counter-Terrorist!", "", 0); menu_additem( menu, "Switch to Specatate!", "", 0); menu_setprop( menu, MPROP_EXIT, MEXIT_ALL); menu_display(id, menu, 0); } return PLUGIN_HANDLED; } public menu_handler(id, menu, item){ switch( item ) { case 0: { cs_set_user_team(id, CS_TEAM_T); get_user_name(id, name, charsmax(name)); client_print(0, print_chat, "[AMXX] %s is now on Terrorist side.", name); ExecuteHamB(Ham_CS_RoundRespawn, id); menu_destroy(menu); } case 1: { cs_set_user_team(id, CS_TEAM_CT); cs_set_user_team(id, CS_TEAM_T); client_print(0, print_chat, "[AMXX] %s is now on Counter-Terrorist side.", name); ExecuteHamB(Ham_CS_RoundRespawn, id); menu_destroy(menu); } case 2: { cs_set_user_team(id, CS_TEAM_SPECTATOR); user_kill(id, 1); menu_destroy(menu); } case MENU_EXIT: { menu_destroy(menu); } } menu_destroy( menu ); return PLUGIN_HANDLED; } public BalanceTeam(id){ if(get_user_flags(id) & ADMIN_KICK){ TeamBalancer(); } } public TeamBalancer(){ new tplayers[32], ctplayers[32], tcount, ctcount, ePlayer, name[32] ; get_players(tplayers, tcount, "e", "TERRORIST"); get_players(ctplayers, ctcount, "e", "CT"); client_print(0, print_chat, "[AMXX] Balancing teams..."); if(tcount == ctcount){ client_print(0, print_chat, "[AMXX] Teams are balanced."); return PLUGIN_HANDLED; }else if(tcount > ctcount){ ePlayer = floatround((tcount-ctcount)/2.0, floatround_floor); if(ePlayer > 0){ for(new i=0; i<ePlayer; i++){ get_user_name(tplayers[i], name, charsmax(name)); client_print(0, print_chat, "[AMXX] Transferring %s to CTs for balancing teams.", name); cs_set_user_team(tplayers[i], CS_TEAM_CT); if(csdm == 1){ ExecuteHamB(Ham_CS_RoundRespawn, tplayers[i]); } } } client_print(0, print_chat, "[AMXX] Teams are balanced."); }else if(ctcount > tcount){ ePlayer = floatround((ctcount-tcount)/2.0, floatround_floor); if(ePlayer > 0){ for(new i=0; i<ePlayer; i++){ get_user_name(ctplayers[i], name, charsmax(name)); client_print(0, print_chat, "[AMXX] Transferring %s to Ts for balancing teams.", name); cs_set_user_team(ctplayers[i], CS_TEAM_T); if(csdm == 1){ ExecuteHamB(Ham_CS_RoundRespawn, tplayers[i]); } } } client_print(0, print_chat, "[AMXX] Teams are balanced."); } return PLUGIN_HANDLED; }
Thank You!
fysiks
07-22-2013 23:09
Re: Please review my plugin, Any room for improvements?
The whitespace and brace placement could be improved (for better readability).
devWaleed
07-23-2013 05:43
Re: Please review my plugin, Any room for improvements?
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?
Shooting King
07-23-2013 11:55
Re: Please review my plugin, Any room for improvements?
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(id, team);
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.
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.
devWaleed
07-23-2013 12:10
Re: Please review my plugin, Any room for improvements?
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.
fysiks
07-23-2013 19:20
Re: Please review my plugin, Any room for improvements?
Quote:
Originally Posted by devWaleed
(Post 1996984)
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.
devWaleed
07-23-2013 22:07
Re: Please review my plugin, Any room for improvements?
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(id, teams[id]);
and for making it useable for more than 1 player, I will have to tweak:
Re: Please review my plugin, Any room for improvements?
Quote:
Originally Posted by devWaleed
(Post 1997183)
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.
devWaleed
07-24-2013 08:08
Re: Please review my plugin, Any room for improvements?
Quote:
Originally Posted by devWaleed
(Post 1997481)
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(id, teams[id]);
and for making it useable for more than 1 player, I will have to tweak:
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
(Post 1997585)
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 == 1)
{
//This makes more sense
}else if(0 == 1){
//Which is not equal, and makes more sense.
}
YamiKaitou
07-24-2013 08:15
Re: Please review my plugin, Any room for improvements?
Pawn takes any Non-Zero integer value to be true and 0 to be false. So, if(0) is the same as if(false)