shavitush / bhoptimer

A bunnyhop timer plugin for Counter-Strike: Source, Counter-Strike: Global Offensive and Team Fortress 2.
https://timer.shav.it
GNU General Public License v3.0
230 stars 94 forks source link

Fix targetname and classname locking #1135

Closed GAMMACASE closed 2 years ago

GAMMACASE commented 2 years ago

This pr prevents issues that were talked about in https://github.com/shavitush/bhoptimer/pull/1134 pr, basically teleporting mid way bhop triggers or any other same trigger setups might cause issues and lock wrong targetname on the player (For example 2 events fired with delay 0.9 and 1.0 where 1st one sets your targetname to bhop for example and other one clears it out, teleporting to start zone in between 0.9 and 1.0 would lock your targetname to being bhop as the still ongoing event with 1.0 delay would be erased). This pr solves this issue by manually resetting targetname and classname in situations when player is teleported to start zone, or when Shavit_OnRestart() is called (only when no start position is used) and all that is done before any events from start triggers are fired, thus meaning all targetnames and classnames that should be applied by start triggers, would be still applied. I've already talked how I see this being an option and decided not to do that before, now it's kind of required as I've oversaw the possibility of targetname locking in this pr https://github.com/shavitush/bhoptimer/pull/1123 .

Walked through the mapfixes list and all maps are working fine, so some more testing would be appreciated in case there are maps that would be highly affected by this.

rtldg commented 2 years ago

seems to introduce a slight race on bhop_appaisaniceman3. https://www.youtube.com/watch?v=OFMkaPOcjEs same thing is possible on the bhop_overthinker bonus although it's a bit tougher to replicate.

probably a couple of different ways to deal with this. mess with the !sp implementation maybe (although checkpoints might trigger it too?), add these maps to mapfixes again, or maybe just add the other maps from #1134 to mapfixes and skip on this pr if this pr would end up with this issue inherently?

GAMMACASE commented 2 years ago

The thing is, adding maps from the other pr wont work, as this issue is currently present on all maps with bhop triggers, thus why adding it simply to mapfixes, wont do much. Same for the issue you are describing, it wont be a fix by readding them to mapfixes, so I'll look into that.

GAMMACASE commented 2 years ago

Alright, figured the main cause of what you were describing, and maps like bhop_overthinker shouldn't have this issue anymore, but on the other hand bhop_appaisaniceman3 doesn't use OnStartTouch to fire its events, instead it uses OnTrigger for some reason which allows you to slip through as there's a m_flWait time window where the touch function would run, and because start zone trigger and trigger that fires events aren't synchronized, you are able to leave with empty targetname there.

GAMMACASE commented 2 years ago

With the latest pr made to eventqueuefix (https://github.com/hermansimensen/eventqueue-fix/pull/17), all of the described issues should be fixed.

rtldg commented 2 years ago

seems fine so far when using that eventqueuefix pr. might as well merge. was hoping carni would merge the eventqueuefix pr first