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

Leave user requested dependencies in changeset #3728

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Nov 30, 2022

Problem

  1. Launch the current master branch build of CKAN (bug does not exist in latest release)
  2. On a fresh game instance, check the three checkboxes to install these mods:
    • Parallax
    • Parallax - Stock Scatter Textures
    • Parallax - Stock Planet Textures
  3. Click Apply changes
  4. The install flow launches, but no mods are installed, and then the changeset is cleared 😭

Reported by @vega-star on Discord. Many thanks for trying out the dev build and contributing feedback in enough detail to pinpoint this bug!

Cause

#3726 made it so that when you click to confirm a changeset, instead of recalculating the changeset from the mod grid as we did previously, it used the data from the same changeset object that the user just viewed and approved in the changeset tab. This allows us to use the changeset tab for changesets other than the one selected on the main mod list (such as reinstallation via the right click menu, which cannot otherwise be represented by the main mod list's checkboxes).

But if we just used the exact same changeset, then the auto-installed flag would never be set, because dependencies are included alongside user requested mods. We need to exclude auto-installed mods from the changeset so the core code can determine that they are in fact auto-installed. #3726 did this by excluding any mod that was a dependency of another mod in the changeset. Turns out that was wrong, because it's possible to pick several mods that are all mutual dependencies, which results in passing an empty changeset to the install flow.

Changes

Now instead of filtering out dependencies, we pick the user-requested mods, which is what #3726 should have done in the first place.

@HebaruSan HebaruSan added Bug Easy This is easy to fix GUI Issues affecting the interactive GUI Pull request Relationships Issues affecting depends, recommends, etc. labels Nov 30, 2022
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.

Ok that makes sense!

@HebaruSan HebaruSan merged commit fa6f9d6 into KSP-CKAN:master Dec 1, 2022
@HebaruSan HebaruSan deleted the fix/install-all-deps branch December 1, 2022 03:13
@HebaruSan HebaruSan mentioned this pull request Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Easy This is easy to fix GUI Issues affecting the interactive GUI Pull request Relationships Issues affecting depends, recommends, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants