[Enhancement] 启动器检查更新失败时显示提示#6286
Conversation
ToobLac
changed the title
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces error handling for update checks and disables updates if the launcher instance cannot be verified. Specifically, it adds an error property to UpdateChecker to capture update failures, updates SettingsPage to display these errors or hide the update UI if self-verification fails, and adds corresponding localization strings. The review feedback points out a potential issue in UpdateChecker.java where latestVersion could be incorrectly set to null if finalResult is null, as the original null check was omitted in the new error-handling branch, and suggests restoring this check.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
# Conflicts: # HMCL/src/main/java/org/jackhuang/hmcl/ui/main/SettingsPage.java
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5515aa9e48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| final StringProperty lblUpdateSubProperty = updatePane.subtitleProperty(); | ||
|
|
||
| { | ||
| if (IntegrityChecker.DISABLE_SELF_INTEGRITY_CHECK || IntegrityChecker.isSelfVerified()) { |
There was a problem hiding this comment.
IntegrityChecker.isSelfVerified() is explicitly blocking and, on its first call, reads and hashes the entire launcher JAR. This condition runs while SettingsPage is being constructed on the JavaFX thread; therefore opening or preloading Settings before the background update check has completed can freeze the UI until verification finishes. Keep this verification in the existing background update-check path and publish its result back to the UI instead.
Useful? React with 👍 / 👎.
| lblUpdateSubProperty.set(i18n("update.newest_version", UpdateChecker.getLatestVersion().version())); | ||
| } else if (UpdateChecker.isCheckingUpdate()) { | ||
| lblUpdateSubProperty.set(i18n("update.checking")); | ||
| } else if (UpdateChecker.errorProperty().get() != null) { |
There was a problem hiding this comment.
When an earlier successful check found an update and a later check fails (for example, after toggling Preview), latestVersion is deliberately retained, so isOutdated() remains true. The preceding outdated branch then wins and this new error branch is never reached, leaving the stale update button and “newest version” text visible even though the most recent request failed. Check the error state before the stale result, or explicitly define how stale results should be presented.
Useful? React with 👍 / 👎.
| }, | ||
| latestVersion); | ||
| private static final ReadOnlyBooleanWrapper checkingUpdate = new ReadOnlyBooleanWrapper(false); | ||
| private static final ReadOnlyObjectWrapper<Throwable> error = new ReadOnlyObjectWrapper<>(); |
There was a problem hiding this comment.
The new wrapper is assigned null on successful checks and the UI tests its value for null, but it is declared as ReadOnlyObjectWrapper<Throwable> and exposed as ReadOnlyObjectProperty<Throwable>. This violates the repository AGENTS.md nullability rule for nullable generic arguments and makes the new API appear non-null to callers; annotate the wrapper/property type argument and nullable locals with @Nullable under a @NotNullByDefault class.
Useful? React with 👍 / 👎.
| }, | ||
| latestVersion); | ||
| private static final ReadOnlyBooleanWrapper checkingUpdate = new ReadOnlyBooleanWrapper(false); | ||
| private static final ReadOnlyObjectWrapper<Throwable> error = new ReadOnlyObjectWrapper<>(); |
There was a problem hiding this comment.
The added error field and errorProperty() method have no /// Markdown-style Javadoc. The repository AGENTS.md requires documentation for every field and method, including newly added internal state and public accessors; add concise documentation describing when this property is set and cleared.
Useful? React with 👍 / 👎.
4 participants
Uh oh!
There was an error while loading. Please reload this page.