Show DLC in recommendations list

[HebaruSan]

Motivation

In KSP-CKAN/NetKAN#7205, we added our first module that recommends a DLC. Currently this is allowed and doesn't crash, and it appears in the relationships tab, but it doesn't appear during the install flow. It would be nice to notify users installing that module that they should consider purchasing the DLC.

Changes

NOTE: If this is merged, the next release must be v1.28.0 due to spec changes!

The spec and schema are updated to allow:

    "kind": "dlc",

When this is set, the module is treated as a DLC and the "download" property is not required. This allows us to represent a DLC with a CkanModule.

The resources property can now contain store and steamstore to hold links for DLC.

Installed DLCs are now represented in the registry as a module in installed_modules with kind set to dlc.

Fixes #2770.

Setup

Testing this requires a bit more preparation than usual due to the schema changes:

1. Edit your CHANGELOG.md file and change the first version listed to v1.28.0, to allow modules with that spec_version to be added to your registry
2. ./build
3. Switch to a repo that has DLC metadata (see Add modules for DLC CKAN-meta#1901) (the install/remove steps are to force ScanDlc to run):

   ckan repo add add-dlc https://github.com/HebaruSan/CKAN-meta/archive/add-dlc.tar.gz
   ckan repo forget default
   ckan update
   ckan install astrogator
   ckan remove --all

I recommend creating an instance with all DLC and an instance with no DLC for ease of testing.

GUI

Now if you install a module that recommends a DLC that you don't have installed, the DLC will appear in the recommendations list:

• The checkbox will always be unchecked regardless of whether it's a recommendation or a suggestion (since we can't install DLC for you, it's just informational)
• The checkbox cannot be checked (it just doesn't react when you click it)
• The checkboxes on the Version tab are hidden for DLC
• Mod info now shows all entries in the resources property, at the bottom of the table, so you can see the store and steamstore links

Apparently the install from .ckan option now allows multi-select as well.

CmdLine

Now if you try to install, remove, or upgrade a DLC, we tell you you can't and provide links where you can purchase:

$ _build/ckan.exe install BreakingGround-DLC
CKAN can't install expansion 'Breaking Ground' for you.
To install this expansion, purchase it from one of its store pages:

- https://www.kerbalspaceprogram.com/product/kerbal-space-program-breaking-ground/
- https://store.steampowered.com/app/982970

$ _build/ckan.exe remove BreakingGround-DLC --ksp steam
CKAN can't remove expansion 'Breaking Ground' for you.
To remove this expansion, follow the instructions for the store page from which you purchased it:

- https://www.kerbalspaceprogram.com/product/kerbal-space-program-breaking-ground/
- https://store.steampowered.com/app/982970

$ _build/ckan.exe upgrade BreakingGround-DLC --ksp steam

Upgrading modules...

CKAN can't upgrade expansion 'Breaking Ground' for you.
To upgrade this expansion, download any updates from the store page from which you purchased it:

- https://www.kerbalspaceprogram.com/product/kerbal-space-program-breaking-ground/
- https://store.steampowered.com/app/982970

The ckan mark command blocks you from marking DLC as auto-installed (because it could cause us to attempt auto removal):

$ _build/ckan.exe mark auto BreakingGround-DLC --ksp steam
Marking BreakingGround-DLC as auto-installed...
Can't mark expansion 'Breaking Ground' as auto-installed.

The ckan show command is updated to print the store and steamstore resources and to hide fields that aren't used:

$ _build/ckan.exe show BreakingGround-DLC
Breaking Ground: The second expansion pack adds new science-collecting equipment to deploy on your missions, surface features to be investigated scattered across distant planets, and a wealth of new robotic parts for players to test their creativity with

Module info:
- version:	1.3.1
- authors:	SQUAD
- status:	
- license:	restricted

Provides:
- BreakingGround-DLC

Resources:
- store: https://www.kerbalspaceprogram.com/product/kerbal-space-program-breaking-ground/
- steamstore: https://store.steampowered.com/app/982970

Netkan

When we validate .ckan files, we now enforce the spec_version requirements documented for the "kind" property in the spec, and tests are included to exercise this.

[DasSkelett]

Nice! Definitely improves UX with DLCs.

Two things:
Mods with missing/incompatible mod dependencies are not selectable for installation.
Mods with DLCs as dependencies are selectable, and throw a misleading/wrong error if you try to install them:
MakingHistoryEngineTweaks v1 depends on MakingHistory-DLC, which is not compatible with the currently installed version of KSP

We should probably also hide the install checkbox for the second group.
I think that's CompatibilitySorter.CheckDepends that needs an update for this.

(An alternative would be displaying a different message, but IMHO the first option is cleaner, code-wise at least.)

---

And it looks like users will have problems after updating the client. My simulated updating process always throws an error after the second time it's started:

1. Create a new instance with the DLCs installed.
2. Run CKAN v1.27.2, switch to the testing repository (this is essentially what happens after we merge Add modules for DLC CKAN-meta#1901)
3. Update repository, close CKAN
4. Start CKAN "v1.28.0". Now the "new" RegisterDlc is called, and adds the DLCs to the installed modules.
5. Problem here: If CKAN doesn't refetch the repos now (same etag + update at startup disabled), it is missing the DLCs in the available_modules. Thus the else part of RegisterDlc is executed, and "fake" DLC modules are added to installed_modules.
   However, this is unvisible at first, everything seems to be fine.
6. Now close CKAN and start it again, it now tries to load the "fake" DLCs in installed_modules. They are invalid metadata though, thus the following exception is thrown:

Unhandled Exception:
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. 
---> CKAN.BadMetadataKraken: The license  is invalid
  at CKAN.License..ctor (System.String license) [0x0002d] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at (wrapper managed-to-native) System.Reflection.RuntimeConstructorInfo.InternalInvoke(System.Reflection.RuntimeConstructorInfo,object,object[],System.Exception&)

[a very lot of inner system functions being called]

  at CKAN.JsonSimpleStringConverter.ReadJson (Newtonsoft.Json.JsonReader reader, System.Type objectType, System.Object existingValue, Newtonsoft.Json.JsonSerializer serializer) [0x00018] in <0b9b648722e24f93be8ee8ef36c6982c>:0 

[a very very long traceback of JsonSerializer functions]

  at CKAN.RegistryManager.Load () [0x0004d] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CKAN.RegistryManager.LoadOrCreate () [0x0004a] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CKAN.RegistryManager..ctor (System.String path, CKAN.KSP ksp) [0x00087] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CKAN.RegistryManager.Instance (CKAN.KSP ksp) [0x0002c] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CKAN.CmdLine.Repo.ForgetRepository (CKAN.CmdLine.Repo+ForgetOptions options) [0x00051] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CKAN.CmdLine.Repo+<>c__DisplayClass7_0.<RunSubCommand>b__0 (System.String option, System.Object suboptions) [0x00143] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CommandLine.Parser.ParseArgumentsStrict (System.String[] args, System.Object options, System.Action`2[T1,T2] onVerbCommand, System.Action onFail) [0x00077] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CKAN.CmdLine.Repo.RunSubCommand (CKAN.KSPManager manager, CKAN.CmdLine.CommonOptions opts, CKAN.CmdLine.SubCommandOptions unparsed) [0x00078] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CKAN.CmdLine.MainClass.Execute (CKAN.KSPManager manager, CKAN.CmdLine.CommonOptions opts, System.String[] args) [0x00174] in <0b9b648722e24f93be8ee8ef36c6982c>:0 
  at CKAN.CmdLine.MainClass.Main (System.String[] args) [0x000a1] in <0b9b648722e24f93be8ee8ef36c6982c>:0

I can see two options to fix this right now:

1. make the "fake" DLCs valid metadata
2. force a refresh after a client update (needs to be done for each instance, of course).

Option one sounds like a good idea, but option two also does 😄 (maybe do both?)
I'd say it's generally a good idea to force repo refreshes after client updates, to fetch all modules that have been ignored before due to a future spec version. This also applies to normal modules, not only DLCs.

[HebaruSan]

Mods with DLCs as dependencies are selectable, and throw a misleading/wrong error if you try to install them:

MakingHistoryEngineTweaks is not installable without MakingHistory-DLC for me:

Are you talking about CmdLine maybe? There I see the same message as for a normal missing dependency.

1. make the "fake" DLCs valid metadata

Done in latest commit.

2. force a refresh after a client update (needs to be done for each instance, of course).

I see the appeal of this, but whether to do a refresh is (and arguably should be) up to the user. The app should still work even if the user chooses not to refresh.

[DasSkelett]

Mods with DLCs as dependencies are selectable, and throw a misleading/wrong error if you try to install them:

MakingHistoryEngineTweaks is not installable without MakingHistory-DLC for me:

[image]

Are you talking about CmdLine maybe? There I see the same message as for a normal missing dependency.

It says not indexed in the picture, did you remember to force-update the repository after switching to the v1.28.0 compiled client again and to update the CHANGELOG.md before compiling?

Because for me MakingHistory does not show as not indexed, and so EngineTweaks is "installable", despite MakingHistory not being installed:

[HebaruSan]

did you remember to force-update the repository after switching to the v1.28.0 compiled client again and to update the CHANGELOG.md before compiling?

Ah! No, I forgot some of the setup; now I see what you see. Will investigate.

[HebaruSan]

CompatibilitySorter needed an update to account for DLCs now being in the provides index. Should be better now.

[DasSkelett]

Think it's good to go now. Really nice addition to our DLC handling, future-proofs us for more DLCs to come. Thanks!