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

Don't fail modpack export when having unindexed mods installed #3139

Merged
merged 2 commits into from
Nov 6, 2020

Conversation

DasSkelett
Copy link
Member

@DasSkelett DasSkelett commented Aug 11, 2020

Problem

When you have mods or mod versions installed that are not in the list of available modules ("DarkKAN modules"), the modpack export tab freezes.
This is caused by registry.LatestAvailable() in AddGroup() throwing an ModuleNotFoundKraken for unknown mods when trying to fetch the abstract for each installed mod.

Changes

  • Now we search the registry for an InstalledModule with the given identifier, and use its abstract property. We should always have an InstalledModule for each module we're handling in the export logic, and if for some reason we haven't, it shouldn't break anything.

And from DasSkelett#9 thanks to @HebaruSan:

  • Non-indexed mods are now left out of the return value of RegistryManager.GenerateModpack, so now as far as Core is concerned, modpacks should consist only of indexed mods
  • The modpack editor control now adds installed non-indexed mods to the Ignored list by default. The user can move them to another group if desired.
  • Now custom modules installed from .ckan files will no longer be added to the list of available modules in the registry (see code review comments for details)

Fixes #3138
...maybe, hard to say without the registry or any information about installed mods, but it solved it for me.

@DasSkelett DasSkelett added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request labels Aug 11, 2020
@HebaruSan
Copy link
Member

("DarkKAN modules")

Is that a thing? Like dark matter, dark energy, and the dark web? 😁

@HebaruSan
Copy link
Member

HebaruSan commented Oct 2, 2020

One thought, the typical user won't be able to install "DarkKAN modules" without adding a new metadata repo (or downloading a .ckan file). Does it make sense to include them in exported modpacks, if they'll cause a failed installation by default? I guess it could, since maybe the modpack creator will include instructions on how to add a repo. But we might want to display a warning about that somewhere.

@HebaruSan
Copy link
Member

Now that I think about it, we have a possibly sensible thing to do with these: Put them in the Ignored group by default. That way the user can just click Export to generate a modpack that will work for other people, but if he or she really wants to have weird custom modules listed in the modpack, the option is there to do that manually.

@HebaruSan
Copy link
Member

Submitted DasSkelett#9 to try to do that...

Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

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

Admittedly I'm biased, but these changes look fantastic to me. 😉

@HebaruSan HebaruSan merged commit f11d8b0 into KSP-CKAN:master Nov 6, 2020
@DasSkelett DasSkelett deleted the fix/unindexed-mod-export branch November 6, 2020 01:47
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 GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Exporting a modpack using CKAN 1.27.2 on Debian 4.19.118-2 locks up CKAN
2 participants