DLC Detection

[dbent]

Basic implementation of a DLC detection scheme. As I was doing this I fixed various annoyances but the meat of the changes are in:

• Core/Registry/Registry.cs
• Core/Registry/RegistryManager.cs
• Core/Relationships/RelationshipResolver.cs
• Core/Relationships/SanityChecker.cs

The basic idea:

• DLC have an identifier and an UnmanagedModuleVersion which extends ModuleVersion.
• DLC participate in relationships like any other module so you can use depends, conflicts, even provides with them and everything should work as expected. Versions are also supported!
• The "detection" code right now is just a stand-in that you can use for testing. Basically create a directory dlc within your installation's CKAN directory. Then inside that directory create a file named Identifier.dlc. Identifier-DLC will become the module's identifier (following the soft convention that DLC should have the postfix -DLC) and the contents of the file will be the module's version.

Other notes:

• DLC are not persisted in the registry. Persistence makes things complicated, introduces sources for bugs, and means we have to deal with upgrades over time. DLC detection should be fast (testing for a presence of a file on disk) and is almost certainly faster than deserializing the registry.json so no real point in storing the results.
• I really don't like how the Version objects are used as sentinels to indicate the type of module, but that's the existing mechanism and changing it would require a hell of a lot of refactoring I don't have the time nor inclination to do right now.

Now for miscellaneous list of changes due to various annoyances:

