[codex] add Windows PC attach-window support to maa-cli#543
Open
Alphayellowcat wants to merge 5 commits into
Open
[codex] add Windows PC attach-window support to maa-cli#543Alphayellowcat wants to merge 5 commits into
Alphayellowcat wants to merge 5 commits into
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些整体层面的反馈：
ConnectionConfig/AttachWindowArgs里的screencap_method、mouse_method和keyboard_method字段是u64，但在maa init的交互提示中使用的是i32；为了安全性和可读性，建议在配置、模板和 MaaCore 调用之间统一这些字段的整数位宽和有符号性。- 在 Windows 的 AttachWindow 路径中，你通过 API 直接传递了原始的
u64方法 ID；建议为这些方法选择器引入轻量的 newtype 或枚举，使调用点具备自描述性，并降低交换或误用这些数值常量的风险。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- The `screencap_method`, `mouse_method`, and `keyboard_method` fields are `u64` in `ConnectionConfig`/`AttachWindowArgs` but are prompted as `i32` in `maa init`; it would be safer and clearer to align these types to a single integer width/signedness across config, template, and MaaCore calls.
- In the Windows AttachWindow path you pass raw `u64` method IDs through the API; consider introducing small newtypes or enums for these method selectors to make call sites self-documenting and reduce the risk of swapping or misusing these numeric constants.
## Individual Comments
### Comment 1
<location path="crates/maa-cli/src/run/windows.rs" line_range="7" />
<code_context>
+ Foundation::{HWND, LPARAM},
+ UI::WindowsAndMessaging::{EnumWindows, GetWindowTextLengthW, GetWindowTextW, IsWindowVisible},
+};
+use windows_sys::core::BOOL;
+
+pub(super) struct WindowMatch {
</code_context>
<issue_to_address>
**issue (bug_risk):** Importing `BOOL` from `windows_sys::core` is likely incorrect and will not compile.
In `windows-sys`, `BOOL` is defined under `windows_sys::Win32::Foundation`, not `windows_sys::core`. Since you already import `HWND` and `LPARAM` from `Foundation`, import `BOOL` there as well:
```rust
use windows_sys::Win32::Foundation::{BOOL, HWND, LPARAM};
```
Then remove the `windows_sys::core::BOOL` import, which does not exist and will cause a compile error.
</issue_to_address>
### Comment 2
<location path="crates/maa-cli/src/config/init.rs" line_range="215-224" />
<code_context>
+ x => insert!(config, "window_title" => x),
+ };
+ }
+ if let Some(screencap_method) = obj.get("screencap_method") {
+ match screencap_method.as_int().unwrap() {
+ 2 => {}
+ x => insert!(config, "screencap_method" => x),
+ };
+ }
+ if let Some(mouse_method) = obj.get("mouse_method") {
+ match mouse_method.as_int().unwrap() {
+ 32 => {}
+ x => insert!(config, "mouse_method" => x),
+ };
+ }
+ if let Some(keyboard_method) = obj.get("keyboard_method") {
+ match keyboard_method.as_int().unwrap() {
+ 2 => {}
+ x => insert!(config, "keyboard_method" => x),
+ };
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** PC input methods are typed as `u64` in config but initialized here via signed integers, which can produce invalid configs.
These fields are `Option<u64>` in `ConnectionConfig`, but the template uses `Input::<i32>` and writes `as_int()` results directly into TOML. If a user enters a negative value, the generated config will contain a negative integer and deserialization into `u64` will fail. Please either reject/guard against negative values here (e.g., bail or clamp on `x < 0`) or update the template to use an unsigned type consistent with the struct definition.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进今后的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The
screencap_method,mouse_method, andkeyboard_methodfields areu64inConnectionConfig/AttachWindowArgsbut are prompted asi32inmaa init; it would be safer and clearer to align these types to a single integer width/signedness across config, template, and MaaCore calls. - In the Windows AttachWindow path you pass raw
u64method IDs through the API; consider introducing small newtypes or enums for these method selectors to make call sites self-documenting and reduce the risk of swapping or misusing these numeric constants.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `screencap_method`, `mouse_method`, and `keyboard_method` fields are `u64` in `ConnectionConfig`/`AttachWindowArgs` but are prompted as `i32` in `maa init`; it would be safer and clearer to align these types to a single integer width/signedness across config, template, and MaaCore calls.
- In the Windows AttachWindow path you pass raw `u64` method IDs through the API; consider introducing small newtypes or enums for these method selectors to make call sites self-documenting and reduce the risk of swapping or misusing these numeric constants.
## Individual Comments
### Comment 1
<location path="crates/maa-cli/src/run/windows.rs" line_range="7" />
<code_context>
+ Foundation::{HWND, LPARAM},
+ UI::WindowsAndMessaging::{EnumWindows, GetWindowTextLengthW, GetWindowTextW, IsWindowVisible},
+};
+use windows_sys::core::BOOL;
+
+pub(super) struct WindowMatch {
</code_context>
<issue_to_address>
**issue (bug_risk):** Importing `BOOL` from `windows_sys::core` is likely incorrect and will not compile.
In `windows-sys`, `BOOL` is defined under `windows_sys::Win32::Foundation`, not `windows_sys::core`. Since you already import `HWND` and `LPARAM` from `Foundation`, import `BOOL` there as well:
```rust
use windows_sys::Win32::Foundation::{BOOL, HWND, LPARAM};
```
Then remove the `windows_sys::core::BOOL` import, which does not exist and will cause a compile error.
</issue_to_address>
### Comment 2
<location path="crates/maa-cli/src/config/init.rs" line_range="215-224" />
<code_context>
+ x => insert!(config, "window_title" => x),
+ };
+ }
+ if let Some(screencap_method) = obj.get("screencap_method") {
+ match screencap_method.as_int().unwrap() {
+ 2 => {}
+ x => insert!(config, "screencap_method" => x),
+ };
+ }
+ if let Some(mouse_method) = obj.get("mouse_method") {
+ match mouse_method.as_int().unwrap() {
+ 32 => {}
+ x => insert!(config, "mouse_method" => x),
+ };
+ }
+ if let Some(keyboard_method) = obj.get("keyboard_method") {
+ match keyboard_method.as_int().unwrap() {
+ 2 => {}
+ x => insert!(config, "keyboard_method" => x),
+ };
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** PC input methods are typed as `u64` in config but initialized here via signed integers, which can produce invalid configs.
These fields are `Option<u64>` in `ConnectionConfig`, but the template uses `Input::<i32>` and writes `as_int()` results directly into TOML. If a user enters a negative value, the generated config will contain a negative integer and deserialization into `u64` will fail. Please either reject/guard against negative values here (e.g., bail or clamp on `x < 0`) or update the template to use an unsigned type consistent with the struct definition.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| Foundation::{HWND, LPARAM}, | ||
| UI::WindowsAndMessaging::{EnumWindows, GetWindowTextLengthW, GetWindowTextW, IsWindowVisible}, | ||
| }; | ||
| use windows_sys::core::BOOL; |
There was a problem hiding this comment.
issue (bug_risk): 从 windows_sys::core 导入 BOOL 很可能是不正确的，并且会导致无法编译。
在 windows-sys 中，BOOL 定义在 windows_sys::Win32::Foundation 下，而不是 windows_sys::core。既然你已经从 Foundation 导入了 HWND 和 LPARAM，也请从这里一并导入 BOOL：
use windows_sys::Win32::Foundation::{BOOL, HWND, LPARAM};然后删除 windows_sys::core::BOOL 的导入；该符号并不存在，会导致编译错误。
Original comment in English
issue (bug_risk): Importing BOOL from windows_sys::core is likely incorrect and will not compile.
In windows-sys, BOOL is defined under windows_sys::Win32::Foundation, not windows_sys::core. Since you already import HWND and LPARAM from Foundation, import BOOL there as well:
use windows_sys::Win32::Foundation::{BOOL, HWND, LPARAM};Then remove the windows_sys::core::BOOL import, which does not exist and will cause a compile error.
Author
|
Thanks for the review.
I did not introduce dedicated enums/newtypes for the method IDs in this patch so I could keep the PR focused on landing PC AttachWindow support and the validation fix. Happy to follow up on that refactor separately if maintainers would prefer it. |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- PC 预设的默认值（窗口标题、截屏/鼠标/键盘方法）以及对非负方法的校验在
ConnectionConfig::attach_window_args和init.rs中都有重复；建议将这些常量和校验逻辑集中到一个单独的辅助函数中，以避免后续出现偏差。 - 在
run/windows.rs中，EnumWindows即使在找到完全匹配的标题后仍会继续枚举；如果实际只会使用第一个匹配项，可以在回调中在第一次精确匹配后提前返回，避免不必要的枚举开销。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- PC 预设的默认值（窗口标题、截屏/鼠标/键盘方法）以及对非负方法的校验在 `ConnectionConfig::attach_window_args` 和 `init.rs` 中都有重复；建议将这些常量和校验逻辑集中到一个单独的辅助函数中，以避免后续出现偏差。
- 在 `run/windows.rs` 中，`EnumWindows` 即使在找到完全匹配的标题后仍会继续枚举；如果实际只会使用第一个匹配项，可以在回调中在第一次精确匹配后提前返回，避免不必要的枚举开销。
## Individual Comments
### Comment 1
<location path="crates/maa-cli/src/config/init.rs" line_range="215-224" />
<code_context>
+ if let Some(screencap_method) = obj.get("screencap_method") {
</code_context>
<issue_to_address>
**suggestion:** 避免为非负的 attach-window 方法重复编写校验逻辑。
对 `screencap_method`、`mouse_method` 和 `keyboard_method` 的非负检查与 `asst.rs` 中的 `validate_attach_window_method` 重复，这在规则变更时有产生不一致的风险。建议将这部分逻辑集中（例如放在 config 模块中的一个共享辅助函数里），这样交互式初始化和运行时解析就能始终使用同一套校验规则。
建议的实现方式：
```rust
if let Some(screencap_method) = obj.get("screencap_method") {
let x = screencap_method.as_int().unwrap();
crate::config::validate_attach_window_method("connection.screencap_method", x)?;
if x != 2 {
insert!(config, "screencap_method" => x);
}
}
```
为了完整应用上述建议，并避免在代码库中重复校验逻辑，还需要：
1. 引入一个共享的辅助函数，例如放在 `crates/maa-cli/src/config/mod.rs`（或其他中心化的 config 模块）中，类似：
```rust
pub fn validate_attach_window_method(key: &str, value: i64) -> anyhow::Result<()> {
if value < 0 {
bail!("{key} must be a non-negative integer, got {value}");
}
Ok(())
}
```
2. 将 `asst.rs` 中现有的运行时校验更新为调用新的 `validate_attach_window_method`，而不是在内部重新实现非负检查，这样交互式初始化和运行时解析就能共享同一套规则。
3. 在这个 `init.rs` 文件中，同样将 `mouse_method` 和 `keyboard_method` 的非负检查替换为调用 `crate::config::validate_attach_window_method("connection.mouse_method", x)?` 和 `crate::config::validate_attach_window_method("connection.keyboard_method", x)?`，同时保留现有的默认值处理逻辑（例如这里的 `2` 的情况）。
4. 确保添加/调整必要的 `use` 语句或使用全限定路径，使所有调用点都能顺利通过编译。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据这些反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The PC preset defaults (window title, screencap/mouse/keyboard methods) and validation for non-negative methods are duplicated between
ConnectionConfig::attach_window_argsandinit.rs; consider centralizing these constants and the validation in a single helper to avoid drift. - In
run/windows.rs,EnumWindowscontinues enumerating even after finding an exact title match; if only the first match is used, you could early-return from the callback after the first exact match to avoid unnecessary enumeration work.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The PC preset defaults (window title, screencap/mouse/keyboard methods) and validation for non-negative methods are duplicated between `ConnectionConfig::attach_window_args` and `init.rs`; consider centralizing these constants and the validation in a single helper to avoid drift.
- In `run/windows.rs`, `EnumWindows` continues enumerating even after finding an exact title match; if only the first match is used, you could early-return from the callback after the first exact match to avoid unnecessary enumeration work.
## Individual Comments
### Comment 1
<location path="crates/maa-cli/src/config/init.rs" line_range="215-224" />
<code_context>
+ if let Some(screencap_method) = obj.get("screencap_method") {
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating validation logic for non-negative attach-window methods.
The non-negative checks for `screencap_method`, `mouse_method`, and `keyboard_method` duplicate `validate_attach_window_method` in `asst.rs`, which risks them diverging if the rules change. Consider centralizing this (e.g., a shared helper in the config module) so interactive init and runtime parsing always use the same validation logic.
Suggested implementation:
```rust
if let Some(screencap_method) = obj.get("screencap_method") {
let x = screencap_method.as_int().unwrap();
crate::config::validate_attach_window_method("connection.screencap_method", x)?;
if x != 2 {
insert!(config, "screencap_method" => x);
}
}
```
To fully apply your suggestion and avoid duplicated validation logic across the codebase, also:
1. Introduce a shared helper function, e.g. in `crates/maa-cli/src/config/mod.rs` (or another central config module), something like:
```rust
pub fn validate_attach_window_method(key: &str, value: i64) -> anyhow::Result<()> {
if value < 0 {
bail!("{key} must be a non-negative integer, got {value}");
}
Ok(())
}
```
2. Update the existing runtime validation in `asst.rs` to call this new `validate_attach_window_method` instead of inlining its own non-negative checks, so interactive init and runtime parsing share the same rules.
3. In this `init.rs` file, similarly replace the non-negative checks for `mouse_method` and `keyboard_method` with calls to `crate::config::validate_attach_window_method("connection.mouse_method", x)?` and `crate::config::validate_attach_window_method("connection.keyboard_method", x)?` respectively, preserving any existing default-value handling (like the `2` case here).
4. Ensure any necessary `use` statements or fully-qualified paths are added/adjusted so all call sites compile cleanly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Author
|
Follow-up in
I did not change the |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #543 +/- ##
===========================================
- Coverage 71.86% 22.45% -49.41%
===========================================
Files 69 6 -63
Lines 6412 285 -6127
Branches 6412 285 -6127
===========================================
- Hits 4608 64 -4544
+ Misses 1477 220 -1257
+ Partials 327 1 -326 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wangl-cc
reviewed
May 15, 2026
| check_interval: Option<time::Duration>, | ||
| ) -> Result<()> { | ||
| let etag_file = dest.with_added_extension("etag"); | ||
| let etag_file = etag_file_path(dest); |
Member
There was a problem hiding this comment.
These changes should be unrelated, and I think dest.with_added_extension should work well. Can you discard them?
Comment on lines
+10
to
+12
| pub(crate) const PC_SCREENCAP_METHOD_DEFAULT: i32 = 2; | ||
| pub(crate) const PC_MOUSE_METHOD_DEFAULT: i32 = 32; | ||
| pub(crate) const PC_KEYBOARD_METHOD_DEFAULT: i32 = 2; |
Member
There was a problem hiding this comment.
Could you please add references for those numbers? If possible, make them enums, similar to what we do for other types in the maa-type crate.
| use super::*; | ||
|
|
||
| const FIXTURE_JSON: &str = include_str!("../../fixtures/version/cli_version.json"); | ||
| const FIXTURE_JSON: &str = include_str!("../../../maa-version/fixtures/cli_version.json"); |
Member
There was a problem hiding this comment.
I think this is unrelated. Does the symlink not work on Windows?
| /// The match is exact and case-sensitive, following MAA GUI behavior. | ||
| /// If omitted, `明日方舟` is used by default. | ||
| #[arg(long, verbatim_doc_comment)] | ||
| pub window_title: Option<String>, |
Member
There was a problem hiding this comment.
Does this value change often? If not, consider setting it only in the configuration; there is no need to add a new CLI argument.
| )?; | ||
| } | ||
| #[cfg(not(target_os = "windows"))] | ||
| crate::config::asst::Preset::Pc => { |
Member
There was a problem hiding this comment.
Considering cfg on Pc variants ensures the variants exist only on Windows, instead of creating a branch here.
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
2 participants
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
This PR adds Windows PC-client support to
maa-cliby exposing MaaCore's AttachWindow path through the CLI runtime and profile configuration.It also updates
maa initso users can generate a PC-oriented connection profile without hand-editing the config.What changed
PCconnection preset for Windows AttachWindow modeconnection.window_title,screencap_method,mouse_method, andkeyboard_methodconnection.type = "PC"throughAsstAsyncAttachWindowinstead of ADB connectPCplatform-diff resource for this preset--window-titleoverride on the CLImaa initso it can generate a PC preset with sensible defaultsWhy
MaaCorealready exposesAsstAsyncAttachWindowon Windows, butmaa-clionly exposed ADB-style connection flows. That meant the GUI could drive the native Windows Arknights client while the CLI could not, even though the core capability already existed.This closes that feature gap and makes the PC client a first-class connection mode in
maa-cli.User impact
Users on Windows can now configure:
and run
maa-cliagainst the native PC client without going through emulator or ADB flows.Validation
cargo +1.88.0 test -p maa-cli --quietmaa.exe明日方舟window by titleNotes
PCpresetSummary by Sourcery
为 maa-cli 新增 Windows PC AttachWindow 连接支持，并通过配置、CLI 和核心 API 全流程打通。
New Features:
window_title、screencap_method、mouse_method、keyboard_method），包括一个--window-titleCLI 覆盖参数。async_attach_windowAPI，并将其集成进 maa-cli 运行时以支持 PC 连接。Enhancements:
.etag文件路径来统一逻辑，并现代化 MaaCore 用户目录初始化流程。Tests:
Original summary in English
Summary by Sourcery
Add Windows PC AttachWindow connection support to maa-cli and wire it through config, CLI, and core APIs.
New Features:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
为 maa-cli 新增 Windows PC AttachWindow 连接支持，并通过配置、CLI 和核心 API 全流程打通。
New Features:
window_title、screencap_method、mouse_method、keyboard_method），包括一个--window-titleCLI 覆盖参数。async_attach_windowAPI，并将其集成进 maa-cli 运行时以支持 PC 连接。Enhancements:
.etag文件路径来统一逻辑，并现代化 MaaCore 用户目录初始化流程。Tests:
Original summary in English
Summary by Sourcery
Add Windows PC AttachWindow connection support to maa-cli and wire it through config, CLI, and core APIs.
New Features:
Enhancements:
Tests:
新功能：
window_title、screencap_method、mouse_method和keyboard_method，并可通过--window-titleCLI 参数覆盖窗口标题。async_attach_windowAPI 进行连接。增强：
maa init和 asst 配置的 schema/示例配置，自动生成并文档化面向 PC 的连接配置以及 platform-diff 资源。.etag辅助文件路径，并现代化 MaaCore 用户目录初始化流程。测试：
Original summary in English
Summary by Sourcery
为 maa-cli 新增 Windows PC AttachWindow 连接支持，并通过配置、CLI 和核心 API 全流程打通。
New Features:
window_title、screencap_method、mouse_method、keyboard_method），包括一个--window-titleCLI 覆盖参数。async_attach_windowAPI，并将其集成进 maa-cli 运行时以支持 PC 连接。Enhancements:
.etag文件路径来统一逻辑，并现代化 MaaCore 用户目录初始化流程。Tests:
Original summary in English
Summary by Sourcery
Add Windows PC AttachWindow connection support to maa-cli and wire it through config, CLI, and core APIs.
New Features:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
为 maa-cli 新增 Windows PC AttachWindow 连接支持，并通过配置、CLI 和核心 API 全流程打通。
New Features:
window_title、screencap_method、mouse_method、keyboard_method），包括一个--window-titleCLI 覆盖参数。async_attach_windowAPI，并将其集成进 maa-cli 运行时以支持 PC 连接。Enhancements:
.etag文件路径来统一逻辑，并现代化 MaaCore 用户目录初始化流程。Tests:
Original summary in English
Summary by Sourcery
Add Windows PC AttachWindow connection support to maa-cli and wire it through config, CLI, and core APIs.
New Features:
Enhancements:
Tests: