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

Frame-waiting and net messages synchronization in main event loop from ioq3 #64

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

karnute
Copy link
Contributor

@karnute karnute commented Jan 21, 2017

This modification uses adapted code from ioquake3 in main event loop frame-waiting and net messages synchronization. It tries to alleviate, in part, the warping that were aggravated when sv_fps were increased in the server version 4.3.1. Code change include a potential better network timing by taking into account the network rate of each client.
Only checked, compile and run, in Linux 64 bits. Client tested in current servers (probably with sv_fps=20). Server needs live tests at increased values of sv_fps (like 25, 30, 40,...) and with players with different pings.

@karnute
Copy link
Contributor Author

karnute commented Jan 21, 2017

Sorry, I realize now that this new pull request includes all my minor changes in the previous pull request #61 (that were done without branch).
The important change in this pull request is the commit 505496d that is in branch ioq3_frame_net_wait

@ThomasBrierley
Copy link
Contributor

ThomasBrierley commented May 8, 2017

Looking forward to seeing the effect of this 😄

If you still wanted to tidy It, should be safe to cherry-pick it:
(505496d appears to be independent)...

git reset 8ed058b --hard
git cherry-pick 505496d
git push origin ioq3_frame_net_wait --force

@karnute karnute force-pushed the ioq3_frame_net_wait branch from dddaee4 to 86b8087 Compare May 9, 2017 17:39
@karnute
Copy link
Contributor Author

karnute commented May 9, 2017

Thanks, Thomas. Now this pull request is only about the isolated important commit (the other minor commits can be dealt just in request #61 as intended).
From some experiments done with the modification in this commit (see UrT forum ) it seems to be only a very small improvement, but in any case results were just equal or slightly better than with the official binaries (in server and in client). I think, the effect of adaptive packet timing could be more important in servers with heterogeneous clients pings. So, probably the changes will not harm, but could be a small hope to a future return of increased snaps in servers.

@ThomasBrierley
Copy link
Contributor

I think, the effect of adaptive packet timing could be more important in servers with heterogeneous clients pings.

Yes, and from my experience this is not an unusual scenario in UrT. I frequently see ranges from 30 - 300ms in packed servers, and this is when the strange hits start happening (even though I usually have ~40ms ping).

It's really difficult to test this stuff because of the "human" factor, it might be good to come up with an objective way to measure the effect.

@ryan-sg
Copy link
Contributor

ryan-sg commented Feb 6, 2018

FWIW -- i have tried a varient of this patch (from @karnute's repo) against a server also running the code, and it does seem to feel better. Unfortunately 'feels' is hard to quantify into words or measurable units :-)

@ThomasBrierley
Copy link
Contributor

ThomasBrierley commented Feb 6, 2018

Side note: I tried OpenArena recently (also based on ioq3) the networking is really smooth, but then it uses unmodified ioq3 engine so they get all the years of patches and improvements for free (as in without integration).

I did have a go at attempting to diff stuff in ioq3 with this repo but honestly i'm just not familiar enough with ioq3 or C, not to mention the divergence seems to go so far back. Maybe someone else with more ioquake coding skillz can give it a pop.

@ryan-sg
Copy link
Contributor

ryan-sg commented Feb 6, 2018

@ThomasBrierley you may be interested in @mickael9's repository: https://github.com/mickael9/ioq3

@Barbatos
Copy link
Member

Barbatos commented Feb 6, 2018

Yeah you should have a look into @mickael9's repository. It was forked off the latest ioq3 engine and the UrT-specific stuff are being implemented. Keeping a close eye on it!

@ThomasBrierley
Copy link
Contributor

Oh nice! thanks. I suppose it does make a bit more sense to take the UrT specific bits and patch them into the latest ioq3 rather than the reverse.

@karnute karnute force-pushed the ioq3_frame_net_wait branch from 86b8087 to 21c9ffe Compare February 16, 2018 20:51
@ThomasBrierley
Copy link
Contributor

@Barbatos It looks to me that Karnute may have gotten to the bottom of the hits issues with sv_fps=20 with his PR on mickael's: mickael9/ioq3#29

It might be worth patching in here for a short term point release if testing turns out to be successful. Unless I'm jumping the gun here and it's not as applicable as I think.

@karnute
Copy link
Contributor Author

karnute commented Apr 3, 2018

The proposed patch mickael9/ioq3#29 could be probably applied here and also in ioquake3.org, but it would be better to do some tests in servers with many players at different latency before applying it here. That patch could improve hits, but it could also affect the calibration of the lag compensation inside the game code, and could require a recalibration.
The ping problem occurs also with any value of sv_fps, so that patch may not solve the glitches observed with higher values of sv_fps.

@ThomasBrierley
Copy link
Contributor

It would be better to do some tests in servers with many players at different latency before applying it here.

I feel like this would be best achieved with longer running public test server just to get enough players. Am I right in thinking that patch doen't affect client binaries? In which case is it ok to setup an appropriately named public server for this purpose?

@karnute
Copy link
Contributor Author

karnute commented Apr 3, 2018

Client engine includes a non-dedicated server, but the changes in mickael9/ioq3#29 patch does not require any change in client players to work, so it could be tested in public servers by any other client.

@ThomasBrierley
Copy link
Contributor

I put a test server together last night building off your branch and it was all working nicely, but then I discovered it was stuck in ffa mode O_o (g_gametype ineffective) which I don't think is going to attract many players.

It's nothing to do with your patch, it's also present in mickaels main branch and I'm pretty sure i've found the offending commits from ioq3 upstream. Anyway, I'll hold off until that issue is addressed separately because team modes are needed to attract enough of those unsuspecting test subjects 😈

@karnute
Copy link
Contributor Author

karnute commented Apr 4, 2018

Thomas, I will reply you about mickael9/ioq3#29 patch in its corresponding thread, not here, because the patch here was only a subset of the mickael9 changes.

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.

4 participants