Skip to content

Make global install filters and presets game-specific#4361

Merged
HebaruSan merged 14 commits into
KSP-CKAN:masterfrom
HebaruSan:feature/install-filter-per-game-presets
May 5, 2025
Merged

Make global install filters and presets game-specific#4361
HebaruSan merged 14 commits into
KSP-CKAN:masterfrom
HebaruSan:feature/install-filter-per-game-presets

Conversation

@HebaruSan
Copy link
Copy Markdown
Member

@HebaruSan HebaruSan commented May 5, 2025

Motivation

  • @linuxgurugamer pointed out that the install filter preset to block MiniAVC also blocks ZeroMiniAVC.dll, which is unnecessary for CKAN-installed mods because the filter itself blocks MiniAVC, but could be desirable for purging files from manually-installed mods.
  • The global install filter applies across all games, but each game has completely different mods and file naming conventions, so it doesn't make sense to treat them the same.
    • Similarly, the MiniAVC button appears even if you're using KSP2, which doesn't make sense because it's a KSP1 mod.
  • @doinkythederp's work on a new Mac GUI proceeds apace (see https://github.com/doinkythederp/KerbalModManager), and one of the screenshots reminded me that ModuleLabel and ModuleLabelList live exclusively in GUI but would be useful for other GUIs.

Changes

  • Now the MiniAVC preset's file paths have / added to the front so they won't match parts of other file names
    • Furthermore, MiniAVC v1 and MiniAVC v2 are broken out into separate preset buttons in case users only want to block one or the other.
    • A test is added to confirm that the install filter presets for KSP1 do not prevent ZeroMiniAVC.dll from being installed.
      Since ZeroMiniAVC and MiniAVC are both GPL-3.0, and the CKAN repo is MIT, we download the mods during the test and cache them in _build/test/cache/<target framework> (the target framework is included to avoid problems with the test clobbering itself when running in parallel for different frameworks). This directory will be cached by the GitHub workflows to reduce network traffic.
  • Now each game has its own global install filter. Any values users currently have will be copied to both games at load, and will be edited separately thereafter.
    • This also applies to the preset buttons. Currently the MiniAVC button appears for KSP1 and nothing appears for KSP2; we can add presets for KSP2 later if a common use case arises.
  • ModuleLabel and ModuleLabelList are moved from GUI to Core.

Side fixes

  • Global install filters #3458 was developed on Linux and left in two compile errors that only occurs on Windows for net8.0.
    Now they're fixed.
  • Some of the tests make API calls out to GitHub, so if you run them too much, you can get throttled.
    Now we can set the GITHUB_TOKEN environment variable to a GitHub API auth token to solve this.
  • The RegistryLive test can fail on Windows if our three target platforms attempt to run it simultaneously, because a FIle.Open call in RepositoryData holds an exclusive lock.
    Now that call uses OpenRead instead.
  • Since 4a6df21, loading the registry can throw this exception:
    System.ArgumentException: An item with the same key has already been added.
       at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
       at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
       at CKAN.SingleAssemblyResourceManager.InternalGetResourceSet(CultureInfo culture, Boolean createIfNotExists, Boolean tryParents)
       at System.Resources.ResourceManager.GetString(String name, CultureInfo culture)
       at CKAN.Properties.Resources.get_UnmanagedModuleVersionKnown()
       at CKAN.Versioning.UnmanagedModuleVersion..ctor(String version)
       at CKAN.CkanModule.DeSerialisationFixes(StreamingContext like_i_could_care)
    
    The loading of CkanModules from registry.json is parallelized for performance, and the construction of a CkanModule for a DLC requires creating an UnmanagedModuleVersion, the constructor of which accesses an i18n resource, which is loaded on the fly and cached by SingleAssemblyResourceManager. If two DLC CkanModules are loaded in parallel, they can end up loading the same resource set simultaneously in different threads and attempting to store both sets to the cache, which throws a duplicate key exception.
    Now SingleAssemblyResourceManager uses lock to control access to its resource set cache so this won't happen.
  • ModuleInstaller.Download and ModuleInstaller.CachedOrDownload are not used, so now they're removed.
    These functions were the only reason ModuleInstaller's constructor needed a user agent, so that parameter is also removed.
  • A translated French string was duplicated at some point. Now the duplicate is removed.
  • The locally cached file for the KSP-Default repository is checked many times when the tests are run, even though in most cases we're not even trying to use that repo.
    Now the actual correct repo to used is passed all the way into the registry during tests, which pre-empts the attempt to check if KSP-Default is cached. In addition, RegistryManagers used in tests will now be Disposed when done, which will ensure that the global in-process cache of RegistryManagers doesn't grow without bound.
  • ModuleInstaller grabbed a reference to the JsonConfiguration internally, global-variable style, which made it impossible to override those settings in tests.
    Now the configuration is a parameter instead of being acquired sneakily, so tests can control the settings used for the test.
  • RegistryManager.AscertainDefaultRepo duplicated some logic from Repository.DefaultGameRepo.
    Now the former calls the latter instead.

@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Registry Issues affecting the registry Network Issues affecting internet connections of CKAN ConsoleUI Issues affecting the interactive console UI i18n Issues regarding the internationalization of CKAN and PRs that need translating labels May 5, 2025
@HebaruSan HebaruSan force-pushed the feature/install-filter-per-game-presets branch from 29afca5 to e66a076 Compare May 5, 2025 19:32
@HebaruSan HebaruSan merged commit 267934a into KSP-CKAN:master May 5, 2025
2 checks passed
@HebaruSan HebaruSan deleted the feature/install-filter-per-game-presets branch May 5, 2025 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working as intended Cmdline Issues affecting the command line ConsoleUI Issues affecting the interactive console UI Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI i18n Issues regarding the internationalization of CKAN and PRs that need translating Network Issues affecting internet connections of CKAN Registry Issues affecting the registry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant