Feat zip peeking #416
Feat zip peeking #416
Conversation
The plan is to automate the creation of hooks for each function in all GDScripts.
Vanilla functions are renamed, and at the bottom of the script, mod loader hooks are generated. These hooks replace the vanilla functions and handle calls to the `callable_stack`.
Currently, I'm stuck on this approach because the method list's return property does not contain any information if there is no return type defined for the method.
That seems to work for filtering out methods that have a return.
Without removing the `class_name` before initializing the script on export, I get this parse error: `hides a global script class`. With these code changes, I get `Value of type [script_path] cannot be assigned to a variable of type [class_name]` if there is a self-assignment somewhere in the script.
Also added a check for static functions and currently working on recognizing inner classes.
check the `type_hint` and add it if it's present.
To check if the function has no space before `func` or `static`, used to ignore functions in inner classes. Later, we might want to figure out a way to include these.
Also removed extra comma in args list
until now all scripts received the "Mod Loader Hooks" header at the bottom even when no changes where made.
used to ignore setter funcs
Also refactored some of the function/field names to improved the modding experience
added getter method, made both static and moved getter/setter check to is_moddable check
improved performance by exiting early when nothing is hooked
and a first style and type pass
| "Loading any resource packs (.zip/.pck) with `load_resource_pack` will WIPE the entire virtual res:// directory. ", | ||
| "If you have any unpacked mods in ", _ModLoaderPath.get_unpacked_mods_dir_path(), ", they will not be loaded. ", | ||
| "Please unpack your mod ZIPs instead, and add them to ", _ModLoaderPath.get_unpacked_mods_dir_path()), LOG_NAME) | ||
| ModLoaderStore.has_shown_editor_zips_warning = true |
There was a problem hiding this comment.
Does only_once not work in this case?
godot-mod-loader/addons/mod_loader/api/log.gd
Line 135 in d7a28c3
There was a problem hiding this comment.
it should, yeah. i didn't change anything here, just moved it.
| # Application's Steam ID, used if workshop is enabled | ||
| steam_id = 0, | ||
| # Application's version following semver | ||
| semantic_version = "0.0.0", |
There was a problem hiding this comment.
what if a game uses non semantic version numbers?
There was a problem hiding this comment.
then they can either leave it like this, which essentially disables version checking, implement their own logic since we can't preemtively cover all strange version standards, or switch to semver, which is probably the easiest. it's the main version standard for a reason
There was a problem hiding this comment.
Maybe we could add an option to enable or disable this, or even allow the developer to implement their own validation. Just a thought for the future.
Co-authored-by: KANAjetzt <41547570+KANAjetzt@users.noreply.github.com>
| "Loading any resource packs (.zip/.pck) with `load_resource_pack` will WIPE the entire virtual res:// directory. ", | ||
| "If you have any unpacked mods in ", _ModLoaderPath.get_unpacked_mods_dir_path(), ", they will not be loaded. ", | ||
| "Please unpack your mod ZIPs instead, and add them to ", _ModLoaderPath.get_unpacked_mods_dir_path()), LOG_NAME) | ||
| ModLoaderStore.has_shown_editor_zips_warning = true |
There was a problem hiding this comment.
it should, yeah. i didn't change anything here, just moved it.
| # Application's Steam ID, used if workshop is enabled | ||
| steam_id = 0, | ||
| # Application's version following semver | ||
| semantic_version = "0.0.0", |
There was a problem hiding this comment.
then they can either leave it like this, which essentially disables version checking, implement their own logic since we can't preemtively cover all strange version standards, or switch to semver, which is probably the easiest. it's the main version standard for a reason
| var game_install_directory := OS.get_executable_path().get_base_dir() | ||
|
|
||
| if OS.get_name() == "OSX": | ||
| if OS.get_name() == "macOS": |
There was a problem hiding this comment.
from what i can see in the c++ code the feature flag is "macos" (no capitalization), is it working correctly with the capitalized OS?
There was a problem hiding this comment.
yep, that's get_name() though, not the flag. confused me too. we should probably decide on a single one - i think it was originally chosen because has_feature was the same with web and without, which has changed since
| return {} | ||
| ModLoaderLog.error("Can't open workshop folder %s (Error: %s)" % [workshop_folder_path, error_string(DirAccess.get_open_error())], LOG_NAME) | ||
| return [] | ||
| var workshop_dir_listdir_error := workshop_dir.list_dir_begin() # TODOGODOT4 fill missing arguments https://github.com/godotengine/godot/pull/40547 |
There was a problem hiding this comment.
Nitpick: workshop_dir_listdir_error could just be result or error
| var game_install_directory := OS.get_executable_path().get_base_dir() | ||
|
|
||
| if OS.get_name() == "OSX": | ||
| if OS.get_name() == "macOS": |
There was a problem hiding this comment.
yep, that's get_name() though, not the flag. confused me too. we should probably decide on a single one - i think it was originally chosen because has_feature was the same with web and without, which has changed since
3 participants
big pr with some suboptimal mixing of things to get them fixed for dome, sorry about those.
the core of the zip peeking pr is about having info about mod metadata before the mod is loaded by load_resource_pack. this lets us compare the game version (and other data, in following prs) from the zip with the one set by the game. if a version mismatch is detected the mod won't be loaded, which will reduce crashes.
this basically flips the order of operations around from previously
to
optimally this would be the new process, but i had to take a few shortcuts to quickly get it ready for the dome patch
shortcut way
Other notes for the future