Skip to content

refactor: replacing GameNetPortal with state machine [MTT-1742] [MTT-3501] [MTT-3502] #666

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 87 commits into from
Jul 11, 2022
Merged

refactor: replacing GameNetPortal with state machine [MTT-1742] [MTT-3501] [MTT-3502] #666

merged 87 commits into from
Jul 11, 2022

Conversation

LPLafontaineB
Copy link
Contributor

@LPLafontaineB LPLafontaineB commented May 30, 2022 •
edited
Loading

Description

This PR aims to simplify and clarify the connection flow in Boss Room. It replaces GameNetPortal (as well as ClientGameNetPortal and ServerGameNetPortal) with a state machine (ConnectionManager). This state machine receives inputs from other parts of our code, and registers to callbacks from NGO. It then redirects these inputs and callbacks to the current state, which then handles it accordingly.

ConnectionManager is responsible for initializing the different states and redirecting inputs and callbacks to them. It also handles our custom messaging for giving a disconnect reason to clients.
The connection logic itself is the responsibility of the different states.

Issue Number(s)

MTT-1742, MTT-3502 & MTT-3501

Contribution checklist

  • Tests have been added for boss room and/or utilities pack
  • Release notes have been added to the project changelog file and/or package changelog file
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • JIRA ticket ID is in the PR title or at least one commit message
  • Include the ticket ID number within the body message of the PR to create a hyperlink

@LPLafontaineB LPLafontaineB added the 5-Design Review Draft to review design ideas before committing to future work label May 30, 2022
@LPLafontaineB
Adding ConnectionManager skeleton
decf8bf 
(cherry picked from commit 4bbe421)
@LPLafontaineB
initial state machine implementation
f774161 
(cherry picked from commit e30c8f9)
@LPLafontaineB
simplifying initial spawn logic
40f7167 
* Removed use of clientSceneMap
* Replaced with OnLoadEventCompleted and OnSynchronizeComplete callbacks

(cherry picked from commit 80fe462)
@LPLafontaineB
Adding disconnect reason handling client-side
0ab59b2 
(cherry picked from commit d094706)
@LPLafontaineB
using state machine's monobehaviour for coroutines
0382392 
(cherry picked from commit 444f436)
@LPLafontaineB
replacing abstract methods with empty virtual ones
2d3b0bb 
(cherry picked from commit 96575b7)
@LPLafontaineB
implementing reconnecting state
314f670 
(cherry picked from commit 0a3680d)
@LPLafontaineB
simplifying reconnect state
385fff2 
(cherry picked from commit 2c83355)
@LPLafontaineB
removing implementation without state machine
5263998 
(cherry picked from commit f49977d)
@LPLafontaineB
cleanup
fb212ec 
(cherry picked from commit 991bd90)
@LPLafontaineB
cleanup
f923235 
(cherry picked from commit 67e2fbe)
@LPLafontaineB
removing duplicated call
bedb352 
(cherry picked from commit 47444a1)
@LPLafontaineB
fix: making sure reconnecting state goes to offline state once attemp…
6237f47 
…ts are exhausted

(cherry picked from commit 6a06365)
@LPLafontaineB
fixing user-requested shutdown
523e077 
(cherry picked from commit 2accfe4)
@LPLafontaineB
Used inheritance to simplify Reconnecting state
ac49f7c 
* Reconnecting now inherits from Offline
* Also extracted GetPlayerId() to Offline state

(cherry picked from commit 4f1c105)
@LPLafontaineB
removing unneeded using directives
c7eaf27 
(cherry picked from commit 429ee19)
@LPLafontaineB
removing unneeded method
5a808c0 
(cherry picked from commit b23b99f)
@LPLafontaineB
small bug fixes
0461c0b 
* shutting down host now changes state to offline
* fixed return value of JoinRelayServerAsync

(cherry picked from commit 593da8a)
@LPLafontaineB
reorganized DI so that only states get injected in
6b3ac0f 
(cherry picked from commit 0e07a94)
@LPLafontaineB
cleaning up
fe1880e 
(cherry picked from commit fd9e635)
@LPLafontaineB
removing GameNetPortal
109833d 
(cherry picked from commit 532bd10)
@LPLafontaineB
removing clientScene from connection payload
64f61d1 
(cherry picked from commit 9d3c1b7)
@LPLafontaineB
cleaning up old implementation
2516936
@LPLafontaineB LPLafontaineB force-pushed the lplb/gamenetportal-refactor branch from 9d3c1b7 to 2516936 Compare June 2, 2022 20:00
@LPLafontaineB LPLafontaineB changed the base branch from pdeschain/folder-assembly-refactor to develop June 2, 2022 20:02
fernando-cortez
fernando-cortez previously approved these changes Jun 2, 2022
View reviewed changes
Copy link
Collaborator

@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.

I really like this! A suggestion would be to include a little blurb (either in each state) or somewhere central where you could detail what state each state can transition to. Even without that, it was real simple to navigate. 🚀

@LPLafontaineB LPLafontaineB 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 Jul 6, 2022
NoelStephensUnity
NoelStephensUnity reviewed Jul 6, 2022
View reviewed changes
Copy link
Contributor

@NoelStephensUnity NoelStephensUnity left a comment

Choose a reason for hiding this comment

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

I like the update and love the new state machine approach!
From what I can tell, it looks like a bunch of really good improvements.

@LPLafontaineB LPLafontaineB changed the title refactor: replacing GameNetPortal with state machine [MTT-1742] refactor: replacing GameNetPortal with state machine [MTT-1742] [MTT-3501] [MTT-3502] Jul 11, 2022
@LPLafontaineB LPLafontaineB merged commit fd60daa into develop Jul 11, 2022
@LPLafontaineB LPLafontaineB deleted the lplb/gamenetportal-refactor branch July 11, 2022 15:15
SamuelBellomo added a commit that referenced this pull request Jul 12, 2022
@SamuelBellomo
Merge branch 'develop' into sam/feat/dedicated-server-support
f4fa6b1 
* develop:
  refactor: replacing GameNetPortal with state machine [MTT-1742] [MTT-3501] [MTT-3502] (#666)
  chore: update NGO, tools, authentication and relay packages (#690)
  feat: replacing discope with vcontainer [MTT-3675] (PR recreated) (#679)
  updated changelogs with fixes made to the release changelog (#688)

# Conflicts:
#	Assets/Scenes/CharSelect.unity
#	Assets/Scenes/PostGame.unity
#	Assets/Scenes/Startup.unity
#	Assets/Scripts/ApplicationLifecycle/ApplicationController.cs
#	Assets/Scripts/Gameplay/ConnectionManagement/ClientGameNetPortal.cs
#	Assets/Scripts/Gameplay/ConnectionManagement/GameNetPortal.cs
#	Assets/Scripts/Gameplay/ConnectionManagement/ServerGameNetPortal.cs
#	Assets/Scripts/Gameplay/DebugCheats/Unity.BossRoom.DebugCheats.asmdef
#	Assets/Scripts/Gameplay/GameState/ClientBossRoomState.cs
#	Assets/Scripts/Gameplay/GameState/ClientCharSelectState.cs
#	Assets/Scripts/Gameplay/GameState/ClientPostGameState.cs
#	Assets/Scripts/Gameplay/GameState/GameStateBehaviour.cs
#	Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs
#	Assets/Scripts/Gameplay/GameState/ServerCharSelectState.cs
#	Assets/Scripts/Gameplay/GameState/ServerPostGameState.cs
#	Assets/Scripts/Gameplay/GameplayObjects/ClientDoorVisualization.cs
#	Assets/Scripts/Gameplay/UI/IPUIMediator.cs
#	Assets/Scripts/Gameplay/UI/PostGameUI.cs
#	Assets/Scripts/Infrastructure/DIScope.cs
#	Assets/Tests/Runtime/Unity.BossRoom.Tests.Runtime.asmdef
@LPLafontaineB LPLafontaineB mentioned this pull request Aug 8, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NoelStephensUnity NoelStephensUnity NoelStephensUnity left review comments

@SamuelBellomo SamuelBellomo SamuelBellomo approved these changes

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

@LukeStampfli LukeStampfli Awaiting requested review from LukeStampfli

@pdeschain pdeschain Awaiting requested review from pdeschain

Assignees
No one assigned
Labels
2-One More Review One review in, one to go
Projects
None yet
Milestone
No milestone
Development

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