search

KSP-CKAN / NetKAN

Metadata files used by the NetKAN/CKAN indexer for KSP
Creative Commons Zero v1.0 Universal
72 stars 338 forks source link

Incomplete compatibility for mods without version files #8291

Closed Sinomen closed 3 years ago

Sinomen commented 3 years ago

CKAN 1.29.2 shows, since KSP 1.11.0 came out, for several mods wrong version compatibility information. In particular, even older mod versions are displayed as only to be compatible with KSP 1.11.0, despite the respective mods being several months old and listed earlier as compatible with older KSP versions.

Examples: Snark's mods Missing History 1.8.2, Indicator Lights 1.7, Better Burn Time 1.10, Variant Persist 1.1.

I first thought it's related to Spacedock, but Chatter as well as other mods, which are also hosted there, show correct data.

Forum discussion: https://forum.kerbalspaceprogram.com/index.php?/topic/126111-111-betterburntime-v110-provides-extra-burn-time-indicators-on-the-navball-for-suicide-burns-target-rendezvous/&do=findComment&comment=3900976

HebaruSan commented 3 years ago

Just explained this on that thread, it's because it's on SpaceDock and @KSPSnark does not use .version files (Chatterer has one):

CKAN needs some trustable source for its compatibility info. In this case, all the author is providing is, "Compatible with 1.11.0" via SpaceDock's version selector. I'm happy to work with Snark to add a .version file, but so far it doesn't sound like he's interested.

Sinomen commented 3 years ago

I see, and thank you already for your efforts and time :)

Btw, another mod coming to my mind is scatterer, latest version 0.0722 is as per the mod author and my own experience perfectly compatible with KSP 1.10.1, yet it is listed in CKAN for KSP 1.11.0 only.

HebaruSan commented 3 years ago

Yes, same thing, Scatterer also doesn't have a version file.

Sinomen commented 3 years ago

I see, yes. And every mod version shows only a single KSP version to be compatible, same with Snark's mods, and in contrast to say Chatterer which shows version ranges.

Long story short: probably not fixable with reasonable efforts?

HebaruSan commented 3 years ago

I would say that from CKAN's point of view, it's more like "already fixed" since there is already a complete and flexible system in place to represent mod compatibility: .version files. Anything we would do on this front would essentially be a re-invention of what .version files already do. The quality of the metadata does suffer because some authors choose not to use them, but all we can do on that front is encourage and assist when someone wants to create one.

Sinomen commented 3 years ago

And manually inserting .version files into the CKAN repository is not feasible, I reckon. Oh well, I can live with that, knowing my mods. And beginners usually always go to the latest KSP version anyways (KSP 1.11.0 isn't an option for me due to too many game breaking bugs).

Question/suggestion: would it be possible to have a feature in CKAN like a checkbox "ignore warnings for this version of this mod" when starting KSP?

HebaruSan commented 3 years ago

And manually inserting .version files into the CKAN repository is not feasible, I reckon.

Right, they need to be inside the mod's downloaded ZIP file.

Question/suggestion: would it be possible to have a feature in CKAN like a checkbox "ignore warnings for this version of this mod" when starting KSP?

There's a request for that on file at KSP-CKAN/CKAN#2955. So far it hasn't been worked on, but pull requests are welcome.

Sinomen commented 3 years ago

Understood. Unfortunately I can't do programming... :(

Thanks again for your time and efforts, much appreciated! :)

HebaruSan commented 3 years ago

Although maybe we can do something with "x_netkan_override": { "ksp_version_min" }...

Sinomen commented 3 years ago

Sounds promising, let me know if I can be of any help.

HebaruSan commented 3 years ago

Notes to self in case I have to repeat any of this...

jq -rM 'select(.author == "Snark") | .identifier' ~/Downloads/KSP/CKAN-meta/*/*.ckan | uniq > identifiers
for ID in $(cat identifiers); do git -C ~/Downloads/KSP/CKAN-meta log -n 1 --name-only --pretty= -- $ID; done > files 
for F in $(cat files) ; do git -C ~/Downloads/KSP/CKAN-meta log --pretty=oneline --reverse -- $F | head -n 1 ; done > commits

That gets me the SHA of the original commit of the current release of each Snark mod. Now just need to pull the contents and extract the .ksp_version_* properties...

for SHA in $(cat commits|awk '{print $1}'); do git -C ~/Downloads/KSP/CKAN-meta show --pretty= $SHA | egrep '^\+[^+]' | sed -e 's/^.//' ; done > json
jq -rM '[.version, .ksp_version, .ksp_version_min, .ksp_version_max, .identifier] | @tsv' < json > compats

Here they are, the original compatibilities of each release!

1.1 1.8.0           AntennaSleep
1.2 1.8.0           AttitudeAdjuster
1.2 1.8.0           AutoAGL
1.10    1.8.0           BetterBurnTime
1.4 1.8.0           BetterCrewAssignment
1.3 1.8.0           DefaultActionGroups
1.0 1.0.5           EveBiomesPlus
1.6.2   1.10.0          IndicatorLightsCommunityExtensions
1.7 1.10.0          IndicatorLights
1.1 1.2.1           JoolBiomes
1.9.1   1.11.0          MissingHistory
1.1 1.0.5           RadiatorToggle
1.4 1.8.0           SimpleFuelSwitch
1.2 1.5.1           Snarkiverse
1.2 1.8.0           VABReorienter
1.1 1.8.0           VariantPersist
1.0 1.0.5           VectorRepurposed
1.1 1.3.0           VesselCategorizer

Ugh.

2571 [1] FATAL CKAN.NetKAN.Program (null) - ksp_version mixed with ksp_version_(min|max)
HebaruSan commented 3 years ago

Xref KSP-CKAN/CKAN#202. SpaceDockTransformer only sets ksp_version (and only if no ksp_version_* properties are already set), VersionedOverrideTransformer can only delete properties and add other properties. What we want in this case is to change ksp_version to ksp_version_max so we can add a minimum, and there's no way to do that.

What might be better would be to replace that "mixed with" check wtih some code to massage the values it finds, e.g. to turn this

    "ksp_version_min": "1.8.0",
    "ksp_version": "1.11",

... into this:

    "ksp_version_min": "1.8.0",
    "ksp_version_max": "1.11",

AvcTransformer already has similar capabilities, maybe they could be made more generic.

Sinomen commented 3 years ago

Maybe we're thinking too complicated, what about if:

Snark marks the existing BBT 1.9.1 at Spacedock as compatible with KSP 1.10.1

He then adds a line to his changelog and uploads the mod again to Spadedock, this time telling Spacedock that the new upload is BBT version 1.9.2, being compatible with KSP 1.11.0

Result: two mod-wise identical uploads, but with different version numbers for different KSP version. And he doesn't have to use .version files for this.

What do you think?

HebaruSan commented 3 years ago

Asking mod authors who don't use or care about CKAN to change what they're doing "just for CKAN" doesn't usually turn out very well.

Sinomen commented 3 years ago

Sigh... I guess you're right. :(

No idea how people can not care or use it, how else could I handle managing and updating 100+ mods?

Sent you a PM in the forums.

HebaruSan commented 3 years ago

OK, the problems with overrides are solved in KSP-CKAN/CKAN#3265, which allows us to add min version overrides to Snark's mods, which we did in #8309. Hopefully folks will point out any additional specific cases that would benefit from this.

Sinomen commented 3 years ago

Hey o/

Big thanks! :)

Sinomen commented 3 years ago

Ah, "Hopefully folks will point out any additional specific cases that would benefit from this." Scatterer is coming to my mind here too, I have 0.0723 installed, no issues with KSP 1.10.1

HebaruSan commented 3 years ago

OK, based on the title of the forum thread and the release notes, it looks like the current Scatterer is supported back to KSP 1.9. Should be trivial to set that up now...

Sinomen commented 3 years ago

And then there are mods which are listed as compatible with older KSP versions, but actually aren't. Totally not a CKAN/NetCAN issue at all, but still annoying. Example: KIS https://forum.kerbalspaceprogram.com/index.php?/topic/149848-minimum-ksp-version-18-kerbal-inventory-system-kis-v126/&do=findComment&comment=3898830 The MJ team has thankfully changed the compatibility information in the meantime, recent dev versions #1035 and onward were flagged as compatible with KSP 1.10.1 but aren't..

Mentioning this as it will almost for sure cause question being posted in the technical support forums where we always tell the folks to be sure to have compatible mod versions to be installed.

HebaruSan commented 3 years ago

KIS has a version file; if you have corrections for its compatibility, you may submit them here:

We have zero control over MJ's dev versions, that's all hand rolled by Sarbian himself.

Sinomen commented 3 years ago

"KIS has a version file" - that's up to Igor as the author, isn't it?

And sarbian & team did correct it, all good :)

As I said, totally not a CKAN issue.

HebaruSan commented 3 years ago

"KIS has a version file" - that's up to Igor as the author, isn't it?

Yes. The way it works is that you can edit the file, and then GitHub will present your changes to the owner to approve or reject them.

HebaruSan commented 3 years ago

Scatterer updated in #8314, should be live in CKAN in around 5-15 minutes.

Sinomen commented 3 years ago

KIS: maybe I should do this, but I'd need to test it before, I think. Proposing not test-fact-checked changes, I don't like the idea, feels wrong.

Sinomen commented 3 years ago

"Scatterer updated in #8314" - no offense, but you did inform Blackrack about this? I think he should know.

HebaruSan commented 3 years ago

"Scatterer updated in #8314" - no offense, but you did inform Blackrack about this? I think he should know.

Why? We're not changing the actual mod, just updating CKAN's metadata to reflect the author's expressed wishes in the forum thread title and release notes.

Sinomen commented 3 years ago

Ohkay... it's probably just old fashioned me then, I'd see it as common courtesy. Then, I'm the guy who still opens the door for the lady, help her with her coat and chair, and insist paying the bill in the restaurant.

HebaruSan commented 3 years ago

Oh believe me, we reach out to mod authors all the time for more complex stuff, for example this chat with Angel-125 about a revamp of how his mods are handled:

But in this case it's as simple as, the mod author said what the compatibility was, CKAN wasn't reflecting it, and now it is. Nobody's going to be confused or inconvenienced. Sometimes it's better to just make things work smoothly and stay out of the way. :service_dog:

Sinomen commented 3 years ago

I take your word for it! :)

I guess you're right. Won't (and can't ) step out of my shoes though, me being me.

And yes, seeing how it went with Snark's mods, which were what made me open this issue in the first place, Which kind of closes the circle.

Sinomen commented 3 years ago

As anticipated, not-so-experienced players run into the issue of mods wrongly being flagged as compatible with older KSP versions when they are actually not. https://forum.kerbalspaceprogram.com/index.php?/topic/149848-minimum-ksp-version-111-kerbal-inventory-system-kis-v128/&do=findComment&comment=3909956

Igor told me "I assume it may not be compatible" when I asked him about KIS 1.27 and KSP 1.10.1 He also stated explicitly that 1.28 is only compatible with KSP 1.11.

May I suggest that you change the CKAN compatibility information accordingly?

HebaruSan commented 3 years ago

@ihsoft is in full control of KIS's compatibility metadata:

https://forum.kerbalspaceprogram.com/index.php?/topic/197082-ckan-the-comprehensive-kerbal-archive-network-v1292-freedman/&do=findComment&comment=3910123

HebaruSan commented 3 years ago

Submitted ihsoft/KIS#385 to try to help. :crossed_fingers:

Sinomen commented 3 years ago

Ok, now that would be my 1st ever PR, can you please check that I didn't botch anything.

HebaruSan commented 3 years ago

Nice! You've got a fork and a branch and your changes are committed, so far so good. But there are a few more steps to get the pull request created. If you go here:

https://github.com/KSP-CKAN/CKAN-meta

... there should be a link or button somewhere that mentions creating a pull request. Clicking that will give you one more set of title/body to edit, and then there's another submit button towards the bottom.

Normally I do this with a button that shows up after saving changes, but it's easy to miss if you aren't used to it.

DasSkelett commented 3 years ago

Alternatively, click on this link and it should take you to the pull request creation page: https://github.com/Sinomen/CKAN-meta/pull/new/patch-1

Sinomen commented 3 years ago

OK, done, is it correct?

HebaruSan commented 3 years ago

Yeah, looks great! The validation scripts are installing KIS with your changes to test them out:

https://github.com/KSP-CKAN/CKAN-meta/pull/2235/checks

Sinomen commented 3 years ago

Thank you for your patience with old me!

But even though I feel a bit like a toddler now, I think it's better to ask before botching something up.

Sinomen commented 3 years ago

Nah, I did botch it up :(

Should have been min. KSP version, but stupid me changed max version.

I'm sorry.

HebaruSan commented 3 years ago

Nah, I did botch it up :(

Should have been min. KSP version, but stupid me changed max version.

I'm sorry.

Don't worry about it, those changes are safely isolated until they're approved and merged. No harm done. And thanks for helping out! Things like this come up all the time and it's great when folks are willing to pitch in!

Sinomen commented 3 years ago

Thanks to @DasSkelett, I could correct that mistake, should be fine now. wiping the sweat from my eye brows

But even as I had a bad start, let me know if I can help in any way.

HebaruSan commented 3 years ago

But even as I had a bad start, let me know if I can help in any way.

Less than an hour for getting your first pull request merged is pretty fast. And the hope is that you will tell us when you can help, since you're clued in to which mods are compatible!

Sinomen commented 3 years ago

Just created another PR :)

Sinomen commented 3 years ago

As Github is not specific to CKAN, is there kind of a manual where I can find these instructions instead of bothering you again and again with such questions?

HebaruSan commented 3 years ago

Yes, but there's quite a lot of it and it might not always be easy to find the one thing you need to know:

Sinomen commented 3 years ago

I meant the CKAN specific stuff, which naturally won't be part of the Github help pages. Like: PR for current versions should be done in the mod author's repository, for older versions at CKAN. These kind of information.

HebaruSan commented 3 years ago

Hmm, good question. We have a wiki wth a "contributing" section, but it looks like those pages are about developing CKAN's C# code rather than maintaining metadata. The closest thing is probably the advanced section of the adding-a-mod-to-CKAN page:

At this point answering your questions as needed is probably going to be a lot easier than trying to write up a complete guide, but we can make a note about doing that at some point.

Sinomen commented 3 years ago

This one might be a bit tricky: https://github.com/KSP-CKAN/CKAN-meta/pull/2237

Like the Snark mods, no min or max information, just ksp_version

As far as I can see, the mod author does not provide a version file, if I'm not mistaken.

Sinomen commented 3 years ago

That was my CKAN version-wise oldest mod (KSP 1.6), I think it's not worth the time to go through the many mods which are flagged compatible for KSP 1.7 and onwards, or is it?