Skip to content

Conversation

@dwoodruffsf
Copy link
Contributor

Main changes:

  • cleaned up miscellaneous todos and fixmes
  • Added plumbing so that the server can explicitly set a DisconnectReason on a client (the motivation for this was the "GUID Already Playing" error condition, where the server wants to boot the old connection, and give a reason).
  • Found ConnectStatus and DisconnectReason to be highly duplicative, so I combined them into ConnectStatus
  • Updated Client logic to detect Host disconnect scenario (e.g. host losing network).
  • ServerGameNetPortal now properly cleans up per-connection state on client disconnect.
  • ServerGameNetPortal now handles two more error cases: (1) server full, and (2) guid already playing (in the latter case, kicks the old connection).
  • ServerGameNetPortal now actually disconnects you if you experience a game-level connection failure.
  • GameNetPortal: Some Named Messages were being missed because they were arriving before the ClientConnected callback. I moved this event registration to Start, and made it unconditional on the role.
  • Brought back workaround in ServerCharSelectState to prevent GOMPS-506 and GOMPS-508 from blocking entry into game.

I have left two FIXME:MLAPI and 1 TODO:MLAPI tag in the code, referencing active MLAPI bugs in the first case, and a feature we are expecting in the future in the second.

…, which had several problems (e.g. if 2 people log in with the same guid, we didn't do anything sensible, but just clobbered our the old guid entry in our hash and got inconsistent state).
@fernando-cortez fernando-cortez added the 2-Reviewed with Comments PR requires owner's attention label Apr 30, 2021
fernando-cortez
fernando-cortez previously approved these changes May 4, 2021
Copy link
Contributor

@fernando-cortez fernando-cortez left a comment

Choose a reason for hiding this comment

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

My comments have been addressed.

@fernando-cortez fernando-cortez added 2-One More Review One review in, one to go and removed 2-Reviewed with Comments PR requires owner's attention labels May 4, 2021
@Cosmin-B Cosmin-B merged commit 602b2d4 into develop May 5, 2021
@fernando-cortez fernando-cortez deleted the fixme_cleanups branch May 5, 2021 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2-One More Review One review in, one to go

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants