-
Notifications
You must be signed in to change notification settings - Fork 239
Update to JDK 24; simplify application launch code #1146
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
Conversation
Build process is broken because the Shade plugin does not support class files compiled by JDK 24. I plan to remove it in the future because I do not think applications should not use "Uber" jars.
The initialization logic is contained in the main method, rather than split into sub-methods. Removed code relating to graphics acceleration, a feature that is enabled by default on Swing; Swing defaults to the best pipeline on the system.
I expect to remove this class soon. Too many indirections to open the UI.
Everything initialized is in the, well, initializer.
This will be merged into ProfileSelectWindow as those are tightly coupled.
…method I doubt there are any issues, and I suspect the issue that Amidst was trying to get around was fixed between JDK 8 and JDK 24.
These interfaces were for lambdas, but it's better to pass objects or primitives through.
I just reviewed your changes. Its great, that you updated the Java version! Maybe, it can be updated again, to the new LTS? You might also want to adjust the file
I would like that, but am a bit worried that this might be harder for Amidst than for other Applications. The reason is, that the bundled JRE does not only need to provide all modules to run Amidst, but it also needs to provide the modules to run Minecraft, in all the different versions. Just a small heads-up. Since you plan to maintain the project, I don't mind the other code adjustments. And I agree, that constructors with lots of parameters are not great. When I try to build it with Java 24, then I get the following error.
Please let me know about your plans for this PR. Would you like me to review (done) and integrate the changes? Also, I recommend to change only one thing per PR. For me it is also fine, if you integrate the changes into master by yourself. I think you have the right to do that. Otherwise, please let me know in #1144. Its great to see this project to get some attention again =) |
Hey Stefan, sorry about the wait. I've been taking a break programming (the past 6 or 7 weeks have not been conducive for it). I had made this PR because I can't push directly to the branch, then forgot about it! oops. The intention is to break the build pipeline as it exists currently, and fix it later with a non-shaded JAR solution. |
The JDK version for Amidst is now 24, up from 8 (released in 2014). Several dependencies have also been upgraded. Moving to Java 24 will improve performance and allow for the use of language features and APIs added in the decade plus since. This is also a part of a plan to bundle the JRE with Amidst in distributing the application, rather than relying on users to download a (unsupported) Java version themselves. Modern Java applications should have the runtime, dependencies, and code bundled into a package, which Uber/shaded JARs do not do (and why I note that the Maven plugin will be removed). (This will also mean that the JRE we distribute can be much smaller, only including the modules necessary.)
Aside from that, I've been working to condense the launch code. Methods used once were mostly inlined, as the code run needn't be abstracted. (Abstracting something away should really only be done when the method is invoked more than once, if the logic is detached from the rest of the code, or to inline something with a result from a more complicated function.) 'Factory' methods were also inlined, as they were only used once and belie what the code is actually doing (creating a
new
object).The three 'Injector' classes were merged with their respective classes. These pairs were so tightly coupled, that it made it more complicated splitting it in two. The result is much cleaner and direct, and will help in my ability to maintain the application. In theory, all these changes should cause no regressions.