Pull ping corrected grenade and timer re-write to master #932
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 16 commits
September 30, 2022 00:58
Use a reliable transmit for death messages as well as logging the time of last death. This allows grenade timers to be more reliably stopped and allows for stronger ordering of events which might be out of order (e.g. prime as dying).
Due to their <prime> <wait> <throw> nature, grenades have all sorts of inconsistencies introduced by delay between client and server around each event. Two players that see each other at the same time, and prime at the same time, will actually not be able to throw at the same time as the player with the lower ping is able to get the server to register their prime first. This is particularly material since grenades don't allow you to throw for the first 800ms, so even 50-100ms ping differences are a large relative advantage. This introduces ping correction, so that the point at which we consider the client to have primed is corrected for their latency when enabled. We ensure that this does not let players prime any faster, or more frequently, than they would have been able to before, only more consistently. One very nice property is that unlike projectile projection, which causes visible projectiles to skip, there are no strange artifacts for other clients as the adjustment occurs before it's possible to throw the grenade. Also rewrite the grenade timer rendering client code to be more compact and to handle the fact that the expiry offset has been adjusted to correct for latency. Clients that do not support CSQC can simply playsound at the point which they prime; this will now be correctly aligned (as the point they primed is properly reflected to the server). Controlled by: localinfo precise_grenades on/off While we'd normally default a server setting on, this is a big enough change that we'll give it a pug or two of testing before pushing it as a default to soak. [For this reason we also temporarily allow mid-round changing of the tunable.]
Trying to debug some inconsistencies with soundupdate.
So a major part of the issue seems to be that the reference "self" was changing due to migration of timers into a class. However, it turns out we were getting fortunate before and that self was actually the world entity and not the player. This turns out to be the entire reason that soundupdate even worked with localsound, as the latter implies a default entity of zero. This explains why we got double sounds even with the identical calls under the new code (as implicit entities from the grentimer entities were substituted. This seems to work, I get very occasional misses, but I suspect those were the ones you were also seeing before. I'm not sure about the PHS stuff, I've gone through the FTE code and it doesn't look like it's strongly interpreted for Q1 despite the docs and other mod's usage. I've restored the extra channels for now. I've left the old sound code as the default while I test the new one in a few pugs before flipping them. Also made sure the timer sound was cached to avoid potential hitching (and anything to generally increase reliability).
own channels and get out of the channel game entirely. Seems to work reliably under testing. Also try to "perfectly" emulate the old grenade timer sounds in case there are still problems. This means pulling them out of the object and back to the CSQC-call chain so that self can be world and not the object. This can be enabled with fo_grentimer_debug 2. (I tried it the other way but under testing the new timers seemed more reliable now that they're on their own ent (from sound system pov) so..... we'll see.)
clients for testing. In effort to mirror original magically working state (which is the default), restore original channels. Fix caching which got dropped in merge. Small cleanups.
Small inconsistencies between client time and server time can result in minute negative offsets that can stop the sound from playing.
Now that we're happy with the new behavior and there's no differences from the old beyond precision, eliminate the old code and precise grenade option, there are now only "FO Grenades".
zel pointed out it's nicer to stack timers by expiry time when some is spamm^Whas several active simultaneously. Also, he thought he might have seen 2 opaque timers. This could happen momentarily if prime and throw messages are reordered, but we can actually hide this easily by always rendering stacked timers as transparent.
Both because cvar string interaction is suspect in a crash and because autocvars are just better, migrate all* of our cvars to use autocvar. Also update fo_grentimer default to 2, it had not been updated to reflect the work on ping correction (either new or old). *: Some of the menu code uses dynamically constructed names which are incompatible with autocvar so we exclude those.
Last step before deleting the old code and merging to master, everything seems to be working well for people now. Also confirmed that there was an FTE use-after-free bug in reading the grenade timer sound from cvar that was occasionally resulting in crashes. That bug is being fixed FTE side and our migration to auto-cvar in the meanwhile is sufficient to fix on ourside. This fixes client hitches so let's make sure everyone gets it again.
drzel
approved these changes
Sep 30, 2022
| * `localinfo project_grenades 0/1` [default: 0] Adjust the point at which | ||
| grenades are primed to correct for client latency. Does not allow players to | ||
| throw grenades any faster, or more frequently; only more consistently. | ||
| * Server option to limit `sv_maxclients` to current number of players during quad gametime. `localinfo limit_quad_players 0/1`. Default: `1`. |
There was a problem hiding this comment.
limit_quad_players has been removed.
|
|
||
| // Don't allow prime to move before throw (or to the same frame). | ||
| offset = min(offset, time - (self.last_throw + 0.013)); |
There was a problem hiding this comment.
0.013 should probably be 1 / serverkeyfloat("maxfps"). I think there are a couple of other places where errant 77 / 0.0(0)13s are lurking, and should probably be updated too.
It's rather frustrating that ceasefire causes all the active grenade timers to lose sync (the visual timer pauses, but the sound just keeps playing). Implement actual pause/unpause of the timer sound around ceasefire start/end.
newbytf
force-pushed
the
gren_dev_master
branch
from
2e94b4c to
1b79e3c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.