Merged
Conversation
Contributor
There was a problem hiding this comment.
嗨,我这边有一些整体上的反馈:
- 你移除了
Common/Region.json,并新增了Interface/SceneRegionalDevelopment.json和SceneManager/SceneRegionalDevelopment.json;请再次确认之前所有对Common/Region.json的引用,现在都通过同一个、一致的入口来访问,避免未来在区域跳转行为上出现分歧。 - 现在有多个流水线(
SellProduct、DeliveryJobs、EnvironmentMonitoring/Region、Resell/ChangeRegion等)都依赖用于地区建设的万能跳转,建议把共用的路由参数或约定(例如区域 ID 字段、路径 key 等)集中管理,这样以后跳转契约发生变更时,就不需要逐一修改每个流水线的 JSON。 - 随着在二级菜单中新增了地区间的最优路径跳转,可以考虑在 JSON 中对一些边界情况做显式处理或说明(例如同区域跳转、目标区域缺失或已废弃等),以避免万能跳转在这些情况下静默失败,或者退回到次优路径。
给 AI Agents 的提示
Please address the comments from this code review:
## Overall Comments
- You’ve removed `Common/Region.json` and introduced `Interface/SceneRegionalDevelopment.json` and `SceneManager/SceneRegionalDevelopment.json`; double-check that all previous references to `Common/Region.json` are now routed through a single, consistent entry point to avoid future divergence in region navigation behavior.
- Since multiple pipelines (`SellProduct`, `DeliveryJobs`, `EnvironmentMonitoring/Region`, `Resell/ChangeRegion`, etc.) now rely on the万能跳转 for地区建设, consider centralizing any shared routing parameters or conventions (e.g., region ID fields, path keys) so that future changes to the跳转契约 don’t require touching every pipeline JSON.
- With the new最优路径跳转 between regions in二级菜单, it may be worth explicitly handling or documenting edge cases in the JSON (e.g., same-region jumps, missing or deprecated target regions) so that the万能跳转 doesn’t silently fail or fall back to non-optimal paths.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的 Review。
Original comment in English
Hey - I've left some high level feedback:
- You’ve removed
Common/Region.jsonand introducedInterface/SceneRegionalDevelopment.jsonandSceneManager/SceneRegionalDevelopment.json; double-check that all previous references toCommon/Region.jsonare now routed through a single, consistent entry point to avoid future divergence in region navigation behavior. - Since multiple pipelines (
SellProduct,DeliveryJobs,EnvironmentMonitoring/Region,Resell/ChangeRegion, etc.) now rely on the万能跳转 for地区建设, consider centralizing any shared routing parameters or conventions (e.g., region ID fields, path keys) so that future changes to the跳转契约 don’t require touching every pipeline JSON. - With the new最优路径跳转 between regions in二级菜单, it may be worth explicitly handling or documenting edge cases in the JSON (e.g., same-region jumps, missing or deprecated target regions) so that the万能跳转 doesn’t silently fail or fall back to non-optimal paths.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You’ve removed `Common/Region.json` and introduced `Interface/SceneRegionalDevelopment.json` and `SceneManager/SceneRegionalDevelopment.json`; double-check that all previous references to `Common/Region.json` are now routed through a single, consistent entry point to avoid future divergence in region navigation behavior.
- Since multiple pipelines (`SellProduct`, `DeliveryJobs`, `EnvironmentMonitoring/Region`, `Resell/ChangeRegion`, etc.) now rely on the万能跳转 for地区建设, consider centralizing any shared routing parameters or conventions (e.g., region ID fields, path keys) so that future changes to the跳转契约 don’t require touching every pipeline JSON.
- With the new最优路径跳转 between regions in二级菜单, it may be worth explicitly handling or documenting edge cases in the JSON (e.g., same-region jumps, missing or deprecated target regions) so that the万能跳转 doesn’t silently fail or fall back to non-optimal paths.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
本 PR 旨在让 SceneManager 的「万能跳转」能力覆盖“地区建设”,并支持从地区建设二级界面(View2)回退到地区建设菜单后再进行跨地区跳转,从而实现更稳定/更优路径的导航。
Changes:
- 新增地区建设相关的 SceneManager 私有实现与 Interface 场景接口(支持 ValleyIV / Wuling 及其二级入口)。
- 统一并重命名地区建设场景命中节点(
InValleyIVRegionalDevelopment/InWulingRegionalDevelopment→InRegionalDevelopmentValleyIV/InRegionalDevelopmentWuling),并同步更新相关业务流水线与测试用例。 - 将通用地区建设场景命中节点从
Common/Region.json移至SceneManager/Region.json,并新增InRegionalDevelopmentView2场景判定。
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/DeliveryJobs/test_region.json | 同步更新地区建设命中节点名称,保持 node test 可用 |
| assets/resource/pipeline/SellProduct/EnterOutpost.json | 更新地区建设场景命中节点引用 |
| assets/resource/pipeline/SellProduct.json | 更新地区建设场景命中节点引用 |
| assets/resource/pipeline/SceneManager/SceneRegionalDevelopment.json | 新增地区建设万能跳转私有实现(含二级界面回退、切区、进入子页面) |
| assets/resource/pipeline/SceneManager/Region.json | 新增/承载地区建设场景命中节点(含 View2)与原 Common/Region 内容迁移 |
| assets/resource/pipeline/Resell/ChangeRegion.json | 更新地区建设场景命中节点引用 |
| assets/resource/pipeline/Interface/SceneRegionalDevelopment.json | 新增对外场景接口:进入地区建设及其二级入口 |
| assets/resource/pipeline/EnvironmentMonitoring/Region.json | 更新地区建设场景命中节点引用 |
| assets/resource/pipeline/DeliveryJobs/EnterDepotNode.json | 更新地区建设场景命中节点引用 |
| assets/resource/pipeline/DeliveryJobs.json | 更新地区建设场景命中节点引用 |
| assets/resource/pipeline/Common/Region.json | 删除旧的地区建设场景命中节点定义文件(已迁移) |
| assets/resource/pipeline/Common/Location.json | 更新地区建设场景命中节点引用(用于切区锚点/流程) |
Comment on lines
+237
to
+268
| "InRegionalDevelopmentView2": { | ||
| "desc": "在地区建设二级界面", | ||
| "recognition": "OCR", | ||
| "roi": [ | ||
| 0, | ||
| 0, | ||
| 400, | ||
| 70 | ||
| ], | ||
| "expected": [ | ||
| "据点", | ||
| "據點", | ||
| "Outpost", | ||
| "拠点", | ||
| "거점", | ||
| "物资调度", | ||
| "物資調度", | ||
| "Stock Redistribution", | ||
| "商品取引", | ||
| "물자 관리", | ||
| "仓储节点", | ||
| "倉儲節點", | ||
| "Depot Node", | ||
| "保管ボックス", | ||
| "저장고 노드", | ||
| "环境监测", | ||
| "環境監測", | ||
| "Environment Monitoring", | ||
| "環境観測", | ||
| "환경 관측" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
新增了场景判定节点 InRegionalDevelopmentView2,但当前仓库对场景/识别节点有静态截图命中测试(例如 tests/DeliveryJobs/test_region.json)。建议为该新节点补一个对应的 node test case(命中/不命中都可以),用于回归防止 OCR/ROI 调整后误伤万能跳转逻辑。
Contributor
There was a problem hiding this comment.
Hey - 我在这里给了一些总体反馈:
resolveGoodsRegion中硬编码的锚点字符串现在变得相当长且具体;建议将这些锚点值集中到一个共享的常量文件或配置中,以减少拼写错误的风险,并保持它们与 pipeline 的 JSON 配置同步。- 鉴于有多个与区域相关的 pipeline 文件被删除或重命名(例如
Common/Region.json、EnvironmentMonitoring/Region.json),可能值得扫描一下其它 pipelines,查找是否还存在对旧条目的引用,以避免出现孤立或失效的导航路径。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- The hardcoded anchor strings in `resolveGoodsRegion` are now quite long and specific; consider centralizing these anchor values in a shared constants file or config to reduce the risk of typos and keep them in sync with the pipeline JSON.
- Given that several region-related pipeline files were removed or renamed (e.g., `Common/Region.json`, `EnvironmentMonitoring/Region.json`), it may be worth scanning other pipelines for any remaining references to the old entries to avoid orphaned or dead navigation paths.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据这些反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The hardcoded anchor strings in
resolveGoodsRegionare now quite long and specific; consider centralizing these anchor values in a shared constants file or config to reduce the risk of typos and keep them in sync with the pipeline JSON. - Given that several region-related pipeline files were removed or renamed (e.g.,
Common/Region.json,EnvironmentMonitoring/Region.json), it may be worth scanning other pipelines for any remaining references to the old entries to avoid orphaned or dead navigation paths.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hardcoded anchor strings in `resolveGoodsRegion` are now quite long and specific; consider centralizing these anchor values in a shared constants file or config to reduce the risk of typos and keep them in sync with the pipeline JSON.
- Given that several region-related pipeline files were removed or renamed (e.g., `Common/Region.json`, `EnvironmentMonitoring/Region.json`), it may be worth scanning other pipelines for any remaining references to the old entries to avoid orphaned or dead navigation paths.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
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.
1、万能跳转支持地区建设
2、地区建设二级菜单跳转到其他地区时支持最优路径跳转
Summary by Sourcery
支持区域发展库存与商业流程的通用导航,并使场景/流水线定义与新的区域发展菜单保持一致。
新功能:
改进:
测试:
Original summary in English
Summary by Sourcery
Support universal navigation for regional development stockpile and commerce flows and align scene/pipeline definitions with the new regional development menus.
New Features:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
支持区域发展库存与商业流程的通用导航,并使场景/流水线定义与新的区域发展菜单保持一致。
新功能:
改进:
测试:
Original summary in English
Summary by Sourcery
Support universal navigation for regional development stockpile and commerce flows and align scene/pipeline definitions with the new regional development menus.
New Features:
Enhancements:
Tests: