Retain tunnel object through refreshes #836
Conversation
Prevent multiple refreshes running at same time. Prevent duplicate tunnels.
|
Nightly build for this pull request:
|
There was a problem hiding this comment.
Pull Request Overview
Retains existing tunnel objects during refresh to preserve PingInMs values and prevents concurrent refresh operations.
- Adds a refresh lock/flag to avoid overlapping tunnel refreshes.
- Merges refreshed tunnel data into existing objects via new UpdateFrom method while filtering duplicates.
- Adjusts ping initiation logic and introduces duplicate suppression using a HashSet.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| DXMainClient/Domain/Multiplayer/CnCNet/TunnelHandler.cs | Adds refresh concurrency guard, merges tunnel lists preserving PingInMs, removes duplicates, and updates ping invocation logic. |
| DXMainClient/Domain/Multiplayer/CnCNet/CnCNetTunnel.cs | Introduces UpdateFrom method to selectively update tunnel metadata while retaining ping/state. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Task.Factory.StartNew(() => | ||
| { | ||
| List<CnCNetTunnel> tunnels = RefreshTunnels(); | ||
| wm.AddCallback(new Action<List<CnCNetTunnel>>(HandleRefreshedTunnels), tunnels); | ||
| try | ||
| { | ||
| List<CnCNetTunnel> tunnels = RefreshTunnels(); | ||
| wm.AddCallback(new Action<List<CnCNetTunnel>>(HandleRefreshedTunnels), tunnels); | ||
| } | ||
| finally | ||
| { | ||
| lock (_refreshLock) | ||
| { | ||
| _refreshInProgress = false; | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Using Task.Factory.StartNew without specifying TaskScheduler.Default can run this work on the current (UI) synchronization context, defeating the intent of making the refresh asynchronous and potentially freezing the UI. Replace with Task.Run(...) or specify TaskScheduler.Default: Task.Factory.StartNew(() => { ... }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Thanks, that is fixed.
Tunnel refresh now reuses existing objects to preserve the PingInMs - this should stop pings changing to unknown in game lobbies.
Also prevents multiple refreshes running at once.