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

Adds KerbalKonstructs #27

Merged
merged 3 commits into from
Nov 25, 2014
Merged

Adds KerbalKonstructs #27

merged 3 commits into from
Nov 25, 2014

Conversation

Ippo343
Copy link
Contributor

@Ippo343 Ippo343 commented Nov 14, 2014

Blocked on KSP-CKAN/CKAN#315

If @medsouz is okay with it, I'd like to add KerbalKonstructs to the CKAN.
This file installs the mod into GameData/KerbalKonstructs: @medsouz, is it ok?

File validated and tested (aside from the issue about whether the path is correct).

@medsouz
Copy link

medsouz commented Nov 14, 2014

Go ahead. :)

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 14, 2014

Thank you! I'd wait before merging this one until we hear from the author of KerbinSide (I'm about to open that one too) to check if the installation path is a problem or not.

@medsouz
Copy link

medsouz commented Nov 14, 2014

The installation path should be fine for both. The only potential issue is that KerbalKonstructs is usually in GameData/medsouz/KerbalKonstructs (I'm planning to take out the medsouz directory anyway) and is packaged with KerbinSide like that so it might cause a conflict.

Poking @AlphaAsh

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 14, 2014

That's not an issue, CKAN just ignores the version packaged with KerbinSide :)

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 14, 2014

Okay I just tried installing KerbinSide. It "mostly" works, but I get a missing texture on the app button, plus a nice and long stream of NREs during flight.

I guess this is staying open until we hear from @AlphaAsh :)

@Ippo343 Ippo343 mentioned this pull request Nov 14, 2014
@medsouz
Copy link

medsouz commented Nov 14, 2014

Okay turns out I lied, it won't work unless its in GameData/medsouz/KerbalKonstructs. I forgot I hard coded image paths.

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 14, 2014

Okay, no problem :) Unfortunately I can't figure out how to put it in the right directory, so we are going to need for @pjf to help us out I think.

@AlphaAsh
Copy link
Contributor

Yeah the pathing in KK is mostly hard-coded. If it needs changing, go ahead. I can update KerbinSide's KK easily enough.

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 14, 2014

Thank you AlphaAsh, but I'd really just wait to hear pjf on this one: one of the main points of CKAN is not forcing you to change your packaging, so even if you are available to do that, it would be really better not :)

@AlphaAsh
Copy link
Contributor

KerbinSide can't be the only mod that already includes a dependency in the package so yeah, it'd probably be sensible for CKAN to be able to handle multiple mods in a single package.

EDIT - MM shows up in every other mod package, for example :D

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 14, 2014

Actually it already does, and very nicely :) It doesn't take KK from your zip, it actually downloads another fresh copy and installs that one.

The real problem here is that CKAN insists on putting KerbalKonstructs in

GameData/KerbalKonstructs

instead of the path it's expecting. I'm not familiar enough with how CKAN resolves the paths in the install stanzas, I hoped it was just a matter of saying

"install_to": "GameData/medsouz"

but apparently that's not the way to do it, because it doesn't work :/

@AlphaAsh
Copy link
Contributor

Ah, I getcha. Yeah, you're right that needs looking at on CKAN's side if it's to stay true to its mission statement of not forcing changes on packaging. Like you said. Thanks for the clarification.

@AlphaAsh
Copy link
Contributor

Oh and thank you Ippo for getting KK and KerbinSide sorted with this. I've only just read up on CKAN and really appreciate you doing it. Saves me a job and I'd probably have forgotten by the time I wake up tomorrow :P

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 14, 2014

No problem, we'll keep you up to date :)

@pjf
Copy link
Member

pjf commented Nov 15, 2014

KSP-CKAN/CKAN#315 may be required if we need to specify an output folder. Let me bump up the priority on that ticket.

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 18, 2014

So, I tried modifying the netkan file. As I understand it, the new install stanza I added should work, but it doesn't. NetKAN validates the file with no problem and creates the CKAN file, but then I get a BadInstallLocationKraken when I try to install that file, and I really don't understand why.

Unknown install_to GameData/medsouz

I must be missing something really obvious, I'm sure...

@AlphaAsh
Copy link
Contributor

Is there a reliance on KK being catalogued as well? Otherwise no other ideas here. I may code but CKAN is magic compared to the stuff I can do.

@AlexanderDzhoganov
Copy link
Member

@Ippo343 are you sure you're running the CKAN version with the install_to changes - KSP-CKAN/CKAN#394 ?

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 18, 2014

@AlexanderDzhoganov: I am not. But I shouldn't need to, because this is a normal install stanza:

"install_to": "GameData"

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 25, 2014

This issue has bothered me for days. I really have no idea why things were failing last week, but I have just finished testing this using the Protium release and it works fine without changes.

@Ippo343
Copy link
Contributor Author

Ippo343 commented Nov 25, 2014

Awwww yisss! :D Travis didn't like it that I pulled the license from KS. Now it's good :)

AlexanderDzhoganov added a commit that referenced this pull request Nov 25, 2014
@AlexanderDzhoganov AlexanderDzhoganov merged commit fee3875 into KSP-CKAN:master Nov 25, 2014
@Ippo343 Ippo343 deleted the KerbalKonstructs branch December 2, 2014 20:15
SpaceTeph pushed a commit to SpaceTeph/NetKAN that referenced this pull request Jan 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants