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

Include invalid instances in KSPManager #2230

Merged
merged 1 commit into from
Dec 29, 2017

Conversation

HebaruSan
Copy link
Member

Problems

If a KSP instance that CKAN knows about is removed manually (i.e., the user deletes the folder), an error message is printed at startup:

194 [1] WARN CKAN.KSPManager (null) - rss at /Users/lamont/ksp_rss is not a vaild install

But it's impossible to remove it from the list of known installs, because it's not added to the list that is manipulated to make that happen. However, if the user adds another instance, then any currently invalid instances are automatically removed.

None of this is particularly desirable. It should always be possible for the user to remove any instance, but no instance should be removed without the user choosing it.

Changes

Now invalid instances are added to the list of known instances at startup; they appear in the instance selection list in Cmdline, ConsoleUI, and GUI. But if the user tries to use an invalid instance or set it as default, an error message appears indicating that the install is not valid.

This is achieved by defining a new CKAN.KSP.Valid property, which guards CKAN.KSP.CkanDir() and a few other spots. If Valid is false when we need it to be true to perform an operation, then NotKSPDirKraken is thrown and (hopefully) caught by the appropriate UI component. An invalid instance also has a null Version property, because you need a valid instance to get the version, so we have to handle that in the instance listing code now.

Linkages

This is the replacement for #2214. (It's actually attempt #3. With any luck, no one will ever see attempt #2, which involved defining a Tuple<string, string, KSP> type and converting KSPManager.Instances to return it. Allowing KSP objects to be invalid is much more straightforward and less ugly.)

Fixes #896.
Fixes #1836.
Fixes #1859.

Merry Christmas, Happy Hannukah, and Happy New Year!

@HebaruSan HebaruSan added Bug Core (ckan.dll) Issues affecting the core part of CKAN Cmdline Issues affecting the command line GUI Issues affecting the interactive GUI Pull request labels Dec 22, 2017
@politas politas merged commit 2cc20b5 into KSP-CKAN:master Dec 29, 2017
politas added a commit that referenced this pull request Dec 29, 2017
@HebaruSan HebaruSan deleted the fix/allow-invalid-ksp branch December 29, 2017 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants