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

Treat AVC min without max as open range #2571

Merged
merged 1 commit into from
Nov 14, 2018

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Nov 7, 2018

Problem

A .version file containing this:

    "KSP_VERSION":
    {
        "MAJOR":1,
        "MINOR":5,
        "PATCH":0
    },
     "KSP_VERSION_MIN":{
         "MAJOR":1,
         "MINOR":5,
         "PATCH":0
     },

Is resulting in this metadata:

    "ksp_version": "1.5.0",

The minimum value is essentially ignored.

Cause

The KSP-AVC spec says:

  • KSP_VERSION - Optional, Required for MIN/MAX
    Version of KSP that the add-on was made to support.
  • KSP_VERSION_MIN - Optional
    Minimum version of KSP that the add-on supports.
    Requires KSP_VERSION field to work.
  • KSP_VERSION_MAX - Optional
    Maximum version of KSP that the add-on supports.
    Requires KSP_VERSION field to work.

This implies that if you want to specify KSP_VERSION_MIN, you must specify KSP_VERSION. This does not make sense, but it's in the spec, and some modders may feel compelled to follow it strictly as written.

Under that interpretation, if you wanted to specify "all versions after 1.5.0", you would create a file as shown above, but it would not translate into equivalent CKAN metadata. This is because the KSP_VERSION value is used as a fallback for both the min and the max if either are absent:

// Get the minimum and maximum KSP versions that are in the AVC file.
// Use specific KSP version if min/max don't exist.
var avcKspMin = avc.ksp_version_min ?? avc.ksp_version;
var avcKspMax = avc.ksp_version_max ?? avc.ksp_version;

Changes

Now if a .version file has KSP_VERSION_MIN and KSP_VERSION but not KSP_VERSION_MAX, we don't set ksp_version_max. (And vice versa for MAX and MIN.)

@HebaruSan HebaruSan added Bug Pull request Netkan Issues affecting the netkan data labels Nov 7, 2018
@politas
Copy link
Member

politas commented Nov 14, 2018

And because we set the other min/max field to a null value, it doesn't populate that field in the .ckan, is that right?

@HebaruSan
Copy link
Member Author

Yup, null will mean that the property isn't set.

@politas politas merged commit 5b6f1c0 into KSP-CKAN:master Nov 14, 2018
politas added a commit that referenced this pull request Nov 14, 2018
@HebaruSan HebaruSan deleted the fix/avc-open-ranges branch November 14, 2018 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Netkan Issues affecting the netkan data Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants