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 EmbeddedKOSKirkuits #9758

Merged
merged 3 commits into from
Aug 13, 2023
Merged

Add EmbeddedKOSKirkuits #9758

merged 3 commits into from
Aug 13, 2023

Conversation

rkunze
Copy link
Contributor

@rkunze rkunze commented Aug 12, 2023

@rkunze
Copy link
Contributor Author

rkunze commented Aug 12, 2023

Oops. Sorry for putting the file at the top level, did not intent to do that. Thanks for fixing.

As for the indentation warning: Is that documented somewhere that you want 4 spaces as indent? I usually use two spaces for YAML to avoid too much blank space at the start of the line (especially for deeply nested structures). I'll try to keep it in mind for the next time.

Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the submission!

There aren't specific formatting requirements, we just run everything through yamllint to keep it relatively tidy. It was complaining because there were 4 spaces before use_source_archive.

@HebaruSan HebaruSan merged commit 3370c27 into KSP-CKAN:master Aug 13, 2023
1 check passed
@rkunze
Copy link
Contributor Author

rkunze commented Aug 14, 2023

@HebaruSan Thanks for merging!

I found a little oddity in "ckan list" behaviour on my dev instance for EmbeddedKOSKirkuits: Now that the mod is officially on CKAN, I'd expect "ckan list" on my dev instance (where it is installed manually) to show it as an "unmanaged" mod. But it does not show up at all there, even after an explicit "ckan update" and "ckan scan".

My guess from the verbose output of "ckan scan": "ckan scan" only registers manually installed mods that contain a DLL, and EmbeddedKOSKirkuits is purely a set of ModuleManager patches and does not contain a DLL. Supporting evidence: Same behavior with manually installed versions of SkyhawkScienceSystem and SkyhawkKerbalism, which are pure ModuleManager patch sets as well.

Question: Is this known (and intended) behavior, or should I report a bug against the CKAN client?

Edit: just found KSP-CKAN/CKAN#3525 - should have checked before asking...

@HebaruSan
Copy link
Member

HebaruSan commented Aug 14, 2023

That's basically right, the detection of manually installed mods only looks at DLLs; a bunch of loose .cfg files can't reliably be traced back to what mod they are.

We started a project along those lines in KSP-CKAN/CKAN#3525. I have concerns about performance (potentially parsing a lot of .cfg files could slow things down significantly) and validity of the data being relied upon (a lot of mods mis-use :FOR[] in a way that would cause that code to report the wrong mods being installed, garbage in garbage out style), so I haven't pushed overly hard to get those changes merged.

@rkunze
Copy link
Contributor Author

rkunze commented Aug 14, 2023

Spontaneous (and probably dumb) idea: How about checking the directory listing of GameData against known mod IDs? Should be fast enough even for a monster install with several hundred mods, and would at least catch all well-behaved mods that install to GameData/{mod-id}...

@HebaruSan
Copy link
Member

I think there were reasons we didn't go with that solution, but it's been a while now since I've concentrated on it, so I don't have a comprehensive explanation ready to go. At a glance, it seems like a big leap to go from "this directory exists (and might have a file in it)" to "this mod is fully and correctly installed such that mods that depend on it will work properly."

@HebaruSan
Copy link
Member

HebaruSan commented Aug 14, 2023

If you just want a quick workaround for your dev environment, note that the DLL file doesn't have to be valid. If you make a 0-byte file at GameData/YourMod/PluginData/YourMod.dll, I think CKAN would detect it and the game wouldn't even try to load it thanks to how it treats PluginData folders.

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.

2 participants