Skip to content

fix: leaving lobby when disconnecting #515

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 21 commits into from
Mar 14, 2022
Merged

fix: leaving lobby when disconnecting #515

merged 21 commits into from
Mar 14, 2022

Conversation

LPLafontaineB
Copy link
Contributor

@LPLafontaineB LPLafontaineB commented Mar 7, 2022 •
edited
Loading

Description (*)

With this PR, the host now removes disconnecting clients from their lobby and deletes it when they themselves leave. Meanwhile, the clients also leave the lobby when disconnecting, and after each failed reconnection attempt. This PR also fixes an infinite loop issue that appeared in certain circumstances with RateLimitCooldown by using two operation queues. This fix allow us to put the number of reconnection attempts back at 2.

Related Pull Requests

Issue Number(s) (*)

Fixes issue(s): MTT-2701 and works around the issues caused by MTT-2667. Also fixes MTT-2794

Manual testing scenarios

  1. Start a game as a host
  2. Join with a client
  3. Pause the client until the host times it out
  4. Unpause the client and see that it is properly reconnected

This also works when pausing the host to cause a timeout, or when quitting the game as the client or host, or disconnecting via the in-game quit button.

Also works when connection gets denied during approval check (i.e. full game or duplicate connection).

Questions or comments

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 2-Easy This PR is trivial and can be reviewed quickly 1-Needs Review PR needs attention from the assignee and reviewers GDC-cherrypick 0-workaround labels Mar 7, 2022
@SamuelBellomo
Copy link
Contributor

same here. cherrypick vs workaround labels

fernando-cortez
fernando-cortez previously approved these changes Mar 8, 2022
View reviewed changes
@fernando-cortez fernando-cortez added 2-One More Review One review in, one to go and removed 1-Needs Review PR needs attention from the assignee and reviewers labels Mar 8, 2022
LukeStampfli
LukeStampfli previously approved these changes Mar 8, 2022
View reviewed changes
@SamuelBellomo SamuelBellomo added 2-Reviewed with Comments PR requires owner's attention and removed 2-One More Review One review in, one to go labels Mar 8, 2022
@LukeStampfli LukeStampfli mentioned this pull request Mar 9, 2022
Merged
4 tasks
@LPLafontaineB LPLafontaineB marked this pull request as draft March 11, 2022 16:38
@LPLafontaineB
Adding leaving lobby then trying to re-join in reconnect coroutine
fa7a9cc
@LPLafontaineB
Squashed commit of the following:
11b6578 
commit a44aeaa
Merge: d66f779 7c5f64e
Author: LPLafontaineB <[email protected]>
Date:   Fri Mar 11 10:30:44 2022 -0500

    Merge branch 'develop' into feature/replace-guid-with-auth-playerid

commit d66f779
Author: LPLafontaineB <[email protected]>
Date:   Fri Mar 11 10:27:22 2022 -0500

    Removed OnlineMode responsability from GameNetPortal

commit 346bb7f
Author: LPLafontaineB <[email protected]>
Date:   Fri Mar 11 09:30:11 2022 -0500

    Simplified ProfileManager property

commit 2973861
Author: LPLafontaineB <[email protected]>
Date:   Fri Mar 11 09:25:09 2022 -0500

    reverted unneeded change

commit 788eb23
Merge: d119ffb ee838ef
Author: Unity Netcode CI <[email protected]>
Date:   Thu Mar 10 18:46:55 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit d119ffb
Author: LPLafontaineB <[email protected]>
Date:   Thu Mar 10 09:56:47 2022 -0500

    cleanup using directives

commit d164192
Author: LPLafontaineB <[email protected]>
Date:   Thu Mar 10 09:53:19 2022 -0500

    refactored GetPlayerId to use AuthService's playerId when signed in on IP connections as well

commit f479872
Author: LPLafontaineB <[email protected]>
Date:   Thu Mar 10 09:45:24 2022 -0500

    Exctracted GetPlayerId to GameNetPortal

commit 9008d77
Merge: 41790b7 4063c42
Author: LPLafontaineB <[email protected]>
Date:   Wed Mar 9 17:47:47 2022 -0500

    Merge branch 'feature/replace-guid-with-auth-playerid' of https://github.com/Unity-Technologies/com.unity.multiplayer.samples.coop into feature/replace-guid-with-auth-playerid

commit 41790b7
Author: LPLafontaineB <[email protected]>
Date:   Wed Mar 9 17:47:30 2022 -0500

    Adding back GUID with IP connections

    When not using UGS, player id is now defined as GUID + profile string

commit 4063c42
Merge: 6f27732 fd74095
Author: Unity Netcode CI <[email protected]>
Date:   Wed Mar 9 22:43:39 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 0d34e9c
Author: LPLafontaineB <[email protected]>
Date:   Wed Mar 9 17:08:01 2022 -0500

    Revert "Removed guid from ClientPrefs"

    This reverts commit 3d9ebfa.

commit 6f27732
Merge: f13fd7a 974b379
Author: Unity Netcode CI <[email protected]>
Date:   Wed Mar 9 21:34:22 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit f13fd7a
Merge: 40d2f89 9978ce0
Author: Unity Netcode CI <[email protected]>
Date:   Wed Mar 9 21:33:51 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 40d2f89
Merge: 8975d19 f346969
Author: Unity Netcode CI <[email protected]>
Date:   Wed Mar 9 21:14:47 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 8975d19
Merge: 1babc3f 31736f3
Author: Unity Netcode CI <[email protected]>
Date:   Wed Mar 9 20:01:35 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 1babc3f
Merge: 5296213 59434fd
Author: Unity Netcode CI <[email protected]>
Date:   Wed Mar 9 19:02:29 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 5296213
Merge: 2d83fcf 4818892
Author: Unity Netcode CI <[email protected]>
Date:   Wed Mar 9 15:56:06 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 2d83fcf
Merge: b974e90 cee7c1a
Author: Unity Netcode CI <[email protected]>
Date:   Wed Mar 9 15:53:54 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit b974e90
Merge: 77b939f d1c55fa
Author: Unity Netcode CI <[email protected]>
Date:   Tue Mar 8 22:47:44 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 77b939f
Merge: 0d874b5 b83d7b4
Author: Unity Netcode CI <[email protected]>
Date:   Tue Mar 8 19:53:32 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 0d874b5
Merge: 020cce1 be3c9c6
Author: Unity Netcode CI <[email protected]>
Date:   Tue Mar 8 18:48:01 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 020cce1
Merge: e6bb5d5 6b3fe09
Author: Unity Netcode CI <[email protected]>
Date:   Tue Mar 8 18:47:35 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit e6bb5d5
Merge: 47b02ba 891bcd8
Author: Unity Netcode CI <[email protected]>
Date:   Tue Mar 8 00:45:38 2022 +0000

    Merge develop into feature/replace-guid-with-auth-playerid

commit 47b02ba
Author: LPLafontaineB <[email protected]>
Date:   Thu Feb 24 15:28:48 2022 -0500

    cleaning up

commit 62d5ea7
Author: LPLafontaineB <[email protected]>
Date:   Thu Feb 24 15:24:44 2022 -0500

    Made ClientPrefs class static

commit 3d9ebfa
Author: LPLafontaineB <[email protected]>
Date:   Thu Feb 24 15:23:54 2022 -0500

    Removed guid from ClientPrefs

commit 5958c9b
Author: LPLafontaineB <[email protected]>
Date:   Thu Feb 24 15:21:13 2022 -0500

    Removed AddHostData method

commit 4ce6534
Author: LPLafontaineB <[email protected]>
Date:   Thu Feb 24 15:15:38 2022 -0500

    Removed code handling duplicate connections in debug builds

commit 97c2ee1
Author: LPLafontaineB <[email protected]>
Date:   Thu Feb 24 15:14:47 2022 -0500

    Replaced custom guid with playerid for session management

