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

Multi-game labels #3885

Merged
merged 4 commits into from
Aug 21, 2023
Merged

Multi-game labels #3885

merged 4 commits into from
Aug 21, 2023

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Aug 20, 2023

Problem

If you add a label to ResonantOrbitCalculator in KSP1, the different mod with the same identifier will also have that label added in KSP2, and vice versa. This is not ideal because these are two different mods, which you should be able to treat separately.

As the number of KSP2 mods grows, so will the number of identifier collisions between the two games.

Cause

The labels.json file just has one list of identifiers per label, with no distinctions between games, so when both games have a mod with the same identifier, they're treated as one mod.

Changes

  • Now Meta.GetProductName() provides a centralized way to get the string "CKAN", CKANPathUtils.AppDataPath uses it to provide a centralized way to get the path of the shared config folder, and code that uses either value is updated to use these properties (I did this initially because I was considering changing the path of the labels.json file, which I decided against, but this clean-up is still worth keeping)
  • The property in the labels JSON file that formerly held a list of identifiers now holds an object with keys corresponding to game names and values corresponding to lists of identifiers:
    "module_identifiers": {
        "KSP": [ "Mod1" ],
        "KSP2": [ "Mod2" ]
    }
    • For users who haven't added labels to mods before, these will start out empty and accumulate mods independently, without affecting the other game. For users who have already added some labels to mods, the existing list of mods will be copied to both games, so nothing in the UI will change immediately, but from that point forward the two lists will be maintained independently, so a mod can be removed from a label in one game without affecting the other game.
    • Since the old ckan.exe client is expecting this property to only be an array, it is renamed to module_identifiers_by_game. This means that you will lose all your label memberships if you temporarily switch back to an old client version, but at least it won't crash.

Fixes #3824.

Considered and not done

Note that any mods that you have added to a label that are not actually available in that game (e.g., if you add MechJeb2 to your Favorites for KSP1, then run these changes for the first time, then load a KSP2 instance) will be permanently stuck in those lists in the labels.json file behind the scenes (unless you edit it manually), since those mods can never appear in the mod list to be removed via the right click menu, and the mod counts in the label filter menu will be inflated by that amount.

I considered various options for trying to resolve this automatically, but none of them are safe. The problem is that there's no reliable way to determine the list of valid identifiers for a game, because users can add custom repos. For example, if someone adds the MechJeb2 dev build repo in one game instance, then the MechJeb2-dev module becomes available, and they could add it to a label. If they then later load a different game instance without that repo, it would be wrong to auto-remove that identifier just because we don't recognize it.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN labels Aug 20, 2023
@HebaruSan

This comment was marked as resolved.

Track labels' module identifiers per game
Create JsonToGamesDictionaryConverter
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

Awesome! Yes, I think that's a sensible approach and hopefully relatively edge case. Thanks @HebaruSan !

@HebaruSan HebaruSan merged commit 858d71f into KSP-CKAN:master Aug 21, 2023
10 checks passed
@HebaruSan HebaruSan deleted the fix/multigame-labels branch August 21, 2023 01:44
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 Core (ckan.dll) Issues affecting the core part of CKAN GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KSP 1 labels are applied to KSP 2 mods
2 participants