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

refactor: ♻️ Changed disabled_mods setting to load metadata #394

Closed

Conversation

KANAjetzt
Copy link
Member

@KANAjetzt KANAjetzt commented Apr 18, 2024

  • Renamed disabled_mods to deactivated_mods.
  • Moved default profile creation in mod_loader.gd from _ready() to _load_mods(). This is required so mod configs can be loaded on the first run of the game.
  • Separated loading of mod manifests and mod configs to allow the creation of the default user profile in between.
  • Demoted "No profile" error from error to info because on the first run of the game, the file will not be there.
  • Fixed the LOG_NAME of _ModLoaderPath.
  • Changed the MOD_CONFIG_DIR_PATH from "user://configs" to "user://mod_configs" to make it clearer what kind of configs are stored.
  • Fixed an issue where current_config was null because it wasn't set in ModData._load_config.

@KANAjetzt KANAjetzt added enhancement New feature or request refactor / cleanup Improves readability or maintainability 4.x labels Apr 18, 2024
@KANAjetzt KANAjetzt added this to the 4.x - 7.0.0 milestone Apr 18, 2024
@KANAjetzt KANAjetzt requested review from Qubus0, otDan and a team April 18, 2024 15:20
@KANAjetzt KANAjetzt added the breaking Breaking change label Apr 18, 2024
## Mods in this array will only load metadata but will not apply any modifications.
## This can be used to deactivate example mods.
## Mods in this array are overridden by the settings in the user profile.
@export var deactivated_mods: Array[String] = []
Copy link
Member

Choose a reason for hiding this comment

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

why are we changing it to deactivated? seems like a pointless breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about adding a "disabled" and "deactivated" setting first, one to keep the current behavior and one for this change.
I'm not sure what is more accurate.

I don't worry too much about breaking changes for 4.x-v7.

Copy link
Collaborator

Choose a reason for hiding this comment

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

deactivated makes more sense to me, so i think the rename is fine. but yes, this is breaking and no we can't overlook that anymore since we have windowkill now which relies on it. so we need a proper deprecation both for that and for mod_configs

Copy link
Collaborator

Choose a reason for hiding this comment

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

unless it's not directly used, then we are fine

@KANAjetzt KANAjetzt force-pushed the feat_deactivate_mods_setting branch from 1ab1f4a to 48d501c Compare April 20, 2024 07:36
@KANAjetzt KANAjetzt self-assigned this Apr 22, 2024
- Renamed `disabled_mods` to `deactivated_mods`. - Moved default profile creation in `mod_loader.gd` from `_ready()` to `_load_mods()`. This is required so mod configs can be loaded on the first run of the game. - Separated loading of mod manifests and mod configs to allow the creation of the default user profile in between. - Demoted "No profile" error from error to info because on the first run of the game, the file will not be there. - Fixed the `LOG_NAME` of `_ModLoaderPath`. - Changed the `MOD_CONFIG_DIR_PATH` from `"user://configs"` to `"user://mod_configs"` to make it clearer what kind of configs are stored. - Fixed an issue where `current_config` is `null` because it didn't get set in `ModData._load_config`.
@KANAjetzt KANAjetzt closed this Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x breaking Breaking change enhancement New feature or request refactor / cleanup Improves readability or maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants