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

Redesigned core application #33

Merged
merged 87 commits into from
Mar 10, 2023
Merged

Redesigned core application #33

merged 87 commits into from
Mar 10, 2023

Conversation

gentlegiantJGC
Copy link
Member

@gentlegiantJGC gentlegiantJGC commented Feb 2, 2023

This is a major redesign of the application architecture.

Overview

The aim here is for the core application to be as minimal as possible.
The actual application is fully implemented in plugins that can depend upon other plugins.
Plugins are individually versioned and must define the plugins they depend upon in order to import them.
This will allow us to make breaking changes to a first party plugin and know that third party plugins are not compatible without loading them.

Architecture workflow

The main script initialises the application as defined in amulet_editor.application._app.
The application initialises the plugin manager which displays a splash screen and enables plugins as their dependencies are satisfied.
The plugin manager can be found in amulet_editor.data.plugin._manager. Perhaps this should move into application.
If an error occurs loading a plugin a traceback dialog is displayed and the plugin is unloaded.
When the last window is closed a closing splash screen is displayed and the plugins are unloaded starting from the most dependent until they are all disabled at which point the application exits.

Plugins

Plugins are a python package containing a plugin.json file to define metadata so that we do not need to evaluate the python code before it is enabled.
Here is an example.

{
	"identifier": "amulet_team_home_page",
	"version": "1.0.0",
	"name": "Amulet Home Page",
	"depends": [
		"amulet_team_locale~=1.0",
		"amulet_team_main_window~=1.0"
	]
}

The root module of the package (__init__.py) can define a load_plugin and unload_plugin function that is called when the plugin is enabled and disabled.

Plugin loading

First party plugins are stored in amulet_editor.plugins and third party plugins in a data directory.
Plugins cannot be directly imported from these locations. They must first be enabled.
We first ensure the dependencies for the plugin are enabled and do some other validation steps.
We then load the package and install it into sys.modules under the identifier field defined in plugin.json
Once that is finished we call load_plugin to tell the plugin to start.

Plugin loading happens

  1. at application startup if the plugin is a fixed first party plugin or was previously enabled.
  2. when the user manually enables it.

Plugin unloading

We first recursively unload all plugins that depend upon the plugin being unloaded.
We then call unload_plugin to notify the plugin it is being unloaded.
We then remove it from sys.modules so that it cannot be imported.

Plugin unloading happens when the application shuts down or the user disables the plugin or a dependent plugin.

Existing plugins

Some plugins are partially implemented but may change.
amulet_team_app - a plugin exposing features from the core application.
amulet_team_locale - a plugin to manage changing the locale and notifying plugins of changes.
amulet_team_main_window - a plugin implementing the main GUI window framework.
amulet_team_home_page - a plugin implementing a home page within the main window.
amulet_team_settings - a plugin implementing a settings button in the main window. Will allow plugins to add settings.

Other

Implemented translation switching at runtime.
Probably other things I have forgotten about.

There is one class per operation mode.
One class is used when the app is in startup mode. This shows the GUI to open a level.
The other is for a process that manages a level.
Initially I thought it was best to make the window as small as possible but now I think it is best to start maximised.
This more closely links the tool buttons and views.
Moved the pages associated with the home view into a new package.
Adds base view class
Migrated the code from the proto-plugin branch.
This translator can read lang files in the key=value per line format.
I find this format to be easier to use than the binary format qt uses.
Added an internal signal to set the locale and a signal to detect when it has been changed.
Set up translations for all components that exist outside of a plugin.
This exposes the locale setter and signal to other plugins.
@jevexendo jevexendo added the enhancement New feature or request label Feb 2, 2023
@jevexendo
Copy link
Member

@gentlegiantJGC I haven't had time to review the actual code implementation yet, but does this system allow plugins to define required Python packages?

@gentlegiantJGC
Copy link
Member Author

It doesn't currently but I could probably implement it fairly easily.
It is a feature we should have.

@gentlegiantJGC
Copy link
Member Author

gentlegiantJGC commented Feb 4, 2023

I think the depends metadata entry should look like this to support specifying python and library versions as well as plugins.

	"depends": {
		"python": "~=3.9",
		"library": [
			"PySide6~=6.2"
		],
		"plugin": [
			"amulet_team_locale~=1.0",
			"amulet_team_main_window~=1.0"
		]
	},

Plugin could originally only define other plugins as dependents.
This refactors the dependency system to include python dependency and the libraries distributed with python, other python libraries and plugins.
This will allow us to update libraries and know if the plugin is compatible.
Updated the import validation to verify if the imported module is listed in the dependents.
Still need to validate this when the plugin is enabled.
Validate the python and library dependencies before activating the plugin
@gentlegiantJGC
Copy link
Member Author

I have implemented python and library dependencies.
A plugin will only be enabled if the python version is compatible and all of the python libraries are installed and compatible.
As before the plugin will only be enabled when all the dependent plugins are enabled.
If a plugin tries importing a library or plugin that it has not listed as a dependency an exception will be raised.
Libraries that are shipped with python (builtin or stdlib) are tied to the python version and should not be specified in the library metadata entry. These can just be imported.

@jevexendo
Copy link
Member

What do you think about using the official pyproject.toml spec for the plugin configuration/settings file? Since it already covers much of the functionality we'd want to implement and since I think that most Python developers should already be familiar with the format, I feel like it might be a nice format to leverage for this purpose.

@StealthyExpertX
Copy link

Jevexendo that idea would be cool and would help expand what libraries we can use since we can set the depends.

My question would be though would pip be installed already somehow so that this can be done so libraries get auto-installed with plugins?

@StealthyExpertX
Copy link

StealthyExpertX commented Feb 6, 2023

Also, another note is there a way to define tags for plugins and also what platforms they can be used and what versions of Minecraft it supports?

Might be a good idea to support those with list formats "mc_versions" : ["1.0.0", "1.19.20"] and ranged formats like so
"mc_versions" :["1.16.10-1.19.0"] and maybe authors as well?

"mc_platforms" :["java","bedrock"]

"tags" :["Chest Editor", "NBT Editing", "LootTables"]

"authors": ["StealthyX", "ThatOtherGuy"]

@jevexendo
Copy link
Member

If we went with the pyproject.toml spec then I believe that we would get those features for free, but regardless, I think that those tags are reasonable things to support.

@gentlegiantJGC
Copy link
Member Author

gentlegiantJGC commented Feb 7, 2023

Our needs are a bit different to the pyproject.toml file.
We need the ability to distinguish normal python libraries from plugins. They are handled differently.
If a plugin depends upon a library that isn't installed or has the wrong version we don't install the dependency, we just don't enable that plugin.
Plugin dependencies on the other hand can be enabled at a later date potentially satisfying the first plugin.
Also when disabling we need to disable all dependent plugins.

I also don't know if we can actually install dependencies from pypi to a frozen bundle and I don't fancy writing a full dependency system to manage requirement conflicts between plugins.
In javascript plugins can include their own dependencies so version conflicts are not an issue but in python all imports are global.

We would need to modify the pyproject.toml so it makes more sense to just start from scratch.

Game version dependencies I don't think should be done in the metadata. I think the plugin should just inspect the level object when loaded and decide if it should do anything or not.

I will add other metadata fields in the future such as author, homepage, bug tracker.

@Podshot
Copy link
Member

Podshot commented Feb 15, 2023

I don't think installing outside packages from PyPi is a good idea. All it would take is a typo-squatted package or a plugin developer intentionally including a malicious package to harvest data or do damage to a users machine.

While installing 3rd-party plugins does have an inherent risk that a user should be aware of, I feel that by implementing additional package installation that is defined by plugins increases that risk to a level I personally feel is too much.

A user can decided whether to trust and download a 3rd-party plugin themselves, but there are both intentional and accidental ways that the installation of additional external packages would be harder for an end user to verify if they wanted to. Even then a seemingly harmless package can turn malicious later if there is some switch/flag hidden within or if a "supply-chain"/"developer-chain" attack occurs (IE: typo-squatting somewhere in the dependency tree or the original authors account/package is hijacked)

@gentlegiantJGC
Copy link
Member Author

Yeh that is another reason that I hadn't considered.

@gentlegiantJGC gentlegiantJGC merged commit 7d4fe14 into main Mar 10, 2023
@gentlegiantJGC gentlegiantJGC deleted the proto-gui-2 branch March 10, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants