Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grenadefixes #479

Closed
wants to merge 4 commits into from
Closed

Grenadefixes #479

wants to merge 4 commits into from

Conversation

Vaqtincha
Copy link
Contributor

No description provided.

@fl0werD
Copy link
Contributor

fl0werD commented Dec 23, 2019

В чом прекол

@etojuice
Copy link
Contributor

etojuice commented Dec 25, 2019

3c6bacb
51c86ad

IMO these features should be optional

@Vaqtincha
Copy link
Contributor Author

No.

3c6bacb - forget code
51c86ad - bug

@s1lentq s1lentq force-pushed the master branch 2 times, most recently from 3dee6bb to 5bbba22 Compare April 2, 2020 11:28
@s1lentq s1lentq force-pushed the master branch 3 times, most recently from 6ae3127 to 5bbba22 Compare April 22, 2020 10:40
@Vaqtincha Vaqtincha closed this May 2, 2020
@Vaqtincha Vaqtincha deleted the grenadefixes branch May 2, 2020 09:13
@Vaqtincha Vaqtincha restored the grenadefixes branch July 1, 2020 10:05
@Vaqtincha
Copy link
Contributor Author

grenade can't deployed on pickup (if the player did not have a weapon)

https://www.youtube.com/watch?v=uSxIaeqMlMU

@Vaqtincha Vaqtincha reopened this Dec 15, 2022
@dystopm
Copy link
Contributor

dystopm commented Jul 13, 2023

grenade can't deployed on pickup (if the player did not have a weapon)

https://www.youtube.com/watch?v=uSxIaeqMlMU

Hi @Vaqtincha , got one question, that hegrenade you wanted to pickup, was originally an armoury_entity or weaponbox? Because it matters, a lot
In case answer is weaponbox, you first had ONLY a hegrenade in your inventory, and then you dropped it and try to pick it up?

In this cases of bugfixes approaches a little of context is always needed. The better communication the best, we can't guess by just looking at the code because it can involve big chains of structures and flows that can be easily broken.

(so this way this will not turn into a real "circus")

@Vaqtincha
Copy link
Contributor Author

@dystopm bug only with armory_entity. on armory touch the player receives via givenameditem and CanDeploy is called before the player is filled with ammo (m_pPlayer->m_rgAmmo[m_iPrimaryAmmoType])

@Vaqtincha
Copy link
Contributor Author

And this is imperceptible when the player has any other weapon.

@dystopm
Copy link
Contributor

dystopm commented Jul 13, 2023

I'm not understanding then. GiveNamedItem creates, Spawn assigns m_iDefaultAmmo, touching called on DefaultTouch where AddPlayerItem is called, then CBasePlayerWeapon::AddToPlayer which gives ammo on ExtractAmmo, gun is linked to player and then inside FShouldSwitchWeapon there is the first call to CanDeploy which returns true because ammo was given on ExtractAmmo/AddPrimaryAmmo. Am I forgetting something? 🤔

@Vaqtincha
Copy link
Contributor Author

@dystopm
CArmoury::ArmouryTouch->CBasePlayer::GiveNamedItem->CBasePlayerItem::DefaultTouch->
CBasePlayer::AddPlayerItem->

BOOL EXT_FUNC CBasePlayer::__API_HOOK(AddPlayerItem)(CBasePlayerItem *pItem)
{
	CBasePlayerItem *pInsert = m_rgpPlayerItems[pItem->iItemSlot()];
	===== this block will be ignored (player don't have this weapon) ======
        while (pInsert)
	{
		if (FClassnameIs(pInsert->pev, STRING(pItem->pev->classname)))
		{
			if (pItem->AddDuplicate(pInsert))
			{
				g_pGameRules->PlayerGotWeapon(this, pItem);
				pItem->CheckRespawn();

				// ugly hack to update clip w/o an update clip message
				pItem->UpdateItemInfo();

				if (m_pActiveItem)
					m_pActiveItem->UpdateItemInfo();

				pItem->Kill();
			}

			return FALSE;
		}

		pInsert = pInsert->m_pNext;
	}
       =====  is new weapon =====
	if (pItem->AddToPlayer(this))
	{
		g_pGameRules->PlayerGotWeapon(this, pItem);

		if (pItem->iItemSlot() == PRIMARY_WEAPON_SLOT)
			m_bHasPrimary = true;

		pItem->CheckRespawn();
		pItem->m_pNext = m_rgpPlayerItems[pItem->iItemSlot()];
		m_rgpPlayerItems[pItem->iItemSlot()] = pItem;

		if (HasShield())
			pev->gamestate = HITGROUP_SHIELD_ENABLED;

                ====== cannot switch to a grenade (but in fact it should) =====
		if (g_pGameRules->FShouldSwitchWeapon(this, pItem)) 
		{
			if (!m_bShieldDrawn)
			{
				SwitchWeapon(pItem);
			}
		}
#ifdef REGAMEDLL_FIXES
		m_iHideHUD &= ~HIDEHUD_WEAPONS;
#endif

		return TRUE;
	}

	return FALSE;
}

->AddDuplicate->ExtractAmmo->AddPrimaryAmmo->m_pPlayer->GiveAmmo
this chain is not called and the player has m_Player->m_rgAmmo[grenadeAmmo] == 0

->CHalfLifeMultiplay::ShouldSwitchWeapon

will return FALSE

@Vaqtincha
Copy link
Contributor Author

this is a problem with weaponbox and not armory_entity (many years have passed and I forgot) but the problem is the same (CanDeploy is called before the player gets ammo)

@dystopm
Copy link
Contributor

dystopm commented Jul 13, 2023

this is a problem with weaponbox and not armory_entity (many years have passed and I forgot) but the problem is the same (CanDeploy is called before the player gets ammo)

Well between these 2 entity there's a huge difference.

armoury_entity gives you ammo on AddWeapon (which is ExtractAmmo), it doesn't need to get into AddDuplicate chain. so this was discarded from before. Was going to ask you if you were using an edited fork or something.

weaponbox gives you gun first, ammo then, but that can be fixed in two ways:

  • Instead of giving weaponbox ammo from hegrenade (PackAmmo on CreateWeaponBox), ammo should go to m_iDefaultAmmo - only on EXHAUSTIBLE weapons
  • Checking THIS particular scenario when player has no guns -> I prefer this one, because the first one can alter some modded behaviour

Stay tuned

@Vaqtincha
Copy link
Contributor Author

Ahahah. "A huge difference between weaponbox and armory_entity." Thanks, I didn't know. 🙃 There is no need to be smart here. I know very well how they work.

@dystopm
Copy link
Contributor

dystopm commented Jul 13, 2023

There is no need to be smart here. I know very well how they work.

@dystopm bug only with armory_entity. on armory touch the player receives via givenameditem and CanDeploy is called before the player is filled with ammo (m_pPlayer->m_rgAmmo[m_iPrimaryAmmoType])

¿?

I couldn't say you know "well" how this works if you were ignoring the behavior of both entities and your proposed fix is only changing CanDeploy to "true" randomlessly. There IS a need to be smart here, what there is no need for is to be spicy.

@dystopm dystopm mentioned this pull request Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants