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

Change ckan relationships #2

Merged
merged 2 commits into from
Aug 31, 2023
Merged

Conversation

Qazerowl
Copy link
Contributor

@Qazerowl Qazerowl commented Aug 29, 2023

I apologize for making a pull request that attempts to address two things at once:

First item:
I have added a couple conflicting packages to the express-install package. I blocked the outdated SmokeScreen-RO and CustomBarnKit-RO packages, as if a user has ckan compatiblity with older versions of ksp enabled, the express install will ask the user to pick between them and the up-to-date versions with the current metadata.
More controversially, I also blocked SCANsat, StageRecovery, and KerbalEngineerRedux. These mods are all of the mods on "The Blacklist" that are available on ckan. SCANsat and KER can both be made compatible with additional work after installation, but this falls into the realm of an "advanced user" who can use the non-express install if they want to use those mods.
It is more important, imo, to assure that the express install "just works" than to make it easy to customize. The adjustments I made to the non-express install make it much easier to use, so advanced users who do want to use those mods shouldn't find it too difficult to switch over from the express install.

Second item:
Currently, the express install package asks you to select a graphics preset, and it is the graphics preset metapackage that does the real "meat and potatoes" of installing ROEngines, ROTanks, etc. In conjunction with this "sister" PR, I would like to move the non-graphical dependencies out of the preset metapackages and into the express install itself. This makes the differences between the graphical presets easier to understand, makes it clearer to users what the express-install is actually doing, and makes maintaining the packages just a touch easier. The non-graphical dependencies between all 3 preset metapackages are already identical, so there are no changes to the actual end result of the installation.

I am happy to resubmit an edited PR if only one of the two changes is desired.

@NathanKell
Copy link
Member

Gotta think about the rest of the PR, but stuff cannot be shuffled between the graphics netkans and this. The only reason that stuff is in the graphics netkans is because the way CKAN dependency resolving works, it has to be, otherwise CKAN gets confused and prompts you for a bunch of different Provides choices. Believe me, I'd much prefer to not have to duplicate all that! :)

@HebaruSan
Copy link

the way CKAN dependency resolving works, it has to be, otherwise CKAN gets confused and prompts you for a bunch of different Provides choices

I thought we fixed that a release or two ago, see KSP-CKAN/CKAN#3476.

@Qazerowl
Copy link
Contributor Author

After running a couple tests (and changing the order the dependencies are listed in for both repos), I can confirm that this pair of PRs does not result in any additional questions during an express install.

@NathanKell
Copy link
Member

@HebaruSan Oooh, that's awesome, I remember that PR--I didn't realize it got merged!

@Qazerowl thanks for doing this! I'm slightly concerned re: the blacklisting but on balance I think it's correct here.

Also, moving all that stuff out of the graphics side will finally let us fairly simply turn this into the RO/RP-1 Express Install, and offer an install path that involves no career, only RO. I'll need to write some code first for that before we can make that change, but what was holding me back before was the terrible prospect of having another three graphics netkans floating around, just lacking RP-1 in them. :D So thank you!

@NathanKell NathanKell merged commit e58740d into KSP-RO:main Aug 31, 2023
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