etlegacy icon indicating copy to clipboard operation
etlegacy copied to clipboard

Issues with hit detection

Open jackeri opened this issue 4 years ago • 39 comments

The "feeling" when shooting is somehow delayed. The way it manifests is just simply: you press the shooting button -> the hit event may or may not register at all or it registers delayed.

Things to look for: antiwarp code, antilag code and the "updated" client & server download code made in ~2015 ish.

jackeri avatar Mar 15 '20 00:03 jackeri

Related UDP download speed change was done in 108ca8503a141f996148b9c5b4dfed6e9ed78f51. See #330.

rmarquis avatar Mar 15 '20 00:03 rmarquis

Some initial changes done in f73b9c85c95f89644bfcf5bed984505afc9f838c.

Early testing indicates better results already.

rmarquis avatar Mar 17 '20 11:03 rmarquis

See also #1201 to potentially improve hit "feeling" by having hitsounds generated server side.

rmarquis avatar Jun 20 '20 09:06 rmarquis

Point of interest ETPub Replace servertime value with attackTime and test the results.

rafal1137 avatar Jul 24 '20 23:07 rafal1137

For the record: some heavy testing was done back in March, with various combination of etded/etlded and Jaymod/Legacy mod (since Jaymod is compatible with etlded, has same hitbox, etc.). The issue has been pigeonholed to the mod code, especially antilag part.

Server Mod Client     Jacker's notes Spyhawk's notes
etded Jaymod 2.60b     With both antilag and antiwarp disabled I was consistenly still hitting the players where I expected to hit. I'm not used to jaymod, so I played a bit to get the feel of it. No noticeable difference between 2.60b and ETL clients. (lag 0, warp 0): There is little lag noticeable. Aiming at or very close to the target pretty much hits most of the time.
    Legacy master        
etlded Jaymod 2.60b     Pretty much the same as the vanilla server (lag 0, warp 0) can't see diff between etlded vs etded (both Jaymod). I'm less sure about the client, but that might be placebo.
    Legacy master        
etlded Legacy 2.60b     With antilag 0 the hitboxes were totally out of place. Headshots were 0.5 meters ahead of the player mode. With antilag 1 the effect was mitigated to some degree but hitboxes were still very inconsestent. (lag 0, warp 0) As one should expect, you need to aim in front of the target to hit it without antilag. (lag 1, warp 0) Legacy antilag does its job, but feels... inconsistent? It feels like only 1 bullet on 3 gets a HS, unlike Jaymod where 3 HS in a row are not an issue. I couldn't feel any difference between 2.60b and ETL client
    Legacy master        

The last change done above however gave mixed results: while it felt better on the testing server, in some way, the feeling was worst once deployed on a populated server.

I do believe reworking the antilag/antiwarp from scratch would be easier than finding out where the original issue has been introduced. Legacy mod implements Zinx antiwarp from etpro (shared publicly), and might have used etpub as a base for its implementation back in ~2013.

However, afaik some people mentioned etpub (and derived mods) implementation has some issues, but Jaymod implementation should be rather independent from etpub codebase family. It might be worth looking at it since it's been opensourced since then by Jaybud.

rmarquis avatar Jul 25 '20 08:07 rmarquis

From @Aranud, and related to @rafal1137 comment above:

  • In ETPub, the historical trace use attack time : https://github.com/BulldogDrummond/etpub/blob/d6e81ec7d8c62a4fab5c970a850aad049e2ed9ee/trunk/src/game/g_antilag.c#L624
  • On Legacy it use server time : https://github.com/etlegacy/etlegacy/blob/7bfb9ffc7443dc92ea2817f47a2b59eb647b452d/src/game/g_antilag.c#L757
  • On NQ it use one or other depending of g_antilag cvar value : https://github.com/core-c/WET-NoQuarter/blob/a56829de1368e8da1bdf93acdeae43bddeed790e/trunk/src/game/g_antilag.c#L656

rmarquis avatar Jul 26 '20 13:07 rmarquis

A few extra resources about existing implementations:

g_skipCorrection [1|0] Set this to 1 to enable Neil Toronto's unlagged2 skip correction. This will smooth out the movement of players with high packet loss (to a degree). This is similar to etpro's antiwarp, but has some differences. Neil likes this version better, bani likes his better. This replaces g_smoothClients from etmain. You can find a demo that shows g_skipCorrection in action at: http://et.tjw.org/etpub/skipCorrection/

Defaults to 1 (on)

g_maxWarp [integer] This allows you to control the amount of "warping" that players with high packet loss can do. The [integer] is the number of server frames that you allow a player to miss before their next movement is put in check.

A server frame is 50ms on a typical server (sv_fps set to 20). This means that if you set g_maxWarp to 5 you won't allow players to warp from point A to point B if that distance takes an normal player 1/4 of a second to travel. Setting this to 1 is a good way to drive off just about everyone from your server.

As far as I can tell, 1000ms is allowed by default in the game, so setting this to any value higher than 39 should have no effect if sv_fps is set to 10.

You can find a demo that shows g_maxWarp in action at: http://et.tjw.org/etpub/skipCorrection/

Defaults to 4

rmarquis avatar Jul 26 '20 17:07 rmarquis

Interesting note in NQ code // tjw: save ucmd->serverTime before it gets mutilated further // tack on 2 server frames time for "50ms built-in lag"

Propably worth implementing it

Ref:

  • b7a90ad78843616e5aedd4d3c0ca08b69c8fe0d6

rafal1137 avatar Jul 27 '20 17:07 rafal1137

I'd heavily suggest to try understanding the code before pushing "fixes", knowing etpub and derived might have issue of their own (see above comment).

In that case, tjw was an etpub coder, and never directly participate to NQ dev afaik. NQ being derived from an old version of etpub, that part of the code might have been dropped from later etpub version for good reason.

rmarquis avatar Jul 27 '20 18:07 rmarquis

@rmarquis Yeah this reply from

  • http://bani.anime.net/banimod/forums/viewtopic.php?p=53470

you fixed a problem you were having giving you 50 ms lag all the time

Might be a reason why it was removed.

rafal1137 avatar Jul 27 '20 19:07 rafal1137

Other differences between ETL / ETPub / NQ for attackTime assignment :

On Pub the value from g_antilagDelay is added (by default the value is set to 0). On NQ, the value added is computed by the delta of current server frame time previous server frame time.

  • ETL : https://github.com/etlegacy/etlegacy/blob/761fded941df1450d5f2e50598b0e01104d44dd1/src/game/g_active.c#L1226

  • Pub : https://github.com/BulldogDrummond/etpub/blob/d6e81ec7d8c62a4fab5c970a850aad049e2ed9ee/trunk/src/game/g_active.c#L1390-L1391

  • NQ : https://github.com/core-c/WET-NoQuarter/blob/a56829de1368e8da1bdf93acdeae43bddeed790e/trunk/src/game/g_active.c#L1971

Aranud avatar Sep 07 '20 11:09 Aranud

Another differences but on Jaymod side this time : there is one different regarding the implementation of unlagged in ETL / ETPUB / NQ compared to Jaymod and it's there : https://github.com/budjb/jaymod/blob/bbe44e0cca806d6b51260d48d4c11eab2b85bfe9/src/cgame/cg_ents.cpp#L2340-L2349

Aranud avatar Sep 10 '20 11:09 Aranud

Bots are not antilagged :

https://github.com/etlegacy/etlegacy/blob/1ee8c7e848d6d8aa1f243b378888d5149952ee20/src/game/g_antilag.c#L78-L82

This could explain why with test on bots the result are mitigated.

Aranud avatar Oct 26 '20 10:10 Aranud

Something went wrong with the refactoring in this commit: 8eff4c672e0455d9df735683663dcfd4c13f44f6 The actual headbox doesn't match the visual head box from g_debugPlayerHitboxes anymore.

rmarquis avatar Jan 01 '21 16:01 rmarquis

@Aranud https://youtu.be/fd0KRnL01Vo

mittermichal avatar Jan 03 '21 23:01 mittermichal

Hello, regarding to this issue, i want to manifest my experience on it aswell Aranud, this demo was made at SCRIM SERVER GRAY TM , but i think we can sense the same at main server for example. This is a big issue, the hit regi, i know is a on going process but yeah, big concern i would say since it gives us players lots os frustration when we are aiming and it does not register! How could this be more calibrated mate? rmarquis gave jaymod example, but nitmode aswell i guess has better experience in the hit regi than legacy! althou bullets are way more faster leaving gun.

And at the time when i was live playing, the player was crouched, and i was aiming head, somehow when got to demo player was not crouched and it seems i was aiming middle boddy in video. ( I was using wireless at the time despite low ping, now i use cable always to avoid any other option of what could have hapened here )

https://www.youtube.com/watch?v=Hr9bLHMkHwQ&feature=youtu.be&ab_channel=Malcreado

Thanks for your support

Cheers!

npapo avatar Feb 09 '21 08:02 npapo

@npapo upload the demo It looks like the version of the legacy you were playing with doesn't match the one where you replay the demo Also might be good idea to record server demos on the scrim servers.

mittermichal avatar Feb 09 '21 09:02 mittermichal

2021-01-10-195425-frostbite.zip

Here u have it

i added to zip but file went to .rar , and gibhud wont recognize it, i changed extension and is fine now, tell me if u were able to open it

Cheers

npapo avatar Feb 09 '21 09:02 npapo

The youtube version is misleading due to the rewinds causing a slight misinterpretation. When you watch the demo and the correct point in slow motion you will actually see that that the crosshair was slightly off the players head, so technically the hit detection was correct, but that said it should have registered as a headshot. Those hitboxes need to be redone.

jackeri avatar Feb 10 '21 12:02 jackeri

Please keep an eye on any possible improvement on TM in the coming days. This issue is notorious for being hard to test, but I think it felt a bit better on my end. I'm however a natural poor shooter, and I didn't play for the past 6+ months, so that might be placebo.

I also updated the official server (2.77.1-400) with the change.

rmarquis avatar Aug 13 '21 20:08 rmarquis

You might be a bit out of the loop. This issue is already fixed, thanks to @ryzyk-krzysiek :) Correct me if wrong.

BystryPL avatar Aug 13 '21 21:08 BystryPL

I'm not sure what changes you are referring to.

The issue above seems to be that some interpolated entities used in bullets trace are simply dropped immediately, so the antilag can't rely on them and hits are not registered or registered later, with a delay.

rmarquis avatar Aug 13 '21 21:08 rmarquis

You might be a bit out of the loop. This issue is already fixed, thanks to @ryzyk-krzysiek :) Correct me if wrong.

And @Aranud !

The body hitbox change that made it not static helped with issues that every mod with static one has, including etpro even though the realhead code comes from it.

The other contributing factor is that sv_fps 40 is playable now, it makes the hit results get to players faster. I think it improves antilag too because there are smaller intervals where player positions are saved, making it more precise and reducing the interpolation needed. This doesn't help public servers, as running at sv_fps 40 with a lot of players is not a good idea.

One thing that could be changed but might have little effect is to revert hitsound changes. The current solution is always run later in the code, so there is no real gain, only around 1ms loss probably. The other difference is that the call to play the sound is different now but I haven't gotten to test it if it makes a difference. The issue with current solution is that on high ping (200+ if I remember correctly) the sound is affected by it, hitsounds start stacking on top of each other, and even with turned off hitsounds the gun sound is stacked on top of each other too.

New hitounds https://streamable.com/3lgggu Old hitsounds https://streamable.com/bytgb1

I'm not sure what changes you are referring to.

The issue above seems to be that some interpolated entities used in bullets trace are simply dropped immediately, so the antilag can't rely on them and hits are not registered or registered later, with a delay.

I will definitely keep an eye on this and try to notice if there is an improvement or not. And test it myself when I get the time and motivation to do so, to understand better what is going on and how it could impact the game.

ryzyk-krzysiek avatar Aug 14 '21 14:08 ryzyk-krzysiek

Ah yes! Increasing sv_fps will indeed contribute to mitigate the interpolation issue, but not solve it. Having both working correctly would indeed be fantastic! Other changes you made are really good, I really like that bodybox height that follows the head movement.

Beware though, a quick look at the code shows there are still code dependent on sv_fps 20 (search for 50 or 1000 / 20 for example).

I believe the hitsound change (#1201) was done so it feels more immediate, but it might have adverse effects with higher sv_fps(?) There was a discussion about it somewhere, but I can't find it now.

rmarquis avatar Aug 14 '21 17:08 rmarquis

Yes there are still elements that sv_fps affects, I think it's mostly scripts and two of them are documented here #1637. We are playing games on sv_fps 40 for about 2 months now and there wasn't any issues noticed that could be related to that setting. Ideally it should all be fixed in the future, but because it makes such a small difference (like that wait command, on sv_fps 40 commands can get executed 25ms sooner) it's not really noticeable.

I misremembered exact issue with new hitsounds initialy and was too lazy to check discord. The issue manifest itself when a player gets rate limited and probably when packets are lost, so it's rather rare and not normal, but still worse than previously in extreme cases.

I might be wrong if I misunderstand something but I don't know how can the new solution be more immediate if the code that plays the hitsounds is simply run later and the old logic doesn't seem to be something complicated to slow it down so much.

Old: https://github.com/etlegacy/etlegacy/blob/6826d05d1cec2fec3d67f967e79afe7ce5142cc3/src/cgame/cg_playerstate.c#L502 New: https://github.com/etlegacy/etlegacy/blob/6826d05d1cec2fec3d67f967e79afe7ce5142cc3/src/cgame/cg_playerstate.c#L547

ryzyk-krzysiek avatar Aug 15 '21 08:08 ryzyk-krzysiek

The issue manifest itself when a player gets rate limited and probably when packets are lost, so it's rather rare and not normal, but still worse than previously in extreme cases.

I might state the obvious here, but... if this happens to high lagging players only, does it really matter? They'd have more worry about any smooth gameplay than some weird sound issue.

I might be wrong if I misunderstand something but I don't know how can the new solution be more immediate if the code that plays the hitsounds is simply run later and the old logic doesn't seem to be something complicated to slow it down so much.

The difference is highlighted in #1201 with the move to hitsound becoming event based, which is theoretically faster at the cost of a slightly higher bandwidth. However, looking at comment from @mittermichal in 94b616a884adb89e26b7d5ba56ccf056caf2ffdc, I'm now wondering if this new implementation has been done correctly.

so basically you removed old detection of hit sounds but kept the old way creating them

I'll have a closer look.

rmarquis avatar Aug 15 '21 09:08 rmarquis

I might state the obvious here, but... if this happens to high lagging players only, does it really matter? They'd have more worry about any smooth gameplay than some weird sound issue.

You are right, it's just another point about new hitsounds not improving anything actually and even making things worse in some cases.

The difference is highlighted in #1201 with the move to hitsound becoming event based, which is theoretically faster at the cost of a slightly higher bandwidth. However, looking at comment from @mittermichal in 94b616a, I'm now wondering if this new implementation has been done correctly.

I still fail to see how they are faster since both old and new solution mean that the hitsound come in the same snapshot and the event one is run later in the code in client. Also not saying it should be reverted, just an observation from when I was testing performance issues, that it seems it's actually marginally worse.

ryzyk-krzysiek avatar Aug 15 '21 10:08 ryzyk-krzysiek

I'm now wondering if this new implementation has been done correctly.

A commit wasn't referenced in #1201 , but the code was corrected later and the implementation is actually correct.

I still fail to see how they are faster since both old and new solution mean that the hitsound come in the same snapshot and the event one is run later in the code in client.

I think you're right here. The discussion about server hitsound came up a few times, but I can't find much on the tracker so I guess it all happened on Discord. I believe the discussion revolved about sound being played slightly faster, but maybe I am mistaken and it was solely about reducing bandwidth. I think you're the first to actually have measured perf difference here.

Also not saying it should be reverted, just an observation from when I was testing performance issues, that it seems it's actually marginally worse.

Note that these performance results might be obsolete now, and new testing be required. As outlined in #1637, if the interpolation correctly works now, using higher sv_fps might be quite different, and might even have adverse effects. In particular, Neil Toronto advises against using higher than 33, though I am not sure how that advice translates when using modern hardware compared to ~15 years ago.

rmarquis avatar Aug 17 '21 14:08 rmarquis

We played a bit of games with the recent change and I can't tell if there is any difference, but it didn't seem to make things worse so it's fine I guess.

The last issue people sometimes bring up is that legacy has more delay with hitsounds than etpro, after few months of trying many things and testing in my opinion legacy is the same or even faster when sv_fps 40 is used. Unless someone will provide proof of etpro having less delay I would say this is fixed.

Note that these performance results might be obsolete now, and new testing be required. As outlined in #1637, if the interpolation correctly works now, using higher sv_fps might be quite different, and might even have adverse effects. In particular, Neil Toronto advises against using higher than 33, though I am not sure how that advice translates when using modern hardware compared to ~15 years ago.

I think you might have misunderstood me with this performance tests. By perfomance I meant issues with server with high number of players, in the end it wasn't really the server, it was players having too low rate.

I don't agree on using sv_fps 40 being not advantageous over standard 20.

Sv_fps 40 means more updates of the game on the server, I would say that leads to more precision and better antilag, it also means more updates sent to players, which a) reduces interpolation from 50ms to 25ms, which can have disadvantages with bad internet (so far noone complained), but players see faster what is actually happening and it reduces peekers advantage (common problem in CS because they have/had interpolation of 100ms) b) lower ping which reduces delay of important events such as hitsounds

I think there is a reason in CS:GO people prefer to play on 128tick servers instead of 64.

Of course this requires better server and more bandwidth, that's why I would say it's not recommended for public servers with a lot of players. I haven't tested when the server would have start having trouble with running game frame in under 25ms, but for 6v6 games and less there is no issues.

This is a good read as reading it is almost like reading what ET has, probably because source engine was based on some quake engine. https://developer.valvesoftware.com/wiki/Source_Multiplayer_Networking

As of now untill someone brings evidence of something not working correctly or working worse than other mods (like the delay I do not have on my end) I think issues have been pretty much solved.

ryzyk-krzysiek avatar Aug 23 '21 19:08 ryzyk-krzysiek

Thanks for the feedback. One important question: did you play on server with sv_fps 20 or 40?

I don't agree on using sv_fps 40 being not advantageous over standard 20.

Just to clarify here: I am not saying it couldn't be advantageous, but that it does mask the underlying antilag issue, that we know exist. As such, it is not a proper fix but more of a band aid solution - which might be sufficient, but indeed not optimal. Having both would be advantageous, if the reserves of Neil are proved to be unworthy or obsolete with modern hardware and bandwidth.

rmarquis avatar Aug 23 '21 19:08 rmarquis