(cherry picked from commit 28e5fc5240050917f81f62107dccbc9b7ae2e856)
@LPLafontaineB
Adding injection to ServerGameNetPortal
4c728e1
@LPLafontaineB
replaced dumb while with less dumb yield waitwhile
eb6cf85
@LPLafontaineB
Removed unnecessary callback registering
7140994
@LPLafontaineB
Fixed infinite loop issue
9d6dd18
@LPLafontaineB
Made sure client leaves lobby by themselves when disconnecting as wel…
0c3d0c8 
…l, and begin tracking after successful reconnect
Base automatically changed from feature/cherrypick/player-id-in-session-manager to release/GDC2022 March 14, 2022 14:28
@LPLafontaineB LPLafontaineB requested a review from pdeschain March 14, 2022 14:28
SamuelBellomo
SamuelBellomo previously approved these changes Mar 14, 2022
View reviewed changes
pdeschain
pdeschain requested changes Mar 14, 2022
View reviewed changes
@SamuelBellomo SamuelBellomo added the 0-URGENT Blocker for a release and needs to be merged ASAP label Mar 14, 2022
@LPLafontaineB LPLafontaineB requested a review from pdeschain March 14, 2022 19:01
@LPLafontaineB LPLafontaineB enabled auto-merge (squash) March 14, 2022 19:21
@LPLafontaineB LPLafontaineB merged commit 93ff764 into release/GDC2022 Mar 14, 2022
@LPLafontaineB LPLafontaineB deleted the fix/leaving-lobby-when-disconnecting branch March 14, 2022 19:21
SamuelBellomo added a commit that referenced this pull request Mar 14, 2022
@SamuelBellomo
Merge branch 'release/GDC2022' into sam/fix/workaround-loading-screen…
e44d423 
…-not-going-away

* release/GDC2022:
  fix: leaving lobby when disconnecting (#515)
  better error messages and lobby fix (#550)
  feat: authentication failure message popups will tell the user to try direct ip connection option (#547)
  feat: replace guid with auth playerid for session management (#488) (#548)

# Conflicts:
#	Assets/BossRoom/Scripts/Shared/Net/ConnectionManagement/ClientGameNetPortal.cs
@LPLafontaineB LPLafontaineB mentioned this pull request Mar 14, 2022
Merged
4 tasks
LPLafontaineB added a commit that referenced this pull request Mar 15, 2022
@LPLafontaineB @SamuelBellomo
fix: leaving lobby when disconnecting (#515)
e598a64 
* Host now removes clients from lobby when they disconnect
* Clients leave the lobby when disconnecting
* Host deletes the lobby when disconnecting

(cherry picked from commit 93ff764)
Co-Authored-By: Sam Bellomo <[email protected]>
@LPLafontaineB LPLafontaineB mentioned this pull request Mar 15, 2022
Merged
4 tasks
SamuelBellomo added a commit that referenced this pull request Mar 15, 2022
@LPLafontaineB @SamuelBellomo
fix: leaving lobby when disconnecting (#515) (#553)
9930593 
* Host now removes clients from lobby when they disconnect
* Clients leave the lobby when disconnecting
* Host deletes the lobby when disconnecting

(cherry picked from commit 93ff764)
Co-Authored-By: Sam Bellomo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pdeschain pdeschain pdeschain approved these changes

@SamuelBellomo SamuelBellomo SamuelBellomo approved these changes

@fernando-cortez fernando-cortez Awaiting requested review from fernando-cortez

@LukeStampfli LukeStampfli Awaiting requested review from LukeStampfli

Assignees

@pdeschain pdeschain

@SamuelBellomo SamuelBellomo

Labels
0-URGENT Blocker for a release and needs to be merged ASAP 1-Needs Review PR needs attention from the assignee and reviewers GDC-cherrypick
Projects
None yet
Milestone
No milestone
Development

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