Override provides prompt with relationship property, check first recommendation in any_of group

[HebaruSan]

Motivation

The RSS/RO/RP-0/RP-1 team reached out to discuss ways to streamline their installation process, see KSP-CKAN/NetKAN#8615 and KSP-CKAN/NetKAN#8688. They hope to reduce the number of decisions the user has to make and provide more information to help the user make good choices.

We're still brainstorming options for improvements, but a few ideas seem to be clear wins so far. This is the first of possibly/hopefully several changes resulting from that discussion.

Problems

• When a depends clause matches multiple modules (via either provides or any_of), only a standard prompt based on the identifier(s) is shown (and it varies between CmdLine/ConsoleUI/GUI). The RSS/RO/RP-0/RP-1 team would like to be able to override that prompt in order to give users additional help and clues about which module to pick.
• The RSS/RO/RP-0/RP-1 team has been taking advantage of any_of relationships for conflicting recommendations because they don't all show up as checked:
  
  Rather, they're all unchecked, which is still not great because these are supposed to be recommendations, on-by-default:
  

Changes

• Now the standard prompt for choosing a dependency is the same across CmdLine/ConsoleUI/GUI (copied from the existing English text for GUI)
• Now you can add a choice_help_text property to a relationship to override the text the user sees when choosing among multiple modules:

  depends:
    - any_of:
      - name: Mod1
      - name: Mod2
      choice_help_text: Pick Mod1 unless you like Mod2
    - name: RSSTextures
      choice_help_text: Pick the 16K version if your computer can handle it

  
• Now if you recommend an any_of group or an identifier provided by multiple modules, the first one that shows up on screen will be checked, and the rest will be unchecked:
  

After this, it should be possible to guide users into a good RSS/RO/RP-0/RP-1 install more often and more easily.

[HebaruSan]

@NathanKell, @ppboyle, @StonesmileGit, @siimav, @jwvanderbeck:
There's a test build here if you want to try these changes:

• https://github.com/KSP-CKAN/CKAN/suites/3403036368/artifacts/80394317

To test the choice_help_text property thing, you can save a copy of the current .ckan files to your local drive, edit them to add the new property, then use File → Install from .ckan to install them (one at a time, unfortunately):

• https://github.com/KSP-CKAN/CKAN-meta/blob/master/RP-0/RP-0-v1.10.5.0.ckan
• https://github.com/KSP-CKAN/CKAN-meta/blob/master/RealismOverhaul/RealismOverhaul-v14.3.0.0.ckan
• https://github.com/KSP-CKAN/CKAN-meta/blob/master/RealSolarSystem/RealSolarSystem-v18.1.5.ckan

[NathanKell]

Awesome! I'll check this out after supper (am currently fighting with porting my Chrome profile over to a new PC, bleh). Thanks!!

[NathanKell]

Worked like a treat! ❤️

[DasSkelett]

Yeah given that the spec defines (a top-level) comment as

A comment field, if included, is ignored. It is not displayed to users, nor used by programs. Its primary use is to convey information to humans examining the CKAN file manually

https://github.com/KSP-CKAN/CKAN/blob/master/Spec.md#comment

I wouldn't reuse the key like that, even if it's not top-level. Maybe help or help_text instead? Or hint?

[HebaruSan]

Maybe help or help_text instead? Or hint?

Those are good. I'd like to add a short adjective prefix as well that explains what the hint is about.
choice_hint? choice_help_text? choice_prompt?

[NathanKell]

I think choice_help_text is clearest. Sent by my thumbs, slowly.

…

On Tue, Aug 3, 2021, 1:45 PM HebaruSan ***@***.***> wrote: Maybe help or help_text instead? Or hint? Those are good. I'd like to add a short adjective prefix as well that explains what the hint is about. choice_hint? choice_help_text? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3426 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABHH4ISBUAXZ2RQYS5TQGHTT3BIP5ANCNFSM5BLZPT6A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

[HebaruSan]

choice_help_text it is! Force-pushed; new build:

• https://github.com/KSP-CKAN/CKAN/suites/3403036368/artifacts/80394317

[DasSkelett]

I was wondering why RCSBuildAid and RCSBuildAidCont both start checked, despite being in an any_of in the RealismOverhaul netkan, turns out they are both listed as first-level recommendations in the RP-0 netkan.

 		{ "name" : "RCSBuildAid" },
		{ "name" : "RCSBuildAidCont" },

... which has been mentioned in the last line here: KSP-CKAN/NetKAN#8688 (comment)

@NathanKell I think you might want to adjust that to be in an any_of as well.

[DasSkelett]

choice_help_text doesn't work yet for any_of:

`any_of` should not be combined with other properties

CKAN/Core/Converters/JsonRelationshipConverter.cs

Lines 29 to 32 in a833159

	if (child.Properties().Count() > 1)
	{
	throw new Kraken("`any_of` should not be combined with other properties");
	}

[HebaruSan]

Fixed, I think.

[DasSkelett]

Should we also deduplicate items from the list? Of course, the metadata should be changed as well, but I think it doesn't hurt to fix it in the client.

Edit: adding .Distinct() to the RelationshipDescriptor.ModuleRelationshipDescriptor.LatestAvailableWithProvides and RelationshipDescriptor.AnyOfRelationshipDescriptor.LatestAvailableWithProvides. It does loose the ordering if there are are equal items, but IMHO you couldn't rely on that anyway with duplicated ones.

[HebaruSan]

I think updating just AnyOfRelationshipDescriptor.LatestAvailableWithProvides is sufficient, we shouldn't be generating duplicates based on provides.

[DasSkelett]

Yeah, that makes sense. Thanks!

[DasSkelett]

Okay, I think this PR works well as is. If we find more fixes that are needed for KSP-CKAN/NetKAN#8701, it might be wise to do them as separate PR, especially if they do dig into the RelationshipResolver.

[HebaruSan]

choice_help_text doesn't work yet for any_of:

`any_of` should not be combined with other properties

CKAN/Core/Converters/JsonRelationshipConverter.cs

Lines 29 to 32 in a833159

	if (child.Properties().Count() > 1)
	{
	throw new Kraken("`any_of` should not be combined with other properties");
	}

@DasSkelett, should we bump the spec version for this? That would give us a way to protect old clients from newer metadata with extra properties alongside any_of. (I.e., netkan could require a module to have spec_version 1.31 or later if any_of has sibling properties.)

[DasSkelett]

Indeed, the same though came to my mind yesterday. I think we should do that.