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 dependencies of upgrading mods in change set #3560

Merged
merged 1 commit into from
Apr 10, 2022

Conversation

HebaruSan
Copy link
Member

Problem

  1. Install xScience 5.26
  2. Upgrade to the latest version
  3. The change set shows just the upgrade:
    image
  4. However, the install screen shows that new dependencies will be installed:
    image
    The user should be told about this in the change set screen.

Cause

The change set screen has always only looked for dependencies for modules being installed, not upgraded.

I think the expectation was that all of the complicated stuff would happen for installs, while upgrades would be simple replacements of one version of a module with a newer version, even though the underlying relationship logic in Core can handle more complicated scenarios.

Changes

Now upgrades will be treated more like installs, in that any new dependencies they introduce will be displayed:

image

To make this work, we also need to fix ComputeChangeSetFromModList to use the upgraded version in calculating the change set. It was using the old version; luckily this only affects the change set screen and not the actual install, which is handled in Core.

Fixes #3559.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix GUI Issues affecting the interactive GUI Pull request Relationships Issues affecting depends, recommends, etc. labels Apr 6, 2022
@DasSkelett
Copy link
Member

This comment needs updating,

// We're going to split our change-set into two parts: updated/removed mods,
// and everything else (which right now is replacing and installing mods, but we may have
// other types in the future).

"updated" is no longer part of the first group

Otherwise looks good to me

@HebaruSan HebaruSan force-pushed the fix/upgrade-deps-in-changeset branch from 1e5d903 to a45bfe6 Compare April 10, 2022 16:18
@HebaruSan
Copy link
Member Author

Updated! Sometimes writing extra documentation can be a liability of sorts...

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.

Doesn't break more complex changeset scenarios, so that's a plus 😄

@HebaruSan HebaruSan merged commit 1bdd5bd into KSP-CKAN:master Apr 10, 2022
@HebaruSan HebaruSan deleted the fix/upgrade-deps-in-changeset branch April 10, 2022 16:26
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 Easy This is easy to fix GUI Issues affecting the interactive GUI Relationships Issues affecting depends, recommends, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] New dependencies of upgraded mods missing from change set
2 participants