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

Remove duplicate Install changes for upgrades #3706

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Nov 14, 2022

Problem

When you upgrade a module, the GUI changeset shows two entries for the same module, an Update step and an Install step.

image

Cause

I think these extra changes existed along, but were hidden in GUI by CreateSortedModList until #3667 simplified the changeset UI to show what's actually being done. The !sortedChangeSet.Any(c => c.Mod.identifier == change.Mod.identifier check would have prevented an Install change in the changeset from being added to the list in the UI if an Update was already present:

private void CreateSortedModList(IEnumerable<ModChange> changes, ModChange parent = null)
{
var notUserReq = changes
.Where(c => !(c.Reason is SelectionReason.UserRequested))
.Memoize();
foreach (ModChange change in changes)
{
bool goDeeper = parent == null || change.Reason.Parent.identifier == parent.Mod.identifier;
if (goDeeper)
{
if (!sortedChangeSet.Any(c => c.Mod.identifier == change.Mod.identifier && c.ChangeType != GUIModChangeType.Remove))
sortedChangeSet.Add(change);
CreateSortedModList(notUserReq, change);
}
}
}

Changes

Now the resolver's Install step is only added to the changeset if there isn't already an Update step for the same mod.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request labels Nov 14, 2022
@HebaruSan HebaruSan added the Easy This is easy to fix label Nov 14, 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.

Aha, that makes sense!

@HebaruSan HebaruSan merged commit bb81737 into KSP-CKAN:master Nov 14, 2022
@HebaruSan HebaruSan deleted the fix/changeset-upgrade-install branch November 14, 2022 06:06
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants