[Feature] 资源包管理 #4980 reforged#5774
Conversation
在BMC4中测试时发现BMC4会判定所有资源包兼容,与原版表现不符,会导致启用/禁用功能失效,故添加警告
Co-authored-by: 3gf8jv4dv <3gf8jv4dv@gmail.com>
Co-authored-by: 3gf8jv4dv <3gf8jv4dv@gmail.com>
…esourcepack-enhancement
Co-authored-by: 3gf8jv4dv <3gf8jv4dv@gmail.com>
Co-authored-by: 3gf8jv4dv <3gf8jv4dv@gmail.com>
removed |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5821e3600
ℹ️ 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".
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant refactoring of the addon management system, establishing a generic base for both mods and resource packs through the LocalAddonFile and LocalAddonManager classes. It replaces the previous resource pack implementation with a more robust system that includes detailed compatibility checks based on PackMcMeta and unified update logic. Review feedback highlights opportunities to improve robustness by logging failures during file deletion, providing user-facing error messages when directory creation fails, and optimizing performance by batching configuration file writes during bulk operations.
|
碎碎念几句。 回顾来看,当时的「撤回」感觉操之过急。 尤其是合并后又突然评论「光顾着看代码了」「合并之后发现还是有很多问题」,给 PR 作者、其他活跃贡献者以及普通用户的观感并不太好。而且撤回理由也很含糊——看到了 mineDiamond 的评论才让人大致明白撤回的一个「动机」。 而且 force push 也不是一个可以随意在默认分支/主分支使用的操作——尤其是对于知名的、有多人协作的项目。这个 PR 也不涉及隐私啥的,哪怕再提交一个 revert commit 也好过 force push。直接 force push,让之前的 PR 看起来「合并」了,但却没有出现在 commit history 中,给其他查阅 commit / issue / PR 的用户带来了困惑和不解。 |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the addon management system by introducing a unified hierarchy for mods and resource packs through the LocalAddonManager and LocalAddonFile classes. It replaces the previous resource pack implementation with a more robust system that includes compatibility checking and update support for CurseForge and Modrinth. The UI has been updated to use generic components for addon updates and downloads. Review feedback suggests improving the UI by displaying filenames instead of full paths in error dialogs and ensuring consistency by using utility methods for zip file operations.
| return i == 0 ? PackMcMeta.PackVersion.UNSPECIFIED : new PackMcMeta.PackVersion(i, 0); | ||
| } | ||
| } | ||
| try (var zipFileTree = new ZipFileTree(CompressingUtils.openZipFile(gameJar))) { |
There was a problem hiding this comment.
For consistency with other parts of the resource pack management code (e.g., ResourcePackZipFile), consider using the CompressingUtils.openZipTree utility method here.
| try (var zipFileTree = new ZipFileTree(CompressingUtils.openZipFile(gameJar))) { | |
| try (var zipFileTree = CompressingUtils.openZipTree(gameJar)) { |
|
又看了看 Glavo 给的图,我产生了一种微妙的感觉。 一个资源包条目的两行文字中,除了扩展名以外,两行文字一模一样。 既然目前第一行确定是显示资源包的文件名,那么可以把第二行改成显示 |
…ement # Conflicts: # HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/DownloadPage.java
Done |
|
有些资源包使用 目前这类资源包会使第二行显示为空。 |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef04e58364
ℹ️ 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".
| if (!resourcePacks.contains(packIdOld) && (supportsNewOptionsFormat || !resourcePacks.contains(packId))) { | ||
| resourcePacks.add(supportsNewOptionsFormat ? packId : packIdOld); |
There was a problem hiding this comment.
Prevent duplicate pack IDs when enabling resource packs
The enable guard uses supportsNewOptionsFormat || !resourcePacks.contains(packId), which is always true on 1.13+ once packIdOld is absent. That means invoking enable on an already-enabled pack appends duplicate file/... IDs to resourcePacks. Since disableResourcePack removes only one occurrence, users can end up with packs that remain enabled after a disable action if they were enabled multiple times (for example via bulk-enable on already enabled selections).
Useful? React with 👍 / 👎.
| modified = true; | ||
| } | ||
| if (!incompatibleResourcePacks.contains(packIdOld) && (supportsNewOptionsFormat || !incompatibleResourcePacks.contains(packId)) && isIncompatible(resourcePack)) { | ||
| incompatibleResourcePacks.add(packId); |
There was a problem hiding this comment.
Write legacy incompatible IDs without the file/ prefix
For legacy options format (supportsNewOptionsFormat == false), this path still writes packId (file/<name>) into incompatibleResourcePacks instead of the legacy <name> format. In the same class, isEnabled only checks the unprefixed ID when running in legacy mode, so incompatible packs can be misclassified and options.txt is saved in a format older versions do not consistently interpret.
Useful? React with 👍 / 👎.
|
@Calboot 这个账号被封了,暂时更新不了该pr :( |
|
@copilot resolve the merge conflicts in this pull request |
|
@codex resolve the merge conflicts in this pull request |
|
Codex couldn't complete this request. Try again later. |
…ement # Conflicts: # HMCL/src/main/resources/assets/css/root.css # HMCLCore/src/main/java/org/jackhuang/hmcl/mod/ModManager.java # HMCLCore/src/main/java/org/jackhuang/hmcl/mod/curse/CurseAddon.java # HMCLCore/src/main/java/org/jackhuang/hmcl/mod/modrinth/ModrinthRemoteModRepository.java
Glavo
left a comment
There was a problem hiding this comment.
这些意见来自 Codex。
我在当前 PR head 3a0beb047c7c86c94afdcab003b6ea165e0611df 上做了静态审查。下面几处会造成资源包启用状态或删除行为与用户预期不一致,建议修正后再合并。
另有一个较小问题:HMCLCore/src/main/java/org/jackhuang/hmcl/mod/modinfo/PackMcMeta.java 中 pack.description 为对象时只解析 text 和 extra,没有处理 Minecraft 文本组件常见的 translate + fallback。这类资源包会在列表和详情页显示空描述,也无法通过 fallback 文本被搜索到。
| String packId = "file/" + resourcePack.getFileNameWithExtension(); | ||
| String packIdOld = resourcePack.getFileNameWithExtension(); | ||
| boolean modified = false; | ||
| if (!resourcePacks.contains(packIdOld) && (supportsNewOptionsFormat || !resourcePacks.contains(packId))) { |
There was a problem hiding this comment.
[P1] supportsNewOptionsFormat || !resourcePacks.contains(packId) 在 1.13+ 恒为 true,已启用的 file/<name> 资源包再次执行启用时会重复 append 到 resourcePacks。disableResourcePack 只 remove 一次,重复启用后用户点禁用仍会残留条目,游戏里资源包继续处于启用状态。
| modified = true; | ||
| } | ||
| if (!incompatibleResourcePacks.contains(packIdOld) && (supportsNewOptionsFormat || !incompatibleResourcePacks.contains(packId)) && isIncompatible(resourcePack)) { | ||
| incompatibleResourcePacks.add(packId); |
There was a problem hiding this comment.
[P2] 旧格式 options(supportsNewOptionsFormat == false)下,resourcePacks 写的是 packIdOld,但不兼容列表仍写入 packId(file/<name>)。后面的 isEnabled 在旧格式下只认可 packIdOld,所以 1.6.1 到 1.12.2 的不兼容资源包启用后会被 UI 判定为未启用,并且保存出旧版本不一致的 options.txt。
| boolean modified = false; | ||
| for (ResourcePackFile resourcePack : resourcePacks) { | ||
| if (resourcePack != null && resourcePack.manager == this) { | ||
| resourcePack.delete(); |
There was a problem hiding this comment.
[P2] 删除资源包只删除文件和 localFiles,没有同步移除 options.txt 里的 resourcePacks / incompatibleResourcePacks 条目。用户删除已启用资源包后配置仍保留旧 ID;之后重新放回同名资源包会被自动视为已启用,造成删除行为和后续游戏状态不一致。


具体改动
LocalAddonFile和LocalFileManager<T extends LocalAddonFile>,使更新对于模组以外的东西(如资源包和光影包)也能适用,从而实现了资源包更新(不保留旧版本)See #4980