[Craxor]

Originally Posted by HamletEagle It will be unapproved until you do the requested changes and PM me.

Yes, i will send a PM after i solve my problems from my real life, thanks

[Craxor]

Update, now we are at version 1.2, thankzzz Bugsy and Hamlet!

[HamletEagle]

From a quick look:

1.

PHP Code:

set_pdata_int( iEntity , m_iCount , 1 , XoCArmoury );
set_pev( iEntity , pev_effects , ( pev( iEntity , pev_effects ) & ~EF_NODRAW ) );
set_pev( iEntity , pev_solid , SOLID_TRIGGER );
} 


Instead of doing that, simply call Ham_Restart on the armoury_entity. That's how the game does this job.
2.I don't like that you duplicate code in addkey and remkey. You could move the message display directly in GetWeaponIndex(add another id param and possible a type param so you know from which function this was called - because the 3rd message is a bit function specific, so you would check that param and decide what message to show).
3.

PHP Code:

while ( ( iEntity = find_ent_by_class( iEntity , "armoury_entity" ) ) ) 


check if iEntity is valid before doing anything with it.
4.Also consider removing the comments that are not explaining code functionality, they are useless(for example:

//Replaced iWeaponType variable with wtType. This way, wtType gets assigned a constant for the weapon type
	//that it is. It makes more sense for people looking at your code to see the actual weapon type instead of

//Eliminated variable for the cvar value. Since the value is used only once, psas it
		//directly into the switch.

).
5.In player_touch, set_pev( ent , pev_solid , SOLID_NOT ); looks like it's not needed. Game should do that on it's own.
6.Just to keep good practices here for ( new i = 1 ; i <= 5 ; i++ ) use < sizeof, don't hardcode the size.
7.Why g_ArmouryType is bi-dimensional? The names are never used. It could simply be

PHP Code:

new const g_ArmouryTypes[] = 
{
    CSW_MP5NAVY ,
    CSW_TMP ,
    CSW_P90 , 
    CSW_MAC10 ,
    CSW_AK47 , 
    CSW_SG552 , 
    CSW_M4A1 , 
    CSW_AUG , 
    CSW_SCOUT , 
    CSW_G3SG1 , 
    CSW_AWP , 
    CSW_M3 , 
    CSW_XM1014 , 
    CSW_M249 , 
    CSW_FLASHBANG , 
    CSW_HEGRENADE , 
    CSW_VEST , 
    CSW_VESTHELM , 
    CSW_SMOKEGRENADE
}; 


Also ArmouryEntities is not directly used and could be removed. You can say it's good as a reference for others. In this case, keep it as a comment. You are just complicating your own code and making it harder to understand.

ArmouryData enum can be removed now.
Then the check would look like:

PHP Code:

g_ArmouryTypes[get_pdata_int( ent , m_iType , XoCArmoury ) ] 


Moved back to new plugins.

[Bugsy]

@HamletEagle

5. In player_touch, set_pev( ent , pev_solid , SOLID_NOT ); looks like it's not needed. Game should do that on it's own.

I was finding that without this, the touch keeps firing. Setting SOLID_NOT makes touch not get fired anymore. If/when it is restored, set_pev( iEntity , pev_solid , SOLID_TRIGGER ); makes it touch again. Like you said, this can be replaced with calling Ham_Restart on it.

[Craxor]

Are you sure about Ham_Restart ? It says 'Undefiend symbol' ham restart it is not for 1.8.3 ? Here's the way i'm using it:

ExecuteHam( Ham_Restart , iEntity )

Or it is Ham_CS_Restart ?

[HamletEagle]

About SOLID_NOT, I was not totally sure, by looking at CArmouryTouch I saw that:

PHP Code:

if (!m_iCount)
    Hide(); 


And:

PHP Code:

void CArmoury::Hide()
{
    pev->effects |= EF_NODRAW;

#ifdef REGAMEDLL_FIXES
    // more not to touch with the world.
    pev->solid = SOLID_NOT;
#endif
} 


I should have paid more attention, the SOLID_NOT flag is not set by default dll, so you need to do that. My bad.

[HamletEagle]

@Craxor, Ham_CS_Restart.

[Craxor]

About '6' - i don't understand ..
About '2' - again, i don't understand, can you reformulate?
Edit: about '2' not more easy to just make some const' messages ? ( joking ... )


Also i just discovered the 'kevlar' is not saving, working to add in bitfields, block it , bla bla .. it shows at blockedlist everything ... But after i change the map is no more saved ( is not blocking it and also not show in blockedlist ) Also never appeart in amx_cspum_keylist , is not appear in the list ... idk why ..

Wait a little.

[HamletEagle]

6.

PHP Code:

for ( new i = 1 ; i <sizeof m_rgpPlayerItems_CWeaponBox; i++ ) 


rocket science, I know.
2. Let's look at addkey and remkey:

addkey:

PHP Code:

iFoundIndex = GetWeaponIndex( szWeaponArg );
    
    switch ( iFoundIndex )
    {
        case GWI_NotFound:
        {
            client_print( id , print_console, "Unable to find the key '%s', type amx_cspum_keylist for all keys available to insert." , szWeaponArg ); 
            return PLUGIN_HANDLED; 
        }
        case GWI_Duplicate:
        {    
            client_print( id , print_console, "Duplicate weapons were found. Please be more specific with the weapon you want to add." , szWeaponArg ); 
            return PLUGIN_HANDLED; 
        }
        default:
        {
            if ( gBlockWeapons & ( 1 << iFoundIndex ) )
            {
                client_print( id , print_console , "'%s' is already saved in the pick up manager list." , gKeyList[ iFoundIndex ] ); 
                return PLUGIN_HANDLED; 
            }
        }
    } 


remkey:

PHP Code:

iFoundIndex = GetWeaponIndex( szWeaponArg );
    

    switch ( iFoundIndex )
    {
        case GWI_NotFound:
        {
            client_print( id , print_console, "Unable to find the key '%s', type amx_cspum_keylist for all keys available to insert." , szWeaponArg ); 
            return PLUGIN_HANDLED; 
        }
        case GWI_Duplicate:
        {    
            client_print( id , print_console, "Duplicate weapons were found. Please be more specific with the weapon you want to add." , szWeaponArg ); 
            return PLUGIN_HANDLED; 
        }
        default:
        {
            if ( !( gBlockWeapons & ( 1 << iFoundIndex ) ) )
            {
                client_print( id , print_console , "'%s' is not currently in the pick up manager list." , gKeyList[ iFoundIndex ] ); 
                return PLUGIN_HANDLED; 
            }
        }
    } 


It's almost the same code, but in two different functions. Generally, you should try to avoid that, especially when the code can be moved in one single function. Something like this can be done:

PHP Code:

//Here so we know from what function we are calling the GetWeaponIndex function
//AddKey is 0, RemKey is 1, helps on readability
enum FunctionType
{
    AddKey,
    RemKey
}

//id param was added because we are going to show the messages from this function
//Type param tells from what function we call GetWeaponIndex(addkey or remkey)
GetWeaponIndex(id, const szKeyword[], FunctionType:Type)
{
    new iFound = GWI_NotFound;
    
    for ( new iWeapon = 1 ; iWeapon < sizeof( gKeyList ) ; iWeapon++ )
    {
        if ( containi( gKeyList[ iWeapon ] , szKeyword ) > -1 )
        {
            if ( iFound > GWI_NotFound )
            {
                iFound = GWI_Duplicate;
                break;
            }
            else
            {
                iFound = iWeapon;
            }
        }
    }
    
    new iReturn = iFound //this will hold what the function returns
    switch ( iFound )
    {
        case GWI_NotFound: //the key is not found, set iReturn to an index that is always invalid(no key can have -1 as index)
        {
            client_print( id , print_console, "Unable to find the key '%s', type amx_cspum_keylist for all keys available to insert." , szKeyword ); 
            iReturn = GWI_NotFound; 
        }
        case GWI_Duplicate: //they key is duplicate, we set the iReturn to -1 too because we just want to know if an error happened
        {    
            client_print( id , print_console, "Duplicate weapons were found. Please be more specific with the weapon you want to add." , szKeyword ); 
            iReturn = GWI_NotFound; 
        }
        default:
        {
            //The message is function dependent, but no matter from what function we call it, if we get here an error occured
            //Set the return to -1 for later checks
            switch(Type)
            {
                case AddKey:
                {
                    if ( gBlockWeapons & ( 1 << iFound ) )
                    {
                        client_print( id , print_console , "'%s' is already saved in the pick up manager list." , gKeyList[ iFound ] ); 
                        iReturn = GWI_NotFound    
                    }
                }
                case RemKey:
                {
                    if ( !( gBlockWeapons & ( 1 << iFound ) ) )
                    {
                        client_print( id , print_console , "'%s' is not currently in the pick up manager list." , gKeyList[ iFound ] ); 
                        iReturn = GWI_NotFound    
                    }
                }
            }    
        }
    }
    
    //Now return iReturn, it may be -1(in case an error occured) or a valid position from gKeyList
    return iReturn
} 


Then, in the functions:

PHP Code:

if(iFoundIndex == GWI_NotFound) //if -1 is returned then an error occured, stop the function
{
    return PLUGIN_HANDLED
} 


Again, the -1 is just a random value that I choosed to know if an error occured so the addkey/remkey functions can be stopped. If the return is not -1, it means a valid entry from the constant was returned.
I hope you will understand, but if you don't just ask.

[Craxor]

Ok hamlet, all done . can you check here if i miss something? I don't want to post a version with mistakes, so if i made them correct as you said leave a reply

Edit: Ofcourse tested, and everything works.