1. Making global variables static
means they are in the scope of current file(they are visible only in the file they are created). Since you don't work with multiple files, static
is misused. Use new
2. Be consistent with your function naming. For example, you have "addkey", "remkey", etc and suddenly "_ShowKeyLists". Keep the same style regarding naming patter and capitalization. "OnPlayerTouchItem" here you start with upper case, in "addkey" for example with lower case. Pick a style and stick to it.
3. Not very important, but since "player" string is used multiple times, I would do something like this:
new const PlayerClass = "player"
register_touch( "armoury_entity" , PlayerClass , "OnPlayerTouchItem" );
4. I would like you to add the cvar description and possible values inside source file. That way, it's easier for anyone to understand how to use your plugin.
5. Inside addkey
- The !read_argv check looks useless. cmd_access makes sure you have enough params. Remove it(also in remkey).
- client_print( id , 2, "Unable to find the key '%s', type amx_cspum_keylist for all keys available to insert." , szWeaponArg ); Don't use magic number, use the associated constant. Here it is print_console, instead of 2.
6. In block_all
- Indent properly.
- Check if weapon is not already blocked before formatting and setting into vault(if(!(gBlockWeapons & (1 << i))). That way you can avoid useless native calls.
7. It's not logical to allow any admin to access amx_cspum_keylist
since only admins with ban access can use the plugin. Apply the same restriction here(add ADMIN_BAN
when registering the command and use cmd_access
instead of is_user_admin
: if ( gKeyList[ i ][ 0 ] )
: since gIgnoreWeapons
exists, use it.
- !ent check is invalid. It should be ent != -1 or simply call pev_valid.
- static is misused, switch to new.
- case 1: engfunc( EngFunc_RemoveEntity, ent ); that's really wrong.
Inside case 1 you have to check what kind of entity you have and deal with it properly.
- When you want to remove the weaponbox entity, you must also remove the weapon from the box, not only the box. The proper way is to make it think immediately(by call_think for example). This triggers internally CWeaponBox::Kill which takes care of things properly.
- If you want to remove an armoury_entity, you should not actually delete the entity. This entities are a bit special, if you remove them they won't respawn the next round again and this is the issue if cvar is changed or key is removed. Use game's own method:
What this does is to set the item count to 0(so entity contain 0 weapons) and make it invisible. Game will automatically respawn them on new round.
const XoCArmoury = 4
const m_iCount = 35
set_pdata_int(ent, m_iCount, 0, XoCArmoury)
set_pev(ent, pev_effects, pev(ent, pev_effects) | EF_NODRAW)
- For shield you can simply call EngFunc_RemoveEntity.
For your information, CSW_SHIELD
should be named to CSW_SHIELDGUN
, because this is the real name. Also it's value is 99, not 31. This constant was added into amxx 1.8.3.
In general, for consistency you should keep it's original name and value, but because here you use it for indexing an array and because there is too much empty space from 30 to 99 you can and should keep it like that.
If you don't understand something ask. Don't forget that everything can be discussed.