fix(launch): Forge 1.17+ on Java 17+ via launch-profile rework#8
Open
objz wants to merge 12 commits into
Open
Conversation
introduce the launch_profile module with the core types and pure functions
needed to parse, evaluate, and substitute against mojang-format version
JSON: Rule/OsCondition/FeatureSet with a single rule evaluator,
TemplateContext + substitute() for ${var} expansion, and the
LaunchProfile schema mirroring mojang's on-disk shape. no consumers yet.
…storage vanilla launches now read upstream mojang JSON via the new LaunchProfile path, render game/JVM args through launch_profile::render with template substitution, and store the cached meta.json byte-for-byte instead of re-serializing through a narrow struct. legacy stripped meta.json files are re-fetched on first launch. adds HttpClient::get_bytes and fetch_version_meta_with_raw; collapses the two library-rule evaluators into one shared launch_profile::rules::evaluate.
new launch_profile::resolve module walks a profile's inheritsFrom chain, loading parents from meta_dir/versions/<id>/meta.json and merging them into a single flat LaunchProfile per mojang's documented semantics. pure merge_into handles field-level merging; async resolve walks the chain with HashSet-based cycle detection and an 8-level depth cap.
stop stripping the installer's version JSON in save_installer_profile - copy it byte-for-byte so the upstream arguments.jvm (with the --add-opens flags forge 1.17+ needs on java 17+) reaches launch time. launch loads the loader profile as LaunchProfile, sets an implicit inheritsFrom = game_version when absent, resolves through vanilla, then renders args. classpath assembly unifies into one loop handling both vanilla-style and loader-style libraries. lazy migration of pre-rework loader-profiles from the installer's version JSON when still on disk. fixes the reported Forge 1.17+ on java 17+ IllegalAccessError.
install_forge_from_profile now writes the upstream versionInfo as compact
JSON (no key reordering, no field stripping) instead of building a custom
{mainClass, libraries, gameArguments} shape. legacy 1.7.10-era Forge
profiles ride the same LaunchProfile + resolve + render pipeline as
modern Forge. drops the no-longer-needed extract_extra_game_args helper.
stop re-serializing FabricProfile/QuiltProfile through narrow structs; new fetch_X_profile_with_raw helpers return both the parsed shape (for library downloads) and the upstream bytes (written byte-for-byte to the loader-profiles cache). also fixes a latent issue surfaced by the change: the legacy-loader migration predicate now skips Fabric and Quilt explicitly since their upstream shape matches the predicate but they have no installer JSON to recover from.
…ary dedup extra_args from the patches module was being passed to build_game_args (which appended it to game_args) AND emitted again via cmd.args before game_args - a regression that doubled legacy Forge + lwjgl3ify shim args. extra_args is now passed only once. also, merge_into now dedups libraries by group:artifact, preferring the child's entry - without this, Forge/NeoForge library overrides of vanilla (e.g. log4j) would silently lose to vanilla because the JVM picks the first classpath match.
several smaller fixes from the code review: - add os_version to RuleContext + OsCondition for mojang rules that constrain natives selection by host version (rare, but in the spec). - factor mojang_os_name/arch/version into a shared launch_profile::system module instead of duplicating between net/mojang.rs and launch/mod.rs. - get_json and get_bytes now retry transient failures (5xx, connect, body decode) via a shared get_with_retry envelope; previously only download_file had retry logic. - migrate_legacy_meta_if_needed falls back to the stale cached profile with a warn log on network failure instead of erroring out (offline launches now work). - tightened legacy-loader detection by requiring the unique-to-rmcl gameArguments field; clearer error when loader_version is absent. - forge legacy versionInfo write uses compact serialization (no pretty-print key reorder). - deleted dead fetch_version_meta and save_profile_json. - documented quick-play templates omission in templates::lookup. - added end-to-end test that loads a vanilla profile from disk, loads a loader profile with inheritsFrom, resolves, renders, and snapshots.
- drop stale phase planning comments from launch_profile module headers. - simplify merge_libraries to HashSet<&str> (no string clones). - rewrite coord_key with match_indices (clearer intent). - read /proc/sys/kernel/osrelease on linux for mojang_os_version instead of shelling out to uname; return empty elsewhere. - add comment explaining why is_retryable excludes Parse. - shorten the launch_profile::... paths in launch/mod.rs with top-level use statements. - derive Default on LaunchProfile + nested types so test fixtures use ..Default::default() instead of listing every None field. - inline account_can_launch at its single call site, delete the helper and three tests that exercised trivially-correct one-line logic. - extract get_json_with_raw on HttpClient and save_profile_bytes in instance::loader; collapses three near-identical fetch_X_profile_with_raw helpers and three near-identical 'create dir + write bytes' blocks.
…t on classpath modern forge's installer places some libraries (notably the bootstrap library that defines ForgeBootstrap / BootstrapLauncher) into the instance's .minecraft/libraries/ rather than the shared meta cache. the unified classpath loop introduced in the rework checked downloads.artifact first and pushed meta_dir/libraries/<path> without ever consulting the local installer dir for libraries that had downloads.artifact set. resolve a single relative path per library (downloads.artifact.path if present, else maven_coord_to_path(name)), then for has_local_libs check the local installer dir first regardless of which path source we used. fixes ClassNotFoundException at launch on forge 1.20.6 and likely similar versions.
two related bugs surfaced by manual testing on 1.20.6 vanilla and fabric:
1. FeatureSet missing has_quick_plays_support / is_quick_play_singleplayer /
is_quick_play_multiplayer / is_quick_play_realms. mojang's profiles gate
the quick-play arguments behind these features. serde silently dropped
the unknown fields during deserialization, leaving an empty FeatureSet
that matched any context, so the conditional arguments emitted raw
${quickPlay*} placeholders to the JVM that template substitution then
warned about. add the fields explicitly and extend features_match to
reject any required feature the context doesn't set.
2. TemplateContext missing version_type. ${version_type} is an
unconditional template in modern vanilla's arguments.game (renders the
profile's "type" field, e.g. "release"). without it, the substitution
emitted a literal ${version_type} to minecraft and warned. add the
field, plumb the merged profile's type_ value through, default to
"release" when absent.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the reported crash where Forge 1.17+ fails to launch on Java 17+ with
IllegalAccessError. The root cause was structural: rmcl only understood Mojang's legacyminecraftArgumentsstring format and silently dropped the modernarguments: {game, jvm}object that ships the required--add-opensflags.The fix is a six-phase rework that adds a unified
launch_profilemodule (rule evaluator, template engine, profile model, inheritsFrom resolver, render layer), then rewires the vanilla launch path, Forge/NeoForge installer paths, legacy Forgeinstall_profile.jsonpath, and Fabric/Quilt install paths to read and write upstream Mojang JSON byte-for-byte instead of re-serializing through narrow structs.Two pre-existing latent bugs collapsed out as a side effect: the two divergent library-rule evaluators became one, and the missing template substitution layer became a real engine.
What this changes for users
arguments.jvm.meta.jsonfrom Mojang, with offline fallback to the stale cache<instance>/.minecraft/versions/<name>/<name>.jsonpatches.rsis untouched.Test plan
cargo clippy --all-targets -- -D warningscleancargo fmt --checkclean for touched files--add-opensin JVM args${quickPlay*}or${version_type}warningsHistory
11 commits, conceptually grouped:
22e4481rules,templates,model)202900563014720d006788d2b5b9a7b378b1988764,412e6ff,363ada8d88ab08,f453518