For approval:
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:
PHP Code:
new const PlayerClass[] = "player"
register_touch( "armoury_entity" , PlayerClass , "OnPlayerTouchItem" );
etc
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).
8.In
_ShowKeyLists:
if ( gKeyList[ i ][ 0 ] ) : since
gIgnoreWeapons exists, use it.
9.In
OnPlayerTouchItem:
- !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.
- 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:
PHP Code:
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)
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.
- For shield you can simply call EngFunc_RemoveEntity.
Inside case 1 you have to check what kind of entity you have and deal with it properly.
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.
back to roumania so i will be realy busy in real life, i'm waiting an response, thank you all .
Linear Mode
