Skip to content

feat: simpler quitting flow #579

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

Merged
merged 7 commits into from
Apr 5, 2022
Merged

Conversation

LPLafontaineB
Copy link
Contributor

@LPLafontaineB LPLafontaineB commented Mar 23, 2022

Description (*)

This PR implements the same feature as #537 did, but without checking scene names in code. To do this, the SettingsPanelCanvas is removed from the DDOL scene, and added to every scene. An enum field in QuitPanel decides if we want to return to the menu, or quit the application, so each scene can specify the expected behavior. In this case, in every scene except the MainMenu, we return to the menu when using the quit button, and in the MainMenu scene, we quit the application.

This PR also extracts the DI scope initialization and injection from ClientMainMenuState and ClientCharSelectState to their parent GameStateBehavior. Now every state has a specific scope and a serialized fields for GameObjects in which to inject.

Related Pull Requests

Issue Number(s) (*)

Fixes issue(s): MTT-2919

Manual testing scenarios

  1. Launch the game and use the quit button in the different scenes
  2. See that it brings you back to the main menu in all scenes except the MainMenu scene, where it quits the application

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)

* Now specifies that it should only quit the game on the prefab inside the MainMenu scene, otherwise always calls LeaveSession()
@LPLafontaineB LPLafontaineB added the 1-Needs Review PR needs attention from the assignee and reviewers label Mar 23, 2022
pdeschain
pdeschain previously approved these changes Mar 29, 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 Mar 29, 2022
@fernando-cortez fernando-cortez added 2-Reviewed with Comments PR requires owner's attention and removed 2-One More Review One review in, one to go labels Mar 30, 2022
@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 Mar 30, 2022
SamuelBellomo
SamuelBellomo previously approved these changes Mar 30, 2022
Copy link
Contributor

@SamuelBellomo SamuelBellomo left a comment

Choose a reason for hiding this comment

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

good to go once we've solved the other conversation

@LPLafontaineB LPLafontaineB added the 1-Needs Review PR needs attention from the assignee and reviewers label Apr 4, 2022
Copy link
Contributor

@pdeschain pdeschain left a comment

Choose a reason for hiding this comment

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

👍

@pdeschain pdeschain added the 2-One More Review One review in, one to go label Apr 4, 2022
@LPLafontaineB LPLafontaineB mentioned this pull request Apr 5, 2022
4 tasks
@LPLafontaineB LPLafontaineB removed the 1-Needs Review PR needs attention from the assignee and reviewers label Apr 5, 2022
@fernando-cortez fernando-cortez added 3-Good to Merge and removed 2-One More Review One review in, one to go labels Apr 5, 2022
@LPLafontaineB LPLafontaineB merged commit 821c8a3 into develop Apr 5, 2022
@LPLafontaineB LPLafontaineB deleted the feature/simpler-quitting-flow branch April 5, 2022 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants