fix(ci): Use Java 8 in CI workflow for ForgeGradle compatibility#97
fix(ci): Use Java 8 in CI workflow for ForgeGradle compatibility#97hattapauzi wants to merge 5 commits into
Conversation
Add a client mixin for multiplayer server-list entries so servers routed through modflared display the existing indicator icon. Position the indicator away from the Forge modded-server compatibility badge and use direct visible strings for the hover and connection status text to avoid untranslated lang keys appearing in-game. Register the new mixin and keep the language entry for the server-list tooltip resource.
Reset the render color to white before binding and drawing the modflared server-list indicator. This prevents the indicator texture from inheriting tint from previous server-list rendering state or other modded indicators.
Restore localization lookups for the server-list tooltip and connection feedback while preserving English fallbacks when the legacy client returns unresolved translation keys. This addresses the review feedback without reintroducing the in-game issue where raw translation keys appeared in the UI.
Update the Forge 1.12.2 artifact version from 1.12.2-legacy.1 to 1.12.2-1 now that the branch is tested and feature complete. This changes the generated jar names to drop the legacy qualifier while preserving the mod id and build configuration.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the mod version and introduces a visual indicator in the multiplayer server list to show when a Cloudflare tunnel is active. It also refactors translation logic to provide fallbacks when localization keys are missing. Feedback was provided regarding a version mismatch between configuration files, a potential server-side crash due to client-only mixins being registered in the common list, and a redundant null check in the translation helper.
| mod_name=Modflared | ||
| mod_license=MIT | ||
| mod_version=1.12.2-legacy.1 | ||
| mod_version=1.12.2-1 |
There was a problem hiding this comment.
The version has been updated to 1.12.2-1 here, but the Modflared.java file (line 25) still contains the old version string 1.12.2-legacy.1. Since the @Mod annotation in Modflared.java relies on that constant, the mod will still report the old version to Forge. Please ensure both are synchronized.
|
Let say if this fix is successful, would it still means that MixinBootstrap still be needed as dependencies? |
|
We should use Java 8 because Minecraft 1.12.2 uses it. |
hattapauzi
changed the title
Per maintainer feedback, switch CI from Java 11 to Java 8 to match the Minecraft 1.12.2 runtime. Also switch the JDK distribution from 'microsoft' (which does not publish Java 8 builds) to 'temurin' (Adoptium). Refs: HttpRafa#97 (comment)
|
Ok, I made the change... but the mod is still has some bugs to it, last time I tested on RLCraft, when I took damage, it disconnected me from the server... I need to test it again on vanilla just to make sure... |
weird, why i cant play it in RLcraft... vanilla works flawlessly |
Probably because of Mixin conflicts. Does the game crash or are you just disconnected. |
Just disconnected, Why is the build's failing? |
2 participants
Summary
Fix CI build failure by using Java 8 instead of Java 21, as ForgeGradle 2.3 and Gradle 4.x require the Pack200 API which was removed in Java 14, and Minecraft 1.12.2 itself runs on Java 8.
Context
The CI workflow was configured to use Java 21, which caused the build to fail with:
The Pack200 API was deprecated in Java 8 and removed in Java 14. Since this project uses ForgeGradle 2.3, Gradle 4.10.3, and targets Minecraft 1.12.2 (an older toolchain that uses Java 8), Java 8 is the appropriate choice.
Changes
21to8in.github/workflows/build_publish.ymlmicrosoft(which does not publish Java 8 builds) totemurin(Adoptium)Testing
The change can be verified by running the CI workflow, which should now complete successfully without Pack200 errors.
Links
java/util/jar/Pack200inapplyBinaryPatchestask