feat(context): 为 MaaContext 添加 wait_freezes 接口支持#1095
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 新的
Context::wait_freezes路径会直接使用可能为 null 的json::value调用PipelineParser::parse_wait_freezes_value（例如MaaContextWaitFreezes在wait_freezes_param == nullptr时被调用），但目前parse_wait_freezes_value会把非数字/非对象视为错误，而不是回退到提供的default_value；这会导致简单的 C API 形式在没有 JSON 对象时无法使用，应当要么对 null 做特殊处理，将其视为“使用default_value”，要么走类似parse_wait_freezes_param的逻辑。 Context::wait_freezes中显式time参数与解析出的param.time之间的互斥检查在默认值场景下会产生不良交互：当调用方传入非零的time但在wait_freezes_param中省略time时，parse_wait_freezes_value通常会从默认 pipeline（非零）中填充param.time，从而触发“二者都非零”的错误；可以考虑要么让显式time无条件覆盖param.time且不视为错误，要么调整解析/合并逻辑，使在外部提供了time参数时不应用默认的time。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new Context::wait_freezes path calls PipelineParser::parse_wait_freezes_value directly with a json::value that may be null (e.g., MaaContextWaitFreezes called with wait_freezes_param == nullptr), but parse_wait_freezes_value currently treats non-number/non-object as an error instead of falling back to the provided default_value; this makes the simple C API form unusable without a JSON object and should either special-case null to mean 'use default_value' or route through parse_wait_freezes_param-like logic.
- The mutual-exclusion check between the explicit time argument and parsed param.time in Context::wait_freezes can interact badly with defaults: when a caller passes a non-zero time but omits time in wait_freezes_param, parse_wait_freezes_value will usually populate param.time from the default pipeline (non-zero), causing the 'both non-zero' error; consider either letting the explicit time unconditionally override param.time without treating this as an error, or adjusting the parser/merge logic so that default time is not applied when an external time argument is provided.
## Individual Comments
### Comment 1
<location> `source/Common/MaaContext.cpp:201-210` </location>
<code_context>
return context->run_action_direct(action_type, *param_opt, cv_box, reco_detail);
}
+MaaBool MaaContextWaitFreezes(MaaContext* context, MaaSize time, const MaaRect* roi, const char* wait_freezes_param)
+{
+ LogFunc << VAR_VOIDP(context) << VAR(time) << VAR(wait_freezes_param);
+
+ if (!context) {
+ LogError << "handle is null";
+ return false;
+ }
+
+ json::value param_json;
+ if (wait_freezes_param) {
+ auto param_opt = json::parse(wait_freezes_param);
+ if (!param_opt) {
+ LogError << "failed to parse wait_freezes_param" << VAR(wait_freezes_param);
+ return false;
+ }
+ param_json = std::move(*param_opt);
+ }
+
+ cv::Rect cv_roi {};
+ if (roi) {
+ cv_roi.x = roi->x;
+ cv_roi.y = roi->y;
+ cv_roi.width = roi->width;
+ cv_roi.height = roi->height;
+ }
+
+ return context->wait_freezes(std::chrono::milliseconds(time), cv_roi, param_json);
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Calling MaaContextWaitFreezes without wait_freezes_param will always fail due to null JSON not being accepted by parse_wait_freezes_value.
Because `param_json` stays default-constructed when `wait_freezes_param` is `nullptr`, `Context::wait_freezes` ends up calling `parse_wait_freezes_value` with a `null` JSON value. The new parser rejects non-number, non-object inputs, logs `"invalid wait_freezes_param"`, and returns `false`, so `MaaContextWaitFreezes(..., nullptr)` will always fail even when defaults are available, despite the C API implying the parameter is optional.
To preserve the existing API behavior, either:
- Treat a `null`/empty `json::value` as "use defaults" inside `Context::wait_freezes`, or
- Initialize `param_json` to an empty object when `wait_freezes_param` is null, so the parser can fall back to defaults.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据这些反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The new Context::wait_freezes path calls PipelineParser::parse_wait_freezes_value directly with a json::value that may be null (e.g., MaaContextWaitFreezes called with wait_freezes_param == nullptr), but parse_wait_freezes_value currently treats non-number/non-object as an error instead of falling back to the provided default_value; this makes the simple C API form unusable without a JSON object and should either special-case null to mean 'use default_value' or route through parse_wait_freezes_param-like logic.
- The mutual-exclusion check between the explicit time argument and parsed param.time in Context::wait_freezes can interact badly with defaults: when a caller passes a non-zero time but omits time in wait_freezes_param, parse_wait_freezes_value will usually populate param.time from the default pipeline (non-zero), causing the 'both non-zero' error; consider either letting the explicit time unconditionally override param.time without treating this as an error, or adjusting the parser/merge logic so that default time is not applied when an external time argument is provided.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new Context::wait_freezes path calls PipelineParser::parse_wait_freezes_value directly with a json::value that may be null (e.g., MaaContextWaitFreezes called with wait_freezes_param == nullptr), but parse_wait_freezes_value currently treats non-number/non-object as an error instead of falling back to the provided default_value; this makes the simple C API form unusable without a JSON object and should either special-case null to mean 'use default_value' or route through parse_wait_freezes_param-like logic.
- The mutual-exclusion check between the explicit time argument and parsed param.time in Context::wait_freezes can interact badly with defaults: when a caller passes a non-zero time but omits time in wait_freezes_param, parse_wait_freezes_value will usually populate param.time from the default pipeline (non-zero), causing the 'both non-zero' error; consider either letting the explicit time unconditionally override param.time without treating this as an error, or adjusting the parser/merge logic so that default time is not applied when an external time argument is provided.
## Individual Comments
### Comment 1
<location> `source/Common/MaaContext.cpp:201-210` </location>
<code_context>
return context->run_action_direct(action_type, *param_opt, cv_box, reco_detail);
}
+MaaBool MaaContextWaitFreezes(MaaContext* context, MaaSize time, const MaaRect* roi, const char* wait_freezes_param)
+{
+ LogFunc << VAR_VOIDP(context) << VAR(time) << VAR(wait_freezes_param);
+
+ if (!context) {
+ LogError << "handle is null";
+ return false;
+ }
+
+ json::value param_json;
+ if (wait_freezes_param) {
+ auto param_opt = json::parse(wait_freezes_param);
+ if (!param_opt) {
+ LogError << "failed to parse wait_freezes_param" << VAR(wait_freezes_param);
+ return false;
+ }
+ param_json = std::move(*param_opt);
+ }
+
+ cv::Rect cv_roi {};
+ if (roi) {
+ cv_roi.x = roi->x;
+ cv_roi.y = roi->y;
+ cv_roi.width = roi->width;
+ cv_roi.height = roi->height;
+ }
+
+ return context->wait_freezes(std::chrono::milliseconds(time), cv_roi, param_json);
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Calling MaaContextWaitFreezes without wait_freezes_param will always fail due to null JSON not being accepted by parse_wait_freezes_value.
Because `param_json` stays default-constructed when `wait_freezes_param` is `nullptr`, `Context::wait_freezes` ends up calling `parse_wait_freezes_value` with a `null` JSON value. The new parser rejects non-number, non-object inputs, logs `"invalid wait_freezes_param"`, and returns `false`, so `MaaContextWaitFreezes(..., nullptr)` will always fail even when defaults are available, despite the C API implying the parameter is optional.
To preserve the existing API behavior, either:
- Treat a `null`/empty `json::value` as "use defaults" inside `Context::wait_freezes`, or
- Initialize `param_json` to an empty object when `wait_freezes_param` is null, so the parser can fall back to defaults.
</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
This PR adds a new wait_freezes method to the MaaContext API, allowing users to wait for screen stabilization at the context level. The implementation includes comprehensive cross-platform support through C API, Python bindings, and agent client/server protocol. The PR also refactors existing freeze-waiting logic into a reusable ActionHelper component and extends the PipelineParser to handle both numeric and object-based wait_freezes parameters.
Changes:
- Added Context-level
wait_freezesAPI with validation for mutually exclusive time and ROI parameters - Refactored freeze-waiting and target ROI calculation logic from Actuator into a new reusable ActionHelper component
- Extended PipelineParser with
parse_wait_freezes_valueto support both simple numeric and advanced object parameter formats
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/python/binding_test.py | Adds test validating parameter validation when both time values are zero |
| test/agent/agent_child_test.py | Adds identical agent test for wait_freezes parameter validation |
| source/modules/MaaFramework.cppm | Exports MaaContextWaitFreezes for C++ module consumers |
| source/include/MaaAgent/Message.hpp | Defines ContextWaitFreezesReverseRequest/Response message structures |
| source/include/Common/MaaTypes.h | Adds wait_freezes virtual method to MaaContext interface |
| source/binding/Python/maa/context.py | Implements Python binding for wait_freezes with JWaitFreezes dataclass support |
| source/MaaFramework/Task/Context.h | Declares wait_freezes method and reorganizes method declarations |
| source/MaaFramework/Task/Context.cpp | Implements wait_freezes with comprehensive parameter validation and merging logic |
| source/MaaFramework/Task/Component/Actuator.h | Changes get_target_rect signature to accept const reference |
| source/MaaFramework/Task/Component/Actuator.cpp | Refactors to delegate to ActionHelper for freeze-waiting and target rect calculations |
| source/MaaFramework/Task/Component/ActionHelper.h | Defines new ActionHelper class with wait_freezes and get_target_rect methods |
| source/MaaFramework/Task/Component/ActionHelper.cpp | Implements ActionHelper with logic extracted from Actuator, returning bool for success/failure |
| source/MaaFramework/Resource/PipelineParser.h | Declares parse_wait_freezes_value for parsing wait_freezes from JSON |
| source/MaaFramework/Resource/PipelineParser.cpp | Refactors parsing logic into parse_wait_freezes_value to handle number and object formats |
| source/MaaAgentServer/RemoteInstance/RemoteContext.h | Declares wait_freezes override for remote context |
| source/MaaAgentServer/RemoteInstance/RemoteContext.cpp | Implements wait_freezes by sending request through agent protocol |
| source/MaaAgentClient/Client/AgentClient.h | Declares handle_context_wait_freezes handler |
| source/MaaAgentClient/Client/AgentClient.cpp | Implements handler to process wait_freezes requests from agent server |
| source/Common/MaaContext.cpp | Implements C API wrapper MaaContextWaitFreezes with JSON parsing |
| include/MaaFramework/Instance/MaaContext.h | Declares C API function MaaContextWaitFreezes |
| docs/en_us/2.2-IntegratedInterfaceOverview.md | Documents MaaContextWaitFreezes parameters and usage constraints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| ActionHelper helper(tasker_); | ||
| cv::Rect roi = helper.get_target_rect(param.target, box); | ||
| helper.wait_freezes(param, roi); |
There was a problem hiding this comment.
The ActionHelper::wait_freezes method now returns bool indicating success/failure, which is a behavior change from the original Actuator::wait_freezes that returned void. This is an improvement, but the Actuator::wait_freezes method that wraps it ignores the return value, potentially silencing errors. Consider handling the return value appropriately, such as logging a warning when wait_freezes fails.
Suggested change
| helper.wait_freezes(param, roi); | |
| bool ret = helper.wait_freezes(param, roi); | |
| if (!ret) { | |
| LogWarn << "wait_freezes failed" << VAR(roi); | |
| } |
Comment on lines
+846
to
+847
| Wait for screen to stabilize. `time` and `wait_freezes_param.time` are mutually exclusive: both cannot be non-zero, and both cannot be zero. `roi` and `wait_freezes_param.target` are mutually exclusive: cannot specify both. | ||
|
|
There was a problem hiding this comment.
The documentation states "both cannot be non-zero, and both cannot be zero" but doesn't clearly explain what happens when one is zero and the other is non-zero, or what the expected behavior is. Consider adding an example showing the correct usage patterns, such as "Pass time=1000 with wait_freezes_param=None for simple usage" or "Pass time=0 with wait_freezes_param=JWaitFreezes(time=1000) for advanced configuration".
Suggested change
| Wait for screen to stabilize. `time` and `wait_freezes_param.time` are mutually exclusive: both cannot be non-zero, and both cannot be zero. `roi` and `wait_freezes_param.target` are mutually exclusive: cannot specify both. | |
| Wait for screen to stabilize. | |
| `time` and `wait_freezes_param.time` are mutually exclusive: exactly one of them must be non-zero. | |
| - Simple usage: pass a non-zero `time` (for example, `time = 1000`) and omit `wait_freezes_param` or set it to `NULL`/`None`. The whole screen (or the specified `roi`) is watched for about 1 second of stability. | |
| - Advanced usage: pass `time = 0` and provide `wait_freezes_param` with a non-zero `time` (for example, `wait_freezes_param = { "time": 1000, ... }`) to further configure `target`, `threshold`, and other options. | |
| If both `time` and `wait_freezes_param.time` are non-zero, or if both are zero, the call is invalid and the behavior is undefined (it may be rejected by the implementation). | |
| `roi` and `wait_freezes_param.target` are also mutually exclusive: specify at most one of them. |
| @@ -56,6 +56,8 @@ extern "C" | |||
| const MaaRect* box, | |||
| const char* reco_detail); | |||
|
|
|||
There was a problem hiding this comment.
The MaaContextWaitFreezes function declaration lacks documentation comments, while other similar functions in this file (like MaaContextRunRecognitionDirect and MaaContextRunActionDirect) have detailed documentation comments. Consider adding a documentation comment describing the function's purpose, parameters, and return value for consistency.
Suggested change
| /** | |
| * @brief Wait until the content within a region of interest appears frozen or until a timeout. | |
| * | |
| * This function monitors the current context and waits for the screen (or image source) | |
| * to become stable, optionally within a specified region of interest (ROI). The waiting | |
| * behavior can be further configured through the @p wait_freezes_param argument. | |
| * | |
| * @param context Context in which the wait operation is performed. | |
| * @param time Maximum time to wait before giving up (timeout). | |
| * @param roi Optional region of interest; if null, the entire area is used. | |
| * @param wait_freezes_param Parameters for the wait operation, typically in JSON or configuration string form. | |
| * | |
| * @return MaaBool indicating whether the wait operation succeeded according to the configured criteria. | |
| */ |
Comment on lines
+154
to
+160
| # 测试 wait_freezes API（参数校验：time 和 wait_freezes_param.time 同时为零应返回 false） | ||
| from maa.pipeline import JWaitFreezes | ||
| wait_result = new_ctx.wait_freezes(time=0, wait_freezes_param=JWaitFreezes(time=0)) | ||
| print(f" wait_freezes (both zero): {wait_result}") | ||
| assert ( | ||
| not wait_result | ||
| ), "wait_freezes should return false when both time are zero" |
There was a problem hiding this comment.
The test code is identical in both test files (Python binding test and agent test). While this ensures consistency, if the test logic needs to be updated, both files will need to be changed. Consider extracting the test into a shared test helper function or a common test module to reduce duplication and maintenance burden.
moomiji
added a commit
to MaaXYZ/MaaFramework.Binding.CSharp
that referenced
this pull request
Feb 14, 2026
moomiji
added a commit
to MaaXYZ/MaaFramework.Binding.CSharp
that referenced
this pull request
Feb 14, 2026
moomiji
added a commit
to MaaXYZ/MaaFramework.Binding.CSharp
that referenced
this pull request
Feb 14, 2026
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
1 participant
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
在上下文级别新增
wait_freezes接口，用于在可配置的时间与 ROI 下等待画面稳定，并通过核心层、各语言绑定以及远程代理层对外提供该能力。New Features:
MaaContext::wait_freezes方法，通过时间与 ROI 或流水线参数来等待屏幕指定区域稳定。wait_freezes能力。Enhancements:
ActionHelper组件，并更新Actuator以使用该组件。PipelineParser，支持从数值型或对象型 JSON 中解析wait_freezes参数，以实现更灵活的配置方式。Documentation:
MaaContextWaitFreezes接口及其参数说明。Tests:
wait_freezes参数的校验行为。Original summary in English
Summary by Sourcery
Add a context-level wait_freezes API to wait for screen stabilization with configurable timing and ROI, and expose it through core, bindings, and remote agent layers.
New Features:
Enhancements:
Documentation:
Tests:
新功能：
wait_freezesAPI，用于在屏幕稳定后继续执行，并支持可配置的时间与 ROI（感兴趣区域）参数。wait_freezes功能。增强：
ActionHelper组件中，并更新Actuator以使用该组件。PipelineParser，使其支持从数值型或对象型 JSON 格式中解析wait_freezes参数。文档：
MaaContextWaitFreezes接口及其参数。测试：
wait_freezes参数校验的行为。Original summary in English
Summary by Sourcery
在上下文级别新增
wait_freezes接口，用于在可配置的时间与 ROI 下等待画面稳定，并通过核心层、各语言绑定以及远程代理层对外提供该能力。New Features:
MaaContext::wait_freezes方法，通过时间与 ROI 或流水线参数来等待屏幕指定区域稳定。wait_freezes能力。Enhancements:
ActionHelper组件，并更新Actuator以使用该组件。PipelineParser，支持从数值型或对象型 JSON 中解析wait_freezes参数，以实现更灵活的配置方式。Documentation:
MaaContextWaitFreezes接口及其参数说明。Tests:
wait_freezes参数的校验行为。Original summary in English
Summary by Sourcery
Add a context-level wait_freezes API to wait for screen stabilization with configurable timing and ROI, and expose it through core, bindings, and remote agent layers.
New Features:
Enhancements:
Documentation:
Tests: