Skip to content

Unknown Players #3035

@LucasAdams

Description

@LucasAdams

In the archives I've noticed clientIDs appearing that are not in the players list.
For example in https://api.openfront.io/public/game/xBfi1ncT the client gZts6g7J seems to connect at turn 3126 (~5minutes into the game?!?!), spawns at turn 3555, and even performs an attack at turn 3783, before disconnecting at turn 4130 (~30secs later).
Additionally in a random selection of ~100 games I find ~5 games where disconnects (without other actions) occurs to ~10 clients.

Looking at GameServer.ts the archiveGame() outputs this.gameStartInfo which is only set in start() and only populated by this.activeClients.
I think this has two issues:
(a) Temporarily disconnected clients are not reported: clients are removed when ws.on('close') (if the client actively closes the connection this can be less than the 30sec disconnectedTimeout and less than the hardcoded 60_000 in phase()). GameManager.ts tick() seems to show that the prestart() period is only 2secs long which reduces exposure.
(b) New clients are not reported: joinClient() allows clients to join after start() called (_hasStarted==True).

If archiveGame() iterates over this.allClients rather than this.gameStartInfo.players then the archive is completely fixed.
If start() iterates over allClients rather than activeClients then (a) is mitigated completely.
However (b) requires sending updates to clients on joinClient() potentially requiring a new message.

Some general thoughts about the GameServer code:

  • sendStartGameMsg() should just accept client as a variable which all of it's callers can provide, rather than searching again for the corresponding websocket.
  • this.websockets should probably be removed in favour of iterating over this.allClients.ws
  • should client.ws.on("close") immediately trigger a mark_disconnected? or is it intended to keep the 60sec delay for temporary disconnects?
  • ideally kickClient would immediately mark them as markClientDisconnected(true) so their territory could be immediately canibalised.
  • the disconnected status and the websocket itself are managed separetely and it might be worth unifying. for example phase() implements a timeout of 60_000 but checkDisconnectedStatus() uses the 30sec disconnectedTimeout but these concepts are somewhat orthogonal.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status

    Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions