修复清理游戏资源 / 库操作在 FXThread 运行的问题#5301
Hidden character warning
Conversation
There was a problem hiding this comment.
Pull request overview
该 PR 旨在修复“清理游戏资源/库文件时在 FXThread 执行导致卡 UI”的问题,将清理操作改为与删除实例类似的异步执行逻辑。
Changes:
- 将清理
libraries目录的删除操作迁移到Schedulers.io()异步执行,并在Schedulers.javafx()回调中处理异常提示 - 将清理
assets与实例resources目录的删除操作迁移到Schedulers.io()异步执行,并在Schedulers.javafx()回调中处理异常提示 - 增加相关 Task/调度/异常展示所需的 import
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FileUtils.deleteDirectoryQuietly(getProfile().getRepository().getBaseDirectory().resolve("libraries")); | ||
| }).whenComplete(Schedulers.javafx(), (exception) -> { | ||
| if (exception != null) { | ||
| Controllers.dialog(i18n("message.failed") + StringUtils.getStackTrace(exception), i18n("message.error"), MessageDialogPane.MessageType.ERROR); | ||
| } |
There was a problem hiding this comment.
FileUtils.deleteDirectoryQuietly swallows IOExceptions and only returns a boolean, so this task will usually complete with exception == null even when deletion fails (permissions/locked files). If you want the error dialog to trigger, use deleteDirectory (let IOException propagate) or check the boolean result and fail the task explicitly when it returns false.
| FileUtils.deleteDirectoryQuietly(getProfile().getRepository().getBaseDirectory().resolve("libraries")); | ||
| }).whenComplete(Schedulers.javafx(), (exception) -> { | ||
| if (exception != null) { | ||
| Controllers.dialog(i18n("message.failed") + StringUtils.getStackTrace(exception), i18n("message.error"), MessageDialogPane.MessageType.ERROR); |
There was a problem hiding this comment.
The error dialog concatenates i18n("message.failed") and the stack trace without a separator, which makes the message hard to read. Add a newline (as done elsewhere, e.g. "..." + "\n" + StringUtils.getStackTrace(...)).
| Task.runAsync(Schedulers.io(), () -> { | ||
| HMCLGameRepository baseDirectory = getProfile().getRepository(); | ||
| FileUtils.deleteDirectoryQuietly(baseDirectory.getBaseDirectory().resolve("assets")); | ||
| if (version.get() != null) { | ||
| FileUtils.deleteDirectoryQuietly(baseDirectory.getRunDirectory(version.get().getVersion()).resolve("resources")); |
There was a problem hiding this comment.
Inside the IO task you read version.get() (a JavaFX property) and derived values from it. Capture the version name (or the resources path) on the FX thread before scheduling to avoid cross-thread access to JavaFX properties and to ensure a consistent target if the selection changes while the task is running.
| Task.runAsync(Schedulers.io(), () -> { | |
| HMCLGameRepository baseDirectory = getProfile().getRepository(); | |
| FileUtils.deleteDirectoryQuietly(baseDirectory.getBaseDirectory().resolve("assets")); | |
| if (version.get() != null) { | |
| FileUtils.deleteDirectoryQuietly(baseDirectory.getRunDirectory(version.get().getVersion()).resolve("resources")); | |
| HMCLGameRepository baseDirectory = getProfile().getRepository(); | |
| String versionId = (version.get() != null) ? version.get().getVersion() : null; | |
| Task.runAsync(Schedulers.io(), () -> { | |
| FileUtils.deleteDirectoryQuietly(baseDirectory.getBaseDirectory().resolve("assets")); | |
| if (versionId != null) { | |
| FileUtils.deleteDirectoryQuietly(baseDirectory.getRunDirectory(versionId).resolve("resources")); |
| FileUtils.deleteDirectoryQuietly(baseDirectory.getBaseDirectory().resolve("assets")); | ||
| if (version.get() != null) { | ||
| FileUtils.deleteDirectoryQuietly(baseDirectory.getRunDirectory(version.get().getVersion()).resolve("resources")); | ||
| } | ||
| }).whenComplete(Schedulers.javafx(), (exception) -> { | ||
| if (exception != null) { | ||
| Controllers.dialog(i18n("message.failed") + StringUtils.getStackTrace(exception), i18n("message.error"), MessageDialogPane.MessageType.ERROR); | ||
| } |
There was a problem hiding this comment.
Same as above: deleteDirectoryQuietly will not throw, so exception will remain null on common deletion failures. If you need to report failure to the user, use a throwing delete or convert the boolean return value into a task failure.
| } | ||
| }).whenComplete(Schedulers.javafx(), (exception) -> { | ||
| if (exception != null) { | ||
| Controllers.dialog(i18n("message.failed") + StringUtils.getStackTrace(exception), i18n("message.error"), MessageDialogPane.MessageType.ERROR); |
There was a problem hiding this comment.
This dialog message also misses a separator between the localized failure text and the stack trace; add a newline for readability.
| Task.runAsync(Schedulers.io(), () -> { | ||
| FileUtils.deleteDirectoryQuietly(getProfile().getRepository().getBaseDirectory().resolve("libraries")); |
There was a problem hiding this comment.
The background task calls getProfile()/version.get() (JavaFX properties) from the IO executor. JavaFX properties are not thread-safe; snapshot the needed values (e.g., repository base path / selected version string) on the FX thread before starting the task, and only use those captured values inside runAsync.
916f23a to
71418ac
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var libraries = getProfile().getRepository().getBaseDirectory().resolve("libraries"); | ||
| Task.runAsync(Schedulers.io(), () -> { | ||
| FileUtils.deleteDirectoryQuietly(libraries); | ||
| }).whenComplete(Schedulers.javafx(), (exception) -> { | ||
| if (exception != null) { | ||
| Controllers.dialog(i18n("message.failed") + StringUtils.getStackTrace(exception), i18n("message.error"), MessageDialogPane.MessageType.ERROR); | ||
| } |
There was a problem hiding this comment.
FileUtils.deleteDirectoryQuietly(...) swallows IOException and returns false, so this Task.runAsync(...) closure will usually complete successfully even when deletion fails. As a result, whenComplete(..., exception -> ...) won’t show an error dialog on common failures. If you want to report failures, use deleteDirectory(...) (let the exception propagate) or check the boolean result and throw/show an error accordingly.
| Task.runAsync(Schedulers.io(), () -> { | ||
| FileUtils.deleteDirectoryQuietly(assetsDir); | ||
| if (resourcesDir != null) { | ||
| FileUtils.deleteDirectoryQuietly(resourcesDir); | ||
| } | ||
| }).whenComplete(Schedulers.javafx(), (exception) -> { | ||
| if (exception != null) { | ||
| Controllers.dialog(i18n("message.failed") + "\n" + StringUtils.getStackTrace(exception), i18n("message.error"), MessageDialogPane.MessageType.ERROR); | ||
| } |
There was a problem hiding this comment.
Same as above: deleteDirectoryQuietly(...) catches IOException, so this task will not surface typical deletion failures via exception. Consider using deleteDirectory(...) or handling the boolean return to ensure the UI can inform users when cleanup fails.
…java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

改为和删除实例一样的逻辑