refactor(mod): 重构模组前置相关#3273
Conversation
Reviewer's Guide通过引入按版本的依赖弹窗、区分列表项的左键/右键/下载按钮行为,并新增针对特定版本的快速下载流程(集成到全局对话框/消息框系统),重构了 Mod 版本依赖处理和 UI。 针对特定版本依赖弹窗及操作的时序图sequenceDiagram
actor User
participant PageDownloadCompDetail
participant ModMain
participant ModDependencyMsgBox
User->>PageDownloadCompDetail: ShowVersionPopup_Click
PageDownloadCompDetail->>PageDownloadCompDetail: ResolveFileFromSender(sender)
PageDownloadCompDetail->>ModMain: ModDependencyMsgBox(file)
ModMain->>ModDependencyMsgBox: create ModDependencyMsgBox(MyMsgBoxConverter)
ModDependencyMsgBox-->>ModMain: Result (CompProject | 1 | 2 | null)
ModMain-->>PageDownloadCompDetail: return Result
alt Result is CompProject
PageDownloadCompDetail->>ModMain: frmMain.PageChange(CompDetail)
else Result == 1
PageDownloadCompDetail->>PageDownloadCompDetail: InstallToInstance(file)
else Result == 2
PageDownloadCompDetail->>PageDownloadCompDetail: SaveCompFile(file)
else Result == null
PageDownloadCompDetail->>PageDownloadCompDetail: [close popup]
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以:
Getting HelpOriginal review guide in EnglishReviewer's GuideRefactors mod version dependency handling and UI by introducing a per-version dependency popup, separating list item click behaviors (left/right/download), and adding a new quick-download flow for specific versions integrated with the global dialog/message-box system. Sequence diagram for per-version dependency popup and actionssequenceDiagram
actor User
participant PageDownloadCompDetail
participant ModMain
participant ModDependencyMsgBox
User->>PageDownloadCompDetail: ShowVersionPopup_Click
PageDownloadCompDetail->>PageDownloadCompDetail: ResolveFileFromSender(sender)
PageDownloadCompDetail->>ModMain: ModDependencyMsgBox(file)
ModMain->>ModDependencyMsgBox: create ModDependencyMsgBox(MyMsgBoxConverter)
ModDependencyMsgBox-->>ModMain: Result (CompProject | 1 | 2 | null)
ModMain-->>PageDownloadCompDetail: return Result
alt Result is CompProject
PageDownloadCompDetail->>ModMain: frmMain.PageChange(CompDetail)
else Result == 1
PageDownloadCompDetail->>PageDownloadCompDetail: InstallToInstance(file)
else Result == 2
PageDownloadCompDetail->>PageDownloadCompDetail: SaveCompFile(file)
else Result == null
PageDownloadCompDetail->>PageDownloadCompDetail: [close popup]
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我在这里提供一些整体性的反馈:
- 在 ModMain.ModDependencyMsgBox 和 ModDependencyMsgBox.Close 中,你都调用了 ComponentDispatcher.PopModal(),这可能导致模态栈不平衡;建议从 Close() 中移除 PopModal 调用,并依赖 ModDependencyMsgBox 中 finally 块来进行清理。
- AppendUniqueNameSuffix 目前对每个元素都使用 ModBase.GetUuid(),而不是已有的 uuid 字段,因此每个控件都会得到不同的后缀;如果你的意图是为每个对话框使用单一的唯一分组 ID,那么应当统一使用 uuid 字段,而不是生成新的值。
- FillDependencySection 不再像旧的 _AddDependencyBar 那样记录缺失的依赖项目 ID;如果在 compProjectCache 不完整时静默失败会带来问题,建议在无法解析依赖时重新加入一条调试日志。
面向 AI Agents 的提示
Please address the comments from this code review:
## Overall Comments
- 在 ModMain.ModDependencyMsgBox 和 ModDependencyMsgBox.Close 中,你都调用了 ComponentDispatcher.PopModal(),这可能导致模态栈不平衡;建议从 Close() 中移除 PopModal 调用,并依赖 ModDependencyMsgBox 中 finally 块来进行清理。
- AppendUniqueNameSuffix 目前对每个元素都使用 ModBase.GetUuid(),而不是已有的 uuid 字段,因此每个控件都会得到不同的后缀;如果你的意图是为每个对话框使用单一的唯一分组 ID,那么应当统一使用 uuid 字段,而不是生成新的值。
- FillDependencySection 不再像旧的 _AddDependencyBar 那样记录缺失的依赖项目 ID;如果在 compProjectCache 不完整时静默失败会带来问题,建议在无法解析依赖时重新加入一条调试日志。帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据这些反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- In ModMain.ModDependencyMsgBox and ModDependencyMsgBox.Close you both call ComponentDispatcher.PopModal(), which can lead to an unbalanced modal stack; consider removing the PopModal call from Close() and rely on the finally block in ModDependencyMsgBox to clean up.
- AppendUniqueNameSuffix currently uses ModBase.GetUuid() for each element instead of the existing uuid field, so each control gets a different suffix; if the intent is to have a single unique group ID per dialog, use the uuid field consistently instead of generating new values.
- FillDependencySection no longer logs missing dependency project IDs like the old _AddDependencyBar did; if silent failures are problematic when compProjectCache is incomplete, consider reintroducing a debug log when a dependency cannot be resolved.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ModMain.ModDependencyMsgBox and ModDependencyMsgBox.Close you both call ComponentDispatcher.PopModal(), which can lead to an unbalanced modal stack; consider removing the PopModal call from Close() and rely on the finally block in ModDependencyMsgBox to clean up.
- AppendUniqueNameSuffix currently uses ModBase.GetUuid() for each element instead of the existing uuid field, so each control gets a different suffix; if the intent is to have a single unique group ID per dialog, use the uuid field consistently instead of generating new values.
- FillDependencySection no longer logs missing dependency project IDs like the old _AddDependencyBar did; if silent failures are problematic when compProjectCache is incomplete, consider reintroducing a debug log when a dependency cannot be resolved.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f857531d71
ℹ️ 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".
| return; | ||
| } | ||
| if (!instance.IsLoaded) instance.Load(); | ||
| var modsFolder = instance.PathIndie + "mods\\"; |
There was a problem hiding this comment.
Route instance installs by resource type
When this popup is opened from the default version-list branch (which also covers resource packs, shaders, and data packs), choosing “Install to current instance” always builds the target under <instance>/mods. For those non-mod resource pages this silently downloads the selected file into the wrong folder, so Minecraft will not load it; use the same type-to-subfolder mapping as the save/quick-download paths or restrict this action to mods.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1e6908047
ℹ️ 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".
| additional = (project, new List<string>(), "", ModComp.CompLoaderType.Any, | ||
| ModComp.CompType.Any, null) |
There was a problem hiding this comment.
Preserve version context when opening dependencies
When this popup is opened from a detail page that already has a target Minecraft version/loader (for example, coming from an instance mod list or install flow), clicking a dependency now navigates with an empty target version and Any loader. The dependency detail page therefore loses the compatibility context and opens the full unfiltered version list instead of the matching version group, making it easy to install the wrong dependency version; the existing detail-card navigation preserves additional.TargetVersion and additional.TargetLoader for this scenario.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb9e86609c
ℹ️ 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".
| new LoaderDownload(Lang.Text("Download.Comp.Detail.DownloadFile"), | ||
| new List<DownloadFile> { file.ToNetFile(target) }) |
There was a problem hiding this comment.
Honor dependency auto-install for popup installs
When AutoInstallDependencies is enabled and the selected mod version has required dependencies, this new install path queues only the selected file and never runs the dependency resolver used by SaveCompFile (ModCompDependency.BuildRequest / ConfirmDependencyInstall). Installing from the popup can therefore leave a missing-dependency mod in the current instance; route this action through the same dependency flow before starting the download.
Useful? React with 👍 / 👎.
This closes #3271
Summary by Sourcery
重新设计模组版本依赖的处理逻辑和界面,引入专门的依赖弹窗,以及针对单个版本的新安装/下载入口。
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
Revamp mod version dependency handling and UI, introducing a dedicated dependency popup and new install/download entry points for individual versions.
New Features:
Enhancements:
Summary by Sourcery
通过将汇总的依赖栏改为按版本的依赖弹窗,并新增安装和下载特定版本的入口,重新设计模组版本依赖处理方式。
新功能:
改进:
ModDependency消息框类型集成到现有对话框基础设施中,并实现其 UI,用于展示版本详情和依赖信息。Original summary in English
Summary by Sourcery
Revamp mod version dependency handling by replacing the aggregated dependency bar with a per-version dependency popup and adding new entry points for installing and downloading specific versions.
New Features:
Enhancements: