search

ec- / Quake3e

Improved Quake III Arena engine
GNU General Public License v2.0
1.15k stars 148 forks source link

Don't spawn loading clients during map restart #282

Closed Chomenor closed 1 month ago

Chomenor commented 1 month ago

This removes the call to SV_ClientEnterWorld during map restart for clients that haven't spawned yet. Fixes map failing to load if a map restart occurs during a UDP download (due to CS_ACTIVE check in SV_DoneDownload_f). Also prevents players from spawning prematurely if a map restart occurs while they are loading the map.

A download-specific fix would also be an option, but this method may be more correct, as there doesn't seem to be a reason to forcibly spawn players if a map restart happens to occur while they are loading the map. It appears that cnq3 uses the same approach.

ec- commented 1 month ago

https://bugzilla.icculus.org/show_bug.cgi?id=4668

Chomenor commented 1 month ago

You're right, this was broken, and now that I checked, cnq3 seems to be broken as well. I added a provisional fix but it will need more testing. It is pretty ugly but still might be arguably better than what was there before.

Alternatively we could just skip SV_ClientEnterWorld for downloading clients only, which was the main problem I was trying to solve.

ec- commented 1 month ago

Please check https://github.com/ec-/Quake3e/commit/ed4af16763684a9b492c4c6737833531b43963e2

Chomenor commented 1 month ago

It appears broken. The client freezes if a map restart occurs during map loading.

I know of three ways to possibly fix it:

1) Keep the call to SV_ClientEnterWorld during map restart but only for non-downloading clients. This fixes the download problem, as downloading clients will now get the gamestate via SV_DoneDownload_f. It still spawns non-downloading clients if a restart happens during map loading, but probably not a big deal.

2) Don't change sv_serverid during map restart at all. Use a different method to drop pre-restart movement commands (to try to be as close as possible to original behavior). This is the method I use in my Elite Force fork. It probably the cleanest approach overall, and simplifies the code considerably by getting rid of sv.restartedServerId and associated checks. One downside would be compatibility if a mod were to explicitly query the sv_serverid cvar expecting a different value after a map restart (although this is probably very unlikely that any mod would do this).

3) Defer calling SV_ClientEnterWorld until a client message is received. This is what this PR contains. It probably also works but maybe the code isn't as clean compared to the other options.

If you have a preference for one of these options I can test it and open a new pull request.

Chomenor commented 1 month ago

Added PR #283 as a potential fix.