Skip to content

Handle keepalive packets in server loading state#864

Merged
notfood merged 2 commits intorwmt:devfrom
MhaWay:server-loading-keepalive
Apr 8, 2026
Merged

Handle keepalive packets in server loading state#864
notfood merged 2 commits intorwmt:devfrom
MhaWay:server-loading-keepalive

Conversation

@MhaWay
Copy link
Copy Markdown

@MhaWay MhaWay commented Apr 7, 2026

Summary

Handle Client_KeepAlive packets while the connection is in ServerLoading.

Fixes #861.

What changed

  • add keepalive handling to ServerLoadingState
  • update latency / behind / simulation tracking in the same way as ServerPlayingState
  • add a regression test that sends a keepalive while the server is blocked waiting for join point completion

Why this fixes the issue

Previously, a reconnecting client could continue sending keepalive packets while the server was still in ServerLoading, and the server would log them as unhandled packets.

After this change, the loading state accepts those packets normally, which removes the server log spam and keeps connection bookkeeping coherent during reconnect/loading.

Validation

  • dotnet test Source/Tests/Tests.csproj --filter "FullyQualifiedName~LoadingStateHandlesKeepAliveWhileWaitingForJoinPoint"
  • dotnet build Source/Server/Server.csproj -c Release

Copilot AI review requested due to automatic review settings April 7, 2026 09:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the server’s ServerLoading connection state to accept and process Client_KeepAlive packets (matching ServerPlaying behavior) to prevent “unhandled packet” log spam during reconnect/loading scenarios (Fixes #861).

Changes:

  • Add a typed handler for ClientKeepAlivePacket in ServerLoadingState and update latency/behind/simulation bookkeeping similarly to ServerPlayingState.
  • Add a regression test client-state that sends a keepalive during the loading phase while the server is blocked waiting for join point completion.
  • Add a server-side test covering the reconnect/loading keepalive scenario.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
Source/Common/Networking/State/ServerLoadingState.cs Adds Client_KeepAlive handling during ServerLoading to avoid unhandled packet errors and keep timing/latency fields updated.
Source/Tests/ServerTest.cs Adds a regression test intended to validate keepalive handling while waiting for join point completion.
Source/Tests/Helper/TestLoadingKeepAliveState.cs Implements a test client connection state that sends Client_KeepAlive during the loading handshake.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +64 to +74
var timeoutWatch = Stopwatch.StartNew();
while (true)
{
if (server.playerManager.Players.Count == 0)
break;

if (timeoutWatch.ElapsedMilliseconds > 2000)
Assert.Fail("Timeout");

Thread.Sleep(50);
}
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The success condition here can be satisfied immediately because Players.Count starts at 0; the loop may break on the first iteration before the client has connected/sent any packets, so the test can pass without exercising the keepalive-in-loading behavior. Consider waiting until the player count becomes >0 (or another “client reached loading” signal) and only then waiting for it to drop back to 0 after disconnect.

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +60
_ = Task.Run(async () =>
{
await Task.Delay(200);
server.worldData.EndJoinPointCreation();
});
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fire-and-forget Task.Run isn’t awaited or tracked, so it can still execute after the test completes/teardown starts, potentially making the test nondeterministic and leaking work into subsequent tests. Store the task and await it (or add it to teardown) so join point completion is guaranteed and scoped to the test.

Copilot uses AI. Check for mistakes.
await TypedPacket<ServerJoinDataPacket>();

connection.Send(Packets.Client_WorldRequest);
connection.Send(new ClientKeepAlivePacket(0, 0, false, 0), false);
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client_WorldRequest is sent reliably, but the keepalive is sent unreliably (reliable: false), so it can be delivered/processed before the world request and state transition on some transports. That can make the regression test flaky and may not guarantee the keepalive is handled specifically in ServerLoading. For determinism, send the keepalive reliably here (or otherwise synchronize so it’s sent only after the server has switched to loading).

Suggested change
connection.Send(new ClientKeepAlivePacket(0, 0, false, 0), false);
connection.Send(new ClientKeepAlivePacket(0, 0, false, 0));

Copilot uses AI. Check for mistakes.
@MhaWay MhaWay force-pushed the server-loading-keepalive branch from e32e439 to 86c0c6a Compare April 7, 2026 09:51
@notfood notfood added fix Fixes for a bug or desync. 1.6 Fixes or bugs relating to 1.6 (Not Odyssey). standalone server Fix or bugs relating to the standalone server. labels Apr 8, 2026
@notfood notfood linked an issue Apr 8, 2026 that may be closed by this pull request
@notfood notfood merged commit be793d1 into rwmt:dev Apr 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.6 Fixes or bugs relating to 1.6 (Not Odyssey). fix Fixes for a bug or desync. standalone server Fix or bugs relating to the standalone server.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ServerLoading should not log Client_KeepAlive as an unhandled packet

3 participants