Support reading BuildIDs

[dbent]

@pjf @politas @plague006 @Dazpoet @NathanKell

This PR allows CKAN to get more accurate KSP version information by reading buildID.txt. This requires the use of a mapping table to map BuildIDs to actual KSP versions. This mapping potentially exists in 3 locations:

• Remotely at https://raw.githubusercontent.com/KSP-CKAN/CKAN-meta/master/builds.json
  • This allows us to update the mapping without requiring users to download a new version of the client.
• Inside the Registry, this is a cached version of the above and is used preferentially. It is updated automatically when a user performs an update operation.
• As an embedded resource in the CKAN.dll assembly, this allows us to have a fallback in case the other remote methods fail. This should be kept up to date with every new CKAN release.

The basic process is:

• Read buildID.txt
• Extract the BuildID string, skipping leading zeros.
• Lookup the corresponding KspVersion from an IBuildMap.
• If that doesn't work, fallback to reading it from readme.txt.

In order to better support this functionality I've introduced two basic new classes: KspVersion and KspVersionRange. KspVersion is a replacement for KSPVersion and is a dumber version of the latter. All it knows is that a KSP version can have four components: a major number, a minor number, a patch number, and a build number. Some of these components may be undefined in which case all less significant components must also be undefined. The following are valid KspVersion strings:

• "" (the empty string)
• "1"
• "1.0"
• "1.0.0"
• "1.0.0.0"

Since some components may be undefined (like the first four examples listed above) a KspVersion can be converted to a KspVersionRange. A KspVersionRange represents two KspVersion objects (a lower and upper bound) which have fully defined components and boolean flags which indicate if those bounds are inclusive or exclusive.

The previous examples have the following KspVersionRange (the range syntax is standard mathematical representation of a range with [, ] representing inclusive bounds and (, ) representing exclusive bounds, this also how nuget does it).

KspVersion	KspVersionRange
""	"(,)"
"1"	"[1.0.0.0, 2.0.0.0)"
"1.0"	"[1.0.0.0, 1.1.0.0)"
"1.0.0"	"[1.0.0.0, 1.0.1.0)"
"1.0.0.0"	"[1.0.0.0, 1.0.0.0]"

This allows us to map BuildIDs like 1028 to version strings like 1.0.5.1028 and have everything JustWork™. Because when a mod says it requires KSP "1.0.5" it's really saying it requires the range "[1.0.5.0, 1.0.6.0)". So a mod could require specifically 1.0.5.1028 and know it works only with that specific build.

Another nice property of this is that if we fallback to the readme.txt and get a version string like "1.1.0" that will automatically translate into the range "[1.1.0.0, 1.1.1.0)". If a mod targets "1.1.0" it will work because "[1.1.0.0, 1.1.1.0)" (game version range) is a subset of "[1.1.0.0, 1.1.1.0)" (mod version range). However, if a mod targets a specific build, say "1.1.0.1196" it will not work because "[1.1.0.0, 1.1.1.0)" (game version range) is not a subset of "[1.1.0.1196, 1.1.0.1196]" (mod version range).

This also gets rid of the hacky-ness of converting "1.0" to "1.0.0" and "1.0.99".

Because this touches such a fundamental part of the client, it will require a lot of testing, but for now the code is in place.

[dbent]

To solve #1545 I recommend adding a mapping for build 1028 to maps it to 1.0.5.1, then any mod which explicitly requires build 1028 should require 1.0.5.1 instead of 1.0.5. This should then be the general strategy going forward with any "silent patches".

[politas]

Does that mean we would need to override every mod that has a ksp_version or ksp_version_max of 1.0.5, or will 1.0.5.1 fit that requirement?

[dbent]

Does that mean we would need to override every mod that has a ksp_version or ksp_version_max of 1.0.5

@politas yes, but should be batchable with @pjf regex-magick.

Alternatively, we just let it be, ignore 1.0.5/1.0.5.1, and use the strategy for any such future occurrences. (I would be A-OK with this). The main issue in #1545 of warning users when they're not completely up-to-date is taken care of by Module Manager anyway, and I doubt there are very many users using mods that don't also have Module Manager installed.

[NathanKell]

Sorry, ModuleManager? What?

[StollD]

@NathanKell He means the warning that MM displays if you are on 1.0.5.1024

[dbent]

Specifically: https://github.com/sarbian/ModuleManager/blob/master/moduleManager.cs#L167

[NathanKell]

Interesting. Because in the last few days two people have had that issue despite having MM, so they must have been ignoring the warning. Ah well... :]

[mheguy]

Awesome, looking forward to this. =)

[pjf]

So, the good news is that @NathanKell assures me that build IDs are unified across operating systems, so we shouldn't have to split on OS. Clearly whatever I was thinking there is either old or wrong.

Also, because we're using IGameComparator classes now, we can do some very fancy things when it comes to comparing KSP versions. So "1.0.5" can mean "1.0.5.1028", and "1.0.5.xxxx" can be an exact build number. This means mod authors can target "1.1.0" and have that mean the stable release when it comes out, and "1.1.0.xxxx" can be if they want to target an exact build.

Likewise, we can probably have a "1.1.0-beta" version, and have our IGameComparator classes do sensible things with that.

I'm also hella jetlagged right now, but in any case having a mapping of builds to versions in the metadata is a great idea.

[pjf]

Hey, that's really cool! I had no idea we could do this. :)

[dbent]

@pjf Ah, yes, indexers are totally a thing. It's why you can do dictionary["foo"]. They can even take multiple parameters: someType["foo", "bar"].

[pjf]

Glancing at the code it looks sensible to me, but I haven't had a chance to test it yet. I do have some build IDs (all from Linux, but which I'm told will be the same everywhere):

464 : 0.23.5
559 : 0.24.2
00642 : 0.25.0
00861 : 1.0.4

I'm guessing this means that for build IDs we should strip leading zeros and/or just treat them as integers.

[dbent]

@pjf Made some changes.

• Remove the platform-specific mappings.
• Introduced a builds key to the builds.json file, this should make it easier to expand its metadata in the future. Similar to how repositories.json has a top-level repositories key.
• I've stripped the leading zeros from the build number and mapped builds to build-specific version numbers (now that we can assume they're the same on all platforms). So 1172 is 1.1.0.1172, 1174 is 1.1.0.1174. However I cannot in good conscience treat identifiers like integers no matter how much they make look like integers, nope, nope, never gonna happen 😄.
• Renamed some types.

I like the idea of mapping 1.0.5 to 1.0.5.1028. This however requires us to know which build is the stable one. Maybe additional metadata like so?

{
  "builds": {
    "1028": "1.0.5.1028",
    "1172": "1.1.0.1172",
    "1174": "1.1.0.1174"
  },
  "short_versions": {
    "1.0.5": "1.0.5.1028"
  }
}

Basically we consider a full KSP version to have four components: <major>.<minor>.<patch>.<build>. If we see a short KSP version (i.e. one that does not have all four components) we use the short_versions map to translate the short version to a build number, which we then translate into a full version.

This screws up the existing short/long logic in KSPVersion though. Right now 1.0 would be internally translated to 1.0.0 when its "minimum" value is considered and 1.0.99 when its "maximum" value is considered. That entire bit of logic could use an overhaul though. I'll have to think on that.

[Ruedii]

I really think we should take this opportunity to switch to an external lookup table in the database.

This would help future-proof major versions of CKAN as we update the client itself less often, not having the version info hardcoded would be good. It might even be good to move the script to determine where to look for the BuildID to the primary database to further future proof it.

[mheguy]

@Ruedii First bullet in @dbent's OP. Lines 14 and 15 of KspBuildMap.cs in the PR.

[Ruedii]

I wouldn't recommend treating them as integers either.

Just use a string conversion lookup table.
If anything they would be an set of integers. (Major, Minor, Point and release)

[politas]

Added all the BuildIDs I have to https://github.com/KSP-CKAN/CKAN-meta/blob/master/builds.json
(And the one extra from @pjf's list above)

[Dazpoet]

I like this idea, sounds awesome 👍

[dbent]

Major update with version range goodness, see first post for details.

[dbent]

Now with glorious 100% statement coverage on the new cases.

[dbent]

@KSP-CKAN/wranglers I've been using the changes in this build as my daily driver for a few weeks now and haven't run into any issues, let me know if you want a build to test this out.

[mheguy]

If you link a full build I'll tinker with it.

[dbent]

• http://build.apokee.com/ckan/buildid/ckan.exe
• http://build.apokee.com/ckan/buildid/netkan.exe

[politas]

You know what the biggest issue with this is going to be? The standard advice for what people should do if they want to install mods that aren't compatible with their KSP version is to change the version in the Readme file. This change is going to break that method.

[mheguy]

Yes, now we'll have to tell them to change the extension of build.txt to build.txt.bak and then change versions in readme.

[politas]

Ok, that's less of a hassle than I was thinking.

[techman83]

From my testing it does what is says on the tin. Falls back to readme if buildID.txt is missing or invalid. Tested a few different buildIDs and got the correct version each time. I've had a skim of the code and nothing glaring stands out.

@plague006 @politas how'd your testing go?

[mheguy]

Been using it for testing PRs and whatnot and haven't run into any issues with it.

[politas]

I haven't tried it.

[techman83]

I've had a really good read of this and really like the design. I can't see anything super obvious that should hold us back. Merging with thanks @dbent !

[pjf]

I'm catching up on PRs, and you are all amazing. Thank you!