Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#4036 closed defect (fixed)

[PATCH] Unknown player has rejoined

Reported by: elexis Owned by: elexis
Priority: Must Have Milestone: Alpha 22
Component: Network Keywords: patch
Cc: Patch:

Description

Sometimes when two clients join simultaneously, it might show "Unkown player has rejoined" or "Bad connection to Unknown Player". This is due to g_Players not being updated.

The server must likely either send another player-assignment netmessage or must process existing messages correctly in the loading screen.

The bug exists at least since alpha 19.

Attachments (2)

0001-Network-server-cleanup.patch (6.0 KB ) - added by elexis 7 years ago.
Removes the peculiarity of Broadcast.
0002-Send-player-assignment-updates-to-rejoining-clients-.patch (971 bytes ) - added by elexis 7 years ago.
Fixes the actual bug.

Download all attachments as: .zip

Change History (8)

comment:1 by elexis, 7 years ago

TLDR: Broadcast doesn't broadcast to rejoining clients, thus those don't receive playerassignment updates.


Observeration: In alpha 21 the bug has shapeshifted and besides showing an unknown player having rejoined, it also throws the following error stack when that client posts chat:

WARNING: JavaScript warning: gui/session/messages.js line 947
reference to undefined property g_PlayerAssignments[msg.guid]
ERROR: JavaScript error: gui/session/messages.js line 947
TypeError: g_PlayerAssignments[msg.guid] is undefined
  formatChatCommand@gui/session/messages.js:947:1
  g_FormatChatMessage.message@gui/session/messages.js:107:20
  addChatMessage@gui/session/messages.js:766:18
  g_NetMessageTypes.chat@gui/session/messages.js:62:1
  handleNetMessages@gui/session/messages.js:562:4
  onTick@gui/session/session.js:621:2
  __eventhandler161 (tick)@sn tick:0:1

On some event, the server sends the new player assignments, so the unknown player becomes identified and is displayed as "has rejoined" (when that feels to have occured long before). After that, the errorspam stops.

Blackbox analysis: Obviously the player GUID isn't found in the player assignments, meaning that the player assignments have not been sent to the rejoined client that experiences these error messages. As the bug only occurs to players who were rejoined when the other client has also rejoined simultaneously, it must mean that the server sends the player assignments message only when new client start to rejoin, but not when the rejoin has finished.

Whitebox analysis: SendPlayerAssignments is sent from AddPlayer, RemovePlayer, SetPlayerReady, ClearAllPlayerReady, AssignPlayer and StartGame. AddPlayer is only called from OnUserJoin which is only called from OnAuthenticate, which is called immediately after the rejoin was requested. The Ready and StartGame messages are never sent in running games. When some other client joins or leaves, AddPlayer and RemovePlayer will trigger a player assignment update and thus fix the bug, matching the observation.

Reproduce:

  1. Host a networked game with late observers enabled. It must be slow to rejoin, so chose Corsica vs Sardinia with 7 AIs on a giant map and 2x gamespeed if needed.
  2. Client 1 starts to rejoin.
  3. Client 2 starts to rejoin, before 1 finishes the rejoin.

Client 1 is getting the undefined player rejoin message and errors if that other player types chat.

Cause: The server does call SendPlayerAssignments which does Broadcast the message. But Broadcast skips all clients that are not in NSS_PREGAME or NSS_INGAME. Since our rejoining client 1 is in the state NSS_JOIN_SYNCING, it will not receive that message.

Origin: The peculiarity must come from the way rejoins work: Rejoining clients don't receive any simulation commands until they have finished deserializing.

(When the client starts to rejoin ReallyStartGame from Game.cpp it first deserializes the savestate, then calls LoadFinished() from NetClient.cpp to send the NMT_LOADED_GAME message to the server who responds with sending the simulation commands of all clients from the turn nr of the deserialized state to the current turn in OnJoinSyncingLoadedGame.)

Resolution: A function like Broadcast should be more general and by default send to all clients that passed the handshake. Otherwise the receiving clients should be selected wisely and explicitly by the caller.

Last edited 7 years ago by elexis (previous) (diff)

by elexis, 7 years ago

Removes the peculiarity of Broadcast.

by elexis, 7 years ago

Fixes the actual bug.

comment:2 by elexis, 7 years ago

Keywords: patch review added
Milestone: BacklogAlpha 22
Summary: Unknown player has rejoined[PATCH] Unknown player has rejoined

comment:3 by Imarok, 7 years ago

Looks good at first glance

comment:4 by elexis, 7 years ago

(The new Broadcast could also be used instead of SendMessage in KickPlayer. All other SendMessage calls don't apply.)

comment:5 by elexis, 7 years ago

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 19171:

Change the NetServer Broadcast function to send the given message to clients that are in one of the states specified by the caller.
Thus remove the peculiarity to broadcast to clients that are in the gamesetup, loading screen or ingame, but not rejoining ones.

Fix "unknown player" errors in the GUI by broadcasting player assignments to rejoining players too. Fixes #4036.

Differential Revision: D17
Reviewed By: Imarok

comment:6 by elexis, 7 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.