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

Improve Package Loader #95

Closed
3 tasks done
zortax opened this issue Jan 10, 2021 · 2 comments · Fixed by #122
Closed
3 tasks done

Improve Package Loader #95

zortax opened this issue Jan 10, 2021 · 2 comments · Fixed by #122
Labels
improvement An improvement to an existing feature

Comments

@zortax
Copy link
Contributor

zortax commented Jan 10, 2021

There are some things that need to be done:

  • Refactor package loading (into a more responsibility driven approach)
  • Fix bugs in package loading
  • implement a better algorithm for dependency resolution (find best loadable dep. graph)
@zortax zortax added improvement An improvement to an existing feature doing labels Jan 10, 2021
@zortax zortax added this to To do in Flint Development via automation Jan 10, 2021
@zortax zortax moved this from To do to In progress in Flint Development Jan 10, 2021
@Janrupf
Copy link
Contributor

Janrupf commented Jan 10, 2021

Please consider implementing at least a basic support for unloading - it doesn't have to be functional yet, but just be there so it can get implemented eventually.

@zortax
Copy link
Contributor Author

zortax commented Jan 10, 2021

unloading is pretty much impossible to implement for packages that use class transformations or anything alike. To properly implement unloading for the other APIs, significant changes in the Service API would be necessary. Proposed architecture would be to differ between ServiceHandler and UnloadableServiceHandler which would then also implement a method to unload a discovered annotation; I could then automatically blacklist packages with one or more annotation discovered by a not unloadable service handler from being unloaded, though this would make it incredibly nontransparent for package authors wether their package is gonna be unloadable. And even if a package should be unloadable by itself, the unloading could be blocked as it could potentially break the dependency graph. The problem gets even more complicated when considering that packages can contain service handlers as well.

So unloading is

  • difficult to implement for packages that only depend on the minecraft API
  • even more difficult for packages that use class transformation APIs
  • pretty much impossible to implement for packages using javassist/ow2 ASM (unless we use instrumentation/hotswapping which I wouldn't)
  • nontransparent and difficult to understand for package authors and users

That's why I personally would not implement unloading at all and I am certainly not gonna do it right now.
We can however talk about a "disabling" API, which optionally allows packages to be disabled. The package won't be unloaded, event listeners etc. would still be registered, the responsibility would be with the package author to just "disable" all package logic. The package author could then decide whether it can be disabled by the user.

@zortax zortax moved this from In progress to Review in progress in Flint Development Jan 28, 2021
@zortax zortax linked a pull request Jan 28, 2021 that will close this issue
Flint Development automation moved this from Review in progress to Done Jan 29, 2021
@jan-br jan-br added improvement An improvement to an existing feature and removed doing improvement An improvement to an existing feature labels Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement An improvement to an existing feature
Projects
Development

Successfully merging a pull request may close this issue.

3 participants