feat: 支持更多 CopilotTask 字段 - #103
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嗨，我发现了两个问题，并给出了一些整体反馈：
SupportUnitUsage枚举的 case 使用了首字母大写的名称，比如.None和.Specific；建议按照 Swift 的 lowerCamelCase 约定重命名（例如.none、.specific），以与常见的 Swift 风格保持一致。- 阵容栏位选择器中的
ForEach(0..<5)直接写死了范围并显示原始索引；建议使用更具描述性的表示方式（例如从 1 开始的显示，或者使用枚举/常量列表来表示栏位），让 UI 更清晰并避免“魔法数字”。
给 AI Agents 的提示
请根据本次代码评审的评论进行修改：
## 整体意见
- `SupportUnitUsage` 枚举的 case 使用了首字母大写的名称，比如 `.None` 和 `.Specific`；建议按照 Swift 的 lowerCamelCase 约定重命名（例如 `.none`、`.specific`），以与常见的 Swift 风格保持一致。
- 阵容栏位选择器中的 `ForEach(0..<5)` 直接写死了范围并显示原始索引；建议使用更具描述性的表示方式（例如从 1 开始的显示，或者使用枚举/常量列表来表示栏位），让 UI 更清晰并避免“魔法数字”。
## 单条评论
### 评论 1
<location path="MeoAsstMac/Views/CopilotView.swift" line_range="55-64" />
<code_context>
viewModel.copilot = .regular(newValue)
}
- HStack {
- Toggle("自动编队", isOn: binding.formation)
- Toggle("信赖干员", isOn: binding.add_trust)
+ VStack {
+ HStack {
+ Toggle("自动编队", isOn: binding.formation)
+ if binding.formation.wrappedValue {
+ Picker("编队栏位", selection: binding.formation_index) {
+ ForEach(0..<5) { index in
+ Text("\(index)").tag(index)
+ }
+ }
+ .pickerStyle(.menu)
+ Toggle("信赖干员", isOn: binding.add_trust)
+ Toggle("忽视练度需求", isOn: binding.ignore_requirements)
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** 将「信赖干员」和「忽视练度需求」的可见性绑定到「自动编队」上，可能会改变之前的行为并降低配置灵活性。
「信赖干员」过去在「自动编队」是否开启的情况下都会显示；现在「信赖干员」和「忽视练度需求」只有在「自动编队」开启时才会出现。如果这些选项是希望在非自动编队场景（例如手动编队）下也能生效，那么这一改动就移除了这种灵活性。建议保持这些开关始终可用，或者确认它们确实是刻意限制为仅在「自动编队」开启时可见。
</issue_to_address>
### 评论 2
<location path="MeoAsstMac/Views/CopilotView.swift" line_range="58-59" />
<code_context>
+ HStack {
+ Toggle("自动编队", isOn: binding.formation)
+ if binding.formation.wrappedValue {
+ Picker("编队栏位", selection: binding.formation_index) {
+ ForEach(0..<5) { index in
+ Text("\(index)").tag(index)
+ }
</code_context>
<issue_to_address>
**suggestion:** 编队栏位从 0 开始可能不符合用户直觉，建议显示为 1–5 或使用更语义化的标签。
当前使用 `ForEach(0..<5)` 并在 UI 中直接显示 `Text("\(index)")`，会把内部的 0–4 暴露给用户。建议保持 selection 的 `tag(index)` 不变，但展示时改为 `Text("\(index + 1)")`（1–5），或改用更语义化的文案如「第 1 队」等，仅在内部继续使用零基索引。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈持续改进评审质量。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The
SupportUnitUsageenum cases use uppercased names like.Noneand.Specific; consider renaming them to follow Swift’s lowerCamelCase conventions (e.g..none,.specific) for consistency with typical Swift style. - The
ForEach(0..<5)for the formation index picker hardcodes the range and presents raw indices; consider using a more descriptive representation (e.g. 1-based display, or an enum/constant list of slots) to make the UI clearer and avoid magic numbers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `SupportUnitUsage` enum cases use uppercased names like `.None` and `.Specific`; consider renaming them to follow Swift’s lowerCamelCase conventions (e.g. `.none`, `.specific`) for consistency with typical Swift style.
- The `ForEach(0..<5)` for the formation index picker hardcodes the range and presents raw indices; consider using a more descriptive representation (e.g. 1-based display, or an enum/constant list of slots) to make the UI clearer and avoid magic numbers.
## Individual Comments
### Comment 1
<location path="MeoAsstMac/Views/CopilotView.swift" line_range="55-64" />
<code_context>
viewModel.copilot = .regular(newValue)
}
- HStack {
- Toggle("自动编队", isOn: binding.formation)
- Toggle("信赖干员", isOn: binding.add_trust)
+ VStack {
+ HStack {
+ Toggle("自动编队", isOn: binding.formation)
+ if binding.formation.wrappedValue {
+ Picker("编队栏位", selection: binding.formation_index) {
+ ForEach(0..<5) { index in
+ Text("\(index)").tag(index)
+ }
+ }
+ .pickerStyle(.menu)
+ Toggle("信赖干员", isOn: binding.add_trust)
+ Toggle("忽视练度需求", isOn: binding.ignore_requirements)
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Tying 信赖干员 and 忽视练度需求 visibility to 自动编队 may change previous behavior and reduce configurability.
信赖干员 used to be visible regardless of 自动编队; now both 信赖干员 and 忽视练度需求 only appear when 自动编队 is on. If these options are meant to apply outside 自动编队 (e.g., for manual formations), this change removes that flexibility. Consider keeping these toggles always available, or confirm they are intentionally restricted to 自动编队 only.
</issue_to_address>
### Comment 2
<location path="MeoAsstMac/Views/CopilotView.swift" line_range="58-59" />
<code_context>
+ HStack {
+ Toggle("自动编队", isOn: binding.formation)
+ if binding.formation.wrappedValue {
+ Picker("编队栏位", selection: binding.formation_index) {
+ ForEach(0..<5) { index in
+ Text("\(index)").tag(index)
+ }
</code_context>
<issue_to_address>
**suggestion:** 编队栏位从 0 开始可能不符合用户直觉，建议显示为 1–5 或使用更语义化的标签。
当前使用 `ForEach(0..<5)` 并在 UI 中直接显示 `Text("\(index)")`，会把内部的 0–4 暴露给用户。建议保持 selection 的 `tag(index)` 不变，但展示时改为 `Text("\(index + 1)")`（1–5），或改用更语义化的文案如「第 1 队」等，仅在内部继续使用零基索引。
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+55
to
+64
| HStack { | ||
| Toggle("自动编队", isOn: binding.formation) | ||
| if binding.formation.wrappedValue { | ||
| Picker("编队栏位", selection: binding.formation_index) { | ||
| ForEach(0..<5) { index in | ||
| Text("\(index)").tag(index) | ||
| } | ||
| } | ||
| .pickerStyle(.menu) | ||
| Toggle("信赖干员", isOn: binding.add_trust) |
There was a problem hiding this comment.
issue (bug_risk): 将「信赖干员」和「忽视练度需求」的可见性绑定到「自动编队」上，可能会改变之前的行为并降低配置灵活性。
「信赖干员」过去在「自动编队」是否开启的情况下都会显示；现在「信赖干员」和「忽视练度需求」只有在「自动编队」开启时才会出现。如果这些选项是希望在非自动编队场景（例如手动编队）下也能生效，那么这一改动就移除了这种灵活性。建议保持这些开关始终可用，或者确认它们确实是刻意限制为仅在「自动编队」开启时可见。
Original comment in English
issue (bug_risk): Tying 信赖干员 and 忽视练度需求 visibility to 自动编队 may change previous behavior and reduce configurability.
信赖干员 used to be visible regardless of 自动编队; now both 信赖干员 and 忽视练度需求 only appear when 自动编队 is on. If these options are meant to apply outside 自动编队 (e.g., for manual formations), this change removes that flexibility. Consider keeping these toggles always available, or confirm they are intentionally restricted to 自动编队 only.
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并且给了一些整体上的反馈：
- 建议对
formation_index相对于numFormationSlots做范围限制或校验（尤其是在加载已有配置时），以避免在变更槽位数量或读取持久化数据后出现越界选择的情况。 CopilotView中编队和助战单位的控制逻辑目前在视图内比较密集，建议抽取为一个独立的子视图，这样可以提升可读性，也更便于之后独立演化这部分逻辑。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- Consider clamping or validating `formation_index` against `numFormationSlots` (especially when loading existing configurations) so that changes to the slot count or persisted values don't result in out-of-range selections.
- The `CopilotView` formation and support-unit controls are getting fairly dense in-line; extracting them into a dedicated subview would improve readability and make it easier to evolve this logic independently.
## Individual Comments
### Comment 1
<location path="MeoAsstMac/Views/CopilotView.swift" line_range="76-78" />
<code_context>
+ Text($0.description).tag($0)
+ }
+ }
+ if binding.support_unit_usage.wrappedValue == .specific {
+ TextField("指定助战干员", text: binding.support_unit_name)
+ .frame(maxWidth: 150)
+ }
+ }
</code_context>
<issue_to_address>
**nitpick:** The fixed `maxWidth: 150` on the TextField may be too restrictive for longer localized names.
This may cause truncation or awkward UX for longer names or in some locales. Consider relying on the HStack’s natural sizing (possibly via layoutPriority) or deriving the width from available space instead of a fixed constant to keep layouts robust across window sizes and languages.
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据你的反馈改进后续的评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Consider clamping or validating
formation_indexagainstnumFormationSlots(especially when loading existing configurations) so that changes to the slot count or persisted values don't result in out-of-range selections. - The
CopilotViewformation and support-unit controls are getting fairly dense in-line; extracting them into a dedicated subview would improve readability and make it easier to evolve this logic independently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider clamping or validating `formation_index` against `numFormationSlots` (especially when loading existing configurations) so that changes to the slot count or persisted values don't result in out-of-range selections.
- The `CopilotView` formation and support-unit controls are getting fairly dense in-line; extracting them into a dedicated subview would improve readability and make it easier to evolve this logic independently.
## Individual Comments
### Comment 1
<location path="MeoAsstMac/Views/CopilotView.swift" line_range="76-78" />
<code_context>
+ Text($0.description).tag($0)
+ }
+ }
+ if binding.support_unit_usage.wrappedValue == .specific {
+ TextField("指定助战干员", text: binding.support_unit_name)
+ .frame(maxWidth: 150)
+ }
+ }
</code_context>
<issue_to_address>
**nitpick:** The fixed `maxWidth: 150` on the TextField may be too restrictive for longer localized names.
This may cause truncation or awkward UX for longer names or in some locales. Consider relying on the HStack’s natural sizing (possibly via layoutPriority) or deriving the width from available space instead of a fixed constant to keep layouts robust across window sizes and languages.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+76
to
+78
| if binding.support_unit_usage.wrappedValue == .specific { | ||
| TextField("指定助战干员", text: binding.support_unit_name) | ||
| .frame(maxWidth: 150) |
There was a problem hiding this comment.
nitpick: TextField 上固定的 maxWidth: 150 对于较长的本地化名称可能过于局限。
这可能会导致名称被截断，或者在名称较长或某些语言环境下产生不太友好的交互体验。建议更多依赖 HStack 的自然布局尺寸（可以结合 layoutPriority），或者根据可用空间动态计算宽度，而不是使用固定常量，以确保在不同窗口尺寸和语言下布局都足够健壮。
Original comment in English
nitpick: The fixed maxWidth: 150 on the TextField may be too restrictive for longer localized names.
This may cause truncation or awkward UX for longer names or in some locales. Consider relying on the HStack’s natural sizing (possibly via layoutPriority) or deriving the width from available space instead of a fixed constant to keep layouts robust across window sizes and languages.
Collaborator
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈：
- 在
CopilotView上对viewModel.copilot使用全局.animation(.default, value: viewModel.copilot)会对副驾驶配置的每一次更改都进行动画（包括文本字段编辑），这可能会让界面显得有些抖动；建议只对影响布局的标志（例如formation）进行动画，而不是对整个配置启用动画。 - 对于
formation_index选择器，在加载已有配置时，将formation_index约束或归一化到0...RegularCopilotConfiguration.formationCount范围内可能更安全，以避免持久化的越界值导致选择器处于无效选项。 - 将
SSSCopilotConfiguration改为RegularCopilotConfiguration的typealias会改变 SSS 配置的编码结构（例如会出现copilot_list、formation等额外字段）；如果序列化兼容性很重要，建议保留一个专用的 SSS 类型，或者实现一个自定义的Codable，在 SSS 情况下省略未使用的字段。
面向 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The global `.animation(.default, value: viewModel.copilot)` on `CopilotView` will animate every change to the copilot config (including text field edits), which can feel jittery; consider scoping animation only to layout-affecting flags like `formation` instead of the entire configuration.
- For the `formation_index` picker, it may be safer to clamp or normalize `formation_index` into `0...RegularCopilotConfiguration.formationCount` when loading existing configs so that out-of-range persisted values don’t lead to an invalid picker selection.
- Changing `SSSCopilotConfiguration` to a `typealias` of `RegularCopilotConfiguration` alters the encoded shape for SSS configs (extra fields like `copilot_list`, `formation`, etc.); if serialized compatibility matters, consider keeping a dedicated SSS type or adding a custom `Codable` implementation that omits unused fields for SSS.帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈来改进之后的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- The global
.animation(.default, value: viewModel.copilot)onCopilotViewwill animate every change to the copilot config (including text field edits), which can feel jittery; consider scoping animation only to layout-affecting flags likeformationinstead of the entire configuration. - For the
formation_indexpicker, it may be safer to clamp or normalizeformation_indexinto0...RegularCopilotConfiguration.formationCountwhen loading existing configs so that out-of-range persisted values don’t lead to an invalid picker selection. - Changing
SSSCopilotConfigurationto atypealiasofRegularCopilotConfigurationalters the encoded shape for SSS configs (extra fields likecopilot_list,formation, etc.); if serialized compatibility matters, consider keeping a dedicated SSS type or adding a customCodableimplementation that omits unused fields for SSS.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The global `.animation(.default, value: viewModel.copilot)` on `CopilotView` will animate every change to the copilot config (including text field edits), which can feel jittery; consider scoping animation only to layout-affecting flags like `formation` instead of the entire configuration.
- For the `formation_index` picker, it may be safer to clamp or normalize `formation_index` into `0...RegularCopilotConfiguration.formationCount` when loading existing configs so that out-of-range persisted values don’t lead to an invalid picker selection.
- Changing `SSSCopilotConfiguration` to a `typealias` of `RegularCopilotConfiguration` alters the encoded shape for SSS configs (extra fields like `copilot_list`, `formation`, etc.); if serialized compatibility matters, consider keeping a dedicated SSS type or adding a custom `Codable` implementation that omits unused fields for SSS.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
2 participants
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.
Summary by Sourcery
增强 Copilot 配置和界面，以支持更多任务字段以及更灵活的编队/支援选项。
新功能：
增强改进：
CopilotConfiguration可哈希，以更好地支持 SwiftUI 绑定。Original summary in English
Summary by Sourcery
Enhance Copilot configuration and UI to support additional task fields and more flexible formation/support options.
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
增强 Copilot 配置和界面，以支持更多任务字段以及更灵活的编队/支援选项。
新功能：
增强改进：
CopilotConfiguration可哈希，以更好地支持 SwiftUI 绑定。Original summary in English
Summary by Sourcery
Enhance Copilot configuration and UI to support additional task fields and more flexible formation/support options.
New Features:
Enhancements:
新功能：
改进：
Original summary in English
Summary by Sourcery
增强 Copilot 配置和界面，以支持更多任务字段以及更灵活的编队/支援选项。
新功能：
增强改进：
CopilotConfiguration可哈希，以更好地支持 SwiftUI 绑定。Original summary in English
Summary by Sourcery
Enhance Copilot configuration and UI to support additional task fields and more flexible formation/support options.
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
增强 Copilot 配置和界面，以支持更多任务字段以及更灵活的编队/支援选项。
新功能：
增强改进：
CopilotConfiguration可哈希，以更好地支持 SwiftUI 绑定。Original summary in English
Summary by Sourcery
Enhance Copilot configuration and UI to support additional task fields and more flexible formation/support options.
New Features:
Enhancements: