Skip to content

feat: auto reconnect #513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 26 commits into from
Mar 8, 2022
Merged

feat: auto reconnect #513

merged 26 commits into from
Mar 8, 2022

Conversation

LPLafontaineB
Copy link
Contributor

Description (*)

This PR adds automatic reconnect attempts when a players loses connection. It will try to reconnect three times before giving up and sending the player back to the main menu (unless the player uses the quit button earlier). As a workaround until a fix for MTT-2684 is available in NGO pre.7, before trying to reconnect we load an empty scene. We will wait until that fix is available before cherrypicking this to develop.

Related Pull Requests

Issue Number(s) (*)

Fixes issue(s): MTT-2617

Manual testing scenarios

  1. Start a game with at least one client
  2. On the host, pause the game until the client disconnects
  3. See the client moved to the new scene with a popup message
  4. Before the end of the third attempt, unpause the host
  5. See the player correctly rejoins the host

Questions or comments

There currently is an issue when testing with debug builds or in the editor. Since the host is paused, it doesn't have the time to time out the clients on its side, so when it is unpaused and the clients reconnect to it, it considers them as duplicate connections. However, in debug builds or in the editor, duplicate connections are allowed and it will be treated as a new player instead. This issue will be fixed when #488 is merged.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@LPLafontaineB LPLafontaineB added 1-Needs Review PR needs attention from the assignee and reviewers GDC-cherrypick 0-workaround labels Mar 7, 2022
@SamuelBellomo
Copy link
Contributor

I see two labels, cherrypick and workaround, which is it? They are mutually exclusive. Knowing this relies on a workaround and that there's an MTT bug we're waiting to fix, this should be just workaround and not cherrypicked to develop correct?

@LPLafontaineB
Copy link
Contributor Author

I see two labels, cherrypick and workaround, which is it? They are mutually exclusive. Knowing this relies on a workaround and that there's an MTT bug we're waiting to fix, this should be just workaround and not cherrypicked to develop correct?

Yes my bad, it should only have had the workaround label

SamuelBellomo
SamuelBellomo reviewed Mar 8, 2022
View reviewed changes
@LPLafontaineB LPLafontaineB marked this pull request as ready for review March 8, 2022 17:15
SamuelBellomo
SamuelBellomo previously approved these changes Mar 8, 2022
View reviewed changes
LukeStampfli
LukeStampfli previously approved these changes Mar 8, 2022
View reviewed changes
@LPLafontaineB LPLafontaineB dismissed stale reviews from LukeStampfli and SamuelBellomo via 677239c March 8, 2022 19:36
SamuelBellomo
SamuelBellomo previously approved these changes Mar 8, 2022
View reviewed changes
LukeStampfli
LukeStampfli previously approved these changes Mar 8, 2022
View reviewed changes
@SamuelBellomo SamuelBellomo added 3-Good to Merge and removed 1-Needs Review PR needs attention from the assignee and reviewers labels Mar 8, 2022
@LPLafontaineB LPLafontaineB dismissed stale reviews from LukeStampfli and SamuelBellomo via 7f7ae4d March 8, 2022 20:21
@LPLafontaineB LPLafontaineB enabled auto-merge (squash) March 8, 2022 21:20
@LPLafontaineB LPLafontaineB added 1-Needs Review PR needs attention from the assignee and reviewers and removed 3-Good to Merge labels Mar 8, 2022
@LPLafontaineB LPLafontaineB merged commit e494c15 into release/GDC2022 Mar 8, 2022
@LPLafontaineB LPLafontaineB deleted the feature/auto-reconnect branch March 8, 2022 22:57
SamuelBellomo added a commit that referenced this pull request Mar 10, 2022
@SamuelBellomo
Merge branch 'release/GDC2022' into sam/fix/adding-transport-RTT-to-U…
761c691 
…I-stats

* release/GDC2022:
  added IP button to main menu (#535)
  fix: postgame menu button (#523)
  chore: adding leak detect (#492)
  Cherry pick: lobby blue banner fix, mainmenu.unity scene merge conflict resolved with unity merge tool (#534)
  Filled in some holes in the floor (#527) (#533)
  fix: populate lobby room name if creation room input field is empty (#522)
  lobby fix: Adding instructions when we get a service error on conflicting joins (#511)
  feat: auto reconnect (#513)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SamuelBellomo SamuelBellomo SamuelBellomo approved these changes

@fernando-cortez fernando-cortez fernando-cortez approved these changes

@LukeStampfli LukeStampfli Awaiting requested review from LukeStampfli

Assignees

@SamuelBellomo SamuelBellomo

@fernando-cortez fernando-cortez

Labels
0-workaround 1-Needs Review PR needs attention from the assignee and reviewers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LPLafontaineB @SamuelBellomo @LukeStampfli @fernando-cortez