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

TTT: Fixed not being able to shoot a weapon when equipped while being held by a magneto stick #685

Closed
wants to merge 1 commit into from

Conversation

Bo98
Copy link
Contributor

@Bo98 Bo98 commented Apr 28, 2014

When a weapon is equipped while it's being held by a magneto stick, errors occur. The player is unable to shoot their weapon and the ammo count is potentially incorrect or not visible at all. The shotgun also spits errors when shot.

The magneto stick calls its Reset function after a weapon is equipped. Because there was no owner when the weapon was first picked up with the magneto stick, the owner is set to nil. Reequipping the weapon worked around this issue.

This fix uses the WeaponEquip hook which is called before the owner it set. This gives us time to call SWEP:Drop() and reset the owner before the new owner is set.

… held by a magneto stick.

When a weapon is equipped while it's being held by a magneto stick, errors occur. The player is unable to shoot their weapon and the ammo count is potentially incorrect or not visible at all. The shotgun also spits errors when shot.

The magneto stick calls its Reset function after a weapon is equipped. Because there was no owner when the weapon was first picked up with the magneto stick, the owner is set to nil. Reequipping the weapon worked around this issue.

This fix uses the WeaponEquip hook which is called before the owner it set. This gives us time to call SWEP:Drop() and reset the owner before the new owner is set.
@svdm
Copy link
Collaborator

svdm commented Apr 28, 2014

How about checking in Reset if the EntHolding's owner is not equal to the magneto's owner, and in that case not resetting it? That situation means the owner has changed outside of the magneto stick's control (which is exactly what happens in a pickup) so leaving it alone makes sense.

@Bo98
Copy link
Contributor Author

Bo98 commented Apr 28, 2014

Good point. I'll give it a try.

@Bo98
Copy link
Contributor Author

Bo98 commented Apr 28, 2014

Actually that won't work if the player magneto sticked it himself. The owner will be the same before and after the weapon is equipped so you won't be able to check by comparing owners.

@MyHatStinks
Copy link
Contributor

In that case, would a check similar to ply:HasWeapon work, checking for an entity instead of a classname?

@robotboy655
Copy link
Collaborator

@svdm please look into this

@robotboy655 robotboy655 added TTT The pull request is for TTT and will be handled by svdm. pending labels Sep 9, 2014
Metapyziks added a commit to Metapyziks/terrortown that referenced this pull request Nov 24, 2014
@Kefta
Copy link
Contributor

Kefta commented Feb 1, 2015

@svdm 9 months later...

@gspetrou
Copy link
Contributor

12 months later... This has been on my server for a year and hasn't caused any problems. A year of testing :P

@svdm
Copy link
Collaborator

svdm commented Apr 26, 2015

Sorry for leaving this hanging for so long, I've been meaning to spend some time on my own testing and then never actually had that time available.

It's tempting to just merge this if evidently it doesn't cause problems. However I still feel this is a hacky solution that could have obscure side effects (e.g. it seems like it would leak hook entries in certain scenario's).

It doesn't seem like this PR will be improved, so I will close it. If someone is willing to investigate a cleaner solution (for example using better checks in Reset() as suggested above) then I would be happy to merge a new PR.

@svdm svdm closed this Apr 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TTT The pull request is for TTT and will be handled by svdm.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants