Skip to content
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.

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

feat: removing bridge classes for game states MTT-4204 #697

Merged
merged 18 commits into from
Aug 17, 2022

Conversation

pdeschain
Copy link
Contributor

@pdeschain pdeschain commented Jul 25, 2022

Description

Removed unnecessary empty Client*** classes that represented game state. Also cleaned up the naming of remaining Server*** state classes.

Replaced a pattern where we use a DontDestroyOnLoad gameobject to pass around networked win/loose data between scenes and replaced it with a static instance which is preserved by BossRoomState and cleaned up by PostGameState.

Issue Number(s)

MTT-4204

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

And renamed ServerBossRoomState to BossRoomState.
Added lifetime scopes to all the subscenes of BossRoom scene. These autoinject gameobjects that live under the "static network objects" groupings.
replaced with BufferedNetworkMessageChannel (an addition in this entry) - this message channel is identical to NetworkMessageChannel, but it inherits from a buffered message channel instead. This is used as a way to pass data that syncs from server to clients between different scenes without the need for SO variables, which are hard to understand.
…meState

also added auto-injected objects to the PostGameState-bearing game object.
…tiplayer.samples.coop into pdeschain/removing-bridge-classes
@pdeschain pdeschain marked this pull request as ready for review July 25, 2022 17:26
@pdeschain pdeschain requested a review from SamuelBellomo as a code owner July 25, 2022 17:26
@pdeschain pdeschain requested review from LPLafontaineB, fernando-cortez and SamuelBellomo and removed request for SamuelBellomo July 25, 2022 17:26
@pdeschain pdeschain added the 1-Needs Review PR needs attention from the assignee and reviewers label Jul 25, 2022
@pdeschain pdeschain changed the title feat: removing bridge classes for game states MTT-4014 feat: removing bridge classes for game states MTT-4204 Jul 25, 2022
Replaced it's usage with WinState with a NetworkWinState that's preserved by BossRoomState to survive onto the next scene, PostGameState, which then destroys that preserved gameobject/NetworkWinState when we exit PostGameState to whatever next state.
Assets/Scripts/Gameplay/GameState/BossRoomState.cs Outdated Show resolved Hide resolved
Assets/Scripts/Gameplay/UI/PostGameUI.cs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@pdeschain pdeschain added the 2-Easy This PR is trivial and can be reviewed quickly label Aug 11, 2022
LPLafontaineB
LPLafontaineB previously approved these changes Aug 12, 2022
@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 Aug 12, 2022
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.

Just nitpicks. Much cleaner.

Assets/Scripts/Gameplay/GameState/GameStateBehaviour.cs Outdated Show resolved Hide resolved
Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs Outdated Show resolved Hide resolved
pdeschain and others added 3 commits August 16, 2022 17:06
CHANGELOG.md Outdated Show resolved Hide resolved
SamuelBellomo
SamuelBellomo previously approved these changes Aug 16, 2022
Co-authored-by: Sam Bellomo <71790295+SamuelBellomo@users.noreply.github.com>
@pdeschain pdeschain enabled auto-merge August 17, 2022 15:06
@pdeschain pdeschain added 3-Good to Merge and removed 2-One More Review One review in, one to go labels Aug 17, 2022
@pdeschain pdeschain disabled auto-merge August 17, 2022 17:03
@pdeschain pdeschain merged commit c7f099f into develop Aug 17, 2022
@pdeschain pdeschain deleted the pdeschain/removing-bridge-classes branch August 17, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2-Easy This PR is trivial and can be reviewed quickly 3-Good to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants