Automatically enable newly-installed plugins #94481
Conversation
While installing an asset, check whether it installs any files matching the target pattern res://addons/*/plugin.cfg. If so, and providing that no errors are encountered while unpacking the asset, enable each plugin plugin once the new files have been scanned and scripts within added to the class database. Inspired by godotengine#35234. Fixes godotengine/godot-proposals#324
|
Thank you for your first PR! Yes, this is a missing feature that should help streamlining the use of plugins. I myself get surprised, sometimes, that my installed plugin isn't active. As we discussed on the dev chat, there's two types of assets in the asset lib:
And there's also the risk of running untrusted code automatically, without the ability to review the files on your disk first. What I suggest is the following. It resembles what was already proposed in godotengine/godot-proposals#324: 
I would check in the downloaded files if there's a |
| @@ -553,6 +554,12 @@ void EditorAssetInstaller::_install_asset() { | |||
| Ref<FileAccess> f = FileAccess::open(target_path, FileAccess::WRITE); | |||
| if (f.is_valid()) { | |||
| f->store_buffer(uncomp_data.ptr(), uncomp_data.size()); | |||
| if (target_path.begins_with("res://addons/")) { | |||
| Vector<String> separated = target_path.substr(13).split("/"); | |||
| if (separated.size() == 2 && separated[1] == "plugin.cfg") { | |||
There was a problem hiding this comment.
Note that this won't work for addons inside subfolders. Check #91337 to see how it can be done more reliably.
There was a problem hiding this comment.
Thanks for this pointer! I'm happy to see an existing use of the magic number 13 :)
Here's what I have in mind. If there's one or more plugin, the dialog would look like this:
with “Install and Enable” focused by default. Otherwise, the two buttons would read “Cancel” and “Install” as today. It's slightly weird that changing the install folder and/or toggling “Ignore asset root” would make one of the buttons appear and disappear. I guess I'll see how this feels. But it would seem weirder to have an always-insensitive button for non-plugin assets. Is there any style guide for UI elements in Godot, similar to the GNOME HIG’s Writing Style guide which specifies that button labels should be titlecased (and sets out what that actually means). Having searched the code a little further, the prevailing style seems to be to use titlecase in button labels but use “&” rather than “and”, so the button would be “Install & Enable”. |
Yes: Editor style guide Some existing messages in Godot don't follow this style guide yet though, especially when it comes to single quotes versus double quotes. |
While installing an asset, check whether it installs any files matching the target pattern
res://addons/*/plugin.cfg. If so, and providing that no errors are encountered while unpacking the asset, enable each plugin plugin once the new files have been scanned and scripts within added to the class database.Inspired by #35234.
Fixes godotengine/godot-proposals#324.
We at Endless are developing a block coding plugin and other learning tools targeting young people who are new to Godot. We keep seeing people (including our colleagues, in one case twice in a single playtesting session) forget to enable the plugin after installing it, and then wondering why the plugin isn't working.
Unlike #35234 I have not included a way to add an asset without enabling any included plugins, on the basis that there is a clear intent to use the plugin – otherwise, why would one be installing it? I guess the counterargument is that someone may wish to review the code before executing it – but the asset library review process is presumably intended in part to catch malicious plugins, and in reality I think it's unlikely that people are reviewing every plugin they use in detail before enabling it. I feel strongly that the default should be to enable the plugin, even if there were a way to download without enabling.
I am very new to this codebase so I'm sure there are shortcomings in the implementation here! It seems to work in testing so far but this is perhaps more of a proof-of-concept/request-for-comment.