Speed up game start in online lobby#946
Conversation
|
Nightly build for this pull request:
|
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce perceived latency when starting online lobby games by ensuring the START CTCP is sent immediately (not delayed behind the queued send pipeline) while flushing any queued game-setting/player-setting CTCP messages first so clients start with consistent settings.
Changes:
- Added a
Connection.FlushMessagesOfTypes(...)helper to immediately emit queued messages of specifiedQueuedMessageTypes. - Added
Channel.FlushGameSettingsMessages()to flush queued game settings/player settings messages. - Updated host game launch flow to flush settings first and send
STARTas anINSTANT_MESSAGE(immediate send).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| DXMainClient/Online/Connection.cs | Adds a new API to flush queued messages by type and send them immediately. |
| DXMainClient/Online/Channel.cs | Exposes a channel-level helper to flush queued game settings/player-setting messages. |
| DXMainClient/DXGUI/Multiplayer/GameLobby/CnCNetGameLobby.cs | Flushes queued settings before host sends START, and changes START to INSTANT_MESSAGE. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| foreach (string message in toSend) | ||
| SendMessage(message); | ||
| } |
There was a problem hiding this comment.
FlushMessagesOfTypes sends messages by calling SendMessage directly, but SendMessage writes to serverStream without any synchronization. Since SendMessage is also invoked from the send-queue thread (RunSendQueue) and from other call sites (e.g. INSTANT_MESSAGE), this can result in concurrent writes to the same NetworkStream, which is not guaranteed to be thread-safe and can interleave/corrupt IRC commands. Consider serializing all serverStream.Write calls via a dedicated send lock inside SendMessage (and/or funneling flush sends through the same single sending mechanism).
| public void FlushMessagesOfTypes(params QueuedMessageType[] types) | ||
| { | ||
| List<string> toSend; | ||
| lock (messageQueueLocker) | ||
| { | ||
| toSend = MessageQueue | ||
| .Where(m => types.Contains(m.MessageType)) | ||
| .Select(m => m.Command) | ||
| .ToList(); | ||
| MessageQueue.RemoveAll(m => types.Contains(m.MessageType)); | ||
| } | ||
| foreach (string message in toSend) | ||
| SendMessage(message); |
There was a problem hiding this comment.
FlushMessagesOfTypes bypasses the normal send-queue pacing (MessageQueueDelay) and will emit all matching queued commands back-to-back. Given the queue exists to avoid IRC flood protection (and other parts of the code even randomize JOIN delays), this introduces a real risk of being throttled/kicked when multiple settings messages are flushed at once. Consider applying a small configurable delay between flushed messages (e.g. reuse MessageQueueDelay or a smaller capped delay) or re-queueing them with a higher priority rather than writing them immediately.
Since we added the STRTD message, game starts have gotten a bit slower to arrive due to the queue delay. This PR changes the game start message to instant so it arrives first and much quicker. We also ensure that game options messages are sent before the game start message to ensure everyone starts with the same settings.
In this PR, when we flush queued options messages, there is no delay between them being sent. I don't anticipate there being any flooding issues but we can revisit if there are reports.