• Now that we're only building on Mono versions after 5 I configured all the projects to explicitly support C# 7.0 and I started using the new features liberally and with great prejudice. I set the version to 7.0, rather than 7.1 or 7.2 (latest) since it's unclear to me what level of support Mono's compiler has for the most recent versions.
• CKAN.Version having the same name as System.Version means we can't use using System; in a lot of files without explicit aliasing. I renamed CKAN.Version to ModuleVersion and moved it and its subclasses into the CKAN.Versioning namespace. I also cleaned up a lot of the versioning code and added documentation.
• I fixed the implementation of GetHashCode() in ModuleVersion nee CKAN.Version. Previously it was only using the hash code from the version string but its equality and comparison methods were using both the epoch and version string for comparisons. This would cause all sorts of wonderful Heisenbugs if we ever used ModuleVersion nee CKAN.Version as the key to a dictionary.
• In a similar vein I also implemented the IEquatable<ModuleVersion> interface on ModuleVersion nee CKAN.Version.
• I made the version regex object static and compiled rather than an instantiating a new one every time. Likely a modest performance improvement there.
• I made the version comparison cache a static object so it can be shared between all ModuleVersion objects. I don't like the idea of this comparison cache, but presumably it was added to imporve performance at some point, and I'm hesitant to remove it without testing that. So if anything this will make it faster.
• ProvidesModuleVersion will now include the version of the module that's providing the provides in its string output.
• UnmanagedModuleVersion nee DllVersion can now take an explicit version string. It continues to use "0" for auto-detected DLLs. Its ToString() implementation has changed to <version> (unmanaged).
• Added a list of DLL files that should be ignored by CKAN. Currently this includes:
  • GameData/Squad/Plugins/KSPSteamCtrlr.dll
  • GameData/Squad/Plugins/Steamworks.NET.dll`

Fixes #2035

[dbent]

Added a new commit which successfully detects the real Making History DLC on my Windows system.

[Olympic1]

I think this should be good for merge if no one wants any changes.

[politas]

Oh, damn. Just realised a small issue - Detected DLC is not appearing in the GUI or consoleui. I think the difference is that while Action/List uses Registry.Installed(), Consoleui/ModListScreen and GUI.MainModList use registry.InstalledModules. Any chance we can normalise the three UIs?

[dbent]

Well from just testing it, that also means those UIs never supported listing autodetected DLLs (which is what these changes are modeled on).

Looking at the code, both the GUI and the CUI expect their mod lists to have"real" CkanModule objects, whereas DLLs and DLC are merely identifiers and (maybe) a version. So that leaves two options:

1. Create "fake" CkanModule objects from the DLLs and DLC and dump them in the same place. I assume that is going to introduce all sorts of nasty bugs since the fake CkanModule objects likely won't have many of the properties and fields the UI code expects them to have.
2. Modify the UI code to enumerate the real installed CkanModule objects, the autodetected DLLs, and the DLC. That would be a lot of careful refactoring.

Since those UIs never supported unmanaged assemblies before, I don't think there's any expectation that DLC will appear in them. I'd consider it low priority to add support to those UIs, and only really consider doing it if there was already an big refactor underway to unify the three UIs. The important thing is that other mods should be able to specify relationships with DLC.

[politas]

I concur.

[HebaruSan]

I thought GUI did support "AD" mods, we have #2186 and #2262 complaining about it.
ConsoleUI definitely doesn't, though, it's very heavily CkanModule-centric.

[dbent]

Then something must have changed because no autodetected mods show up in the GUI for me after testing and while I can see in the code where it checks for it a mod is autodetected or not among a list of mods, I don't see where autodetected mods would be populated in that list.

[politas]

I'm glad we have this working, but I think we would do better if we could get the DLC/expansion/version/compatibility details into metadata, like our KSP version info. We've now got two releases of Making History, tied to specific KSP versions. It would be nice if we flagged any incompatibility for our users.

[HebaruSan]

For what it's worth, I don't think we've had a mod submission yet with a DLC dependency. I haven't seen a mission pack mod, or anything modding the mission builder itself, or anything building directly on the DLC's parts. I'm sure it will happen, but folks don't seem to be in a hurry for some reason.

[politas]

I suspect the first thing we'll see is some missions being included in existing mods, which we hopefully will be handling perfectly {fingers crossed}

[dbent]

I'm glad we have this working, but I think we would do better if we could get the DLC/expansion/version/compatibility details into metadata, like our KSP version info. We've now got two releases of Making History, tied to specific KSP versions. It would be nice if we flagged any incompatibility for our users.

I'm not sure what you mean? That's the whole purpose of this. If a mod requires Making History it should just be added to its dependency list like any other mod:

{
  "depends": [
    {
      "name": "MakingHistory-DLC"
    }
  ]
}

If it depends on a particular version, just add that as well, per usual:

{
  "depends": [
    {
      "name": "MakingHistory-DLC",
      "version": "1.1.0"
    }
  ]
}

For reference, the version of MH that shipped with KSP 1.4.1 was 1.0.0, and the version that shipped with KSP 1.4.2 was 1.1.0. Versions are extracted from the README associated with the DLC.

[Olympic1]

There are being missions released on the forums, but I think people dont know or bother to put them on ckan.

[HebaruSan]

If we made an "Adding a mission pack to the CKAN" wiki page, I bet we could get a forum thread about it stickied.

[politas]

I'm not sure what you mean? That's the whole purpose of this.

I mean, instead of the detailed detection being in Core/DLC/MakingHistoryDlcDetector.cs, we should have the specifics in a file in CKAN-Meta repo, like we have for the KSP versions (and we can include a default copy in the code like we do with Core/builds.json) Obviously it's going to be a bit more complicated than builds.json. but not hugely different to ckanmodules.

[dbent]

I mean, instead of the detailed detection being in Core/DLC/MakingHistoryDlcDetector.cs, we should have the specifics in a file in CKAN-Meta repo, like we have for the KSP versions (and we can include a default copy in the code like we do with Core/builds.json) Obviously it's going to be a bit more complicated than builds.json. but not hugely different to ckanmodules.

I considered this, but decided against it.

The advantage of keeping builds.json in the repository is that new builds can be updated without having to release a new client. However, (I'm assuming) DLC will be much rarer than new KSP builds. At best probably one every six months? And more realistically probably one every 12-18 months. Plus, they're announced with plenty of lead time for us to get ready. Also, as we've seen with Making History, once new DLC is released, it's not like there's a super pressing need to have it available in metadata immediately.

The disadvantage though, is that you introduce more potential sources of failure and bugs, need to persist and refresh the data, and have to have a fallback baked into the assembly that has to be kept in sync with the live data. There's also no guarantee that you won't have to write new code anyway to support a new way of detecting the DLC and extracting the version information.

Now that the code is written though, supporting new DLC should literally be a 10-minute job to add a new implementation of IDlcDetector. Sure, users will have to download a new client to detect the new DLC, but I don't think that's such an onerous burden.

Maybe in the distant future with a CKAN 2.0 that's KSP-agnostic and supports a variety of games, having some declarative way to detect DLC/expansions through metadata would be better, but for now I think simplicity wins out.

[politas]

Reasonably argued. Thanks for the response!

[yalov]

what about recommending or suggesting a DLC ?
I could use it in the Water Launch Sites

[dbent]

what about recommending or suggesting a DLC ?

That should be fine. It would be purely informational though.