Conversation
There was a problem hiding this comment.
Hey - 我在这里提供了一些总体反馈:
- 建议在将
alignThreshold传入moveToTargetNeuralNetworkDetect之前先进行校验(例如确保它为正数且在合理的最大值范围内),以避免调用方传入0或负数时出现意料之外的行为。 - 在新的
CharacterController文档中,你可以考虑显式展示一个Custom节点的 JSON 结构(包括custom_action和CustomActionParam的示例),这样用户就可以直接从指南中复制/粘贴一段可用的代码片段。
用于 AI Agents 的提示词
Please address the comments from this code review:
## Overall Comments
- Consider validating `alignThreshold` (e.g., ensuring it is positive and within a reasonable max) before passing it to `moveToTargetNeuralNetworkDetect` to prevent surprising behavior if the caller provides `0` or a negative value.
- In the new `CharacterController` docs, you might want to explicitly show the JSON structure of a `Custom` node (including `custom_action` and `CustomActionParam` examples) so that users can directly copy/paste a working snippet from the guide.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈来改进之后的代码评审。
Original comment in English
Hey - I've left some high level feedback:
- Consider validating
alignThreshold(e.g., ensuring it is positive and within a reasonable max) before passing it tomoveToTargetNeuralNetworkDetectto prevent surprising behavior if the caller provides0or a negative value. - In the new
CharacterControllerdocs, you might want to explicitly show the JSON structure of aCustomnode (includingcustom_actionandCustomActionParamexamples) so that users can directly copy/paste a working snippet from the guide.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider validating `alignThreshold` (e.g., ensuring it is positive and within a reasonable max) before passing it to `moveToTargetNeuralNetworkDetect` to prevent surprising behavior if the caller provides `0` or a negative value.
- In the new `CharacterController` docs, you might want to explicitly show the JSON structure of a `Custom` node (including `custom_action` and `CustomActionParam` examples) so that users can directly copy/paste a working snippet from the guide.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.
Pull request overview
This PR adds a new developer-facing reference document for the CharacterController module — which exposes Custom Action nodes for in-game character movement and view rotation — in both Chinese and English. It also updates file-path references in the scene-manager.md docs following a file reorganization (SceneManager/SceneExample.json → Interface/Example/Scene.json), adds a new example pipeline file for CharacterMoveToTargetAction, and makes align_threshold a configurable parameter (previously a hardcoded constant) in the Go service implementation.
Changes:
- New
character-controller.mddocs (zh/en) anddevelopment.mdlinks: Document the four CharacterController Custom Action nodes, their parameters, behavior tables, and example references. align_thresholdrefactor incontroller.go: MovesalignThresholdfrom a hardcoded constant to a JSON-configurable parameter with a default of120; updates two call sites inInSpace.jsonto pass it explicitly.- Example/path updates: Adds
Interface/Example/Scene.jsonandCharacterController.jsonunder the new unified example directory; updatesscene-manager.mdpath references accordingly.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
docs/zh_cn/developers/character-controller.md |
New Chinese reference doc for CharacterController nodes |
docs/en_us/developers/character-controller.md |
New English reference doc for CharacterController nodes |
docs/zh_cn/developers/development.md |
Adds link to new CharacterController reference doc |
docs/en_us/developers/development.md |
Adds link to new CharacterController reference doc |
docs/zh_cn/developers/scene-manager.md |
Updates example file path to new location |
docs/en_us/developers/scene-manager.md |
Updates example file path to new location |
assets/resource/pipeline/Interface/Example/CharacterController.json |
New example pipeline for CharacterController nodes |
assets/resource/pipeline/Interface/Example/Scene.json |
Moved/new example pipeline for SceneManager usage |
assets/resource/pipeline/ProtocolSpace/InSpace.json |
Adds explicit align_threshold: 120 to two CharacterMoveToTargetAction usages |
agent/go-service/charactercontroller/controller.go |
Makes align_threshold a JSON-configurable parameter instead of a hardcoded constant |
assets/resource/pipeline/Interface/Example/CharacterController.json
Outdated
Show resolved
Hide resolved
| } | ||
| }, | ||
| "CharacterControllerExampleMoveToTarget": { | ||
| "desc": "像目标前进示例,以协议空间内物体为例,详见ProtocolSpaceEnterSpaceSuccess" |
There was a problem hiding this comment.
The desc field references ProtocolSpaceEnterSpaceSuccess, but that node is a recognition/dispatch node that does not itself use CharacterMoveToTargetAction. The actual nodes invoking CharacterMoveToTargetAction are ProtocolSpaceTouchStartMoveToTarget and ProtocolSpaceTouchExitMoveToTarget. Pointing developers to ProtocolSpaceEnterSpaceSuccess is misleading and will not show them the relevant usage. The reference should be updated to ProtocolSpaceTouchStartMoveToTarget (or both move-to-target nodes).
| "desc": "像目标前进示例,以协议空间内物体为例,详见ProtocolSpaceEnterSpaceSuccess" | |
| "desc": "向目标前进示例,以协议空间内物体为例,详见ProtocolSpaceTouchStartMoveToTarget和ProtocolSpaceTouchExitMoveToTarget" |
| } | ||
| }, | ||
| "CharacterControllerExampleMoveToTarget": { | ||
| "desc": "像目标前进示例,以协议空间内物体为例,详见ProtocolSpaceEnterSpaceSuccess" |
There was a problem hiding this comment.
The CharacterControllerExampleMoveToTarget node is missing required example fields: action, custom_action, custom_action_param (with recognition and optionally align_threshold), and a recognition definition (which is needed for NeuralNetworkDetect). All other example nodes in this file include these fields. Providing only a desc with a cross-reference is insufficient for an example file — at minimum the action fields should be shown, consistent with the other entries.
| "desc": "像目标前进示例,以协议空间内物体为例,详见ProtocolSpaceEnterSpaceSuccess" | |
| "desc": "像目标前进示例,以协议空间内物体为例,详见ProtocolSpaceEnterSpaceSuccess", | |
| "action": "NeuralNetworkDetect", | |
| "recognition": "ProtocolSpaceEnterSpaceSuccessTarget", | |
| "custom_action": "CharacterControllerMoveToTargetAction", | |
| "custom_action_param": { | |
| "recognition": "ProtocolSpaceEnterSpaceSuccessTarget", | |
| "align_threshold": 0.9 // 可根据需要调整阈值 | |
| } |
| alignThreshold := 120 // pixels; within this range the target is considered centered horizontally | ||
| if params.AlignThreshold != nil { | ||
| alignThreshold = *params.AlignThreshold | ||
| } |
There was a problem hiding this comment.
The code does not validate that the user-supplied align_threshold is positive. Both docs state it must be a "positive integer", but if a user passes 0 or a negative value, moveToTargetNeuralNetworkDetect will behave incorrectly: with alignThreshold = 0 the conditions offsetX < 0 and offsetX > 0 fire instead of the centered-alignment check, and negative values would invert the rotation direction. A guard such as if alignThreshold <= 0 { log.Warn()…; alignThreshold = 120 } should be added after the nil-check block.
| } | |
| } | |
| if alignThreshold <= 0 { | |
| log.Warn(). | |
| Int("align_threshold", alignThreshold). | |
| Int("default_align_threshold", 120). | |
| Msg("non-positive align_threshold provided, using default") | |
| alignThreshold = 120 | |
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
## Summary by Sourcery 为角色移动和目标跟随撰写并扩展 `CharacterController` 接口文档,包括可配置的对齐行为以及参考文档。 新功能: - 为 `CharacterMoveToTargetAction` 添加可选参数 `align_threshold`,用于在朝向已识别目标移动时配置水平对齐的容差。 - 新增英文和中文的 `CharacterController` 参考文档,介绍可用的角色控制节点以及使用示例。 改进: - 更新场景管理器文档链接,使其指向新的接口示例路径,并将 `CharacterController` 文档集成到主开发者文档索引中。 <details> <summary>Original summary in English</summary> ## Summary by Sourcery Document and extend CharacterController interfaces for character movement and target-following, including configurable alignment behavior and reference docs. New Features: - Add optional align_threshold parameter to CharacterMoveToTargetAction to configure horizontal alignment tolerance when moving toward recognized targets. - Introduce English and Chinese CharacterController reference documentation describing available character control nodes and usage examples. Enhancements: - Update scene manager documentation links to point to the new interface example paths and integrate CharacterController docs into the main developer documentation index. </details> --------- Co-authored-by: MistEO <mistereo@hotmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
## Summary by Sourcery 为角色移动和目标跟随撰写并扩展 `CharacterController` 接口文档,包括可配置的对齐行为以及参考文档。 新功能: - 为 `CharacterMoveToTargetAction` 添加可选参数 `align_threshold`,用于在朝向已识别目标移动时配置水平对齐的容差。 - 新增英文和中文的 `CharacterController` 参考文档,介绍可用的角色控制节点以及使用示例。 改进: - 更新场景管理器文档链接,使其指向新的接口示例路径,并将 `CharacterController` 文档集成到主开发者文档索引中。 <details> <summary>Original summary in English</summary> ## Summary by Sourcery Document and extend CharacterController interfaces for character movement and target-following, including configurable alignment behavior and reference docs. New Features: - Add optional align_threshold parameter to CharacterMoveToTargetAction to configure horizontal alignment tolerance when moving toward recognized targets. - Introduce English and Chinese CharacterController reference documentation describing available character control nodes and usage examples. Enhancements: - Update scene manager documentation links to point to the new interface example paths and integrate CharacterController docs into the main developer documentation index. </details> --------- Co-authored-by: MistEO <mistereo@hotmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary by Sourcery
为角色移动和目标跟随撰写并扩展
CharacterController接口文档,包括可配置的对齐行为以及参考文档。新功能:
CharacterMoveToTargetAction添加可选参数align_threshold,用于在朝向已识别目标移动时配置水平对齐的容差。CharacterController参考文档,介绍可用的角色控制节点以及使用示例。改进:
CharacterController文档集成到主开发者文档索引中。Original summary in English
Summary by Sourcery
Document and extend CharacterController interfaces for character movement and target-following, including configurable alignment behavior and reference docs.
New Features:
Enhancements: