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

✔ added checks for override.cfg setup #110

Merged
merged 2 commits into from
Feb 14, 2023
Merged

✔ added checks for override.cfg setup #110

merged 2 commits into from
Feb 14, 2023

Conversation

KANAjetzt
Copy link
Member

mod_loader_setup.gd

Prevents the setup script from running the setup again, if the mod loader was installed via the override.cfg.

mod_loader.gd

Prevents the error logging in _check_first_autoload(), if a override.cfg file is present.

@KANAjetzt KANAjetzt added bug Something isn't working enhancement New feature or request installation labels Feb 13, 2023
@KANAjetzt KANAjetzt requested review from ithinkandicode, Qubus0 and a team February 13, 2023 19:55
@KANAjetzt
Copy link
Member Author

I think this code will never run, if there is no autoloads there will be no mod_loader.gd right?

if autoload_array.size() == 0:
ModLoaderUtils.log_fatal(str(base_msg, "but no autoloads are currently set up. ", help_msg), LOG_NAME)

@Qubus0
Copy link
Collaborator

Qubus0 commented Feb 13, 2023

you are right, that will never run. the mod loader will always occupy one slot or not be around to tell anyone. this should be in the setup, if at all.

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.

looks good

addons/mod_loader/mod_loader.gd Show resolved Hide resolved
If there is no autoloads there will be no mod_loader.gd
@KANAjetzt KANAjetzt added this pull request to the merge queue Feb 14, 2023
Merged via the queue into GodotModding:develop with commit a3ff6c4 Feb 14, 2023
@KANAjetzt KANAjetzt deleted the check_for_override_cfg_setup branch February 14, 2023 10:27
@KANAjetzt KANAjetzt mentioned this pull request Feb 15, 2023
KANAjetzt added a commit that referenced this pull request Feb 16, 2023
* set more static types, follow style guide more closely, improve readability

* fix missing %s in format strings

* upgrade semver validation to disallow leading zeros and overly long versions

* replace printerr with log_fatal

* use PoolStringArray where applicable

* ✔ added file and dir existence check utils

* ✅ added overwrite handling

* add simple loader self setup through script flag

* don't attempt setup if the autoload already exists

* fix mods dir path not being overridden

* 🔨 renamed functions to be more semantic

* 🔨 2 blank lines after functions

* 🔨 static typing

* fix type inference of game_mod_folder_path

* remove run.gd, as it's no longer needed

* 🔨 removed unnecessary blank lines

* 🔨 fill the void

* Add credits

* fix typo in setup where `ModLoaderUtils` should be `modloaderutils`

* lint - fix not storing returned value in a variable for certain methods

* lint - fix var type set not being able to be inferred

Fixes error: The assigned value doesn't have a set type; the variable type can't be inferred

* fix errors due to func args having the same name as local vars

Fixes 2 `SHADOWED_VARIABLE` errors:
- The local variable 'name' is shadowing an already-defined variable at line 9.
- The local variable 'version_number' is shadowing an already-defined variable at line 15/

* revert non-inferred var type fix

* ✔ added reorder_autoloads()

* ✔ added file data setup

* ✔ added project.binary injection

* ✔ added godotPckTool

* ✔ cleanup project.binary file after injection

* fix typo in `_handle_missing_dependency`

* ✔ added --only-setup cli arg

* validate dependencies and incompatibilities (closes #23)

* remove old version from the intro comment

* add constant for version (`MODLOADER_VERSION`)

* fix: prevent duplicate global class registration

* refactor: utils global class registration does not save anymore, mod_loader exposes a new method for mods to use it and save

* log info instead of warning

* dependency validation: use `get_mod_id`

* shorten `is_dependency_or_incompatibility_valid` to `is_mod_id_valid`, and make the `type` arg optional

* assert that ModLoader is the first autoload

* rename var to `is_mod_loader_first`

* ✔ added --setup-create-override-cfg cli arg

* store log backups and rotate them like godot

* 🔨 fixed the double log

* don't log the full manifest by default, add option to do this

* fixes `log_fatal` to add the missing log to file

Previously this func would not log to the file, which means it wouldn't show in the godot.log during mod development. Note: Unlike the `"error"` condition, I've only used `push_error` instead of also using `printerr`, because `push_error` also seems to print the error already (incl. the `ERROR: ` prefix, which `printerr` doesn't include)

* ✔ added --setup-create-override-cfg cli arg

* 🔨 fixed the double log

* ✔ added get_autoload_array and get_autoload_index

* ✔ updated `_check_first_autoload` to use utils

* 🔨 added back autoload order logging

* 🔨 .trim_prefix("autoload/") instead of .split('/')[1]

* 🔨 might as well just use an array

* 🔨 trim prefix "autoload/" in get_autoload_array()

* 🧹 removed unnecessary var value in get_autoload_array()

* fix: properly parse all types of command line arg values (#103)

* fix: properly parse all types of command line arg values

* refactor: adjust code line in doc comments

* refactor: add cmdline arg fixing wrapper function

* fix: also cover args without =

* ✔ run setup based on position in autoloads (#105)

* downgrade the missing "mods" dir error to a warning (#107)

because the notice is most likely to be seen by modders in the editor, who won't be using the `mods` dir

* Config JSON Tweaks (#108)

* `get_mod_config` - fix missing space in log string

* `get_mod_config` - log improvements

- Generally improve log messages, giving them better user notices that are more accurate to the current issue.
- Use assert for cases where there are definite code errors that need addressing (eg. an invalid mod ID)

* `get_mod_config` - use enums for the error codes

* `get_mod_config` - rename "error" (eg. "error code") to "status"

* `get_mod_config` - remove a redundant bit of text

* ✔ added checks for override.cfg setup (#110)

* ✔ added checks for override.cfg setup

* 🧹 removed logging when no autoloads are set up

If there is no autoloads there will be no mod_loader.gd

---------

Co-authored-by: Qubus0 <steen.rickmer@gmx.de>
Co-authored-by: otDan <otdanofficial@gmail.com>
Co-authored-by: Chris Bloomfield <43499897+ithinkandicode@users.noreply.github.com>
KANAjetzt added a commit that referenced this pull request Feb 16, 2023
…ource_path: String) (#120)

* set more static types, follow style guide more closely, improve readability

* fix missing %s in format strings

* upgrade semver validation to disallow leading zeros and overly long versions

* replace printerr with log_fatal

* use PoolStringArray where applicable

* ✔ added file and dir existence check utils

* ✅ added overwrite handling

* add simple loader self setup through script flag

* don't attempt setup if the autoload already exists

* fix mods dir path not being overridden

* 🔨 renamed functions to be more semantic

* 🔨 2 blank lines after functions

* 🔨 static typing

* fix type inference of game_mod_folder_path

* remove run.gd, as it's no longer needed

* 🔨 removed unnecessary blank lines

* 🔨 fill the void

* Add credits

* fix typo in setup where `ModLoaderUtils` should be `modloaderutils`

* lint - fix not storing returned value in a variable for certain methods

* lint - fix var type set not being able to be inferred

Fixes error: The assigned value doesn't have a set type; the variable type can't be inferred

* fix errors due to func args having the same name as local vars

Fixes 2 `SHADOWED_VARIABLE` errors:
- The local variable 'name' is shadowing an already-defined variable at line 9.
- The local variable 'version_number' is shadowing an already-defined variable at line 15/

* revert non-inferred var type fix

* ✔ added reorder_autoloads()

* ✔ added file data setup

* ✔ added project.binary injection

* ✔ added godotPckTool

* ✔ cleanup project.binary file after injection

* fix typo in `_handle_missing_dependency`

* ✔ added --only-setup cli arg

* validate dependencies and incompatibilities (closes #23)

* remove old version from the intro comment

* add constant for version (`MODLOADER_VERSION`)

* fix: prevent duplicate global class registration

* refactor: utils global class registration does not save anymore, mod_loader exposes a new method for mods to use it and save

* log info instead of warning

* dependency validation: use `get_mod_id`

* shorten `is_dependency_or_incompatibility_valid` to `is_mod_id_valid`, and make the `type` arg optional

* assert that ModLoader is the first autoload

* rename var to `is_mod_loader_first`

* ✔ added --setup-create-override-cfg cli arg

* store log backups and rotate them like godot

* 🔨 fixed the double log

* don't log the full manifest by default, add option to do this

* fixes `log_fatal` to add the missing log to file

Previously this func would not log to the file, which means it wouldn't show in the godot.log during mod development. Note: Unlike the `"error"` condition, I've only used `push_error` instead of also using `printerr`, because `push_error` also seems to print the error already (incl. the `ERROR: ` prefix, which `printerr` doesn't include)

* ✔ added --setup-create-override-cfg cli arg

* 🔨 fixed the double log

* ✔ added get_autoload_array and get_autoload_index

* ✔ updated `_check_first_autoload` to use utils

* 🔨 added back autoload order logging

* 🔨 .trim_prefix("autoload/") instead of .split('/')[1]

* 🔨 might as well just use an array

* 🔨 trim prefix "autoload/" in get_autoload_array()

* 🧹 removed unnecessary var value in get_autoload_array()

* fix: properly parse all types of command line arg values (#103)

* fix: properly parse all types of command line arg values

* refactor: adjust code line in doc comments

* refactor: add cmdline arg fixing wrapper function

* fix: also cover args without =

* ✔ run setup based on position in autoloads (#105)

* downgrade the missing "mods" dir error to a warning (#107)

because the notice is most likely to be seen by modders in the editor, who won't be using the `mods` dir

* Config JSON Tweaks (#108)

* `get_mod_config` - fix missing space in log string

* `get_mod_config` - log improvements

- Generally improve log messages, giving them better user notices that are more accurate to the current issue.
- Use assert for cases where there are definite code errors that need addressing (eg. an invalid mod ID)

* `get_mod_config` - use enums for the error codes

* `get_mod_config` - rename "error" (eg. "error code") to "status"

* `get_mod_config` - remove a redundant bit of text

* ✔ added checks for override.cfg setup (#110)

* ✔ added checks for override.cfg setup

* 🧹 removed logging when no autoloads are set up

If there is no autoloads there will be no mod_loader.gd

* Validate file exists for `add_translation_from_resource` (#118)

* `add_translation_from_resource` - Validate file exists

* `add_translation_from_resource` - fix log name in previous commit

* ✔ bump version string to 5.0.0

* ⚙ Bump version string to 5.0.1

---------

Co-authored-by: Qubus0 <steen.rickmer@gmx.de>
Co-authored-by: otDan <otdanofficial@gmail.com>
Co-authored-by: Chris Bloomfield <43499897+ithinkandicode@users.noreply.github.com>
@KANAjetzt KANAjetzt mentioned this pull request Feb 23, 2023
KANAjetzt added a commit to GDami/godot-mod-loader that referenced this pull request Feb 23, 2023
* ✔ added checks for override.cfg setup

* 🧹 removed logging when no autoloads are set up

If there is no autoloads there will be no mod_loader.gd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request installation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants