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

Add support for Mangler charged shots when attack2fire is enabled #1

Merged
merged 6 commits into from
Mar 9, 2023

Conversation

jedso
Copy link
Contributor

@jedso jedso commented Mar 6, 2023

Currently alt-fire charged shots do not work with the Cow Mangler 5000 when attack2fire is enabled because m_flNextSecondaryAttack will always be set to g_globals.curtime + 0.5:

jumpqol/jumpqol.sp

Lines 2838 to 2841 in 306739d

if (GetEntProp(entity, Prop_Data, "m_iClip1") != 0)
SetEntPropFloat(entity, Prop_Send, "m_flNextSecondaryAttack", g_globals.curtime + 0.5);
else
SetEntPropFloat(entity, Prop_Send, "m_flNextSecondaryAttack", g_globals.curtime - g_globals.interval_per_tick);

This PR adds a check for if the Mangler is fully charged, and if so, sets m_flNextSecondaryAttack to g_globals.curtime - g_globals.interval_per_tick.

The Mangler is not affected by the same reload problem fixed in 9002a10 when all ammo runs out (+ m_iClip1 doesn't decrement), so that check no longer runs.

@chrb22
Copy link
Owner

chrb22 commented Mar 7, 2023

I did think about it when I was implementation attack2fire, but I decided against it. The setting is really only meant for people with butterfingers and if you're running mangler with the intention of using a charged shot at some point, then you don't fall into that category. I suppose you could switch to mangler for a moment to do a charged bounce setup. If something like that is your intention, then I'd say it would be better to make the setting integer valued and having attack2fire = 2 use your fix, while keeping attack2fire = 1 as-is for its intended purpose.

@jedso
Copy link
Contributor Author

jedso commented Mar 8, 2023

The setting is really only meant for people with butterfingers and if you're running mangler with the intention of using a charged shot at some point, then you don't fall into that category

Fair enough. The bigger issue for me was that I was trying to do a charged shot, it wasn't working and I didn't immediately know why. Currently attack2fire is on by default, and without knowing that or knowing more broadly about this plugin and how each of the settings work, I would argue it can be a bit of a confusing experience (as it was for me). In my view, as long as attack2fire remains opt-out by default, core weapon functionality such as alt-fire charged shots should continue to work as expected.

If something like that is your intention, then I'd say it would be better to make the setting integer valued and having attack2fire = 2 use your fix, while keeping attack2fire = 1 as-is for its intended purpose.

I don't mind this solution, although I would argue that the default should become 2 in that case for better UX (i.e. it would make disabling charged shots a purposeful decision rather than one where the user has to hunt down a reason for why it's not working). All good if you disagree with this assessment, though. After all, choosing to do a charged setup is reasonably uncommon. You make the call.

@chrb22
Copy link
Owner

chrb22 commented Mar 8, 2023

To me it makes the most sense to have attack2fire enabled by default because that would be preferable for the vast majority of time you're playing. I think it would be better to just have a small chat message the first time a player hits secondary fire with a fully loaded mangler for them to learn about the setting.

@jedso
Copy link
Contributor Author

jedso commented Mar 9, 2023

I think it would be better to just have a small chat message the first time a player hits secondary fire with a fully loaded mangler for them to learn about the setting.

OK, sounds good. Feel free to refactor/change any of how I implemented this.

@chrb22
Copy link
Owner

chrb22 commented Mar 9, 2023

Seems good. Going to also make it override +attack2 while +attack is held and some other misc stuff. Also going to remove the GetEntProp(entity, Prop_Data, "m_iClip1") check because I have no idea why I put that there.

@chrb22 chrb22 merged commit 42698f8 into chrb22:main Mar 9, 2023
@jedso
Copy link
Contributor Author

jedso commented Mar 9, 2023

Also going to remove the GetEntProp(entity, Prop_Data, "m_iClip1") check because I have no idea why I put that there.

I think you added it to handle when clip ammo and reserve ammo reach 0 for all non-Mangler launchers. Without that check, once you reach 0 ammo, the weapon stays equipped rather than switching to the next usable weapon (CBaseCombatWeapon::ReloadOrSwitchWeapons).

@chrb22
Copy link
Owner

chrb22 commented Mar 9, 2023

I don't remember that being the reason, but it's enough reason to add it back. Thanks for checking.

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.

2 participants