Merged
Conversation
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并给出了一些整体性的反馈:
selectAllTasks和Toolbar中关于 controller/resource 的兼容性检查是重复的,而且存在细微差异;建议抽取一个共享的工具函数(例如isTaskCompatible(taskDef, controllerName, resourceName))来保持逻辑一致,并且更易维护。- 在
allEnabled的useMemo中,将projectInterface?.task放在依赖数组里可能不满足 hook 的 lint 规则,而且一旦projectInterface的引用发生变化会比较脆弱;更安全的做法是依赖projectInterface(或某个稳定的派生值)。
给 AI 代理的提示
请根据下面的代码评审评论进行修改:
## 总体评论
- `selectAllTasks` 和 `Toolbar` 中关于 controller/resource 的兼容性检查是重复的,而且存在细微差异;建议抽取一个共享的工具函数(例如 `isTaskCompatible(taskDef, controllerName, resourceName)`)来保持逻辑一致,并且更易维护。
- 在 `allEnabled` 的 `useMemo` 中,将 `projectInterface?.task` 放在依赖数组里可能不满足 hook 的 lint 规则,而且一旦 `projectInterface` 的引用发生变化会比较脆弱;更安全的做法是依赖 `projectInterface`(或某个稳定的派生值)。
## 逐条评论
### 评论 1
<location path="src/stores/appStore.ts" line_range="618-619" />
<code_context>
// 检查是否有保存的设备和资源配置(用于权限检查等)
const currentControllerName =
selectedController[instanceId] || projectInterface?.controller[0]?.name;
+ const currentResourceName =
</code_context>
<issue_to_address>
**suggestion:** controller/resource 的解析逻辑与 Toolbar 中的逻辑重复,可以进行集中抽取。
这里的逻辑与 `Toolbar` 中的解析逻辑是镜像关系(包括回退顺序和 `projectInterface` 的使用)。建议抽取一个共享工具函数(例如 `getCurrentControllerAndResource(state, instanceId)`),这样规则可以保持一致,而且只需要在一个地方更新。
推荐实现:
```typescript
export const getCurrentControllerAndResource = (state: AppState, instanceId: string) => {
const pi = state.projectInterface;
const instance = state.instances.find((i) => i.id === instanceId);
const controllerName =
state.selectedController[instanceId] ||
instance?.controllerName ||
pi?.controller[0]?.name;
const resourceName =
state.selectedResource[instanceId] ||
instance?.resourceName ||
pi?.resource[0]?.name;
return { controllerName, resourceName };
};
set((state) => {
```
```typescript
const { controllerName: currentControllerName, resourceName: currentResourceName } =
getCurrentControllerAndResource(state, instanceId);
```
1. 在 `Toolbar` 组件中(或当前存在重复 controller/resource 解析逻辑的任何地方),用对 `getCurrentControllerAndResource(state, instanceId)` 的调用替换内联逻辑,以保持行为一致。
2. 确保在该工具函数的签名中,`AppState`(或正确的 store state 类型)已经被导入或在作用域内;如果类型名不同,请将 `AppState` 调整为 `appStore.ts` 中实际使用的类型。
3. 从 `src/stores/appStore.ts` 中导出 `getCurrentControllerAndResource`(如果你的打包器/TS 设置要求在声明之外显式导出),并在 `Toolbar` 中导入它。
4. 如果 `Toolbar` 无法直接访问原始的 store state,你可能需要一个小的包装 selector,通过 store 调用 `getCurrentControllerAndResource`(例如:`useAppStore((state) => getCurrentControllerAndResource(state, instanceId)))`。
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The compatibility checks for controller/resource in
selectAllTasksandToolbarare duplicated and subtly different; consider extracting a shared helper (e.g.isTaskCompatible(taskDef, controllerName, resourceName)) to keep the logic consistent and easier to maintain. - In the
useMemoforallEnabled, usingprojectInterface?.taskin the dependency array may not satisfy hook lint rules and can be fragile ifprojectInterfacechanges identity; it’s safer to depend onprojectInterface(or a stable derived value) instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The compatibility checks for controller/resource in `selectAllTasks` and `Toolbar` are duplicated and subtly different; consider extracting a shared helper (e.g. `isTaskCompatible(taskDef, controllerName, resourceName)`) to keep the logic consistent and easier to maintain.
- In the `useMemo` for `allEnabled`, using `projectInterface?.task` in the dependency array may not satisfy hook lint rules and can be fragile if `projectInterface` changes identity; it’s safer to depend on `projectInterface` (or a stable derived value) instead.
## Individual Comments
### Comment 1
<location path="src/stores/appStore.ts" line_range="618-619" />
<code_context>
// 检查是否有保存的设备和资源配置(用于权限检查等)
const currentControllerName =
selectedController[instanceId] || projectInterface?.controller[0]?.name;
+ const currentResourceName =
</code_context>
<issue_to_address>
**suggestion:** The controller/resource resolution logic is duplicated with Toolbar and could be centralized.
This mirrors the resolution logic in `Toolbar` (including fallback order and `projectInterface` usage). Consider extracting a shared helper (e.g., `getCurrentControllerAndResource(state, instanceId)`) so the rules stay consistent and only need updating in one place.
Suggested implementation:
```typescript
export const getCurrentControllerAndResource = (state: AppState, instanceId: string) => {
const pi = state.projectInterface;
const instance = state.instances.find((i) => i.id === instanceId);
const controllerName =
state.selectedController[instanceId] ||
instance?.controllerName ||
pi?.controller[0]?.name;
const resourceName =
state.selectedResource[instanceId] ||
instance?.resourceName ||
pi?.resource[0]?.name;
return { controllerName, resourceName };
};
set((state) => {
```
```typescript
const { controllerName: currentControllerName, resourceName: currentResourceName } =
getCurrentControllerAndResource(state, instanceId);
```
1. In the `Toolbar` component (or wherever the duplicated controller/resource resolution logic currently lives), replace the inline logic with a call to `getCurrentControllerAndResource(state, instanceId)` to keep behavior consistent.
2. Ensure `AppState` (or the correct store state type) is imported or in scope for the helper signature; if the type name differs, adjust `AppState` to the appropriate type used in `appStore.ts`.
3. Export `getCurrentControllerAndResource` from `src/stores/appStore.ts` (if your bundler/TS setup requires explicit exports beyond this declaration) and import it in `Toolbar`.
4. If `Toolbar` does not have access to the raw store state, you may need a small wrapper selector that calls `getCurrentControllerAndResource` via your store (e.g., `useAppStore((state) => getCurrentControllerAndResource(state, instanceId)))`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
修复任务列表“全选”在批量切换时未正确遵循当前控制器/资源兼容性约束的问题,并让工具栏的“全部已选中”状态计算与该约束保持一致。
Changes:
selectAllTasks全选启用时跳过与当前控制器/资源不兼容的任务- 工具栏
allEnabled计算仅统计与当前控制器/资源兼容的任务(使用useMemo)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/stores/appStore.ts | 调整批量启用逻辑,使全选时考虑 controller/resource 兼容性 |
| src/components/Toolbar.tsx | 调整“全选已选中”状态计算逻辑以匹配兼容性约束 |
MistEO
approved these changes
Apr 2, 2026
Contributor
Author
|
准备提取个工具函数 |
Owner
|
不过其实全选上也无所谓吧,后面实际执行阶段是会忽略掉不支持的 task 的 |
Owner
|
57e1896 这条 commit 导致 mxu 启动后画面是白屏,请检查一下 revert 掉这条 commit 后恢复正常 |
Contributor
Author
end那边实时辅助标着不支持后台,但是确实可以在后台控制器用来确认滑索 |
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
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
确保在切换所有任务时,任务的批量选择会遵循控制器和资源兼容性。
Bug 修复:
增强功能:
Original summary in English
Summary by Sourcery
Ensure task bulk selection respects controller and resource compatibility when toggling all tasks.
Bug Fixes:
Enhancements: