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

(WIP) ModLauncher 11 #3

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

PaintNinja
Copy link
Contributor

@PaintNinja PaintNinja commented Nov 23, 2024

This draft PR proposes some breaking changes in order to further clean-up and simplify ModLauncher.

Main changes:

  • Java 21 baseline
  • Removed almost all deprecated code
  • Removed most unused/dead code
  • Use jopt-simple's native module name
  • More records and static finals
  • Slightly more javadocs
  • Fail-fast service loading

Open questions:

  • Should I leave the ILaunchHandlerService#launchService method returning ServiceRunner until the ModLauncher 12 rewrite?
    • The ServiceRunner was always immediately called after being retrieved, so calling directly rather than forcing indirection through a ServiceRunner is simpler. But this is a wide-reaching breaking change that is likely to break again in the ModLauncher 12 rewrite anyway.
  • What else should ILaunchHandlerService have added to it? Forge's usages says it needs more things, but doesn't say what exactly
  • Can I change ModuleLayerHandler to static init inside the Launcher main class? This would remove the need for passing a lot of objects around everywhere internally (both the ModuleLayerHandler itself and things that rely on it), which in turn could simplify things much further
  • Can ProtectionDomainHelper be removed?
  • Can the two classes in the log package be removed?

Still to do:

  • Changes based on answers to the open questions above
  • Look into unifying TransformTargetLabel.LabelType and ITransformer.TargetType
  • PR to SecureModules' SecureModuleFinder#of to accept an Iterable

- Reuse the same `OptionParser` and default `OptionSpec`s
- Feed args directly into the constructor
- Store DiscoveryData instance in one place, use it as a cache for things like `getSpecialJars()` and `getLaunchTarget()`
The `ServiceRunner` was always immediately called after being retrieved, so calling directly rather than forcing indirection through a `ServiceRunner` is simpler.

The `TestingLaunchHandlerService` continues calling `public static ServiceRunner supplier()`, but now uses `invokeExact()` instead of `invoke()`
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