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

Qubus0 · 2023-02-08T10:51:43Z

This can now properly account for args in format --arg="value" and also --arg "value" (like --script)
It also reverses the Godot bug present in editor builds for Linux and Mac where it would split quoted args at whitespace in this format --arg="value with whitespace"
The last thing is that is_running_with_command_line_arg will now also return true for args in this format --arg="value", which it didn't before since it had to be an exact match

testing: just put this into the mod loader _init

func _init() -> void:
	var test_args := ["--script", "--fake-arg", "--test-mods-path", "--log-info", "--log-ignore"]
	for test_arg in test_args:
		printt(
			"running with %s?" % test_arg,
			ModLoaderUtils.is_running_with_command_line_arg(test_arg),
			ModLoaderUtils.get_cmd_line_arg_value(test_arg)
		)

I used these args
--script addons/mod_loader/mod_loader_setup.gd --log-info --log-ignore=ModLoader:ModData --test-mods-path="C :\s ome\path wit h\whi te space\"

addons/mod_loader/mod_loader_utils.gd

ithinkandicode

👍

* 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>

…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>

…g#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 =

fix: properly parse all types of command line arg values

1ad5185

Qubus0 added the bug Something isn't working label Feb 8, 2023

Qubus0 requested a review from a team February 8, 2023 10:51

ithinkandicode reviewed Feb 8, 2023

View reviewed changes

addons/mod_loader/mod_loader_utils.gd Outdated Show resolved Hide resolved

addons/mod_loader/mod_loader_utils.gd Outdated Show resolved Hide resolved

Qubus0 added 2 commits February 8, 2023 12:38

refactor: adjust code line in doc comments

9f42c18

refactor: add cmdline arg fixing wrapper function

3bb05c7

ithinkandicode approved these changes Feb 8, 2023

View reviewed changes

fix: also cover args without =

0a266ac

Qubus0 requested a review from ithinkandicode February 9, 2023 00:56

Qubus0 added this pull request to the merge queue Feb 9, 2023

Merged via the queue into GodotModding:develop with commit 0b76416 Feb 9, 2023

Qubus0 requested a review from a team February 9, 2023 09:21

Qubus0 deleted the fix_cmd_line_arg_value_parsing branch February 9, 2023 09:21

ithinkandicode mentioned this pull request Feb 22, 2023

v4.2.0 #112

Merged

KANAjetzt mentioned this pull request Feb 23, 2023

v5.0.1 #123

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Qubus0 commented Feb 8, 2023

ithinkandicode left a comment

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

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

Conversation

Qubus0 commented Feb 8, 2023

ithinkandicode left a comment

Choose a reason for hiding this comment