Skip to content

不再用 Version 表示游戏实例 - #6322

Merged
Glavo merged 121 commits into
HMCL-dev:mainfrom
Glavo:instance
Jul 29, 2026
Merged

不再用 Version 表示游戏实例#6322
Glavo merged 121 commits into
HMCL-dev:mainfrom
Glavo:instance

Conversation

@Glavo

@Glavo Glavo commented Jul 6, 2026

Copy link
Copy Markdown
Member

过去,HMCL 中使用 Version 类表示游戏实例 Json 内容,并且大量使用类似 versionversionId 这样的名字来表示一个实例的 ID。

由于 version 是一个非常常见的概念,这种用法造成了大量的混淆,开发者很容易把实例 ID 和游戏版本等内容混淆。

本 PR 中更新了代码,用 instance 代替 version 来表示一个游戏实例。本 PR 中创建了一个新的类 GameInstanceID,用来表示一个实例的 ID,更清晰的表示它的用途。

Version 类现在被拆分成了以下几个类:

  • GameInstanceManifest:用来表示一个裸的实例 JSON。
  • GameInstanceManifest.Resolved:用来表示经过解析,不再带有 inheritsFrom 的实例 JSON。
  • GameInstancePatch:用来表示 GameInstanceManifestpatches 字段的一项元素。

Glavo added 30 commits July 4, 2026 15:32
…date GameInstanceManifest to use GameInstanceID for jar references
…, GameAssetDownloadTask, GameRepository, HMCLGameLauncher, and Launcher
@Glavo

Glavo commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/game/DefaultGameRepository.java Outdated
@Glavo

Glavo commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +401 to +402
Status currentStatus = status;
currentStatus.instances.remove(id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

Comment on lines +687 to +689
if (!resolvedSoFar.add(manifest.id())) {
LOG.warning("Found circular dependency versions: " + resolvedSoFar);
launchManifest = manifest.jar() == null ? manifest.withJar(manifest.id()) : manifest;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

Comment on lines +180 to +182
public @Unmodifiable Set<Entry<K, V>> entrySet() {
// The caller should not modify the returned set
return map.entrySet();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

Comment on lines +69 to +70
public GameInstanceID getInstanceId() {
return instanceId;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@Glavo
Glavo merged commit 5cde4ce into HMCL-dev:main Jul 29, 2026
3 checks passed
@Glavo
Glavo deleted the instance branch July 29, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant