Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: ✨ mod reloading #203

Merged
merged 7 commits into from
Apr 4, 2023
Merged

Conversation

otDan
Copy link
Member

@otDan otDan commented Mar 31, 2023

This enables people to reload mods at runtime, currently the only changes made by mods that will be reset by this are extensions, which is the major changing factor in most cases. Opens up for next PRs that can reset nodes and hopefully all changes that mod developers want to make.
An example use case scenario would be: user downloads or updates mod, without needing to restart the whole game, the user could click a button in game that reloads all mods. Another example could be a mod that introduces in game mod downloading and updating, allowing for a better user experience.
This PR also renames
_remove_extension > _remove_specific_extension_from_script and
_reset_extension > _remove_all_extensions_from_script for easier functionality comprehension.
Adds a fix for #198 that returns null in a return void function.

@otDan otDan added the enhancement New feature or request label Mar 31, 2023
@otDan otDan added this to the v6.0.0 milestone Mar 31, 2023
@otDan otDan requested a review from a team March 31, 2023 19:26
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.

2 little things

Did a run test in brotato no errors 👍
I did not test the reload_mods()

addons/mod_loader/mod_loader.gd Outdated Show resolved Hide resolved
addons/mod_loader/mod_loader.gd Outdated Show resolved Hide resolved
@ithinkandicode
Copy link
Collaborator

I'm a bit behind on the latest development, but is my understanding of these funcs correct?

Func Purpose
_remove_specific_extension Removes an extended script from the given script
_remove_all_extensions Removes all extended scripts from the given script
_clear_extensions Resets every script, removing all extensions from every script

If that's right, I'm wondering if the first 2 funcs could be affixed with _from_script? Because the func name remove_all_extensions in particular sounds very similar to clear_extensions, as in, one might presume that "remove all extensions" means removing all extensions from all scripts. That was my initial thought when I saw the func name. Perhaps the _clear_extensions func name could be clearer too, eg. _remove_all_extensions_from_all_scripts?

So my proposed func names would be this:

Func Purpose
_remove_specific_extension_from_script Removes a single extended script from the given script
_remove_all_extensions_from_script Removes all extended scripts from the given script
_remove_all_extensions_from_all_scripts Removes all extended scripts from every script

@otDan otDan requested a review from KANAjetzt April 1, 2023 12:20
@ithinkandicode ithinkandicode linked an issue Apr 1, 2023 that may be closed by this pull request
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.

Tested in brotato no errors 👍
I did not test the reload_mods() func

@KANAjetzt KANAjetzt added this pull request to the merge queue Apr 4, 2023
Merged via the queue into GodotModding:development with commit d3c35cf Apr 4, 2023
@ithinkandicode ithinkandicode changed the title feat: mod reloading feat: ✨ mod reloading Jun 18, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

_remove_extension - "A void function cannot return a value"
3 participants