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

Sort GUI KSP version column correctly #3106

Merged
merged 3 commits into from
Jul 12, 2020

Conversation

HebaruSan
Copy link
Member

Problem

If you sort by Max KSP version, 1.10 comes between 1.1 and 1.2, which is wrong.

image

image

Cause

Currently this column sorts as a string, which only works if the lexicographical sort order of the strings corresponds to the true sorting of the versions, which it doesn't.

Changes

This gets a bit tricky. We need to expose the GUIMod's KspVersion object, but we can't just sort by that object naïvely, because KspVersion.CompareTo sorts undefined values inappropriately for upper bounds; it considers 1.8 < 1.8.0, and any < everything. We want the opposite, so we can't use .OrderBy or .OrderByDescending for this.

Instead, ManageMods now has an alternate Sort overload that takes a Comparison<DataGridViewRow>, which can handle comparisons between two objects rather than just returning a sort key for one object. The KSP version column uses this Sort with a new function that treats undefined components of KspVersion as maximum rather than minimum. This causes the column to sort according to actual compatibility.

Fixes #3105.

Quirks

Note that this logic makes finer distinctions in mod compatibility than the .ToYalovString() display function does. This means that the alphabetical fallback sorting may sometimes appear to reset unexpectedly. For example, consider this block of mods that all display as "1.10.9":

image

The Versions tab reveals:

  • SmokeScreen is compatible up to 1.10.90
  • AblativeAirbrake through Tundra Technologies are compatible up to 1.10.99
  • Astrogator is compatible up to 1.10

So while the column's strings are superficially the same, the introduction of a KSP 1.10.95 or KSP 1.10.100 would draw out real distinctions in their compatibility, which this sort order reflects.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request labels Jul 9, 2020
@HebaruSan HebaruSan requested a review from DasSkelett July 9, 2020 19:32
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.

Version comparison is always fun!

@HebaruSan HebaruSan merged commit 11adad5 into KSP-CKAN:master Jul 12, 2020
@HebaruSan HebaruSan deleted the fix/kspver-sort branch July 12, 2020 18:00
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Sort by KSP version not working as expected with 1.10.x
2 participants