Weapons that have a secondary ammo type have buggy ammo pickup behavior

[SamVanheer]

Weapons that have a secondary ammo type (the MP5 in Half-Life) will sometimes leave an empty weapon entity behind when picking up ammo from them.

This happens under the following circumstances:

• The player has the maximum amount of ammo of the secondary ammo type (10 for the MP5 grenades)
• The primary ammo picked up from the weapon (25 in Half-Life, 50 in Opposing Force) is sufficient to top off the player's ammo amount for that ammo type (250 for the MP5)

In this case it will leave the weapon entity behind, but with no ammo in it. If the player uses up some primary ammo, reloads and then touches the weapon again it will simply disappear.

This happens because of this code:

halflife/dlls/weapons.cpp

Lines 1140 to 1158 in c7240b9

	int CBasePlayerWeapon::ExtractAmmo( CBasePlayerWeapon *pWeapon )
	{
	int iReturn = 0;
	if ( pszAmmo1() != NULL )
	{
	// blindly call with m_iDefaultAmmo. It's either going to be a value or zero. If it is zero,
	// we only get the ammo in the weapon's clip, which is what we want.
	iReturn = pWeapon->AddPrimaryAmmo( m_iDefaultAmmo, (char *)pszAmmo1(), iMaxClip(), iMaxAmmo1() );
	m_iDefaultAmmo = 0;
	}
	if ( pszAmmo2() != NULL )
	{
	iReturn = pWeapon->AddSecondaryAmmo( 0, (char *)pszAmmo2(), iMaxAmmo2() );
	}
	return iReturn;
	}

The iReturn variable is supposed to indicate if any ammo type could be given to the player, but instead reports this:

• If the player has a secondary ammo type, return true if the player does not have the maximum amount of ammo for that ammo type
• Otherwise, return true if the player does not have the maximum amount of ammo for the primary ammo type

To fix this the code needs to be changed to this:

int CBasePlayerWeapon::ExtractAmmo( CBasePlayerWeapon *pWeapon )
{
	int			iReturn = 0;

	if ( pszAmmo1() != NULL )
	{
		// blindly call with m_iDefaultAmmo. It's either going to be a value or zero. If it is zero,
		// we only get the ammo in the weapon's clip, which is what we want. 
		iReturn = pWeapon->AddPrimaryAmmo( m_iDefaultAmmo, (char *)pszAmmo1(), iMaxClip(), iMaxAmmo1() );
		m_iDefaultAmmo = 0;
	}

	if ( pszAmmo2() != NULL )
	{
		iReturn |= pWeapon->AddSecondaryAmmo( 0, (char *)pszAmmo2(), iMaxAmmo2() );
	}

	return iReturn;
}

The use of |= causes iReturn to contain the value 1 if either AddPrimaryAmmo and/or AddSecondaryAmmo returned true.