Conversation
delay/timeout/duration/rate_limit 等字段可填写单个整数或上下限二元数组,运行时在闭区间内均匀随机取值。 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
嘿——我发现了 2 个问题
给 AI Agent 的提示
请处理此次代码审查中的评论:
## 单独评论
### 评论 1
<location path="source/MaaFramework/Resource/PipelineDumper.cpp" line_range="70-75" />
<code_context>
return order_by_map.at(order_by);
}
+PipelineV2::JDuration dump_duration(const DurationRange& r)
+{
+ if (r.min == r.max) {
+ return r.min;
+ }
+ return std::array<int64_t, 2> { r.min, r.max };
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `dump_duration` 会将所有固定范围(`[x, x]`)重新转换为标量 `x`,因此当用户明确提供了上下界相等的范围时,`get_node_data` 和 `get_node_object` 将不再返回原始配置。
**触发条件:** 当管线包含类似 `[100, 100]` 的范围时。
**建议修复:** 保留源值是标量还是数组的信息;或者当原始输入使用范围语法时,将上下界相等的范围输出为 `[x, x]`。
</issue_to_address>
### 评论 2
<location path="source/MaaFramework/Resource/PipelineTypes.h" line_range="78-99" />
<code_context>
+ int64_t lo = 0;
+ int64_t hi = 0;
+
+ if (value.is_number()) {
+ lo = hi = value.as_long_long();
+ }
+ else if (value.is_array()) {
+ const auto& arr = value.as_array();
+ if (arr.size() != 2 || !arr[0].is_number() || !arr[1].is_number()) {
+ return false;
+ }
+ lo = arr[0].as_long_long();
+ hi = arr[1].as_long_long();
+ }
+ else {
+ return false;
+ }
+
+ if (lo > hi) {
+ return false;
+ }
+
+ min = lo;
+ max = hi;
+ return true;
+ }
+};
</code_context>
<issue_to_address>
**issue (bug_risk):** `DurationRange::from_json` 会接受所有时间字段的负值,随后 `sample_uint` 会将负数形式的动作持续时间转换为 `uint`;例如,`duration: -1` 会变成约 40 亿毫秒的控制器持续时间,而不是被拒绝。
**触发条件:** 当持续时间、延迟、速率限制或其他文档规定为无符号的字段收到负值时。
**建议修复:** 在存储范围之前,验证无符号时间字段的上下界均为非负值,并避免在未验证的情况下将有符号值转换为 `uint`。
</issue_to_address>Original comment in English
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="source/MaaFramework/Resource/PipelineDumper.cpp" line_range="70-75" />
<code_context>
return order_by_map.at(order_by);
}
+PipelineV2::JDuration dump_duration(const DurationRange& r)
+{
+ if (r.min == r.max) {
+ return r.min;
+ }
+ return std::array<int64_t, 2> { r.min, r.max };
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `dump_duration` converts every fixed range (`[x, x]`) back to the scalar `x`, so `get_node_data` and `get_node_object` no longer return the original configuration when a user explicitly supplies an equal-bound range.
**Triggers:** When a pipeline contains a range such as `[100, 100]`.
**Suggested fix:** Preserve whether the source value was scalar or an array, or dump equal-bound ranges as `[x, x]` when the original input used range syntax.
</issue_to_address>
### Comment 2
<location path="source/MaaFramework/Resource/PipelineTypes.h" line_range="78-99" />
<code_context>
+ int64_t lo = 0;
+ int64_t hi = 0;
+
+ if (value.is_number()) {
+ lo = hi = value.as_long_long();
+ }
+ else if (value.is_array()) {
+ const auto& arr = value.as_array();
+ if (arr.size() != 2 || !arr[0].is_number() || !arr[1].is_number()) {
+ return false;
+ }
+ lo = arr[0].as_long_long();
+ hi = arr[1].as_long_long();
+ }
+ else {
+ return false;
+ }
+
+ if (lo > hi) {
+ return false;
+ }
+
+ min = lo;
+ max = hi;
+ return true;
+ }
+};
</code_context>
<issue_to_address>
**issue (bug_risk):** `DurationRange::from_json` accepts negative values for every time field, and `sample_uint` then casts negative action durations to `uint`; for example, `duration: -1` becomes an approximately 4-billion-millisecond controller duration instead of being rejected.
**Triggers:** When a duration, delay, rate limit, or other field documented as unsigned receives a negative value.
**Suggested fix:** Validate non-negative bounds for unsigned time fields before storing the range, and avoid casting signed values to `uint` without validation.
</issue_to_address>| PipelineV2::JDuration dump_duration(const DurationRange& r) | ||
| { | ||
| if (r.min == r.max) { | ||
| return r.min; | ||
| } | ||
| return std::array<int64_t, 2> { r.min, r.max }; |
There was a problem hiding this comment.
issue (bug_risk): dump_duration 会将所有固定范围([x, x])重新转换为标量 x,因此当用户明确提供了上下界相等的范围时,get_node_data 和 get_node_object 将不再返回原始配置。
触发条件: 当管线包含类似 [100, 100] 的范围时。
建议修复: 保留源值是标量还是数组的信息;或者当原始输入使用范围语法时,将上下界相等的范围输出为 [x, x]。
Original comment in English
issue (bug_risk): dump_duration converts every fixed range ([x, x]) back to the scalar x, so get_node_data and get_node_object no longer return the original configuration when a user explicitly supplies an equal-bound range.
Triggers: When a pipeline contains a range such as [100, 100].
Suggested fix: Preserve whether the source value was scalar or an array, or dump equal-bound ranges as [x, x] when the original input used range syntax.
| if (value.is_number()) { | ||
| lo = hi = value.as_long_long(); | ||
| } | ||
| else if (value.is_array()) { | ||
| const auto& arr = value.as_array(); | ||
| if (arr.size() != 2 || !arr[0].is_number() || !arr[1].is_number()) { | ||
| return false; | ||
| } | ||
| lo = arr[0].as_long_long(); | ||
| hi = arr[1].as_long_long(); | ||
| } | ||
| else { | ||
| return false; | ||
| } | ||
|
|
||
| if (lo > hi) { | ||
| return false; | ||
| } | ||
|
|
||
| min = lo; | ||
| max = hi; | ||
| return true; |
There was a problem hiding this comment.
issue (bug_risk): DurationRange::from_json 会接受所有时间字段的负值,随后 sample_uint 会将负数形式的动作持续时间转换为 uint;例如,duration: -1 会变成约 40 亿毫秒的控制器持续时间,而不是被拒绝。
触发条件: 当持续时间、延迟、速率限制或其他文档规定为无符号的字段收到负值时。
建议修复: 在存储范围之前,验证无符号时间字段的上下界均为非负值,并避免在未验证的情况下将有符号值转换为 uint。
Original comment in English
issue (bug_risk): DurationRange::from_json accepts negative values for every time field, and sample_uint then casts negative action durations to uint; for example, duration: -1 becomes an approximately 4-billion-millisecond controller duration instead of being rejected.
Triggers: When a duration, delay, rate limit, or other field documented as unsigned receives a negative value.
Suggested fix: Validate non-negative bounds for unsigned time fields before storing the range, and avoid casting signed values to uint without validation.
There was a problem hiding this comment.
🟡 Changes recommended
The new DurationRange parsing/sampling introduces concrete validation and conversion issues (notably negative/overflow behavior and a NodeJS typing mismatch) that can produce incorrect runtime durations or misleading API surface.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends MaaFramework’s Pipeline protocol so most time-like fields (e.g., delay/timeout/duration/rate_limit) can be configured as either a scalar integer or an inclusive [min, max] range, with runtime uniform sampling while preserving the original config in get_node_data / get_node_object.
Changes:
- Introduce a
DurationRangerepresentation across pipeline data structures and parsing/dumping paths to preserve scalar vs range configuration. - Apply runtime sampling at execution sites (sleep, rate-limit sleep-until, wait_freezes, actuator actions like LongPress/Swipe/Shell).
- Update schema, bindings (Python/NodeJS typings), docs (ZH/EN), and add Python tests for range behavior and invalid ranges.
File summaries
| File | Description |
|---|---|
| tools/pipeline.schema.json | Adds jsonDuration* defs and updates time-field schemas/docs to allow scalar or [min,max]. |
| test/python/pipeline_test.py | Adds test_duration_range covering scalar compatibility, dump preservation, Swipe nesting semantics, and invalid ranges. |
| source/MaaFramework/Task/TaskBase.cpp | Samples delays at runtime via DurationRange::random() before sleeping. |
| source/MaaFramework/Task/PipelineTask.cpp | Samples timeout and rate_limit at runtime during recognition loops. |
| source/MaaFramework/Task/Context.cpp | Updates wait_freezes merging/validation to work with DurationRange. |
| source/MaaFramework/Task/Component/Actuator.cpp | Samples durations/holds/starting and shell timeouts at runtime before invoking controller actions. |
| source/MaaFramework/Task/Component/ActionHelper.cpp | Samples wait_freezes time/rate_limit/timeout at runtime and uses sampled values for logic + telemetry. |
| source/MaaFramework/Resource/PipelineTypesV2.h | Introduces JDuration variant to preserve scalar vs range in dumped V2 objects. |
| source/MaaFramework/Resource/PipelineTypes.h | Adds DurationRange and replaces multiple time fields with it across pipeline types. |
| source/MaaFramework/Resource/PipelineParser.cpp | Parses DurationRange for node timing + wait_freezes numeric/object forms. |
| source/MaaFramework/Resource/PipelineDumper.cpp | Dumps DurationRange back to scalar or [min,max] to preserve original config shape. |
| source/binding/Python/maa/pipeline.py | Updates Python dataclasses/typing to allow duration ranges and preserves them in parsing. |
| source/binding/NodeJS/src/apis/pipeline.d.ts | Updates TS types to allow duration ranges in pipeline configs. |
| docs/zh_cn/3.1-任务流水线协议.md | Documents [min,max] time ranges and the Swipe/MultiSwipe nesting rule (ZH). |
| docs/en_us/3.1-PipelineProtocol.md | Documents [min,max] time ranges and the Swipe/MultiSwipe nesting rule (EN). |
Review details
- Files reviewed: 14/15 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| uint sample_uint(const MAA_RES_NS::DurationRange& r) | ||
| { | ||
| return static_cast<uint>(r.random().count()); | ||
| } |
| if (!get_and_check_value(input, "rate_limit", data.rate_limit, default_value.rate_limit)) { | ||
| LogError << "failed to get_and_check_value rate_limit" << VAR(input); | ||
| return false; | ||
| } | ||
| data.rate_limit = std::chrono::milliseconds(rate_limit); | ||
|
|
||
| auto timeout = default_value.reco_timeout.count(); | ||
| if (!get_and_check_value(input, "timeout", timeout, timeout)) { | ||
| if (!get_and_check_value(input, "timeout", data.reco_timeout, default_value.reco_timeout)) { | ||
| LogError << "failed to get_and_check_value timeout" << VAR(input); | ||
| return false; | ||
| } | ||
| data.reco_timeout = std::chrono::milliseconds(timeout); | ||
|
|
||
| auto pre_delay = default_value.pre_delay.count(); | ||
| if (!get_and_check_value(input, "pre_delay", pre_delay, pre_delay)) { | ||
| if (!get_and_check_value(input, "pre_delay", data.pre_delay, default_value.pre_delay)) { | ||
| LogError << "failed to get_and_check_value pre_delay" << VAR(input); | ||
| return false; | ||
| } | ||
| data.pre_delay = std::chrono::milliseconds(pre_delay); | ||
|
|
||
| auto post_delay = default_value.post_delay.count(); | ||
| if (!get_and_check_value(input, "post_delay", post_delay, post_delay)) { | ||
| if (!get_and_check_value(input, "post_delay", data.post_delay, default_value.post_delay)) { | ||
| LogError << "failed to get_and_check_value post_delay" << VAR(input); | ||
| return false; | ||
| } |
| "jsonDuration": { | ||
| "anyOf": [ | ||
| { | ||
| "$ref": "#/$defs/jsonNInt64" | ||
| }, | ||
| { | ||
| "type": "array", | ||
| "minItems": 2, | ||
| "maxItems": 2, | ||
| "items": { | ||
| "$ref": "#/$defs/jsonNInt64" | ||
| } | ||
| } | ||
| ], | ||
| "default": 0 | ||
| }, |
Summary
[min, max]二元数组表示上下限。get_node_data/get_node_object仍返回原始配置。duration/end_hold保持[a, b]为两段滑动的旧语义,单段随机请写成[[a, b]]。Test plan
test/python/pipeline_test.py中新增的test_duration_range(标量兼容、区间 dump、Swipe 嵌套、min>max 拒绝)Made with Cursor
Sourcery 总结
在整个流水线协议中增加可配置的随机时间范围,同时保留现有的标量值和多段行为。
新功能:
[min, max]范围,并在运行时进行均匀采样。增强功能:
文档:
测试:
Original summary in English
Summary by Sourcery
Add configurable random timing ranges throughout the pipeline protocol while preserving existing scalar and multi-segment behavior.
New Features:
Enhancements:
Documentation:
Tests: