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

Choose game instance if default is locked #3382

Merged
merged 1 commit into from
May 28, 2021

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented May 25, 2021

Problem

If you have multiple game instances, you've selected one as the default instance, and you open that instance in CKAN, then opening another copy of CKAN will cause it to print an error about the instance being locked and quit.

However, if you have no default instance, then in the same flow you will be prompted to choose an instance. The user shouldn't lose options because they've selected a default instance.

Changes

Now if the default instance is locked by another process but there are other unlocked instances, the Manage Game Instances popup appears. Instances that have a CKAN/registry.locked file will have "(LOCKED)" appended to their names, so you can tell which ones are open in other instances of CKAN:

image

Also the Linux man page's documentation for the ckan ksp subcommand is updated to the current ckan instance naming.

Fixes #3375.

@CodeLeopard, there should be a test build under the Checks tab shortly after this is submitted, if you wouldn't mind giving it a try.

@HebaruSan HebaruSan added Enhancement New features or functionality GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Pull request Registry Issues affecting the registry labels May 25, 2021
@HebaruSan HebaruSan requested a review from DasSkelett May 25, 2021 18:56
@CodeLeopard
Copy link

I tested it and it works for the most part, but I did ran into a problem:

If I select a locked instance it just re-opens the selection dialog, in an ideal situation it would show the "Instance is locked" dialog at this point (but then still show the selection dialog again).

If I then select the locked instance again (after the selector re-appears) the full GUI starts up and immediately throws an exception: RegisteryInUseKraken. If I select continue the GUI has no window title and it does not populate the mods list.

@HebaruSan

This comment has been minimized.

@HebaruSan HebaruSan added the In progress We're still working on this label May 27, 2021
@HebaruSan

This comment has been minimized.

@HebaruSan HebaruSan removed the In progress We're still working on this label May 27, 2021
@CodeLeopard
Copy link

CodeLeopard commented May 27, 2021

Okay I just tested the new version and it works properly in the startup case.

However I did discover another issue that occurs when switching between instances (not opening a new one):
In the full GUI, go to: file -> manage instances and select a locked instance, it throws RegisteryInUseKraken.

The 'instance is locked' error not appearing on the correct screen did happen to me, but it does put that message on top so I saw it immediately, I don't think it's a big problem.

@HebaruSan
Copy link
Member Author

However I did discover another issue that occurs when switching between instances (not opening a new one):
In the full GUI, go to: file -> manage instances and select a locked instance, it throws RegisteryInUseKraken.

OK, now the GUI catches that exception, displays an error for it, and goes back to working with the original instance. There's a little delay because technically just the attempt to open the other instance invalidates certain things and we need to make sure everything is set up properly again.

https://github.com/KSP-CKAN/CKAN/suites/2848264095/artifacts/63574915

@HebaruSan HebaruSan force-pushed the fix/gui-locked-inst branch 2 times, most recently from 634e067 to 52204c1 Compare May 27, 2021 21:33
@DasSkelett
Copy link
Member

Just pushed the German translation, and also renamed ManageGameInstancesInvalid/ManageGameInstancesLocked -> ManageGameInstancesNameColumnInvalid/ManageGameInstancesNameColumnLocked to differentiate them a bit from ManageGameInstancesNotValid.
The new name is admittedly not the best, but I couldn't think of a better one.

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Yup, should be good now. Nice enhancement, thanks for the suggestion @CodeLeopard!

@DasSkelett DasSkelett merged commit 45710ef into KSP-CKAN:master May 28, 2021
@HebaruSan HebaruSan deleted the fix/gui-locked-inst branch May 29, 2021 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] When the default instance is locked: open Manage Game Instances instead of quitting
3 participants