-
Notifications
You must be signed in to change notification settings - Fork 922
perf(download): skip re-installing loader if identical version patch … #6594
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
base: main
Are you sure you want to change the base?
Changes from all commits
f66592e
fd56d5c
f351e1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,9 +27,11 @@ | |
| import org.jackhuang.hmcl.game.Artifact; | ||
| import org.jackhuang.hmcl.game.DefaultGameRepository; | ||
| import org.jackhuang.hmcl.game.GameInstanceManifest; | ||
| import org.jackhuang.hmcl.game.GameInstancePatch; | ||
| import org.jackhuang.hmcl.game.Library; | ||
| import org.jackhuang.hmcl.task.Task; | ||
| import org.jackhuang.hmcl.util.io.FileUtils; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Files; | ||
|
|
@@ -155,8 +157,50 @@ public Task<GameInstanceManifest> installLibraryAsync(String gameVersion, GameIn | |
| .withStage(String.format("hmcl.install.%s:%s", libraryId, libraryVersion)); | ||
| } | ||
|
|
||
| /// Checks if a matching library patch is already installed for the target instance on disk and intact. | ||
| /// | ||
| /// @param baseVersion the base game instance manifest | ||
| /// @param libraryVersion the remote library version to install | ||
| /// @return the existing intact matching patch, or `null` if the library is not installed, version mismatches, or files need repair | ||
| private @Nullable GameInstancePatch getIntactMatchingPatch(GameInstanceManifest baseVersion, RemoteVersion libraryVersion) { | ||
| if (LibraryAnalyzer.LibraryType.MINECRAFT.getPatchId().equals(libraryVersion.getLibraryId()) || !repository.hasInstance(baseVersion.id())) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| GameInstanceManifest existingManifest = repository.getInstanceManifest(baseVersion.id()); | ||
| String currentGameVersion = repository.getGameVersion(existingManifest).orElse(null); | ||
| if (!java.util.Objects.equals(currentGameVersion, libraryVersion.getGameVersion())) { | ||
|
Comment on lines
+171
to
+173
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| return null; | ||
| } | ||
|
|
||
| GameInstancePatch matchingPatch = existingManifest.getPatches().stream() | ||
| .filter(patch -> libraryVersion.getLibraryId().equals(patch.id()) | ||
| && java.util.Objects.equals(patch.version(), libraryVersion.getSelfVersion())) | ||
| .findFirst() | ||
| .orElse(null); | ||
| if (matchingPatch == null) { | ||
| return null; | ||
| } | ||
|
|
||
| List<Library> patchLibraries = matchingPatch.libraries(); | ||
| boolean needsRepair = patchLibraries != null && patchLibraries.stream() | ||
| .filter(Library::appliesToCurrentEnvironment) | ||
| .anyMatch(lib -> GameLibrariesTask.shouldDownloadLibrary(repository, existingManifest, lib, true)); | ||
|
|
||
| return needsRepair ? null : matchingPatch; | ||
| } catch (Exception ignored) { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public Task<GameInstanceManifest> installLibraryAsync(GameInstanceManifest baseVersion, RemoteVersion libraryVersion) { | ||
| GameInstancePatch matchingPatch = getIntactMatchingPatch(baseVersion, libraryVersion); | ||
| if (matchingPatch != null) { | ||
| return Task.completed(baseVersion.addPatch(matchingPatch)); | ||
| } | ||
|
|
||
| AtomicReference<GameInstanceManifest> removedLibraryVersion = new AtomicReference<>(); | ||
|
|
||
| return removeLibraryAsync(baseVersion, libraryVersion.getLibraryId()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,7 +72,14 @@ public void execute() throws Exception { | |
| GameInstancePatch.PRIORITY_MC).withJar(null); | ||
| setResult(patch); | ||
|
|
||
| GameInstanceManifest version = new GameInstanceManifest(this.manifest.id()).addPatch(patch); | ||
| GameInstanceManifest existingManifest = null; | ||
| if (gameRepository.hasInstance(this.manifest.id())) { | ||
| try { | ||
| existingManifest = gameRepository.getInstanceManifest(this.manifest.id()); | ||
| } catch (Exception ignored) { | ||
| } | ||
| } | ||
| GameInstanceManifest version = (existingManifest != null ? existingManifest : this.manifest).addPatch(patch); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a modpack update removes a loader or switches loader families, this uses the complete existing manifest as the base and replaces only the Minecraft patch. The builder subsequently installs only loaders listed by the new pack, so an omitted old Forge/Fabric/OptiFine patch is never removed and is saved into the final manifest; switching Forge to Fabric can therefore leave both loaders active. Preserve only patches that are part of the requested build, rather than every existing patch. Useful? React with 👍 / 👎. |
||
| dependencies.add(Task.allOf( | ||
| new GameDownloadTask(dependencyManager, remote.getGameVersion(), version), | ||
| Task.allOf( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.