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

feat: ✨ ModLoaderStore: New singleton to store data #172

Merged
merged 8 commits into from
Mar 9, 2023

Conversation

ithinkandicode
Copy link
Collaborator

@ithinkandicode ithinkandicode commented Mar 6, 2023

Adds a new singleton called ModLoaderStore, which comes before ModLoader in the autoload order. This new singleton is a data store, and lets us reference data that was previously held as local vars in ModLoader, and which therefore may not have been accessible to all methods due to ModLoader not being initialised during its _init() phase.

I have implemented the following:

  • Create the new ModLoaderStore singleton (closes #161).
  • Moved some local variables from ModLoader to ModLoaderStore (closes #161).
  • Implemented the path override options that were missing from the recent ML Options work (follows #145).
  • Implemented the option to disable mods by mod ID strings (via #160).
  • Options data can still be overridden by options.tres, but the stored data can also now be overridden by CLI args (with CLI args having priority).
    • This means we have one primary place to get/set data, and we don't need to keep checking CLI args (eg. for log verbosity levels, which previously checked the CLI args every time a log was used).
  • New utility funcs to get critical paths have also been added, which apply the overrides, so you don't need to check for overrides every time you get the mods/configs/workshop paths:
    • ModLoaderUtils.get_path_to_mods
    • ModLoaderUtils.get_path_to_configs
    • ModLoaderSteam.get_path_to_workshop (renamed from get_steam_workshop_dir to match this new convention)
  • Renamed path override options in options.tres to override_path_to_* for consistency
  • Updated the setup process to include ModLoaderStore
  • Updated the autoload order check, and added a new utility method called check_autoload_position to a new class called ModLoaderGodot

Notes:

  • ModLoader still has lots of other local vars that we may want to move to ModLoaderStore. We may also want to provide utility funcs to retrieve this data, eg. is_mod_active(mod_id: String), but all this can be handled in subsequent PRs.

Closes:

Works towards:

Finishes:

Subsequent Work

As raised in the review comments, there are a few things that we can do following this PR:

  • The things noted in the "works towards" section above can be addressed (eg via)
  • mod_loader_setup.gd - The new autoload check can be used here (via)
  • REQUIRE_CMD_LINE - This could be a configurable option (via)

@ithinkandicode ithinkandicode added enhancement New feature or request refactor / cleanup Improves readability or maintainability labels Mar 6, 2023
@ithinkandicode ithinkandicode marked this pull request as ready for review March 6, 2023 01:45
@KANAjetzt KANAjetzt requested review from Qubus0, KANAjetzt and a team March 6, 2023 06:40
Copy link
Collaborator

@Qubus0 Qubus0 left a comment

Choose a reason for hiding this comment

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

Very nice work

addons/mod_loader/classes/options_profile.gd Show resolved Hide resolved
addons/mod_loader/mod_loader.gd Show resolved Hide resolved
addons/mod_loader/mod_loader_store.gd Outdated Show resolved Hide resolved
Copy link
Member

@KANAjetzt KANAjetzt left a comment

Choose a reason for hiding this comment

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

Some minor stuff, but if everything is setup correctly it works flawless 👍
Thanks Darkly great work 🥇

addons/mod_loader/api/config.gd Outdated Show resolved Hide resolved
addons/mod_loader/mod_loader.gd Show resolved Hide resolved
addons/mod_loader/api/godot.gd Show resolved Hide resolved
addons/mod_loader/mod_loader.gd Show resolved Hide resolved
addons/mod_loader/mod_loader.gd Show resolved Hide resolved
Copy link
Collaborator

@Qubus0 Qubus0 left a comment

Choose a reason for hiding this comment

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

This is the one

@ithinkandicode ithinkandicode changed the title ModLoaderStore: New singleton to store data feat: ✨ ModLoaderStore: New singleton to store data Mar 9, 2023
Copy link
Member

@KANAjetzt KANAjetzt left a comment

Choose a reason for hiding this comment

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

Nice work - tested and approved 👍

@ithinkandicode ithinkandicode added this pull request to the merge queue Mar 9, 2023
Merged via the queue into GodotModding:development with commit 4f2ea00 Mar 9, 2023
@ithinkandicode ithinkandicode deleted the store branch March 9, 2023 14:41
KANAjetzt added a commit to KANAjetzt/godot-mod-loader that referenced this pull request Mar 30, 2023
The ModLoader self setup script uses util function from ModLoaderUtils in GodotModding#172 ModLoaderStore was introduced into utility functions that where used by the self setup. Because the setup is run before the ModLoaderStore is initialized the setup was stuck in an infinite loop. To fix that `get_modloader_store()` was added and is now used in `_get_verbosity()`, `get_path_to_mods()` and `get_path_to_configs()`. Also updated the the autoload index check in the `_init()` of *mod_loader_setup.gd*.

closes GodotModding#200
KANAjetzt added a commit that referenced this pull request Mar 30, 2023
The ModLoader self setup script uses util function from ModLoaderUtils in #172 ModLoaderStore was introduced into utility functions that where used by the self setup. Because the setup is run before the ModLoaderStore is initialized the setup was stuck in an infinite loop. To fix that `get_modloader_store()` was added and is now used in `_get_verbosity()`, `get_path_to_mods()` and `get_path_to_configs()`. Also updated the the autoload index check in the `_init()` of *mod_loader_setup.gd*.

closes #200
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor / cleanup Improves readability or maintainability
Projects
None yet
3 participants