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

Fix max version column for wildcard compat #4142

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Aug 5, 2024

Problem

Before KSP-CKAN/NetKAN#10149, StockWaterfallEffects's latest version was compatible with all of 1.11–1.12, but the max compat game version column showed 1.12.2.

Cause

AvailableModule.LatestCompatibleGameVersion tries to avoid checking compatibility of every single version of every mod against every known game version by quitting early if any of them are marked as compatible with all game versions:

public GameVersion LatestCompatibleGameVersion(List<GameVersion> realVersions)
{
// Cheat slightly for performance:
// Find the CkanModule with the highest ksp_version_max,
// then get the real latest compatible of just that one mod
GameVersion best = null;
CkanModule bestMod = null;
foreach (var mod in module_version.Values)
{
var v = mod.LatestCompatibleGameVersion();
if (v.IsAny)
{
// Can't get later than Any, so stop
return mod.LatestCompatibleRealGameVersion(realVersions);
}
else if (best == null || best < v)
{
best = v;
bestMod = mod;
}
}
return bestMod.LatestCompatibleRealGameVersion(realVersions);
}

As it loops through that list, it tries to remember the highest max bound and then uses it at the end to get the compatibility. Unfortunately, the best < v comparison of two GameVersions can give wrong answers because GameVersion.Undefined is -1, which makes unbounded versions always compare as less than bounded ones, despite being able to match later versions. This makes 1.12.2 compare as greater than 1.12, so it's not a good idea to directly compare mixed bounded and unbounded game versions.

Changes

Now that comparison logic is performed by GameVersionBound.Highest, which properly handles comparisons with unbounded values.

Fixes #4141.

@HebaruSan HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN labels Aug 5, 2024
@HebaruSan HebaruSan merged commit 1327aee into KSP-CKAN:master Aug 5, 2024
3 checks passed
@HebaruSan HebaruSan deleted the fix/max-compat branch August 5, 2024 20:12
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 Core (ckan.dll) Issues affecting the core part of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: max game version column can show an incorrect value when a mod uses major.minor ksp_max_version
1 participant