Port modflared from NeoForge 1.21.11 to Forge 1.12.2#95
Port modflared from NeoForge 1.21.11 to Forge 1.12.2#95HttpRafa merged 10 commits intoHttpRafa:forge/1.12.2from
Conversation
![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 ports the modflared mod to Minecraft 1.12.2 (Legacy). The changes involve a complete overhaul of the build system to use ForgeGradle 2.3, downgrading the project to Java 8, and reimplementing the core logic and mixins to align with the legacy Minecraft codebase. Feedback focuses on a critical bug in the binary update logic where the new version string is discarded, and a regression regarding the removal of macOS support.
|
@hattapauzi Can you rebase your branch on top of the new forge/1.12.2 branch? I merged the latest changes from NeoForge 1.20.11 into it so your PR doesn't include them. |
- Switch build.gradle to ForgeGradle 2.3 with MCP mappings - Set Java source/target compatibility to 1.8 - Add Mixin 0.8.2 dependency with shadowJar bundling - Replace neoforge.mods.toml with legacy mcmod.info - Replace JSON language files with .lang format - Update modflared.mixins.json for 1.12.2 MCP class names
- Convert Java 21 records to standard Java 8 classes - Replace var keyword with explicit types - Replace Files.writeString/readString with FileWriter/BufferedReader - Replace String.strip() with String.trim() - Preserve deterministic CRC32 port algorithm - Fix cloudflared access tcp command to use --destination host:port
- Replace CompletableFuture chaining with explicit BiConsumer callbacks - Replace Platform.OS with System.getProperty(os.name) checks - Update Gson API from JsonParser.parseReader to new JsonParser().parse - Replace SLF4J with Log4j2 LogManager.getLogger - Replaced var keyword with explicit types
- Replace NeoForge FMLPaths with Minecraft.getMinecraft().gameDir - Replace ServerAddress.parseString with ServerAddress.fromString + try/catch - Update all record-style accessors to getter methods (getAccess, getState) - Replace modern toast notifications with logging-only error reporting - Update JsonParser API for Gson Java 8 compatibility
- Replace ConnectionMixin with NetworkManagerMixin (func_150718_a) - Replace ConnectScreenMixin with GuiConnectingMixin (func_73863_a) - Replace ConnectScreenRunnableMixin with GuiConnectingThreadMixin (redirects func_181124_a / createNetworkManagerAndConnect) - Replace ServerStatusPingerMixin with ServerPingerMixin (func_147224_a) - Add null guards in mixins for failed tunnel creation - Remove obsolete 1.21 mixin classes
- Add RunningTunnelAccessTest for deterministic CRC32 port generation - Add CloudflaredDownloadTest for OS/arch binary selection logic - Add ForcedTunnelsJsonTest for forced_tunnels.json array validation
hattapauzi
force-pushed
the
forge/1.12.2-port
branch
from
50d4af9 to
c5fd330
Compare
Am I doing this right? I am not quite sure |
You got it right. |
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request ports modflared to Minecraft 1.12.2, downgrading the project to Java 8 and Gradle 4.10.3 while migrating from NeoForge to Forge and Mixins. The changes involve replacing modern Java syntax with legacy-compatible alternatives and updating Mixin targets for the 1.12.2 environment. Feedback highlights several issues, including a hardcoded Java path in the build configuration that hinders portability and a bug in the update logic that prevents downloading new versions. Additionally, the reviewer noted misleading error messages, a regression in user-facing error notifications, and improper exception handling that disrupts the original connection error UI.
…dCloudflared.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…dCloudflared.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request downgrades the project from NeoForge (Java 21) to Forge 1.12.2 (Java 8), requiring a complete overhaul of the build system, mixins, and codebase for legacy compatibility. Modern Java features like records, the var keyword, and stream APIs have been replaced with Java 8 compatible constructs. Feedback identifies a critical issue where the shadowJar task lacks necessary reobfuscation for production environments and a bug in the tunnel command generation where an unsupported --destination flag would cause the cloudflared process to fail.
|
@hattapauzi It might take a minute before this version is available on Modrinth, because I still need to fix the CI and a few other things. |
Its okay, take your time... I am just happy that my pr is merged... |
2 participants
Summary
Port the modflared Minecraft mod from NeoForge 1.21.11 to Forge 1.12.2, downgrading from Java 21 to Java 8 compatibility while preserving all core functionality including tunnel management, cloudflared binary download, and Mixin-based connection interception.
Changes
Build System Migration
neoforge.mods.tomlwith legacymcmod.info.langformatMod Entrypoint & Mixin Bootstrap
@Modentrypoint with legacy@Mod+IFMLLoadingPluginModflaredMixinLoadercoremod for Mixin initializationMixin Renaming (1.21 → 1.12.2)
ConnectionMixin→NetworkManagerMixin(func_150718_a)ConnectScreenMixin→GuiConnectingMixin(func_73863_a)ConnectScreenRunnableMixin→GuiConnectingThreadMixin(redirects func_181124_a)ServerStatusPingerMixin→ServerPingerMixin(func_147224_a)Java 8 Compatibility Refactoring
varkeyword with explicit typesFiles.writeString/readStringwithFileWriter/BufferedReaderString.strip()withString.trim()CompletableFuturechaining with explicitBiConsumercallbacksPlatform.OSwithSystem.getProperty(os.name)checksJsonParser.parseReadertonew JsonParser().parseLogManager.getLoggerTunnel Manager Updates
FMLPathswithMinecraft.getMinecraft().gameDirServerAddress.parseStringwithServerAddress.fromString+ try/catchgetAccess,getState)Testing
RunningTunnelAccessTestfor deterministic CRC32 port generationCloudflaredDownloadTestfor OS/arch binary selection logicForcedTunnelsJsonTestfor forced_tunnels.json array validationManual Testing
As for now, I just tested a successful connectivity from client machine -> server in which playable in vanilla Minecraft...
(I use arch btw)
I have not got the chance to do the testing for Windows machine(would be in the near future) + Mac machine (which I dont have)...
Have serveral small details in need of fixing which I did not make changes for yet:
Disclosure
Yes, I did vibe my way out with AI to make this possible... I am not a skilled Java programmer(still actively learning and wanted to contribute) but I did do a proper way of prompting(they call it agentic engineering for some reason)
If you interested in looking into this(I just wanted to be transparent), it is called Spec-Driven Development...
https://github.com/hattapauzi/modflared/tree/spec-driven-dev
I would like to contribute more in the future as I would use this mod regularly(me and the boys love to play RLcraft)...
Dependencies
https://www.curseforge.com/minecraft/mc-mods/mixinbootstrap/files/3437402
Yeah, unfortunately this is the side effect of using coding agent... For some reason Kimi K2.6 decided to use another mod as a dependency... I used a free credits given by Fireworks AI (10$ worth 😆 took me 2 attempt to make this work)... If there is a work around to do this that uses only one single .jar file like the other modflared versions please let me know!