[Liverwiz]

Originally Posted by Arkshine Kind of useless, poorly coded, and it's clear you are a beginner and you don't know really what you're doing. Unapproved.

how is it poorly coded?

[ConnorMcLeod]

For show activity, you should use amxmisc.inc stocks :

PHP Code:

/**
 * Standard method to show activity to one single client. 
 * This is useful for messages that get pieced together by many language keys.
 * This depends on the amx_show_activity cvar.  See documentation for more details.
 *
 * @param idtarget    The user id of the person to display to.  0 is invalid.
 * @param idadmin    The user id of the person doing the action.
 * @param name        The name of the person doing the action.
 * @param fmt        The format string to display.  Do not put the "ADMIN:" prefix in this.
 */
stock show_activity_id(idtarget, idadmin, const name[], const fmt[], any:...) 


And when performed on a whole team or on all players :

PHP Code:

/**
 * Standard method to show activity to clients connected to the server.
 * This depends on the amx_show_activity cvar.  See documentation for more details.
 *
 * @param id        The user id of the person doing the action.
 * @param name        The name of the person doing the action.
 * @param fmt        The format string to display.  Do not put the "ADMIN:" prefix in this.
 */
stock show_activity( id, const name[], const fmt[], any:... ) 


[Arkshine]

• You check one or two letters with a native. A bit overkill, simply check one letter. Like :

  Replace that :
  

  Spoiler

          if (equali(sz_name[1], "CT"))
              Team = 2
          else if (equali(sz_name[1], "T"))
              Team = 1

  
  by
  

  Spoiler

          switch( sz_name[1] )
          {
              case 'C', 'c' : Team = 2;
              case 'T', 't' : Team = 1;
          }

• Don't re-index arrays. In the loop below, cache the value of players[i]. (optimization)
• Use the existing define for the flags in cmd_target. For a better readability.
• Your TELLALL code : it should be moved inside your first check ( sz_name[0] == '@' ). Here, you could use a switch and checking the first letter too, and using formatex.
• In set_money function : the use of pcvar is a must. You could use the show_activity stock, since it's ready for that and will reflect all the possible value for the cvar.

Btw, why are you give only 16k money ? There are plugins which removes the limit.

[Liverwiz]

Originally Posted by ConnorMcLeod For show activity, you should use amxmisc.inc stocks : PHP Code: /**  * Standard method to show activity to one single client.   * This is useful for messages that get pieced together by many language keys.  * This depends on the amx_show_activity cvar.  See documentation for more details.  *  * @param idtarget    The user id of the person to display to.  0 is invalid.  * @param idadmin    The user id of the person doing the action.  * @param name        The name of the person doing the action.  * @param fmt        The format string to display.  Do not put the "ADMIN:" prefix in this.  */ stock show_activity_id(idtarget, idadmin, const name[], const fmt[], any:...)  And when performed on a whole team or on all players : PHP Code: /**  * Standard method to show activity to clients connected to the server.  * This depends on the amx_show_activity cvar.  See documentation for more details.  *  * @param id        The user id of the person doing the action.  * @param name        The name of the person doing the action.  * @param fmt        The format string to display.  Do not put the "ADMIN:" prefix in this.  */ stock show_activity( id, const name[], const fmt[], any:... )

much thanks! i was unaware of those stocks.

Originally Posted by Arkshine You check one or two letters with a native. A bit overkill, simply check one letter. Like : Replace that : Spoiler         if (equali(sz_name[1], "CT"))             Team = 2         else if (equali(sz_name[1], "T"))             Team = 1 by Spoiler         switch( sz_name[1] )         {             case 'C', 'c' : Team = 2;             case 'T', 't' : Team = 1;         } Don't re-index arrays. In the loop below, cache the value of players[i]. (optimization) Use the existing define for the flags in cmd_target. For a better readability. Your TELLALL code : it should be moved inside your first check ( sz_name[0] == '@' ). Here, you could use a switch and checking the first letter too, and using formatex. In set_money function : the use of pcvar is a must. You could use the show_activity stock, since it's ready for that and will reflect all the possible value for the cvar. Btw, why are you give only 16k money ? There are plugins which removes the limit.

Oh! i didn't know switch statements could use chars. That'll def make that easier. As for that code, i just pulled it from the documentation; didn't put much thought into it.
What do you mean about using the existing flags for cmd_target()?
That's a pretty good idea....completely skip that check by using the original one.
As for the default 16000....that's just the engine's default max; and the plugin's name. Woudln't you think this plugin would be obsolete in servers that mod the max money anyway? Though i think i might look into getting the server's max money, rather than a hard-coded value.

EDIT: looking at the optimization wiki about re-indexing arrays....i wouldn't have though that would be more efficient. Things like this is why i love programming. the most random things are the most efficient.

[Arkshine]

About cmd_target, you should always look at the available .inc, and you would see the constants :

Spoiler

/* Flags:
*  1 - obey immunity
*  2 - allow yourself
*  4 - must be alive
*  8 - can't be bot */
#define CMDTARGET_OBEY_IMMUNITY (1<<0)
#define CMDTARGET_ALLOW_SELF    (1<<1)
#define CMDTARGET_ONLY_ALIVE    (1<<2)
#define CMDTARGET_NO_BOTS       (1<<3)

About 16000, well, since the limit can be removed, there is no really reason to keep giving 16000 instead of letting the admin choosing what they want. What I say is : be customizable. Just saying, the plugin is still kind of redundant, but just because it's unapproved, doesn't mean you can't improve it and maybe turn it out in a new thing.

[Liverwiz]

Originally Posted by Arkshine About cmd_target, you should always look at the available .inc, and you would see the constants : Spoiler /* Flags: *  1 - obey immunity *  2 - allow yourself *  4 - must be alive *  8 - can't be bot */ #define CMDTARGET_OBEY_IMMUNITY (1<<0) #define CMDTARGET_ALLOW_SELF    (1<<1) #define CMDTARGET_ONLY_ALIVE    (1<<2) #define CMDTARGET_NO_BOTS       (1<<3) About 16000, well, since the limit can be removed, there is no really reason to keep giving 16000 instead of letting the admin choosing what they want. What I say is : be customizable. Just saying, the plugin is still kind of redundant, but just because it's unapproved, doesn't mean you can't improve it and maybe turn it out in a new thing.

i think the wiki should have those constants, it'd make me cry less.
And i agree that customization is always the way to go. But some things you just don't want people to change. And i don't really care much about it getting "approved" i'm just learning, growing, and having fun.

[Exolent[jNr]]

Originally Posted by Liverwiz i think the wiki should have those constants

They've been added now.

[Liverwiz]

Originally Posted by Exolent[jNr] They've been added now.

Sweet! i love it when things get done. Much thanks!



EDIT: UPDATED! version 2.3
Optimized, added MONEY define to customize how much money is given (default 16000)
Let me know what yoda thinks.