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

feat: enable and disable plugins at will! #243

Open
wants to merge 39 commits into
base: master
Choose a base branch
from

Conversation

thennothinghappened
Copy link
Contributor

Plugins can now be disabled rather than being forced to remove them from the directory to prevent them from being initialised.

Disabled plugins have their groups minimised by default to show this visually.

If a plugin does not implement cleanup, the reload button will not display unless the plugin has not successfully loaded at all.

Additionally, plugins that do not implement cleanup have a tooltip on the disable button to indicate that toggling them requires a restart.

Plugins can now be disabled rather than being forced to remove them from the directory to prevent them from being initialised.

Disabled plugins have their groups minimised by default to show this visually.

If a plugin does not implement `cleanup`, the `reload` button will not display unless the plugin has not successfully loaded at all.

Additionally, plugins that do not implement `cleanup` have a tooltip on the `disable` button to indicate that toggling them requires a restart.
Deprecates `PluginEvents.preferencesBuilt` - directly provides plugin with their preferences element to append to, rather than a blanket event for informing plugins. This fixes preference elements not being re-appended-to when reloading in-place.
now redundant since you can just disable the plugin.
stops plugin-handled types from lingering if they're disabled
Avoids the need of using `PluginManager.ready`, as this value was just being used to determine whether it was safe to use `PluginEvents.*`.
@thennothinghappened thennothinghappened marked this pull request as draft January 17, 2025 08:38
@thennothinghappened
Copy link
Contributor Author

Marked this as a draft for the moment as it breaks the plugin dependency system. I have a fix in the works for that, but it had a knock-on effect of needing to be able to enable and disable multiple plugins at once, which currently is not accounted for in the UI, so requires a way of notifying the UI of multiple plugin preference groups at once to update. GMEdit seems to lean on Ace for implementing an Event Emitter, I'm not entirely sure why this is the case, but when I get back to this I might go ahead and implement one in GMEdit itself as there's a few different places that use callback ping-pong and manually update this and that which make more sense to me to be event-based. With that, it'd then be pretty simple to do the rest of the job.

workaround for `builder` assuming that there will always be a current project.
This is a rather large way to go about doing this - in essence this is a re-write of the whole plugin loader execution flow, which does a pass of resolving configs *first*, and then once registry names are known, goes back and does a loading sweep.

Another change is that dependency order is resolved *only* at plugin *start* time, rather than at load time. Plugins that depend on others should resolve their dependencies during their `init` block. Doing this cuts off one extra way for plugins to "leak" when they should be unloaded.

Finally, callback hell has been replaced with `Promise` hell. Unfortunately, Haxe does not natively support `async`/`await`, so we move from nesting callbacks, to chaining `then`, which is... marginally better?

The main good part about this is being able to use `Promise.all()` and related constructs, along with preparing the plugin manager to support a potential soft-dependency system that'd use promises within `init` to gain access to another plugin's data (ideally an intentionally exposed API.)

Finally, there's an unresolved bug currently where `builder` fails to initialise. I believe this is due to it relying on image assets for icons which Electron appears to require to be loaded immediately, or will throw an error.
public API is in fact public API (fixes issue with builder)
@thennothinghappened thennothinghappened marked this pull request as ready for review January 18, 2025 10:03
@thennothinghappened
Copy link
Contributor Author

thennothinghappened commented Jan 18, 2025

I have found one notable issue with this at the moment - currently plugins are still handed the entire project properties menu and are expected to build a group for themselves. Out of a sample size of two that actually use this menu (builder and Constructor), neither currently behave correctly when toggling them if the project properties menu has already been built: Constructor does correctly remove its group if disabled, but isn't informed to go and re-add its group if enabled after the properties have been built once, and builder has the same issue, but doesn't remove its group as it cannot clean up.

The solution for this would just be to go in and do a similar thing to how I handled reloading with preferences groups, but I'm unfortunately a bit busy now - and I'm also not overly happy with the way the preference groups are synced, though didn't have any particularly better ideas for it, that wouldn't majorly affect the current system. I don't feel this is a particularly big issue for the moment and could be resolved in a separate PR, as it only comes into effect in very specific circumstances and doesn't really impact usability that much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant