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

Allow GUI users to delete registry lockfiles #3829

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Apr 21, 2023

Motivation

ConsoleUI can do one thing that GUI can't:

image

image

It would be helpful if GUI users also had the option to delete the lockfile after an explanatory warning.

Problems

  • ConsoleUI's lockfile message confusingly says "instance" twice with different meanings
  • The GUI window take a long time to appear at startup (for KSP1); as a rule, the app should be visible as early as possible so users can see it's running, and they should be able to tell what it's doing if it's busy.
  • Main's constructor was calling Application.Run, which is not good coding style; a constructor should just create the object (even under RAII), not trigger side effects such as running the entire application

Cause

  • GUI calls Registry.Instance before the window appears, which makes it load registry.json which is around 30 MiB nowadays and causes a substantial delay

Changes

  • ConsoleUI's lockfile message moved to Core.RegistryInUseKraken.ToString to be shared, and the English text is edited to be clearer (one "instance" is now "CKAN process" and the other is "game folder")
  • Now GUI's lockfile error popup is replaced with a yes/no dialog with custom button captions (translations copied from ConsoleUI where available):
    image
    If you click Cancel, the manage game instances popup will appear so you can pick a different game instance (if you have any others). If you click Force, it'll try to delete the lockfile and proceed with the chosen instance.
  • Now the GUI window appears before registry.json is loaded, and a progress bar is shown while it loads
  • The sometimes-slow GameInstance.Scan call is moved from startup to UpdateModsList with its own log message to keep the GUI responsive while it happens (since this was previously between the two progress bar sections)
  • We now pass the parent form to all the calls of ShowDialog that I was able to find, which is good practice in case we want to center the child dialogs on the parents
  • Main.manager is now private and external users are switched to using the public Main.Manager instead, and various other members of Main are also made private or readonly
  • All potentially slow actions are removed from Main's constructor and from OnLoad and moved to OnShown, where they now occur within a Wait.StartWaiting block that shows a progress bar. The roles of these functions are now:
    • Program.Main_: Instantiate the main form, hide the console window, and run the application
    • Constructor: Just set up the basic core properties so Main can be loaded, such as the GUI events
    • OnLoad: Get the GUI configuration (without locking the instance!) so we can get the saved window geometry/layout before the window appears
    • OnShown: Load the registry once the window appears and prompt the user for various things
  • GUI configuration is now only saved at exit and when we change instances rather than every time we change any setting
  • Main.tabController and Main.Instance are now marked with the [Obsolete] attribute to generate code style warnings, and the Main*.cs file now all have #pragma warning disable 0618 to suppress the warning when used by the same class

This will generally make GUI better behaved, since it will appear very soon after launch and tell the user what it's doing. And if the lockfile exists, users will have the option to delete it.

Fixes #3558.

Deferred to later

We should also show a progress bar when the user changes instances via the manage game instances popup. I looked into that and it was more complicated than I could manage at this point due to conflicting usages of the progress bar tab.

@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN ConsoleUI Issues affecting the interactive console UI i18n Issues regarding the internationalization of CKAN and PRs that need translating labels Apr 21, 2023
@HebaruSan HebaruSan requested a review from techman83 April 21, 2023 20:40
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

Yep, that will solve a few discord/issue raises. LGTM!

@HebaruSan HebaruSan merged commit e3baf64 into KSP-CKAN:master Apr 26, 2023
@HebaruSan HebaruSan deleted the feature/gui-del-lockfile branch April 26, 2023 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended ConsoleUI Issues affecting the interactive console UI Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI i18n Issues regarding the internationalization of CKAN and PRs that need translating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Option to delete registry.locked and continue in GUI
2 participants