不再用 Version 表示游戏实例 - #6322
Conversation
…bleSequencedMap class
…r improved configuration
…est for enhanced structure
…class for game instance management
…ve list handling in GameInstanceManifest
…aultGameRepository2 for thread safety
…ling for improved clarity
…date GameInstanceManifest to use GameInstanceID for jar references
…est handling and patch preservation
…meInstancePatch structure
…ameInstanceManifest for patch serialization
…stanceManifest and GameInstancePatch
…nownFields handling in GameInstancePatch
…stanceManifest and GameInstancePatch
…date related references
…tance renaming support
…ust related types
…update references
…ask and GameAssetIndexDownloadTask
…, GameAssetDownloadTask, GameRepository, HMCLGameLauncher, and Launcher
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23b6a5b268
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 065233e669
ℹ️ 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".
| Status currentStatus = status; | ||
| currentStatus.instances.remove(id); |
There was a problem hiding this comment.
Keep the instance cached when the disk move fails
When deletion is attempted while the instance directory cannot be renamed—for example, because a file is locked on Windows—the instance is removed from currentStatus.instances before Files.move runs. The catch then returns false without restoring the entry or scheduling refreshAsync, so a failed deletion makes the instance disappear from the UI until a later refresh or restart. Remove the cached entry only after the move succeeds, as the previous implementation did.
Useful? React with 👍 / 👎.
| if (!resolvedSoFar.add(manifest.id())) { | ||
| LOG.warning("Found circular dependency versions: " + resolvedSoFar); | ||
| launchManifest = manifest.jar() == null ? manifest.withJar(manifest.id()) : manifest; |
There was a problem hiding this comment.
Clear inheritance when breaking circular chains
For manifests with circular inheritsFrom links, this fallback retains the non-null parent ID. Resolution subsequently constructs GameInstanceManifest.Resolved, whose constructor rejects any launch manifest with a non-null inheritsFrom, so one cyclic pair now throws IllegalArgumentException out of refreshImpl and prevents the repository from loading any instances. Clear the inherited ID in this cycle-breaking branch before constructing the resolved view.
Useful? React with 👍 / 👎.
| public @Unmodifiable Set<Entry<K, V>> entrySet() { | ||
| // The caller should not modify the returned set | ||
| return map.entrySet(); |
There was a problem hiding this comment.
Wrap collection views before returning them
Despite the class's unmodifiable contract, entrySet() returns the mutable backing-map view directly; callers can remove entries or call Map.Entry.setValue, mutating the map without going through the throwing mutators. keySet() and values() have the same issue. This can mutate manifest download/logging maps that are exposed as immutable, so return genuinely unmodifiable views or delegate to an unmodifiable map.
Useful? React with 👍 / 👎.
| public GameInstanceID getInstanceId() { | ||
| return instanceId; |
There was a problem hiding this comment.
Mark the optional instance ID nullable
Builder#create() can return an options object without setInstanceId being called—the updated GameCrashWindowTest does exactly that—so this getter can return null even though its signature implicitly promises a non-null value. Mark the added field and return type @Nullable (or validate it as required when creating the options) so the nullability contract reflects actual builder behavior.
AGENTS.md reference: AGENTS.md:L8-L9
Useful? React with 👍 / 👎.
过去,HMCL 中使用
Version类表示游戏实例 Json 内容,并且大量使用类似version、versionId这样的名字来表示一个实例的 ID。由于 version 是一个非常常见的概念,这种用法造成了大量的混淆,开发者很容易把实例 ID 和游戏版本等内容混淆。
本 PR 中更新了代码,用 instance 代替 version 来表示一个游戏实例。本 PR 中创建了一个新的类
GameInstanceID,用来表示一个实例的 ID,更清晰的表示它的用途。Version类现在被拆分成了以下几个类:GameInstanceManifest:用来表示一个裸的实例 JSON。GameInstanceManifest.Resolved:用来表示经过解析,不再带有inheritsFrom的实例 JSON。GameInstancePatch:用来表示GameInstanceManifest中patches字段的一项元素。