重构启动器配置模型#6122
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
…or improved clarity
…ames over built-in names
…or improved readability
…ssary parameters and unused methods
…eters for improved safety
…newer HMCL versions
- replace the warning-only dialog with a confirmation flow before overwriting read-only account storage - remove the stale account entry after forcing an overwrite so the list reflects the new storage state - update account list page and item UI wiring to support the revised restricted-account interaction flow - improve the account management experience when offline authentication is restricted or storage cannot be written safely
- move page-specific bindings and drag-and-drop handling out of `Controllers` and into the corresponding page classes - simplify controller setup by replacing verbose lazy initializers with constructor references where possible - keep profile and account state management closer to the UI that consumes it to reduce central coupling and improve maintainability
- add overwrite actions for instance and preset settings when the current format cannot be saved safely - keep unsupported settings pages read-only while exposing a recovery path instead of forcing manual file edits - back up existing instance settings before replacement to reduce the risk of data loss during migration
…rectories are saved
- save migrated instance settings synchronously so migration receipts are not written before the new game settings reach disk - switch shutdown-critical settings writes to synchronous saves to reduce data loss when the process exits immediately afterward - treat unreadable account storage as unavailable and disable further writes instead of reporting read-write access for a broken file
![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: 8309b23ff9
ℹ️ 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".
| profile.setName(LocalizedText.plain(txtProfileName.getText())); | ||
| if (StringUtils.isNotBlank(getLocation())) { | ||
| profile.setGameDir(Path.of(getLocation())); | ||
| profile.setPath(createPortableLocation()); |
There was a problem hiding this comment.
When an existing profile is saved after changing between an absolute path and a relative path, this branch only mutates the Profile already held by its current backing store. New profiles below are routed to user vs. local storage based on newProfile.getPath().isAbsolute(), so edits that cross that boundary leave the profile in the wrong JSON file and skip the read-only/overwrite checks, e.g. a shared profile can become workspace-relative but still be saved in user-game-directories.json.
Useful? React with 👍 / 👎.
- rename the modded isolation enum value to `MODDED` for consistency and correctness - reorder default isolation options so labels and semantics match the actual behavior shown in the UI - add descriptions for each preset to explain when automatic isolation applies to new instances - update localization strings to reflect the new wording and avoid misleading isolation labels
- scope the list-cell padding rule to `no-padding` combo box popups - keep the shared list cell background and selection styles intact - prevent global list cell padding changes from affecting other views
Align `switch` case branches and closing braces in `GameSettingsPage` to keep formatting consistent and improve readability.
- replace `nativesDirectoryType` with `useCustomNatives` in game settings and schema definitions - align launcher behavior with the new flag so the default native directory is used unless a custom path is explicitly enabled - tighten schema validation for UUID fields to match the serialized settings format and reduce invalid configuration values
…or inherited instance settings
…moving redundant method
…stency in game settings
|
/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 refactors the configuration and settings management of HMCL by replacing the legacy Config and ConfigHolder classes with a modular settings architecture. Configuration domains such as launcher settings, user settings, game directories, presets, accounts, and authlib-injector servers are now persisted in separate, detached JSON files managed by SettingsManager. Additionally, the PR introduces robust migration utilities for legacy configuration formats. The code review identified three critical issues: a compilation error in LineComponent due to calling protected Region methods on container, a potential null pointer or logic bug in HMCLGameRepository when retrieving parent game settings, and a potential unhandled UnsupportedOperationException in Launcher when retrieving file owners on unsupported filesystems.
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.
| double horizontalInsets = container.snappedLeftInset() + container.snappedRightInset(); | ||
| double verticalInsets = container.snappedTopInset() + container.snappedBottomInset(); |
There was a problem hiding this comment.
在 LineComponent 中，container 是一个 HBox 实例。由于 LineComponent 位于 org.jackhuang.hmcl.ui.construct 包中，而 Region.snappedLeftInset() 等方法在 javafx.scene.layout.Region 中是 protected 的，且 LineComponent 并不是 HBox 的子类，因此在非同包下无法通过 container 引用直接调用这些 protected 方法。这会导致编译错误（例如：snappedLeftInset() has protected access in Region）。
建议使用公开的 container.getInsets().getLeft() 等方法来代替，以避免编译失败。
| double horizontalInsets = container.snappedLeftInset() + container.snappedRightInset(); | |
| double verticalInsets = container.snappedTopInset() + container.snappedBottomInset(); | |
| double horizontalInsets = container.getInsets().getLeft() + container.getInsets().getRight(); | |
| double verticalInsets = container.getInsets().getTop() + container.getInsets().getBottom(); |
| GameSettings.Preset profilePreset = SettingsManager.getGameSettings(profile.getLegacyGameSettings()); | ||
| if (profilePreset != null && profilePreset.defaultIsolationTypeProperty().getValue() == DefaultIsolationType.ALWAYS) { |
There was a problem hiding this comment.
此处使用 SettingsManager.getGameSettings(profile.getLegacyGameSettings()) 来获取配置预设。如果当前游戏文件夹（Profile）没有关联特定的旧版游戏设置（即 profile.getLegacyGameSettings() 返回 null），则 profilePreset 将为 null。这会导致即使全局默认预设启用了“始终隔离（ALWAYS）”，也无法为该实例应用隔离设置。
建议使用已有的 getParentGameSettings(null) 方法，它会在 profile.getLegacyGameSettings() 为 null 时自动回退到默认预设，从而保证逻辑的正确性。
| GameSettings.Preset profilePreset = SettingsManager.getGameSettings(profile.getLegacyGameSettings()); | |
| if (profilePreset != null && profilePreset.defaultIsolationTypeProperty().getValue() == DefaultIsolationType.ALWAYS) { | |
| GameSettings.Preset profilePreset = getParentGameSettings(null); | |
| if (profilePreset.defaultIsolationTypeProperty().getValue() == DefaultIsolationType.ALWAYS) { |
| try { | ||
| owner = Files.getOwner(ConfigHolder.configLocation()).getName(); | ||
| owner = Files.getOwner(configDirectory).getName(); | ||
| } catch (IOException ioe) { | ||
| LOG.warning("Failed to get file owner", ioe); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Files.getOwner(configDirectory) 在某些不支持文件所有者属性的文件系统（如 FAT32、某些网盘同步目录或特定的内存文件系统）上运行时，会抛出 UnsupportedOperationException。由于当前只捕获了 IOException，这可能会导致程序在这些环境下意外崩溃。
建议同时捕获 UnsupportedOperationException，以提高程序的健壮性。
| try { | |
| owner = Files.getOwner(ConfigHolder.configLocation()).getName(); | |
| owner = Files.getOwner(configDirectory).getName(); | |
| } catch (IOException ioe) { | |
| LOG.warning("Failed to get file owner", ioe); | |
| return; | |
| } | |
| try { | |
| owner = Files.getOwner(configDirectory).getName(); | |
| } catch (IOException | UnsupportedOperationException ioe) { | |
| LOG.warning("Failed to get file owner", ioe); | |
| return; | |
| } |
…r improved validation
…and update related classes
…hema to launcher-settings
…code and documentation
4 participants
No description provided.