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

Treat metapackage depends as not auto installed #3008

Merged
merged 1 commit into from
Feb 17, 2020

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Feb 17, 2020

Problem

If you install a depends-based modpack, its mods will be queued for removal automatically:

changeset

This may also cause a crash:

System.ArgumentOutOfRangeException: InvalidArgument=Verdi 2 er en ugyldig for index.
Parameternavn: index
   ved System.Windows.Forms.TabControl.GetTabPage(Int32 index)
   ved System.Windows.Forms.TabControl.TabPageCollection.get_Item(Int32 index)
   ved CKAN.ThemedTabControl.OnDrawItem(DrawItemEventArgs e)
   ved System.Windows.Forms.TabControl.WmReflectDrawItem(Message& m)
   ved System.Windows.Forms.TabControl.WndProc(Message& m)
   ved System.Windows.Forms.Control.ControlNativeWindow.OnMessage(Message& m)
   ved System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m)
   ved System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

Cause

After #2753, we track which mods are auto-installed to satisfy dependencies, so they can be uninstalled when the depending modules are uninstalled.

A modpack is represented as a CkanModule in which IsMetapackage is true, and it is not saved to the registry when it is installed. So its dependencies are all flagged as auto-installed, but their depending module is missing, and they are queued for removal.

The crash indicates that the GUI runtime is asking us to draw an item that doesn't exist. Maybe this is caused by a weird timing issue, because having auto-removable modules brings the change set back at the end of installation?

Changes

Now we can ask the relationship resolver whether a module is auto-installed. To make this determination, in addition to checking whether it's being installed to satisfy a dependency (the previous logic) it also checks whether the depending module is a metapackage; if so, then the dependency is considered as not auto-installed. This will force modpack dependencies to be considered not auto-installed.

As well, the ThemedTabControl now protects itself from runtime malfeasance by ignoring ArgumentOutOfRangeException. This should prevent issues like this from causing crashes in the future.

Fixes #3007.

@HebaruSan HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Pull request labels Feb 17, 2020
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.

Works!

@HebaruSan HebaruSan merged commit e24a628 into KSP-CKAN:master Feb 17, 2020
@HebaruSan HebaruSan deleted the fix/auto-meta-dep branch March 1, 2020 20:50
@Thorimus
Copy link

Thorimus commented Mar 1, 2020

I don't do any coding a so I dont know what the above means, but was this fixed? I encountered this on CKAN v 1.27.0 with KSP 1.9.1 and filed a bug report but it was immediately closed and marked as a duplicate of #3007

@HebaruSan
Copy link
Member Author

HebaruSan commented Mar 1, 2020

Yes, the fix will be in the next release (which will probably be v1.27.2). Issues remain open while we still need to work on them.

@Growflavor
Copy link

Yes, the fix will be in the next release (which will probably be v1.27.1). Issues remain open while we still need to work on them.

Great! & Thank you for the clarification. I also look forward to this next release then as I have been having the same issue (every day, since I am constantly exporting/importing configs to various computers :-) Thanks for making CKAN so good & consequently such an 'essential' KSP management tool.

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 Core (ckan.dll) Issues affecting the core part of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Crash after importing .ckan file, and on launch afterwards
4 participants