Skip to content

支持清理游戏文件#5814

Open
CiiLu wants to merge 21 commits intoHMCL-dev:mainfrom
CiiLu:cleanroom!
Open

支持清理游戏文件#5814
CiiLu wants to merge 21 commits intoHMCL-dev:mainfrom
CiiLu:cleanroom!

Conversation

@CiiLu
Copy link
Copy Markdown
Contributor

@CiiLu CiiLu commented Mar 20, 2026

image

@zkitefly
Copy link
Copy Markdown
Member

zkitefly commented Mar 20, 2026

image

应该优化一下 0 mb 的提示?

@CiiLu CiiLu marked this pull request as draft March 21, 2026 03:11
@CiiLu CiiLu marked this pull request as ready for review March 21, 2026 03:27
@Glavo Glavo requested a review from Copilot March 21, 2026 13:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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);
Copy link

Copilot AI Mar 21, 2026

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.

Suggested change
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())
Copy link

Copilot AI Mar 21, 2026

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.

Suggested change
.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());
Copy link

Copilot AI Mar 21, 2026

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));
Copy link

Copilot AI Mar 21, 2026

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.

Suggested change
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
Copy link

Copilot AI Mar 21, 2026

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.

Suggested change
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
Copy link

Copilot AI Mar 21, 2026

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.

Suggested change
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
Copy link

Copilot AI Mar 21, 2026

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.

Suggested change
download.size.gibabyte=%.1f GiB
download.size.gibibyte=%.1f GiB

Copilot uses AI. Check for mistakes.

game=遊戲
game.clean=清理遊戲文件
game.clean.loading=统计中...
Copy link

Copilot AI Mar 21, 2026

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.

Suggested change
game.clean.loading=统计中...
game.clean.loading=統計中...

Copilot uses AI. Check for mistakes.
Comment thread HMCL/src/main/resources/assets/lang/I18N.properties Outdated
@Glavo
Copy link
Copy Markdown
Member

Glavo commented Apr 4, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java
return stream
.filter(Files::isRegularFile)
.filter(path -> {
String relative = root.relativize(path).toString().replace("\\", "/");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

在清理库文件(libraries)时,目前的逻辑会删除所有不在 activeLibraries 中的文件。然而,库目录通常包含 .sha1 等元数据文件,这些文件在 Library::getPath 中可能未被包含。直接删除这些文件可能会导致后续的完整性检查失败。建议在过滤时保留与活跃库文件相关的元数据文件。

@CiiLu CiiLu marked this pull request as draft April 18, 2026 13:22
@CiiLu
Copy link
Copy Markdown
Contributor Author

CiiLu commented Apr 20, 2026

依赖库清理坑有点多,先撤了,之后单独开 PR 做

@CiiLu CiiLu marked this pull request as ready for review April 20, 2026 13:48
Copy link
Copy Markdown
Member

@Glavo Glavo left a comment

Choose a reason for hiding this comment

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

本建议由 Codex (GPT-5.4 xhigh) 生成。

[P2] HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java:215, HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java:287, HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/MessageDialogPane.java:123
这里把 buttonPane 容器传给了 MessageDialogPane.Builder#addAction(Node)。该方法会在传入节点上注册 ActionEvent 后关闭对话框,因此点击 okButton 时事件会冒泡到 buttonPane,对话框会在异步删除刚启动时立即关闭,预期中的 “先显示 spinner,完成后再关闭” 交互无法成立。

[P2] HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java:228-241
读取某个 asset index 失败时,这里直接 return Stream.empty() 继续执行,相当于把该版本视为“不需要任何资源”。随后 findUnlistedFiles(...) 会把这些版本实际仍在使用的 assets/objects 判成未使用并删除。结果是下次离线启动可能直接缺资源,在线启动也会触发整批重新下载。

[P3] HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java:245-248, HMCL/src/main/java/org/jackhuang/hmcl/game/HMCLGameRepository.java:90-100
清理目录是按 getVersionRoot(v.getId()).resolve(path) 构造的,但 HMCL 支持 GameDirectoryType.CUSTOM。这类实例的 logscrash-reports.fabric 等实际位于 getRunDirectory(v.getId()) 下,因此这个入口不会清理到它们,释放空间统计也会明显偏小。

我本地补充跑了 ./gradlew compileJava,编译通过;当前问题主要是行为和覆盖范围层面的回归风险。

@Glavo
Copy link
Copy Markdown
Member

Glavo commented Apr 25, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 'Cleanup Game Files' feature to HMCL, allowing users to remove redundant resource files, logs, and crash reports to free up disk space. The implementation includes UI updates such as a new cleanup icon and a confirmation dialog, along with the logic to identify and delete unused files. Feedback from the review identifies several areas for improvement: handling potential null pointers in asset index retrieval, preventing accidental mass deletion of assets in case of IO errors, optimizing directory traversal when version isolation is disabled, and ensuring empty directories are also removed. A typo in the size formatting utility was also noted.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/util/i18n/translator/Translator.java Outdated
@Glavo
Copy link
Copy Markdown
Member

Glavo commented Apr 26, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 'Cleanup Game Files' feature to HMCL, allowing users to reclaim disk space by deleting unused assets and temporary directories like logs and crash reports. The changes include a new toolbar action in the game list, a confirmation dialog showing estimated space savings, and the underlying logic for identifying and deleting redundant files. Review feedback identifies several critical issues: a potential NullPointerException when handling null asset indices, a logic error that prevents the identification of nested 'useless' folders, and a safety concern where failed index loading could lead to the accidental deletion of active assets. It was also recommended to ensure empty directories are removed alongside their contents.

.filter(Objects::nonNull)
.map(Version::getAssetIndex)
.distinct()
.flatMap(idx -> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The idx object (of type AssetIndexReference) can be null for some game versions (e.g., very old versions or custom versions where the asset index is not specified in the JSON). Calling idx.getId() without a null check will result in a NullPointerException.

                    .filter(Objects::nonNull)
                    .flatMap(idx -> {

Comment on lines +272 to +273
LOG.warning("Failed to get asset index", e);
return Stream.empty();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If an asset index fails to load (e.g., due to a corrupted file or network issue), returning Stream.empty() will cause all assets associated with that index to be excluded from the activeAssets set. Consequently, findUnlistedFiles will identify these assets as unused and delete them. This could lead to unintended deletion of game assets, forcing a re-download later. It is safer to skip the asset cleaning process entirely if any index fails to load.

unusedFolders.addAll(walker
.filter(it -> {
var name = it.getFileName().toString();
return (name.startsWith("natives-") || name.endsWith("-natives") || uselessFolderNames.contains(name)) && Files.isDirectory(it);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The check uselessFolderNames.contains(name) will fail for nested paths like mods/.connector or CustomSkinLoader/caches because name only represents the last component of the path (the filename). This means these nested directories will not be identified for cleanup in independent version directories. Consider iterating through the set of paths and resolving them against each runDir instead of using a simple filename check in the walker.

for (Path dir : unusedFolders) {
if (Files.exists(dir)) {
try (var s = Files.walk(dir)) {
s.filter(Files::isRegularFile).forEach(unusedFiles::add);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The current implementation only adds regular files to the unusedFiles list for deletion. This leaves behind empty directory structures for the identified 'useless' folders (like logs, crash-reports, etc.). It would be cleaner to delete the directories themselves after their contents have been removed.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/Versions.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants