Skip to content

feat: 下载页面折叠快照 Minecraft 版本 #5601

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.

Sign up for GitHub

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

Merged
Glavo merged 9 commits into from
Mar 27, 2026
Merged

feat: 下载页面折叠快照 Minecraft 版本 #5601

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9963f37
update
CiiLu Feb 21, 2026
41b0b8f
update
CiiLu Feb 21, 2026
281c1c5
Update HMCLCore/src/test/java/org/jackhuang/hmcl/util/versioning/Game…
CiiLu Feb 25, 2026
abd9eb8
Merge branch 'main' into releas
CiiLu Feb 26, 2026
fe3ad4f
Merge remote-tracking branch 'upstream/main' into releas
Glavo Mar 13, 2026
880ca03
Merge remote-tracking branch 'upstream/main' into releas
Glavo Mar 27, 2026
7bf2c31
Refactor `getReleaseOfSnapshot` to improve code readability and add n…
Glavo Mar 27, 2026
77516d4
Refactor version listing logic in `DownloadPage` and optimize `getRel…
Glavo Mar 27, 2026
e02c5a8
update
Glavo Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 41 additions & 39 deletions HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/DownloadPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,9 @@
import javafx.geometry.Insets;
import javafx.geometry.Pos;
import javafx.scene.Node;
import javafx.scene.control.Control;
import javafx.scene.control.Label;
import javafx.scene.control.ScrollPane;
import javafx.scene.control.Skin;
import javafx.scene.control.SkinBase;
import javafx.scene.control.*;
import javafx.scene.image.ImageView;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.HBox;
import javafx.scene.layout.Priority;
import javafx.scene.layout.Region;
import javafx.scene.layout.StackPane;
import javafx.scene.layout.VBox;
import javafx.scene.layout.*;
import javafx.stage.FileChooser;
import org.jackhuang.hmcl.download.LibraryAnalyzer;
import org.jackhuang.hmcl.game.HMCLGameRepository;
Expand All @@ -55,26 +46,15 @@
import org.jackhuang.hmcl.ui.SVG;
import org.jackhuang.hmcl.ui.construct.*;
import org.jackhuang.hmcl.ui.decorator.DecoratorPage;
import org.jackhuang.hmcl.util.Lang;
import org.jackhuang.hmcl.util.Pair;
import org.jackhuang.hmcl.util.SimpleMultimap;
import org.jackhuang.hmcl.util.StringUtils;
import org.jackhuang.hmcl.util.TaskCancellationAction;
import org.jackhuang.hmcl.util.*;
import org.jackhuang.hmcl.util.i18n.I18n;
import org.jackhuang.hmcl.util.io.FileUtils;
import org.jackhuang.hmcl.util.javafx.BindingMapping;
import org.jackhuang.hmcl.util.versioning.GameVersionNumber;
import org.jetbrains.annotations.Nullable;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.List;
import java.util.Set;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -318,25 +298,47 @@ protected ModDownloadPageSkin(DownloadPage control) {
}
}

for (String gameVersion : control.versions.keys().stream()
List<String> versionList = control.versions.keys().stream()
.map(gv -> Objects.requireNonNullElse(GameVersionNumber.getReleaseOfSnapshot(gv), gv))
.distinct()
.sorted(Collections.reverseOrder(GameVersionNumber::compare))
.toList()) {
List<RemoteMod.Version> versions = control.versions.get(gameVersion);
if (versions == null || versions.isEmpty()) {
continue;
}
.toList();
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This builds versionList by calling getReleaseOfSnapshot() for every key, and then later the code calls getReleaseOfSnapshot() again inside nested loops over versionList × versions.keys(). For mods with many supported versions this becomes unnecessarily expensive (and getReleaseOfSnapshot() may scan DEFAULT_GAME_VERSIONS for legacy snapshots). Consider computing a Map<String, String> gvToRelease once, then grouping keys in a single pass (e.g., Map<String, List<String>>) to avoid repeated parsing/lookups.

Copilot uses AI. Check for mistakes.

for (String release : versionList) {
List<String> releases = new ArrayList<>();
List<String> snapshots = new ArrayList<>();

var sublist = new ComponentSublist(() -> {
ArrayList<ModItem> items = new ArrayList<>(versions.size());
for (RemoteMod.Version v : versions) {
items.add(new ModItem(control.addon, v, control));
for (String gv : control.versions.keys()) {
if (release.equals(Objects.requireNonNullElse(GameVersionNumber.getReleaseOfSnapshot(gv), gv))) {
(gv.equals(release) ? releases : snapshots).add(gv);
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method GameVersionNumber.getReleaseOfSnapshot is called multiple times for the same version string (once on line 302 and again on line 312 for each game version). Consider caching the results in a Map to avoid redundant computation, especially since this involves parsing and comparison operations that could be expensive for large lists of versions.

Copilot uses AI. Check for mistakes.
}
return items;
});
sublist.getStyleClass().add("no-padding");
sublist.setTitle("Minecraft " + gameVersion);
}
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop iterates through control.versions.keys() for each release version, which creates an O(nm) time complexity where n is the number of unique releases and m is the total number of game versions. Consider building a map from release to game versions in a single pass through control.versions.keys() to improve performance from O(nm) to O(m).

Copilot uses AI. Check for mistakes.

for (int i = 0; i <= 1; i++) {
// i = 0 -> releases, i = 1 -> snapshots
List<String> target = (i == 0) ? releases : snapshots;
if (target.isEmpty()) continue;

String title = (i == 0) ? "Minecraft " + release
: String.format("Minecraft %s - %s", release, i18n("version.game.snapshot"));

var sublist = new ComponentSublist(() -> {
ArrayList<ModItem> items = new ArrayList<>();
for (String gv : target) {
List<RemoteMod.Version> versions = control.versions.get(gv);
if (versions != null) {
for (RemoteMod.Version v : versions) {
items.add(new ModItem(control.addon, v, control));
Comment thread
Glavo marked this conversation as resolved.
}
}
}
Comment on lines +299 to +308
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sorting of versions within each group (releases and snapshots) is not preserved. The target lists are built by iterating through control.versions.keys(), which may not maintain the original sort order. Consider sorting the releases and snapshots lists before creating ModItems to ensure versions are displayed in the correct order (newest first).

Copilot uses AI. Check for mistakes.
Comment on lines +301 to +308
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sublist aggregates multiple game-version buckets into one list, but sortVersions() stores the same RemoteMod.Version under every compatible game version. As a result, the same mod file can appear multiple times in this combined list (and ordering becomes dependent on key iteration order). Consider de-duplicating by a stable identifier (e.g., file URL/hash or getSelf()), then sorting the merged list by datePublished before creating ModItems.

Suggested change
for (String gv : target) {
List<RemoteMod.Version> versions = control.versions.get(gv);
if (versions != null) {
for (RemoteMod.Version v : versions) {
items.add(new ModItem(control.addon, v, control));
}
}
}
// Merge versions from all matching game-version buckets, de-duplicating
// by a stable identifier (e.g., the "self" URL), then sort by datePublished.
Map<Object, RemoteMod.Version> uniqueVersions = new LinkedHashMap<>();
for (String gv : target) {
List<RemoteMod.Version> versions = control.versions.get(gv);
if (versions != null) {
for (RemoteMod.Version v : versions) {
if (v == null) continue;
Object key = v.getSelf() != null ? v.getSelf() : v;
uniqueVersions.putIfAbsent(key, v);
}
}
}
List<RemoteMod.Version> sortedVersions = new ArrayList<>(uniqueVersions.values());
sortedVersions.sort(Comparator.comparing(
RemoteMod.Version::getDatePublished,
Comparator.nullsLast(Comparator.naturalOrder())
).reversed());
for (RemoteMod.Version v : sortedVersions) {
items.add(new ModItem(control.addon, v, control));
}

Copilot uses AI. Check for mistakes.
return items;
});

list.getContent().add(sublist);
sublist.getStyleClass().add("no-padding");
sublist.setTitle(title);
Comment thread
Glavo marked this conversation as resolved.
Outdated
list.getContent().add(sublist);
}
}
});
}
Expand Down
33 changes: 33 additions & 0 deletions HMCLCore/src/main/java/org/jackhuang/hmcl/util/versioning/GameVersionNumber.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,39 @@ public final String toDebugString() {
return buildDebugString().toString();
}

public static String getReleaseOfSnapshot(String version) {
if (version == null || version.isEmpty()) {
return null;
}

GameVersionNumber gvn = asGameVersion(version);

if (gvn instanceof Release release) {
if (release.getEaType() == Release.ReleaseType.GA) {
return null;
}
if (release.getPatch() > 0) {
return release.getMajor() + "." + release.getMinor() + "." + release.getPatch();
} else {
return release.getMajor() + "." + release.getMinor();
}
}

if (gvn instanceof LegacySnapshot snapshot) {
String[] defaultVersions = Versions.DEFAULT_GAME_VERSIONS;
for (int i = defaultVersions.length - 1; i >= 0; i--) {
String gaStr = defaultVersions[i];
Release gaRel = Release.parseSimple(gaStr);

if (gaRel.compareToSnapshot(snapshot) > 0) {
return gaStr;
}
}
}

Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method getReleaseOfSnapshot does not handle Special versions (e.g., April Fools versions like "20w14infinite"). These versions will fall through and return null, which is consistent with the test case on line 516. However, it would be beneficial to add a code comment explaining this behavior for maintainability.

Suggested change
// Special or non-standard versions (e.g. April Fools snapshots like "20w14infinite")
// are intentionally not mapped to a GA release and will fall through to return null.

Copilot uses AI. Check for mistakes.
return null;
}
Comment thread
Glavo marked this conversation as resolved.

public static final class Old extends GameVersionNumber {
static Old parse(String value) {
if (value.isEmpty())
Expand Down
12 changes: 12 additions & 0 deletions HMCLCore/src/test/java/org/jackhuang/hmcl/util/versioning/GameVersionNumberTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.function.Supplier;

import static org.jackhuang.hmcl.util.versioning.GameVersionNumber.asGameVersion;
import static org.jackhuang.hmcl.util.versioning.GameVersionNumber.getReleaseOfSnapshot;
import static org.junit.jupiter.api.Assertions.*;

/**
Expand Down Expand Up @@ -503,4 +504,15 @@ public void isAtLeast() {
assertThrows(IllegalArgumentException.class, () -> asGameVersion("17w43a").isAtLeast("1.13", "22w13oneblockatatime", true));
assertThrows(IllegalArgumentException.class, () -> asGameVersion("17w43a").isAtLeast("1.13", "22w13oneblockatatime", false));
}

@Test
public void testGetReleaseOfSnapshot() {
assertEquals("1.21.11", getReleaseOfSnapshot("25w45a"));
assertEquals("1.21.11", getReleaseOfSnapshot("25w45a_unobfuscated"));
assertEquals("1.21.11", getReleaseOfSnapshot("1.21.11-pre3"));
assertEquals("1.21.11", getReleaseOfSnapshot("1.21.11-pre3"));
Comment thread
CiiLu marked this conversation as resolved.
Outdated
assertEquals("26.1", getReleaseOfSnapshot("26.1-snapshot-9"));
assertNull(getReleaseOfSnapshot("26.1"));
assertNull(getReleaseOfSnapshot("20w14infinite"));
}
}