perf(download): skip re-installing loader if identical version patch … - #6594
perf(download): skip re-installing loader if identical version patch …#6594fatelove42 wants to merge 2 commits into
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1d891c6ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (existingPatch.isPresent() && java.util.Objects.equals(existingPatch.get().version(), libraryVersion.getSelfVersion())) { | ||
| return Task.completed(baseVersion); |
There was a problem hiding this comment.
When importing an HMCL modpack for the first time, HMCLModpackInstallTask.execute starts with the manifest embedded in minecraft/pack.json and deliberately calls this method for every loader because Forge and OptiFine libraries must be obtained through their installers. That embedded manifest already contains the matching patch ID and version, so this return bypasses getInstallTask and all of its download tasks, leaving a fresh imported instance without the required loader libraries. Restrict this shortcut to updates where the loader was already installed in the existing instance rather than treating a patch in the desired manifest as proof of installation.
Useful? React with 👍 / 👎.
| if (existingPatch.isPresent() && java.util.Objects.equals(existingPatch.get().version(), libraryVersion.getSelfVersion())) { | ||
| return Task.completed(baseVersion); |
There was a problem hiding this comment.
When a user changes the Minecraft version but reselects the same Fabric or Quilt loader version, AdditionalInstallersPage requires that loader to be reinstalled for compatibility. The patch version compared here identifies only the loader version, while the generated patch contains game-specific metadata such as Fabric's intermediary library; therefore this equality succeeds against the old patch even though libraryVersion.getGameVersion() targets a different Minecraft release. The upgrade then retains the old game-specific loader metadata and can produce an unlaunchable instance, so the shortcut must also establish that the loader patch belongs to the same Minecraft version.
Useful? React with 👍 / 👎.
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23ee8091d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
41a17dd to
6499b20
Compare
|
哈基米把我总test合到branch了（） |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6499b200e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
94b082f to
bc9b086
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc9b086a49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| boolean needsRepair = resolved.launchManifest().getLibraries().stream() | ||
| .filter(lib -> isLoaderLibrary(lib, libraryVersion.getLibraryId())) | ||
| .anyMatch(lib -> GameLibrariesTask.shouldDownloadLibrary(repository, existingManifest, lib, true)); |
There was a problem hiding this comment.
When rebuilding an existing instance with the same Minecraft version, DefaultGameBuilder also passes the "game" remote version through this method. The default matcher then searches for library group IDs containing "game", normally finds none, and declares the patch intact without checking the Minecraft JAR, vanilla libraries, or assets. Consequently, an update of an instance whose game files are missing or corrupt skips GameInstallTask, which previously repaired those files, and leaves the instance unlaunchable; either limit this shortcut to actual loaders or perform the game-specific completion checks before skipping.
Useful? React with 👍 / 👎.
bc9b086 to
fd56d5c
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd56d5c7cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .filter(lib -> isLoaderLibrary(lib, libraryVersion.getLibraryId())) | ||
| .anyMatch(lib -> GameLibrariesTask.shouldDownloadLibrary(repository, existingManifest, lib, true)); |
There was a problem hiding this comment.
When a same-version Forge installation is missing an installer-generated dependency outside net.minecraftforge—for example the net.minecraft:client:...:srg JAR—while its Forge-group JARs remain intact, this filter excludes the missing file and needsRepair stays false. The shortcut then bypasses ForgeNewInstallTask and its processors while preserving a patch that still references the absent output, leaving the instance unlaunchable; determine integrity from all libraries contributed by the matching patch rather than group-ID heuristics.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| try { | ||
| GameInstanceManifest existingManifest = repository.getInstanceManifest(baseVersion.id()); |
There was a problem hiding this comment.
For the modpack-update path in DefaultGameBuilder, the "game" task always runs before the loader tasks, and GameInstallTask saves a manifest containing only the new game patch. Consequently this lookup observes that intermediate game-only manifest rather than the pre-update one, finds no matching loader patch, and reinstalls every loader anyway. Preserve or inspect the original manifest before the game task overwrites the repository state so the intended same-version modpack optimization can take effect.
Useful? React with 👍 / 👎.
2 participants
整合包升级时跳过版本重复的modloader下载安装