支持清理游戏文件 by CiiLu · Pull Request #5814 · HMCL-dev/HMCL
应该优化一下 0 mb 的提示？
CiiLu
marked this pull request as draft
CiiLu
marked this pull request as ready for review
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
该 PR 为启动器新增“清理游戏文件”能力：在版本列表页提供入口，统计可清理文件占用空间并提示用户确认后执行清理，同时补充了文件大小格式化与相关本地化文案。
Changes:
- 新增游戏文件清理入口与实现（扫描冗余 assets/libraries、日志/缓存、natives 目录等并删除）。
- 增加文件大小的本地化格式化能力（
I18n.formatSize/Translator.formatSize+ 对应多语言 key）。 - UI 支持动态更新 MessageDialog 文本，并新增清理图标与多语言文案。
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| HMCL/src/main/resources/assets/lang/I18N_zh_CN.properties | 添加 size 格式化 key 与“清理游戏文件”文案（简中） |
| HMCL/src/main/resources/assets/lang/I18N_zh.properties | 添加 size 格式化 key 与“清理游戏文件”文案（繁中） |
| HMCL/src/main/resources/assets/lang/I18N.properties | 添加 size 格式化 key 与“清理游戏文件”文案（英文） |
| HMCL/src/main/java/org/jackhuang/hmcl/util/i18n/translator/Translator.java | 新增 formatSize(long)，使用 i18n key 格式化大小 |
| HMCL/src/main/java/org/jackhuang/hmcl/util/i18n/I18n.java | 暴露 I18n.formatSize(long) 静态入口 |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java | 实现清理逻辑与统计/确认对话框交互 |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/GameListPage.java | 在版本列表页工具栏新增“清理游戏文件”按钮入口 |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/MessageDialogPane.java | 支持 setText 动态更新对话框文本 |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/SVG.java | 新增 CLEAN 图标枚举值 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| JFXButton okButton = new JFXButton(i18n("button.yes")); | ||
| okButton.getStyleClass().add("dialog-accept"); | ||
|
|
||
| dialogBuilder.addAction(buttonPane); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MessageDialogPane.Builder#addAction(Node) registers an ActionEvent handler on the provided node to close the dialog. Passing a container (buttonPane) means the ActionEvent from okButton will bubble up and close the dialog immediately on click, preventing the intended “show spinner while deleting, then close” flow. Prefer adding the actual button as the action node (and suppress auto-close), or expose/update the dialog action area via an API instead of wrapping with a pane.
| dialogBuilder.addAction(buttonPane); | |
| dialogBuilder.addAction(okButton); |
Copilot uses AI. Check for mistakes.
| .collect(Collectors.toSet()); | ||
|
|
||
| Set<String> activeLibraries = versions.parallelStream() | ||
| .flatMap(version -> version.getLibraries().stream()) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
activeLibraries is derived from version.getLibraries() only, but HMCL stores extra libraries in version.getPatches() (e.g., modpack/loader patches). Those patched libraries live under libraries/ too, so this cleanup can misclassify them as unused and delete required files. Build activeLibraries from the resolved version (e.g., resolvePreservingPatches(...)) or explicitly include libraries from patches when collecting paths.
| .flatMap(version -> version.getLibraries().stream()) | |
| .flatMap(version -> { | |
| Stream<Library> baseLibraries = version.getLibraries().stream(); | |
| Stream<Library> patchedLibraries = version.getPatches().stream() | |
| .flatMap(patch -> patch.getLibraries().stream()); | |
| return Stream.concat(baseLibraries, patchedLibraries); | |
| }) |
Copilot uses AI. Check for mistakes.
Comment on lines +254 to +259
| try (var walker = Files.walk(repository.getVersionRoot(v.getId()), 1)) { | ||
| unusedFolders.addAll(walker | ||
| .filter(it -> { | ||
| var name = it.getFileName().toString(); | ||
| return Files.isDirectory(it) && (name.startsWith("natives-") || name.endsWith("-natives")); | ||
| }).toList()); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files.walk(versionRoot, 1) includes versionRoot itself. If a user’s version id happens to start with natives- or end with -natives, this filter will treat the entire version directory as a “natives” folder and enqueue all its files for deletion. Use Files.list(versionRoot) (children only) or add an explicit !it.equals(versionRoot) guard before applying the name predicate.
Copilot uses AI. Check for mistakes.
| } else if (bytes < 1024 * 1024 * 1024) { | ||
| return supportedLocale.i18n("download.size.megabyte", (double) bytes / (1024 * 1024)); | ||
| } else { | ||
| return supportedLocale.i18n("download.size.gibabyte", (double) bytes / (1024 * 1024 * 1024)); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The i18n key name download.size.gibabyte appears to be a typo (GiB is “gibibyte”, matching the existing kibibyte/megabyte naming). Since this is newly introduced, consider renaming the key (and all translations/usages) to download.size.gibibyte to avoid confusion and keep key naming consistent.
| return supportedLocale.i18n("download.size.gibabyte", (double) bytes / (1024 * 1024 * 1024)); | |
| return supportedLocale.i18n("download.size.gibibyte", (double) bytes / (1024 * 1024 * 1024)); |
Copilot uses AI. Check for mistakes.
| download.size.byte=%d B | ||
| download.size.kibibyte=%.1f KiB | ||
| download.size.megabyte=%.1f MiB | ||
| download.size.gibabyte=%.1f GiB |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The i18n key name download.size.gibabyte appears to be a typo (GiB is “gibibyte”, matching the existing kibibyte/megabyte naming). Since this is newly introduced, consider renaming the key to download.size.gibibyte (and update all locales/usages) for consistency.
| download.size.gibabyte=%.1f GiB | |
| download.size.gibibyte=%.1f GiB |
Copilot uses AI. Check for mistakes.
| download.size.byte=%d B | ||
| download.size.kibibyte=%.1f KiB | ||
| download.size.megabyte=%.1f MiB | ||
| download.size.gibabyte=%.1f GiB |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The i18n key name download.size.gibabyte appears to be a typo (GiB is “gibibyte”, matching the kibibyte/megabyte naming). Since this is newly introduced, consider renaming the key to download.size.gibibyte (and update all locales/usages) for consistency.
| download.size.gibabyte=%.1f GiB | |
| download.size.gibibyte=%.1f GiB |
Copilot uses AI. Check for mistakes.
| download.size.byte=%d B | ||
| download.size.kibibyte=%.1f KiB | ||
| download.size.megabyte=%.1f MiB | ||
| download.size.gibabyte=%.1f GiB |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The i18n key name download.size.gibabyte appears to be a typo (GiB is “gibibyte”, matching the kibibyte/megabyte naming). Since this is newly introduced, consider renaming the key to download.size.gibibyte (and update all locales/usages) for consistency.
| download.size.gibabyte=%.1f GiB | |
| download.size.gibibyte=%.1f GiB |
Copilot uses AI. Check for mistakes.
|
|
||
| game=遊戲 | ||
| game.clean=清理遊戲文件 | ||
| game.clean.loading=统计中... |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the Traditional Chinese locale file, but the new string uses the Simplified character “统计”. Consider changing it to Traditional (“統計”) to keep the locale consistent.
| game.clean.loading=统计中... | |
| game.clean.loading=統計中... |
Copilot uses AI. Check for mistakes.
| folder.world=World Directory | ||
|
|
||
| game=Games | ||
| game.clean=Clean Game Files |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
建议用 Cleanup
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a game file cleanup utility to remove redundant assets, libraries, logs, and temporary folders. It includes UI updates to the game list and enhancements to the message dialog system for dynamic content. The review feedback highlights several optimization opportunities, such as deduplicating asset index parsing and folder traversal to improve performance. Additionally, it is recommended to ensure empty directories are removed and to protect library metadata files, such as checksums, from being deleted to maintain file integrity.
Comment on lines +244 to +251
| List<Path> unusedFolders = new ArrayList<>(); | ||
|
|
||
| for (String path : List.of("logs", "crash-reports", "modernfix", "mods/.connector", "CustomSkinLoader/caches", ".fabric")) { | ||
| unusedFolders.add(repository.getBaseDirectory().resolve(path)); | ||
| versions.forEach(v -> { | ||
| unusedFolders.add(repository.getVersionRoot(v.getId()).resolve(path)); | ||
| }); | ||
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在循环中嵌套 versions.forEach 可能会导致 unusedFolders 列表中出现重复路径（特别是当多个版本共享同一个游戏目录或版本根目录时）。随后对这些重复路径调用 Files.walk 会造成不必要的性能开销，并可能导致计算出的总大小不准确。建议使用 Set<Path> 来存储 unusedFolders，并优化遍历逻辑以减少冗余操作。
Comment on lines +223 to +232
| Set<String> activeAssets = versions.parallelStream() | ||
| .flatMap(version -> { | ||
| try { | ||
| var index = repository.getAssetIndex(version.getId(), version.getAssetIndex().getId()); | ||
| return index.getObjects().values().stream().map(AssetObject::getLocation); | ||
| } catch (IOException ignored) { | ||
| return Stream.empty(); | ||
| } | ||
| }) | ||
| .collect(Collectors.toSet()); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
activeAssets 的计算逻辑对每个版本都重新获取并解析了资产索引（Asset Index）。如果多个版本使用相同的资产索引（这在 Minecraft 中非常常见），会导致重复的磁盘 I/O 和解析开销。建议先按资产索引 ID 进行去重，然后再获取资产列表以提高效率。
Comment on lines +290 to +294
| Task.runAsync(() -> list.forEach(path -> { | ||
| try { | ||
| Files.deleteIfExists(path); | ||
| } catch (IOException ignored) { | ||
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
目前的清理逻辑仅删除了文件，但没有清理空的目录（例如 logs、crash-reports 以及 natives- 文件夹本身）。这会导致清理后留下大量空文件夹。建议在删除文件后，也尝试删除 unusedFolders 中涉及的目录（如果它们已为空）。
| return stream | ||
| .filter(Files::isRegularFile) | ||
| .filter(path -> { | ||
| String relative = root.relativize(path).toString().replace("\\", "/"); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在清理库文件（libraries）时，目前的逻辑会删除所有不在 activeLibraries 中的文件。然而，库目录通常包含 .sha1 等元数据文件，这些文件在 Library::getPath 中可能未被包含。直接删除这些文件可能会导致后续的完整性检查失败。建议在过滤时保留与活跃库文件相关的元数据文件。
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