Skip to content

Fix/legacy modpack null type - #6599

Open
fatelove42 wants to merge 1 commit into
HMCL-dev:mainfrom
fatelove42:fix/legacy-modpack-null-type
Open

Fix/legacy modpack null type#6599
fatelove42 wants to merge 1 commit into
HMCL-dev:mainfrom
fatelove42:fix/legacy-modpack-null-type

Conversation

@fatelove42

Copy link
Copy Markdown
Contributor

概述

修复了由于旧版本 HMCL 或第三方工具导出的整合包配置文件 modpack.cfg 中缺失 type 字段,导致在最新版本 HMCL 中更新整合包时抛出 IllegalArgumentExceptionJsonParseException 并中断更新的问题。


根本原因

  1. JSON 反序列化校验限制:先前 ModpackConfiguration.validate() 强制要求 type 字段不能为 null。在解析缺乏 "type" 键的旧版 modpack.cfg 时,校验失败导致反序列化抛出异常并落空。
  2. 未判空的类型断言拦截CurseInstallTask 及其他整合包安装任务在比对提供商类型时没有检查 type 是否为 null。在旧配置中 typenull 时,!"Curse".equals(null) 被计算为 true,从而误判并抛出 IllegalArgumentException(“Instance is not a CurseForge modpack. Cannot update this instance.”)。

主要修改

  1. 放宽 ModpackConfiguration.validate():移除了 ModpackConfiguration.validate() 中对 type == null 的强行抛错限制,允许旧版本整合包配置成功反序列化。
  2. 安全的提供商类型校验:在所有整合包 installer 任务(包括 CurseInstallTaskModrinthInstallTaskMcbbsModpackLocalInstallTaskMcbbsModpackRemoteInstallTaskMultiMCModpackInstallTaskServerModpackLocalInstallTaskServerModpackRemoteInstallTask)中增加了 config.getType() != null 的前置检查。只有当 type 存在且明确与当前提供商不匹配时才抛出异常拦截;老整合包(type == null)将顺畅放行并在更新完成后自动写回包含 type 的规范新配置。

@github-actions github-actions Bot added the 10+ label Aug 3, 2026
@3gf8jv4dv

Copy link
Copy Markdown
Contributor

维护者已经开始猜疑有 AI 特征的 PR 了:#6598

不过我先帮作者解释一下:

  1. 标题看起来是 PR 的分支名,没有编辑导致的
  2. 作者之前在 HMCL 的 GitHub 有交流记录:https://github.com/HMCL-dev/HMCL/issues?q=sort%3Aupdated-desc%20involves%3Afatelove42

@Glavo

Glavo commented Aug 4, 2026

Copy link
Copy Markdown
Member

@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: e71bb584e5

ℹ️ 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 90 to 93
public void validate() throws JsonParseException {
if (manifest == null)
throw new JsonParseException("MinecraftInstanceConfiguration missing `manifest`");
if (type == null)
throw new JsonParseException("MinecraftInstanceConfiguration missing `type`");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Infer the provider before accepting a null type

When an existing modpack.cfg omits type, this relaxed validation lets finishModpackInstallingAsync proceed, but it then calls ModpackHelper.getUpdateTask before any of the modified installer checks run. The local-file path passes null to getProviderByType and reports the pack as unsupported, while the server-manifest path switches on null and throws an uncaught NullPointerException; consequently, the legacy configurations targeted by this change still cannot be updated. The provider must be inferred or normalized before these dispatch points rather than merely allowing type to remain null.

Useful? React with 👍 / 👎.

@fatelove42
fatelove42 force-pushed the fix/legacy-modpack-null-type branch from e71bb58 to 7c58b5a Compare August 4, 2026 13:28
@github-actions github-actions Bot added 40+ and removed 10+ labels Aug 4, 2026
@fatelove42
fatelove42 force-pushed the fix/legacy-modpack-null-type branch 2 times, most recently from 3e0e463 to da6266f Compare August 4, 2026 13:57
@Glavo

Glavo commented Aug 4, 2026

Copy link
Copy Markdown
Member

@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: da6266f7dc

ℹ️ 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 +73 to +74
if (map.containsKey("manifestType") || (map.containsKey("files") && map.containsKey("minecraft"))) {
return "Curse";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Check MCBBS signatures before classifying Curse manifests

When a legacy MCBBS modpack.cfg without type is loaded through readModpackConfiguration, its manifest is a Map containing both manifestType and addons. This first condition therefore returns Curse, so archive updates dispatch through CurseModpackProvider and fail with a mismatched-pack error, while launch-time MCBBS option injection is also skipped. Use signatures that distinguish the shared manifestType field, such as checking MCBBS-specific fields first.

Useful? React with 👍 / 👎.

Comment on lines +76 to +77
if (map.containsKey("addons")) {
return "MCBBS";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Recognize server manifests before the shared addons field

A legacy server manifest contains both addons and fileApi, so this branch runs before the server check and returns MCBBS. Besides identifying the wrong format, that spelling does not match the registered provider name Mcbbs, causing local updates to report the pack as unsupported and server-manifest updates to reject it instead of accepting the inferred Server type.

Useful? React with 👍 / 👎.

@fatelove42
fatelove42 force-pushed the fix/legacy-modpack-null-type branch from da6266f to 0845bf7 Compare August 4, 2026 14:29
@fatelove42
fatelove42 force-pushed the fix/legacy-modpack-null-type branch from 0845bf7 to b7b6513 Compare August 4, 2026 14:50
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.

3 participants