Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RO/RP-1 express install netkans #8701

Merged
merged 1 commit into from
Aug 10, 2021
Merged

Add RO/RP-1 express install netkans #8701

merged 1 commit into from
Aug 10, 2021

Conversation

NathanKell
Copy link
Contributor

@NathanKell NathanKell commented Aug 8, 2021

Purpose

This adds "express install" packages (a main package and three choices for graphics level) to allow express installation of RO/RP-1/RSS/etc.

Motivations

RO/RP-1 users often ask for a simple gamedata zip, or "what mods should I install", or the like--to the point of asking which recommended or suggested mods to install, which graphics mods, etc.

Changes

This adds a single express install netkan that depends on the core mods for an RP-1 install, and then depends on an RSSGraphics package. That package is provided by three express packages, each reporting only as a detail level, which depend the various RSS-Textures packages, visual enhancements, and Canaveral terrain/pads necessary for a complete experience.

Metanetkans for easy reference:

Issue ref: #8688

Once KSP-CKAN/CKAN#3426 is merged, I will update the local netkans with comments on the graphics choice.

@NathanKell
Copy link
Contributor Author

Side note: I just learned NetKAN's inflation error reports the line number and error for an internal JSON representation of the YAML spec. I was at first quite surprised to hear of an error on a line, and with a character, that my file did not include!

@NathanKell
Copy link
Contributor Author

I believe this is now correct, though the checks are still failing--this seems to be because the CLI version of CKAN defaults to installing recommended mods, and the main netkan depends on both Ven's New Parts (which recs Ven's Stock Revamp) and on ReStock, and of course VSR and ReStock conflict.

Is there some way to get the checker to run without auto-installing recs?

@HebaruSan
Copy link
Member

Side note: I just learned NetKAN's inflation error reports the line number and error for an internal JSON representation of the YAML spec. I was at first quite surprised to hear of an error on a line, and with a character, that my file did not include!

Does it? I don't think that's the case.

@HebaruSan
Copy link
Member

HebaruSan commented Aug 8, 2021

I believe this is now correct, though the checks are still failing--this seems to be because the CLI version of CKAN defaults to installing recommended mods, and the main netkan depends on both Ven's New Parts (which recs Ven's Stock Revamp) and on ReStock, and of course VSR and ReStock conflict.

Is there some way to get the checker to run without auto-installing recs?

The checker is highlighting a real problem. These probably shouldn't recommend (depend on? not sure yet) conflicting modules.

@HebaruSan
Copy link
Member

Oops, forgot these were metanetkans, I'll remove those depends, but can you add them to your repo?

@HebaruSan
Copy link
Member

We'll need to copy this metadata locally and try installing with GUI to make sure the desired work flow happens. The command line tools are already easier for the RP-0 identifier because they don't interactively prompt for recommends or suggests.

@HebaruSan
Copy link
Member

We'll need to copy this metadata locally and try installing with GUI to make sure the desired work flow happens. The command line tools are already easier for the RP-0 identifier because they don't interactively prompt for recommends or suggests.

What I'm trying in case anyone wants to replicate it:

  1. Make a CKAN-meta-rp1express folder somewhere locally (doesn't matter where)
  2. Create RP-1-ExpressInstall-v1.0.ckan, RSSGraphics-ExpressInstall-Low-v1.1.ckan, RSSGraphics-ExpressInstall-High-v1.1.ckan, and RSSGraphics-ExpressInstall-Medium-v1.1.ckan in that folder with their contents as reported by the validation script
  3. Make an archive of that folder: tar czf rp1express.tar.gz CKAN-meta-rp1express
  4. Add the archive as a CKAN repo: ckan repo add rp1express 'file:///full/path/to/rp1express.tar.gz'
  5. Launch GUI, click Refresh to load the new repo, start testing
    image

@HebaruSan
Copy link
Member

My compatible versions are 1.10–1.12. Initial change set:

image

Click Apply, one recommendation appears:

image

Click Continue, prompted to choose RSS textures, I'll install the 2K package:

image

Prompted to choose KSPBurst or KSPBurst Lite, I'll pick the non-lite one:

image

Prompted to choose RSS Graphics, I'll try High to see how it handles a mismatch with the textures:

image

Install fails:

image

So it doesn't look like the RSSGraphics-ExpressInstall relationships are working as intended. I was under the impression that that choice was supposed to be presented early and drive most of the rest of it. Probably need to reorder the depends list a bit.

@HebaruSan
Copy link
Member

You don't need to use any_of for MechJeb because the dev builds have "provides": "MechJeb2". A simple depends on MechJeb2 will prompt the user to choose if they have the dev repo configured. The any_of just makes the dev build show up twice:

image

@DasSkelett
Copy link
Member

So it doesn't look like the RSSGraphics-ExpressInstall relationships are working as intended. I was under the impression that that choice was supposed to be presented early and drive most of the rest of it. Probably need to reorder the depends list a bit.

Even reordered it prompts me for a RSSTextures resolution later on. We might have to fix our RelationshipResolver behavior to recalculate it after TooManyModsProvides selections have been made.
Although thinking about it, doesn't it basically restart the installation from start after a selection has been made? So it should already include them for relationship resolution, no? 🤔

@HebaruSan
Copy link
Member

@HebaruSan
Copy link
Member

HebaruSan commented Aug 8, 2021

I wonder if the ordering of the depends inside the low/med/high modules is affecting it. Depending on RSS before the textures might make it pull in RSSTextures before the specific texture dependency is applied:

      "depends": [
          {
              "name": "RealSolarSystem"
          },
          {
              "name": "RSSTextures4096"
          }
      ],

EDIT: Nope, fixing that (putting RSSTextures4096 first) still doesn't suppress the RSSTextures prompt. 😕

@NathanKell
Copy link
Contributor Author

I believe this is now correct, though the checks are still failing--this seems to be because the CLI version of CKAN defaults to installing recommended mods, and the main netkan depends on both Ven's New Parts (which recs Ven's Stock Revamp) and on ReStock, and of course VSR and ReStock conflict.
Is there some way to get the checker to run without auto-installing recs?

The checker is highlighting a real problem. These probably shouldn't recommend (depend on? not sure yet) conflicting modules.

Perhaps the later bit (where you're attempting to install via GUI) supersedes this but, see here for RP-0's netkan. The issue arises because the express install netkan depends on Restock, but in RP-0.netkan Ven's is the first of the two any_of recs so it is preferentially installed as the CLI blasts through every single mod's recommendeds.

@HebaruSan
Copy link
Member

Ven's is the first of the two any_of recs so it is preferentially installed

I'm looking for code that could do this, and I'm not finding it. The resolution of the any_of rec should return both modules, which should then cause the user to be prompted to choose.

@NathanKell
Copy link
Contributor Author

@HebaruSan ah then I misunderstood, so the issue is actually worse in CLI than I thought, i.e. it will turn "any_of" into "all_of"? :D

@HebaruSan
Copy link
Member

@HebaruSan ah then I misunderstood, so the issue is actually worse in CLI than I thought, i.e. it will turn "any_of" into "all_of"? :D

No, it should be prompting.

@NathanKell
Copy link
Contributor Author

Oh, even prompting in CLI in the action? Yeah that does seem weird.

Well, I'm happysad I provided a great way to find client bugs, I guess? :\

@NathanKell
Copy link
Contributor Author

( MM dependency added here KSP-RO/RP-1-ExpressInstall-Graphics@3121b77 per note upthread.)

@HebaruSan
Copy link
Member

EDIT: Nope, fixing that (putting RSSTextures4096 first) still doesn't suppress the RSSTextures prompt.

I think we're getting tripped up by the relationship resolver working depth-first, essentially. After we pick RSSGraphics-Whatever out of the list, we restart the install with two modules in the change set, RP-1-ExpressInstall and RSSGraphics-Whatever, in that order. RSSGraphics-Whatever is able to satisfy relationships itself (this is why we don't immediately get prompted to make the same choice again), but its dependencies are not looked at yet; rather, the relationship resolver tries to satisfy all of RP-1-ExpressInstall's dependencies (and depends-of-depends) before it looks at any of RSSGraphics-Whatever's depends.

So the current RP-1-ExpressInstall module approach does not seem to be workable. Rather, it seems that the module the user chooses up front needs to have all the "deep depends" in it, so the new modules could be something like:

  • RP-1-ExpressInstall-Low
  • RP-1-ExpressInstall-Medium
  • RP-1-ExpressInstall-High

Each of those modules would have the depends that are currently in the RSSGraphics modules (with the detail-specific depends such as the 4K textures as early as possible in the list), and then the depends that are currently in RP-1-ExpressInstall, later in the list. That way it should be possible to pre-satisfy the RSSTextures and other dependencies early on.

Sorry for all the confusion, but this is getting really deep into the functionality of the relationship resolver, and we usually treat it as a black box not to be touched unless we're really sure what we're doing.

@HebaruSan
Copy link
Member

Alternately, if you really want to have a "main" module branching into "detail level" modules, the "main" module could depend only on those "detail level" modules and not on RP-0, RO, RSS, etc.

@NathanKell
Copy link
Contributor Author

Ah, I see what you mean. And you answered the question I was in the middle of asking. It is definitely the case that I want a main module, because as I understand the spec, I need to include an installable with each package or it gets treated as a metapackage and (per an issue from lamont a few years back) conflicts won't work.

I'll make that change.

@NathanKell
Copy link
Contributor Author

Also, that route is required as well to take advantage of KSP-CKAN/CKAN#3426 - we need a main package to have a depends with a comment.

@NathanKell
Copy link
Contributor Author

@HebaruSan this change is now done. 🤞

@HebaruSan
Copy link
Member

OK, I'm taking a look now and I restarted the validation script. We might still need to reorder some of the depends, but the overall structure is a better fit to how the relationship resolver works now.

@NathanKell
Copy link
Contributor Author

Thanks! And thanks for taking the time to dig through this all (especially the deep rabbit holes of CKAN black magic :D )

@HebaruSan
Copy link
Member

Next test. Initial change set is a lot smaller, as expected:

image

Click Apply. One recommendation again:

image

Then the detail level prompt comes up:

image

I'll pick Low. Womp womp, the next prompt is the textures again:

image

Try putting the 4K/8K/whatever depends as early as possible in the list, before anything that could possibly depend on anything that depends on RSSTextures.

@NathanKell
Copy link
Contributor Author

@HebaruSan Done, and inflator restarted.

@HebaruSan
Copy link
Member

I need to take off for an event in a bit, but I'll be back in a few hours. The steps I listed above should still work if you're feeling adventurous.

@HebaruSan
Copy link
Member

@DasSkelett, if ckan_meta_tester published its repo tar.gz file as a downloadable artifact, that could make it a lot easier to do client testing like this.

@NathanKell
Copy link
Contributor Author

WOOHOO! (although sad robot noises at the beleaguered actionrunner :D )

@NathanKell
Copy link
Contributor Author

Per my mistaken PR, in trying the express installer out I still got the prompt to choose KSPBurst version. This is despite KSPBurst-Full being the second line of the depends, here: https://github.com/KSP-RO/RP-1-ExpressInstall-Graphics/blob/main/RP-1-ExpressInstall-Graphics-High.netkan#L15

@HebaruSan
Copy link
Member

I think that means you've got some older versions of the metadata active, from before the depends was updated to -Full. Make sure to remove any testing repos and click Refresh to get the latest official metadata.

@NathanKell
Copy link
Contributor Author

Nope, that was the first thing I did on launching CKAN. :(

@NathanKell
Copy link
Contributor Author

(To double check I closed CKAN, started it again, and refreshed again, same flow.)

@HebaruSan
Copy link
Member

Check your repositories.

@NathanKell
Copy link
Contributor Author

Only one listed, default, pointing to the official archive.

@NathanKell
Copy link
Contributor Author

image

@HebaruSan
Copy link
Member

OK fine, but it still isn't possible to pull in KSPBurst-Lite by depending on KSPBurst-Full. Some other weird thing is happening that you're not hinting at. Is anyone else able to reproduce this?

@NathanKell
Copy link
Contributor Author

It is not possible to pull it in from depending on KSPBurst-Full, but RealAntennas depends on just KSPBurst, and we depend on RealAntennas. Which I think would make it the same sort of issue as the RSS-Textures thing?

I asked on the RO discord for anyone available to try installing in a blank KSP 1.10 install, hope I'm just cursed. :D

@NathanKell
Copy link
Contributor Author

Ugh, and same with the sunflare. Yep, looks to be exactly the same dependency-resolving issue as before with RSS-Textures, and ordering in the netkan has no effect.

@HebaruSan
Copy link
Member

It is not possible to pull it in from depending on KSPBurst-Full, but RealAntennas depends on just KSPBurst, and we depend on RealAntennas. Which I think would make it the same sort of issue as the RSS-Textures thing?

If that was the case, it would happen when I try it.

@NathanKell
Copy link
Contributor Author

I have no explanation for why I am enountering that and you are not, unless I somehow have cached metadata that removing the repo it is from did not fix. Gonna clear my CKAN cache.

@HebaruSan
Copy link
Member

You really were trying all of this in parallel while we were looking at it, though? At which point did this prompt re-appear for you?

@NathanKell
Copy link
Contributor Author

Ah, I think we had a miscommunication then maybe? I had the prompt for KSPBurst, you suggest depending on KSPBurst-Full and moving it above. I did so and you responded that it fixed the GUI here ( #8701 (comment) ) , so I did not do a local retest at that point. Once it was merged down and in the main CKAN repo I tested now and got the prompts.

@NathanKell
Copy link
Contributor Author

Ok, this just got weirder. DRVeyl reports:
"I was presented with EER and one other as recommended mods... then the textures [which we all expect]... then the TF configs. Then it started." With this image:
image

@HebaruSan
Copy link
Member

Yeah, that one I saw. I assumed you changed something after my last test.

@NathanKell
Copy link
Contributor Author

I do not get that prompt.

Wait, what KSP version were you testing against, and what compat versions? I bet that's it. I am on 1.10 with no compat versions; DRVeyl is on 1.10 but with 1.8 and 1.9 checked.

@HebaruSan
Copy link
Member

HebaruSan commented Aug 10, 2021

Yup, TestFlight got moved around in the High Low metanetkan:

image

@NathanKell
Copy link
Contributor Author

Ugh, that would do it. Thought I discarded that. :(

@HebaruSan
Copy link
Member

I do not get that prompt.

That's even more evidence that you're not testing with the current metadata. Maybe try a completely fresh game install?

@NathanKell
Copy link
Contributor Author

Will do.

@NathanKell
Copy link
Contributor Author

Ok, a completely fresh install and linking that to CKAN fixed the issue, so now I'm only seeing evidence of my other stupidity 😂 (the TestFlight thing) rather than that weird situation with busted metadata. I fixed the local netkans so that should go away as well.

@NathanKell
Copy link
Contributor Author

Circling back to say--it's working! Thank you again, from all of us--besides being I think a very positive option for our users, it'll make supporting them way easier on us. CHeers!

@HebaruSan
Copy link
Member

HebaruSan commented Aug 10, 2021

That's awesome and great to hear! 🎉
Where are you expecting to see the bulk of your support feedback? The KSP-RO Discord maybe? I can stop by to observe now and then to see how it's going rather than making you serve as a go-between for everything.

@NathanKell
Copy link
Contributor Author

Yeah, on the RO discord and sometimes on reddit. The forums are pretty quiet these days. But the issues are often less specific questions from installation (though as I've mentioned we do get the "which of these recommended mods should I really install" :D ), and more we find out a good few messages in that it's actually a broken install in one way or another, not a bug in (whatever submod).

And that's super kind of you! You should know that (AFAIK) next to none of the support load is actually from how to use CKAN--your docs, and our CKAN-101, seems to work pretty well, the only issue there has been walking people through adding the MJ-dev repo (which is why I've been poking at sarbs and lamont to merge dev to release, which they did so yay!). Well, that and the "ok but what should I really pick", so having a one-stop "just click this one thing" answer will be super awesome.

@HebaruSan
Copy link
Member

I always forget KSP-CKAN/CKAN#2682 when repository shenanigans happen. 🙄

I think your CKAN client thought your metadata was up to date, because the Etag was the same as the last time you updated, and so it didn't actually do a refresh. Meanwhile you had removed a testing repo but its metadata was still in your registry, and would remain until the master repo's Etag changed. We don't have a good workaround for this currently; I sometimes edit my <KSP>/CKAN/registry.json file and delete the last_server_etag property to force a refresh.

@NathanKell
Copy link
Contributor Author

AHA! Yes that makes perfect sense. I'll keep that in mind in future, it perfectly explains what was going on. The fresh install (or nuking the local CKAN folder, which presumably would have fixed it too but I wasn't taking chances, fresh install it was!) sorts that out by dint of utterly annihilating the json rather than tweaking it. :]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants