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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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
Copy link

sonarqubecloud bot commented Feb 6, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
5.0% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@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

Detected 70 dependencies

Third-party software list

This page lists the third-party software dependencies used in CollapseLauncher

Dependency Version Licenses
CommunityToolkit.Common 8.4.0 MIT
CommunityToolkit.Mvvm 8.4.0 MIT
CommunityToolkit.WinUI.Animations 8.2.250129-preview2 MIT
CommunityToolkit.WinUI.Behaviors 8.2.250129-preview2 MIT
CommunityToolkit.WinUI.Controls.Primitives 8.2.250129-preview2 MIT
CommunityToolkit.WinUI.Controls.Sizers 8.2.250129-preview2 MIT
CommunityToolkit.WinUI.Converters 8.2.250129-preview2 MIT
CommunityToolkit.WinUI.Extensions 8.2.250129-preview2 MIT
CommunityToolkit.WinUI.Helpers 8.2.250129-preview2 MIT
CommunityToolkit.WinUI.Media 8.2.250129-preview2 MIT
CommunityToolkit.WinUI.Triggers 8.2.250129-preview2 MIT
Costura.Fody 6.0.0 MIT
DependencyPropertyGenerator 1.5.0 MIT
DotNet.ReproducibleBuilds 1.2.25 MIT
EventGenerator.Generator 0.13.1 MIT
Fody 6.9.1 MIT
GitInfo 3.5.0 MIT
Google.Protobuf.Tools 3.29.3 PROTOBUF
Google.Protobuf 3.29.3 BSD-3-Clause
Hi3Helper.ZstdNet 1.6.3 BSD-3-Clause
HtmlAgilityPack 1.11.72 MIT
Markdig.Signed 0.40.0 BSD-2-Clause
Microsoft.CSharp 4.7.0 MIT
Microsoft.DotNet.ILCompiler 9.0.1 MIT
Microsoft.Extensions.DependencyInjection.Abstractions 9.0.1 MIT
Microsoft.Extensions.DependencyInjection 9.0.1 MIT
Microsoft.Extensions.Logging.Abstractions 9.0.1 MIT
Microsoft.Extensions.Logging 9.0.1 MIT
Microsoft.Extensions.Options 9.0.1 MIT
Microsoft.Extensions.Primitives 9.0.1 MIT
Microsoft.Graphics.Win2D 1.3.2 MS-ASP-NET-WEB-OPTIMIZATION
Microsoft.NET.ILLink.Tasks 9.0.1 MIT
Microsoft.NETCore.Targets 6.0.0-preview.4.21253.7 MIT
Microsoft.Web.WebView2 1.0.2957.106 BSD-3-Clause
BSD-MYLEX
Microsoft.Win32.SystemEvents 9.0.1 MIT
Microsoft.Windows.CsWin32 0.3.162 Apache-2.0
Microsoft.Windows.CsWinRT 2.2.0 MIT
Microsoft.Windows.SDK.BuildTools 10.0.22621.3233 PROPRIETARY-LICENSE
Microsoft.Windows.SDK.Win32Docs 0.1.42-alpha PROPRIETARY-LICENSE
Microsoft.Windows.SDK.Win32Metadata 61.0.15-preview MIT
Microsoft.Windows.WDK.Win32Metadata 0.12.8-experimental MIT
Microsoft.WindowsAppSDK 1.6.250108002 MIT
MS-DXSDK-D3DX-9.29.952.3
Microsoft.Xaml.Behaviors.WinUI.Managed 3.0.0 MIT
MinVer 6.0.0 Apache-2.0
NuGet.Versioning 6.12.1 Apache-2.0
PhotoSauce.MagicScaler 0.15.0 MIT
PhotoSauce.NativeCodecs.Libwebp 1.4.0-preview1 MIT
Roman-Numerals 2.0.1 MIT
Sentry 5.1.0 MIT
SharpCompress 0.39.0 MIT
SharpHDiffPatch.Core 2.2.8 MIT
System.Buffers 4.6.0 MIT
System.Drawing.Common 9.0.1 MIT
System.IO.Hashing 9.0.1 MIT
System.Net.Http 4.3.4 MIT
System.Security.AccessControl 6.0.1 MIT
System.Security.Cryptography.Algorithms 4.3.0 MIT
System.Security.Cryptography.Encoding 4.3.0 MIT
System.Security.Cryptography.Primitives 4.3.0 MIT
System.Security.Cryptography.ProtectedData 9.0.1 MIT
System.Security.Cryptography.X509Certificates 4.3.0 MIT
System.Text.Encoding.CodePages 9.0.1 MIT
System.Text.Json 9.0.1 MIT
System.Text.RegularExpressions 4.3.1 MIT
System.Threading.Tasks.Extensions 4.5.4 MIT
TaskScheduler 2.11.0 MIT
ThisAssembly.Constants 2.0.6 MIT
Velopack 0.0.1053 MIT
ZstdSharp.Port 0.8.4 MIT
runtime.win-x64.Microsoft.DotNet.ILCompiler 9.0.1 MIT
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!

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