Skip to content

Refactor module installer to use relative paths internally#4469

Merged
HebaruSan merged 9 commits into
KSP-CKAN:masterfrom
HebaruSan:refactor/modinst-rel-paths
Dec 9, 2025
Merged

Refactor module installer to use relative paths internally#4469
HebaruSan merged 9 commits into
KSP-CKAN:masterfrom
HebaruSan:refactor/modinst-rel-paths

Conversation

@HebaruSan
Copy link
Copy Markdown
Member

@HebaruSan HebaruSan commented Dec 9, 2025

Motivation

I tried switching to a different ZIP handling library after @JonnyOThan suggested supporting 7zip. That effort is still in extended debugging limbo, but one piece of it that seemed valuable enough to pull out on its own was refactoring ModuleInstaller.FindInstallableFiles to return relative install paths instead of absolute. This has a number of benefits:

  • Generating the list of installable files in the ZIP and translating them to absolute paths are two distinct conceptual steps, and it's better to avoid tightly coupling such things
  • FindInstallableFiles no longer needs a game instance to work
  • The output of FindInstallableFiles is now invariant per CkanModule, opening the door to possible future caching if it looks like that would help
  • Often we only need the relative path anyway, so now we don't have to do extra work to re/un/de-convert (which requires having a GameInstance)

Changes

  • Now ModuleInstaller.FindInstallableFiles returns relative install paths instead of absolute. This will be more flexible and easier to maintain.

Side refactorings

  • CkanModule.ProvidesList is now cached instead of being recomputed every time
  • ModuleInstallDescriptor.EnsurePattern is replaced by a cached InstallPattern property
  • CkanModule.GetInstallStanzas is created to encapuslate the logic of falling back to the default install stanza when install isn't set
  • ModuleInstallDescriptor.FindInstallableFiles now checks for updirs using a single pass regex instead of one pass with Contains and another with EndsWith
  • ModuleInstallDescriptor.FindInstallableFiles now returns a lazily generated sequence insetad of a List
  • SpaceWarpInfoLoader is created to encapsulate the operation of parsing a swinfo.json file and retrieving its remote counterpart via the version_check property, so we don't need to duplicate that logic in the SpaceWarpInfoTranslator and the SpaceWarpInfoValidator

@HebaruSan HebaruSan added the Core (ckan.dll) Issues affecting the core part of CKAN label Dec 9, 2025
@coveralls
Copy link
Copy Markdown

coveralls commented Dec 9, 2025

Pull Request Test Coverage Report for Build 20049619596

Details

  • 87 of 98 (88.78%) changed or added relevant lines in 10 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.04%) to 85.3%

Changes Missing Coverage Covered Lines Changed/Added Lines %
Netkan/Services/ModuleService.cs 3 4 75.0%
Netkan/Services/SpaceWarpInfoLoader.cs 12 16 75.0%
Core/Types/ModuleInstallDescriptor.cs 30 36 83.33%
Files with Coverage Reduction New Missed Lines %
Core/IO/ModuleInstaller.cs 1 88.37%
Totals Coverage Status
Change from base Build 20046688194: -0.04%
Covered Lines: 11928
Relevant Lines: 14164

💛 - Coveralls

@HebaruSan HebaruSan force-pushed the refactor/modinst-rel-paths branch from 7936642 to f51877a Compare December 9, 2025 02:18
@HebaruSan HebaruSan merged commit c506710 into KSP-CKAN:master Dec 9, 2025
9 of 12 checks passed
@HebaruSan HebaruSan deleted the refactor/modinst-rel-paths branch December 9, 2025 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Core (ckan.dll) Issues affecting the core part of CKAN

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants