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

Prevent closing app while doing critical process #681

Merged
merged 3 commits into from
Feb 6, 2025
Merged

Conversation

bagusnl
Copy link
Member

@bagusnl bagusnl commented Feb 6, 2025

Main Goal

Show dialog confirming user before exiting app while critical process is happening
image

Close #671

Currently implemented in:

  • Game Install
  • Game Update
  • Cache Update
  • Game Repair

PR Status :

  • Overall Status : Done
  • Commits : Done
  • Synced to base (Collapse:main) : Yes
  • Build status : OK
  • Crashing : No
  • Bug found caused by PR : 0

Sidenote:

  • Bug found when making this PR, unrelated to the PR itself
    When closing app while some UI progress is happening, there is a chance that it throws bunch of errors on XAML thread before it exit
    image

image

Templates

Changelog Prefixes
  **[New]**
  **[Imp]**
  **[Fix]**
  **[Loc]**
  **[Doc]**

@bagusnl bagusnl added the Area: UI/UX Issue labeled for User Interface/eXperience related issue label Feb 6, 2025
@bagusnl bagusnl requested a review from a team February 6, 2025 04:36
@bagusnl bagusnl self-assigned this Feb 6, 2025
@bagusnl
Copy link
Member Author

bagusnl commented Feb 6, 2025

Need consultation about this @neon-nyan

image

is it safe to return Task for the Close method instead, making it detached?

Copy link

github-actions bot commented Feb 6, 2025

Qodana for .NET

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

@neon-nyan
Copy link
Member

Need consultation about this @neon-nyan

image

is it safe to return Task for the Close method instead, making it detached?

IMO, it should be safe to use Task for the CloseApp() method and running it detached.
The thing to consider is that, since Task should always be awaited and by running it detached, it will give a warning.
To get rid of that, you can use something like this everytime you call the method:

_ = CloseApp();

@bagusnl
Copy link
Member Author

bagusnl commented Feb 6, 2025

Need consultation about this @neon-nyan
image
is it safe to return Task for the Close method instead, making it detached?

IMO, it should be safe to use Task for the CloseApp() method and running it detached. The thing to consider is that, since Task should always be awaited and by running it detached, it will give a warning. To get rid of that, you can use something like this everytime you call the method:

_ = CloseApp();

dune tank!

Copy link
Member

@neon-nyan neon-nyan left a comment

Choose a reason for hiding this comment

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

Just one review for the overall changes. Others are looking good to me

CollapseLauncher/XAMLs/MainApp/MainWindow.xaml.cs Outdated Show resolved Hide resolved
@bagusnl bagusnl requested a review from neon-nyan February 6, 2025 08:25
@bagusnl
Copy link
Member Author

bagusnl commented Feb 6, 2025

Ready to merge!

Copy link

sonarqubecloud bot commented Feb 6, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
4.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@bagusnl bagusnl merged commit 0d37d44 into main Feb 6, 2025
6 of 7 checks passed
@bagusnl bagusnl deleted the ensure-exit branch February 6, 2025 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: UI/UX Issue labeled for User Interface/eXperience related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Prevent user to close the app when patching
2 participants