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

Show download errors for upgrades #3840

Merged

Conversation

HebaruSan
Copy link
Member

Problem

If an upgrade's download fails, the failed downloads popup from #3635 appears, but empty:

image

This dead-ends any investigation of such problems.

Originally reported by forum user jost.

Cause

The rows of that table are built based on the mods in the changeset, but only the mods to install were included. The mods to upgrade were not, so if one of them had an error, it wouldn't be shown.

Changes

Now the mods we're upgrading are also passed to the form, so the error appears as expected:

image

While investigating this, I found that the popup could cause CKAN to freeze before it opened, and that passing itself to Util.Invoke instead of a reference to the Main form fixed it, so that's included here as well.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Network Issues affecting internet connections of CKAN labels May 18, 2023
@HebaruSan HebaruSan requested a review from techman83 May 18, 2023 03:26
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.

Good find! Clean fix.

@HebaruSan HebaruSan merged commit c5ba72c into KSP-CKAN:master May 18, 2023
@HebaruSan HebaruSan deleted the fix/empty-upgrade-download-errs branch May 18, 2023 13:16
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 Network Issues affecting internet connections of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants