Conversation
Co-worked with Xiaomi MiMo v2.6 Flash Medium
pcl-ce-automation
Bot
added
🛠️ 等待审查
Contributor
审查者指南本 PR 从实例状态恢复、MultiMC JSON Patch 安装、Java 兼容版本选择、依赖补全和配置持久化多个环节修复整合包支持，使缺失配置或非标准压缩包结构能够正常安装，并确保生成的实例可正确启动及应用自定义设置。 MultiMC 整合包安装与启动时序图sequenceDiagram
participant User
participant Installer as InstallPackMMC
participant JsonMerge as MergeJson
participant Storage as ConfigStorage
participant Launcher as McLaunchJava
participant Java as ModJava
User->>Installer: InstallPackMMC(fileAddress)
Installer->>Installer: Read mmc-pack.json and optional instance.cfg
Installer->>Installer: Apply JSON patches and compatibleJavaMajors
Installer->>JsonMerge: MergeJson(outputFolder, minecraftFolder, ...)
JsonMerge->>JsonMerge: Merge patched fields with vanilla JSON
JsonMerge->>JsonMerge: Remove conflicting LWJGL2 libraries
Installer->>Storage: InvalidateCache(versionFolder)
User->>Launcher: Launch instance
Launcher->>Launcher: Read compatibleJavaMajors
Launcher->>Java: Select Java within compatible major range
实例自定义图标恢复状态图stateDiagram-v2
[*] --> LoadInstance
LoadInstance --> CustomLogo: PCL/Logo.png exists
CustomLogo --> CustomFlagEnabled: Set IsLogoCustom true
CustomFlagEnabled --> [*]
LoadInstance --> DefaultLogo: No custom logo and custom flag is false
LoadInstance --> MissingCustomLogo: Custom flag is true but file is missing
MissingCustomLogo --> DefaultLogo: Clear IsLogoCustom and Logo
DefaultLogo --> [*]
MultiMC JSON Patch Java 兼容性流程图flowchart TD
A[Read compatibleJavaMajors from JSON patches] --> B[Parse, deduplicate, and sort major versions]
B --> C[Intersect ranges from multiple patches]
C --> D{Intersection exists?}
D -->|Yes| E[Use intersected Java major range]
D -->|No| F[Use current patch range]
E --> G[Write javaVersion and compatibleJavaMajors]
F --> G
G --> H[McLaunchJava selects compatible Java]
文件级变更
提示与命令与 Sourcery 交互
自定义使用体验访问你的控制面板即可：
获取帮助Original review guide in EnglishReviewer's Guide本 PR 从实例状态恢复、MultiMC JSON Patch 安装、Java 兼容版本选择、依赖补全和配置持久化多个环节修复整合包支持，使缺失配置或非标准压缩包结构能够正常安装，并确保生成的实例可正确启动及应用自定义设置。 Sequence diagram for MultiMC pack installation and launchsequenceDiagram
participant User
participant Installer as InstallPackMMC
participant JsonMerge as MergeJson
participant Storage as ConfigStorage
participant Launcher as McLaunchJava
participant Java as ModJava
User->>Installer: InstallPackMMC(fileAddress)
Installer->>Installer: Read mmc-pack.json and optional instance.cfg
Installer->>Installer: Apply JSON patches and compatibleJavaMajors
Installer->>JsonMerge: MergeJson(outputFolder, minecraftFolder, ...)
JsonMerge->>JsonMerge: Merge patched fields with vanilla JSON
JsonMerge->>JsonMerge: Remove conflicting LWJGL2 libraries
Installer->>Storage: InvalidateCache(versionFolder)
User->>Launcher: Launch instance
Launcher->>Launcher: Read compatibleJavaMajors
Launcher->>Java: Select Java within compatible major range
State diagram for instance custom logo recoverystateDiagram-v2
[*] --> LoadInstance
LoadInstance --> CustomLogo: PCL/Logo.png exists
CustomLogo --> CustomFlagEnabled: Set IsLogoCustom true
CustomFlagEnabled --> [*]
LoadInstance --> DefaultLogo: No custom logo and custom flag is false
LoadInstance --> MissingCustomLogo: Custom flag is true but file is missing
MissingCustomLogo --> DefaultLogo: Clear IsLogoCustom and Logo
DefaultLogo --> [*]
Flow diagram for MultiMC JSON patch Java compatibilityflowchart TD
A[Read compatibleJavaMajors from JSON patches] --> B[Parse, deduplicate, and sort major versions]
B --> C[Intersect ranges from multiple patches]
C --> D{Intersection exists?}
D -->|Yes| E[Use intersected Java major range]
D -->|No| F[Use current patch range]
E --> G[Write javaVersion and compatibleJavaMajors]
F --> G
G --> H[McLaunchJava selects compatible Java]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
您好——我发现了 3 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论：
## 个别评论
### 评论 1
<location path="Plain Craft Launcher 2/Modules/Minecraft/ModLaunch.cs" line_range="1033-1034" />
<code_context>
+ .ToList();
+ if (majors.Count > 0)
+ {
+ var compatibleMinVer = new Version(majors.Min(), 0, 0, 0);
+ var compatibleMaxVer = new Version(majors.Max(), 999, 999, 999);
+ if (ModBase.modeDebug)
+ ModBase.Log("[Launch] [Debug] JSON 中要求的兼容 Java 主版本范围：" + majors.Min() + " - " + majors.Max());
</code_context>
<issue_to_address>
**issue (bug_risk)：** `compatibleJavaMajors` 中的 Java 8 被转换为 `new Version(8, 0, 0, 0)`，而启动器的其他部分将 Java 8 表示为 `new Version(1, 8, ...)`；因此，版本为 `1.8.x` 的已安装 Java 8 运行时会低于所需的最低版本，从而被拒绝。
**触发条件：** 当 MMC 整合包声明 Java 8 为兼容的主版本时。
**建议修复：** 将不超过 8 的主版本转换为旧版 `Version(1, major, ...)` 表示方式，与现有的 `javaVersion` 处理逻辑保持一致。
```suggestion
var compatibleMinVer = majors.Min() <= 8
? new Version(1, majors.Min(), 0, 0)
: new Version(majors.Min(), 0, 0, 0);
var compatibleMaxVer = majors.Max() <= 8
? new Version(1, majors.Max(), 999, 999)
: new Version(majors.Max(), 999, 999, 999);
```
</issue_to_address>
### 评论 2
<location path="Plain Craft Launcher 2/Modules/Minecraft/ModLaunch.cs" line_range="1025-1028" />
<code_context>
if (ModInstanceList.McMcInstanceSelected.JsonObject["javaVersion"] is not null)
{
- var majorVersion = ModBase.Val(ModInstanceList.McMcInstanceSelected.JsonObject["javaVersion"]["majorVersion"]);
+ // Val 无法解析 JsonNode（会返回 0），必须先转为字符串
+ var majorVersion = ModBase.Val(ModInstanceList.McMcInstanceSelected.JsonObject["javaVersion"]["majorVersion"]
+ ?.ToString());
</code_context>
<issue_to_address>
**issue (bug_risk)：** 明确指定的兼容 Java 主版本集合被压缩成了最小值到最大值的范围，因此声明 `[8, 17]` 等主版本的整合包也会接受不受支持的 Java 9 到 16 运行时。
**触发条件：** 当 `compatibleJavaMajors` 跳过最小值与最大值之间的一个或多个主版本时。
**建议修复：** 使用原始的兼容主版本集合检查候选 Java 运行时，而不是将该集合表示为连续的 `Version` 范围。
</issue_to_address>
### 评论 3
<location path="Plain Craft Launcher 2/Modules/Minecraft/ModModpack.cs" line_range="1498-1500" />
<code_context>
- {
- javaVersion = (int)javaMajors[0];
- javaComponent = null;
+ javaMajorMin = majorMin;
+ javaMajorMax = majorMax;
+ javaMajorsJson = patchJson["compatibleJavaMajors"]?.DeepClone();
+ ModBase.Log($"[ModPack] JSON-Patch {patchJson["uid"]} 兼容的 Java 主版本：" +
+ string.Join(", ", majors));
</code_context>
<issue_to_address>
**issue (bug_risk)：** 当多个 JSON 补丁声明兼容的 Java 主版本时，代码会在 `javaMajorMin` 和 `javaMajorMax` 中计算交集，但只将当前补丁的原始数组存储在 `javaMajorsJson` 中；因此生成的实例会丢失交集，启动时使用的是最后一个补丁更宽泛的范围。
**触发条件：** 当两个或更多组件声明不同的 `compatibleJavaMajors` 列表时。
**建议修复：** 将实际的交集集合存储在 `javaMajorsJson` 中，或保留每个组件的约束，并在选择 Java 时强制执行这些约束的交集。
</issue_to_address>Original comment in English
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="Plain Craft Launcher 2/Modules/Minecraft/ModLaunch.cs" line_range="1033-1034" />
<code_context>
+ .ToList();
+ if (majors.Count > 0)
+ {
+ var compatibleMinVer = new Version(majors.Min(), 0, 0, 0);
+ var compatibleMaxVer = new Version(majors.Max(), 999, 999, 999);
+ if (ModBase.modeDebug)
+ ModBase.Log("[Launch] [Debug] JSON 中要求的兼容 Java 主版本范围：" + majors.Min() + " - " + majors.Max());
</code_context>
<issue_to_address>
**issue (bug_risk):** Java 8 in `compatibleJavaMajors` is converted to `new Version(8, 0, 0, 0)`, while the rest of the launcher represents Java 8 as `new Version(1, 8, ...)`; consequently an installed Java 8 runtime with version `1.8.x` falls below the required minimum and is rejected.
**Triggers:** When an MMC pack declares Java 8 as a compatible major.
**Suggested fix:** Convert majors up to 8 to the legacy `Version(1, major, ...)` representation, matching the existing `javaVersion` handling.
```suggestion
var compatibleMinVer = majors.Min() <= 8
? new Version(1, majors.Min(), 0, 0)
: new Version(majors.Min(), 0, 0, 0);
var compatibleMaxVer = majors.Max() <= 8
? new Version(1, majors.Max(), 999, 999)
: new Version(majors.Max(), 999, 999, 999);
```
</issue_to_address>
### Comment 2
<location path="Plain Craft Launcher 2/Modules/Minecraft/ModLaunch.cs" line_range="1025-1028" />
<code_context>
if (ModInstanceList.McMcInstanceSelected.JsonObject["javaVersion"] is not null)
{
- var majorVersion = ModBase.Val(ModInstanceList.McMcInstanceSelected.JsonObject["javaVersion"]["majorVersion"]);
+ // Val 无法解析 JsonNode（会返回 0），必须先转为字符串
+ var majorVersion = ModBase.Val(ModInstanceList.McMcInstanceSelected.JsonObject["javaVersion"]["majorVersion"]
+ ?.ToString());
</code_context>
<issue_to_address>
**issue (bug_risk):** The explicit set of compatible Java majors is collapsed to a minimum-to-maximum range, so a pack declaring majors such as `[8, 17]` also accepts unsupported Java 9 through 16 runtimes.
**Triggers:** When `compatibleJavaMajors` skips one or more major versions between its minimum and maximum.
**Suggested fix:** Check candidate Java runtimes against the original compatible-major set instead of representing the set as a continuous `Version` range.
</issue_to_address>
### Comment 3
<location path="Plain Craft Launcher 2/Modules/Minecraft/ModModpack.cs" line_range="1498-1500" />
<code_context>
- {
- javaVersion = (int)javaMajors[0];
- javaComponent = null;
+ javaMajorMin = majorMin;
+ javaMajorMax = majorMax;
+ javaMajorsJson = patchJson["compatibleJavaMajors"]?.DeepClone();
+ ModBase.Log($"[ModPack] JSON-Patch {patchJson["uid"]} 兼容的 Java 主版本：" +
+ string.Join(", ", majors));
</code_context>
<issue_to_address>
**issue (bug_risk):** When multiple JSON patches declare compatible Java majors, the code computes an intersection in `javaMajorMin` and `javaMajorMax` but stores only the current patch's original array in `javaMajorsJson`; the resulting instance therefore loses the intersection and launch uses the last patch's broader range.
**Triggers:** When two or more components declare different `compatibleJavaMajors` lists.
**Suggested fix:** Store the actual intersection set in `javaMajorsJson`, or retain per-component constraints and enforce their intersection during Java selection.
</issue_to_address>![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: ca885889e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-worked with Xiaomi MiMo v2.6 Flash Medium
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc7388a57b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-worked with Xiaomi MiMo v2.6 Flash Medium
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eddd951dc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
Author
|
@PinkYuDeer 打扰了，我再次对 GTNH 等 MultiMC 整合包的支持进行了修复。之前因为大家时间都很紧张，一直没机会管这个问题。 我使用 GTNH 2.8.4 + Zulu JDK 17 / GTNH 2.8.4 + Zulu JDK 25 / GTNH 2.9.0-RC-1 + Liberica JRE 26 的组合进行了测试，均可直接安装启动，看起来没有其他问题。 不过我后来还用 GTNH 2.9.0-RC-1 + Zulu JDK 17 测试，发现无法正常启动。看起来这似乎是 Java 版本问题，但我还是告知一下。 使用 此处的 Action 构建 即可进行测试，有问题直接在这里回复就行。我期望在这个 PR 合并后尽快发布一个正式版。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
自不知道什么时候起，MMC 整合包支持的一些特性被破坏了。本 PR 对其进行了修复，使其能够正常安装与启动，并能正确应用相关实例自定义设置。
经过测试，本 PR 的构建产物能够正常安装 GTNH 的 MMC / Prism 整合包。
关于部分首行改动：移除了这些文件的 BOM。
Sourcery 总结
修复并完善 MultiMC/Prism 整合包的安装、启动及实例自定义设置支持。
新功能：
错误修复：
改进：
维护：
Original summary in English
Sourcery 总结
恢复并改进 MultiMC/Prism 模组包的安装、启动和实例自定义支持。
新功能：
错误修复：
改进：
杂项：
Original summary in English
Summary by Sourcery
Restore and improve MultiMC/Prism modpack installation, launch, and instance customization support.
New Features:
Bug Fixes:
Enhancements:
Chores: