Tips to speed up review, or to minimize post-tag between authors and reviewers (based on common flaws/mistakes)
(more guidelines to consider when writing plugins)
Consistency! Consistency! Consistency!
- to elaborate on the "Write Neat Code" guideline above:
Messy code takes much longer to both read and understand than cleanly formatted code. Inconsistent formatting slows things down even more.
Please use only one of each of the following throughout a whole plugin
- Tabs/spaces
- Block style (all brackets on own line, no brackets on own line, only end brackets on own line)
- Variable naming style var_name/varName/iVarName
- Padding inside parenthesis (this)/( this )
- Space after keyword if() for()/ if () for ()
- Semicolons at line end or not
Pick your poison and stick with it.
Make use of the safe defines/constants that you're given
- Sourcemod provides many safe-to-use defines and constants to make your life easier and protect against unexpected issues.
MAXPLAYERS
Max number of players supported across all current versions of the source engine. (useful for defining array size. don't forget to +1 if using client ids as index)
MaxClients
Max number of clients that can currently be in the game. This is updated as map start and will be 0 during OnPluginStart if not a late load. (useful for looping through all clients ingame combined with a IsClientInGame() check).
PLATFORM_MAX_PATH
Max length of a file path. (useful for declaring string size)
MAX_NAME_LENGTH
Max length of a player name. (useful for declaring string size)
On a similar note, when escaping a sql parameter, always using (size*2)+1 to leave room for all possible escape characters.
Use the admin API to your advantage.
Don't hardcode access levels or use flag letters/strings as config values
The CheckCommandAccess and CheckAccess natives allow you to check for access to an existing admin command or even just a named feature. You can still also choose default access flags.
If you hardcode a flag, users would have to edit the source to change it. If you make an admin flag cvar or config value, you bypass the overrides system, not allowing users to 1) utilize the same admin_overrides config to change flag access, and 2) not allowing group overrides at all. Work with the admin API instead of against it.
Do not hardcode offsets or signatures in a plugin
These should be stored in and accessed from gamedata files.
This automatically abstracts the OS (and game/engine where applicable) from SDKCalls and allows for easier updates of this data.
Check your client and entity indexes
Make use of the IsClientInGame, IsClientConnect, IsPlayerAlive, IsValidEdict, and/or IsValidEntity functions where applicable.
Other common checks are that a client index is greater than 0 (not "world") or that players are on a non-spectator team (team index equals 2 or 3 in most games).
This will save you from runtime errors or other unintended effects.
Beware of passing client index to async callbacks
Pass userid rather than client index to asynchronous callbacks (timers, threaded sql calls).
Even with relatively short lengths of time between the calling function and the callback, it is very possible for a client to leave (the client will no longer be "InGame") or worse, a client leave and another client join with the same index, causing you to act upon a player you did not intend to.
SM provides a GetClientSerial function that will be unique per player. Alternatively, userids do not get reused for a long time (after tens of thousands), and it is safe to use GetClientOfUserId on a user that has left (the return will be 0).
Don't write data twice
As a small optimization, you may use "decl" (rather than "new") when creating arrays (including strings and vectors).
This is only safe to do if you are populating the array before accessing it (filling it with values or a string, rather than assuming it will start as 0/blank).
The best places in which to see if this is applicable are
- when creating large arrays
- in loops with many repetitions
- in callbacks that occur often (OnGameFrame, repeated timers with short timing)
(See
http://wiki.alliedmods.net/Introduct...ourcePawn#decl for more info)