feat: 重构MaaToolkitDesktopWindow相关API #1228
Draft
YJBeetle wants to merge 3 commits into
Draft
Conversation
本提交将会在API层面发生一些变动 本提交旨在提升相关的API的通用性和可扩展性，同时更语意化。 以下为具体变更： 将DesktopWindow划分为通用字段和平台专有用段。 原有的class_name是win专有，现API层已冠以平台前缀。 GetHandle返回值类型从指针换成uint64整数以适应多种平台，同时保证了不同语言binding的一致性。 添加了MacOS的专有字段 Linux的专有字段进行重命名
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
你好——我发现了 1 个问题，并给出了一些整体层面的反馈：
- 类似
win32_class_name(),macos_pid()和linux_socket_path()这样的特定平台访问器，在其他平台上被调用时会记录"Only available on ..."日志，但更高层的代码（Python 绑定、NodeJS 控制器）现在会无条件调用所有这些函数；建议要么在不支持的平台上移除这些日志，要么在绑定层按平台做调用控制，以避免运行时出现大量噪声错误输出。 - 在将
MaaToolkitDesktopWindowGetHandle的返回值改为uint64_t之后，若干调用点现在使用reinterpret_cast<void*>(MaaToolkitDesktopWindowGetHandle(...))；为了更好的可移植性和可读性，建议统一通过uintptr_t做中间转换（例如reinterpret_cast<void*>(static_cast<uintptr_t>(...))），并且/或者增加一个静态断言sizeof(void*) <= sizeof(uint64_t)，把依赖的前提条件显式表达出来。
供 AI 代理使用的提示
Please address the comments from this code review:
## Overall Comments
- Platform-specific accessors like `win32_class_name()`, `macos_pid()`, and `linux_socket_path()` log `"Only available on ..."` when called on other platforms, but higher-level code (Python bindings, NodeJS controllers) now calls all of them unconditionally; consider either removing these logs on unsupported platforms or gating the binding calls by platform to avoid noisy error output at runtime.
- After changing `MaaToolkitDesktopWindowGetHandle` to return `uint64_t`, several call sites now do `reinterpret_cast<void*>(MaaToolkitDesktopWindowGetHandle(...))`; for better portability and clarity, consider consistently casting through `uintptr_t` (e.g., `reinterpret_cast<void*>(static_cast<uintptr_t>(...))`) and/or adding a static assertion that `sizeof(void*) <= sizeof(uint64_t)` to make the intended assumptions explicit.
## Individual Comments
### Comment 1
<location path="source/binding/Python/maa/toolkit.py" line_range="184-193" />
<code_context>
.MaaToolkitDesktopWindowGetWindowName(window_handle)
.decode()
)
+ win32_class_name = (
+ Library.toolkit()
+ .MaaToolkitDesktopWindowGetWin32ClassName(window_handle)
+ .decode()
+ )
+ macos_pid = int(
+ Library.toolkit().MaaToolkitDesktopWindowGetMacOSPID(window_handle)
+ )
+ macos_bundle_id = (
+ Library.toolkit()
+ .MaaToolkitDesktopWindowGetMacOSBundleID(window_handle)
+ .decode()
+ )
+ macos_application_name = (
+ Library.toolkit()
+ .MaaToolkitDesktopWindowGetMacOSApplicationName(window_handle)
+ .decode()
+ )
+ linux_socket_path = (
+ Library.toolkit()
+ .MaaToolkitDesktopWindowGetLinuxSocketPath(window_handle)
+ .decode()
+ )
</code_context>
<issue_to_address>
**建议：** 无条件调用特定平台的 getter 可能会在其他平台上产生大量日志噪声。
这些访问器会针对每一个窗口被调用，即使当前平台并不支持它们。在这些平台上，C++ 层会记录一条错误日志并返回一个默认值，因此 `find_desktop_windows()` 会为每个窗口、针对这些本身与当前平台无关的字段生成一条错误日志。
建议要么使用 `sys.platform` 对这些调用做条件判断，只执行当前平台适用的 getter，要么调整 C++ 访问器，使其在不支持的平台上只返回默认值而不记录错误日志（或者仅在调试模式下记录）。这样可以避免在正常使用中出现误导性的错误噪声。
建议的实现方式：
```python
handle = int(
Library.toolkit().MaaToolkitDesktopWindowGetHandle(window_handle)
)
window_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetWindowName(window_handle)
.decode()
)
if sys.platform.startswith("win"):
win32_class_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetWin32ClassName(window_handle)
.decode()
)
else:
win32_class_name = ""
if sys.platform == "darwin":
macos_pid = int(
Library.toolkit().MaaToolkitDesktopWindowGetMacOSPID(window_handle)
)
macos_bundle_id = (
Library.toolkit().MaaToolkitDesktopWindowGetMacOSBundleID(window_handle)
.decode()
)
macos_application_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetMacOSApplicationName(window_handle)
.decode()
)
else:
macos_pid = 0
macos_bundle_id = ""
macos_application_name = ""
if sys.platform.startswith("linux"):
linux_socket_path = (
Library.toolkit()
.MaaToolkitDesktopWindowGetLinuxSocketPath(window_handle)
.decode()
)
else:
linux_socket_path = ""
handle = int(
Library.toolkit().MaaToolkitDesktopWindowGetHandle(window_handle)
)
window_name = (
Library.toolkit()
```
1. 确保在 `source/binding/Python/maa/toolkit.py` 文件顶部（与其他标准库导入放在一起）添加了 `import sys`。
2. 如果这些新变量（`win32_class_name`, `macos_pid`, `macos_bundle_id`, `macos_application_name`, `linux_socket_path`）稍后会被用于构建字典或对象，请确认默认值（`""` 或 `0`）与该结构期望的 schema/类型一致，并根据需要进行调整（例如，如果其他代码期望可空字段，可以改用 `None`）。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Platform-specific accessors like
win32_class_name(),macos_pid(), andlinux_socket_path()log"Only available on ..."when called on other platforms, but higher-level code (Python bindings, NodeJS controllers) now calls all of them unconditionally; consider either removing these logs on unsupported platforms or gating the binding calls by platform to avoid noisy error output at runtime. - After changing
MaaToolkitDesktopWindowGetHandleto returnuint64_t, several call sites now doreinterpret_cast<void*>(MaaToolkitDesktopWindowGetHandle(...)); for better portability and clarity, consider consistently casting throughuintptr_t(e.g.,reinterpret_cast<void*>(static_cast<uintptr_t>(...))) and/or adding a static assertion thatsizeof(void*) <= sizeof(uint64_t)to make the intended assumptions explicit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Platform-specific accessors like `win32_class_name()`, `macos_pid()`, and `linux_socket_path()` log `"Only available on ..."` when called on other platforms, but higher-level code (Python bindings, NodeJS controllers) now calls all of them unconditionally; consider either removing these logs on unsupported platforms or gating the binding calls by platform to avoid noisy error output at runtime.
- After changing `MaaToolkitDesktopWindowGetHandle` to return `uint64_t`, several call sites now do `reinterpret_cast<void*>(MaaToolkitDesktopWindowGetHandle(...))`; for better portability and clarity, consider consistently casting through `uintptr_t` (e.g., `reinterpret_cast<void*>(static_cast<uintptr_t>(...))`) and/or adding a static assertion that `sizeof(void*) <= sizeof(uint64_t)` to make the intended assumptions explicit.
## Individual Comments
### Comment 1
<location path="source/binding/Python/maa/toolkit.py" line_range="184-193" />
<code_context>
.MaaToolkitDesktopWindowGetWindowName(window_handle)
.decode()
)
+ win32_class_name = (
+ Library.toolkit()
+ .MaaToolkitDesktopWindowGetWin32ClassName(window_handle)
+ .decode()
+ )
+ macos_pid = int(
+ Library.toolkit().MaaToolkitDesktopWindowGetMacOSPID(window_handle)
+ )
+ macos_bundle_id = (
+ Library.toolkit()
+ .MaaToolkitDesktopWindowGetMacOSBundleID(window_handle)
+ .decode()
+ )
+ macos_application_name = (
+ Library.toolkit()
+ .MaaToolkitDesktopWindowGetMacOSApplicationName(window_handle)
+ .decode()
+ )
+ linux_socket_path = (
+ Library.toolkit()
+ .MaaToolkitDesktopWindowGetLinuxSocketPath(window_handle)
+ .decode()
+ )
</code_context>
<issue_to_address>
**suggestion:** Unconditional calls to platform-specific getters may cause noisy logging on other platforms.
These accessors are invoked for every window, even when the current platform doesn’t support them. On those platforms the C++ layer logs an error and returns a default, so `find_desktop_windows()` will produce an error log per window for fields that are inherently irrelevant.
Consider either guarding these calls with `sys.platform` so only platform-appropriate getters run, or adjusting the C++ accessors so unsupported-platform calls return defaults without error logging (or only log in debug). This would avoid misleading error noise in normal usage.
Suggested implementation:
```python
handle = int(
Library.toolkit().MaaToolkitDesktopWindowGetHandle(window_handle)
)
window_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetWindowName(window_handle)
.decode()
)
if sys.platform.startswith("win"):
win32_class_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetWin32ClassName(window_handle)
.decode()
)
else:
win32_class_name = ""
if sys.platform == "darwin":
macos_pid = int(
Library.toolkit().MaaToolkitDesktopWindowGetMacOSPID(window_handle)
)
macos_bundle_id = (
Library.toolkit()
.MaaToolkitDesktopWindowGetMacOSBundleID(window_handle)
.decode()
)
macos_application_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetMacOSApplicationName(window_handle)
.decode()
)
else:
macos_pid = 0
macos_bundle_id = ""
macos_application_name = ""
if sys.platform.startswith("linux"):
linux_socket_path = (
Library.toolkit()
.MaaToolkitDesktopWindowGetLinuxSocketPath(window_handle)
.decode()
)
else:
linux_socket_path = ""
handle = int(
Library.toolkit().MaaToolkitDesktopWindowGetHandle(window_handle)
)
window_name = (
Library.toolkit()
```
1. Ensure `import sys` is present near the top of `source/binding/Python/maa/toolkit.py` (alongside the other standard-library imports).
2. If these new variables (`win32_class_name`, `macos_pid`, `macos_bundle_id`, `macos_application_name`, `linux_socket_path`) are later used to build a dict or object, confirm that the default values (`""` or `0`) align with the expected schema/typing for that structure, and adjust as needed (e.g., use `None` instead if the rest of the code expects nullable fields).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+184
to
+193
| win32_class_name = ( | ||
| Library.toolkit() | ||
| .MaaToolkitDesktopWindowGetWin32ClassName(window_handle) | ||
| .decode() | ||
| ) | ||
| macos_pid = int( | ||
| Library.toolkit().MaaToolkitDesktopWindowGetMacOSPID(window_handle) | ||
| ) | ||
| macos_bundle_id = ( | ||
| Library.toolkit() |
Contributor
There was a problem hiding this comment.
建议： 无条件调用特定平台的 getter 可能会在其他平台上产生大量日志噪声。
这些访问器会针对每一个窗口被调用，即使当前平台并不支持它们。在这些平台上，C++ 层会记录一条错误日志并返回一个默认值，因此 find_desktop_windows() 会为每个窗口、针对这些本身与当前平台无关的字段生成一条错误日志。
建议要么使用 sys.platform 对这些调用做条件判断，只执行当前平台适用的 getter，要么调整 C++ 访问器，使其在不支持的平台上只返回默认值而不记录错误日志（或者仅在调试模式下记录）。这样可以避免在正常使用中出现误导性的错误噪声。
建议的实现方式：
handle = int(
Library.toolkit().MaaToolkitDesktopWindowGetHandle(window_handle)
)
window_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetWindowName(window_handle)
.decode()
)
if sys.platform.startswith("win"):
win32_class_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetWin32ClassName(window_handle)
.decode()
)
else:
win32_class_name = ""
if sys.platform == "darwin":
macos_pid = int(
Library.toolkit().MaaToolkitDesktopWindowGetMacOSPID(window_handle)
)
macos_bundle_id = (
Library.toolkit().MaaToolkitDesktopWindowGetMacOSBundleID(window_handle)
.decode()
)
macos_application_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetMacOSApplicationName(window_handle)
.decode()
)
else:
macos_pid = 0
macos_bundle_id = ""
macos_application_name = ""
if sys.platform.startswith("linux"):
linux_socket_path = (
Library.toolkit()
.MaaToolkitDesktopWindowGetLinuxSocketPath(window_handle)
.decode()
)
else:
linux_socket_path = ""
handle = int(
Library.toolkit().MaaToolkitDesktopWindowGetHandle(window_handle)
)
window_name = (
Library.toolkit()- 确保在
source/binding/Python/maa/toolkit.py文件顶部（与其他标准库导入放在一起）添加了import sys。 - 如果这些新变量（
win32_class_name,macos_pid,macos_bundle_id,macos_application_name,linux_socket_path）稍后会被用于构建字典或对象，请确认默认值（""或0）与该结构期望的 schema/类型一致，并根据需要进行调整（例如，如果其他代码期望可空字段，可以改用None）。
Original comment in English
suggestion: Unconditional calls to platform-specific getters may cause noisy logging on other platforms.
These accessors are invoked for every window, even when the current platform doesn’t support them. On those platforms the C++ layer logs an error and returns a default, so find_desktop_windows() will produce an error log per window for fields that are inherently irrelevant.
Consider either guarding these calls with sys.platform so only platform-appropriate getters run, or adjusting the C++ accessors so unsupported-platform calls return defaults without error logging (or only log in debug). This would avoid misleading error noise in normal usage.
Suggested implementation:
handle = int(
Library.toolkit().MaaToolkitDesktopWindowGetHandle(window_handle)
)
window_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetWindowName(window_handle)
.decode()
)
if sys.platform.startswith("win"):
win32_class_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetWin32ClassName(window_handle)
.decode()
)
else:
win32_class_name = ""
if sys.platform == "darwin":
macos_pid = int(
Library.toolkit().MaaToolkitDesktopWindowGetMacOSPID(window_handle)
)
macos_bundle_id = (
Library.toolkit()
.MaaToolkitDesktopWindowGetMacOSBundleID(window_handle)
.decode()
)
macos_application_name = (
Library.toolkit()
.MaaToolkitDesktopWindowGetMacOSApplicationName(window_handle)
.decode()
)
else:
macos_pid = 0
macos_bundle_id = ""
macos_application_name = ""
if sys.platform.startswith("linux"):
linux_socket_path = (
Library.toolkit()
.MaaToolkitDesktopWindowGetLinuxSocketPath(window_handle)
.decode()
)
else:
linux_socket_path = ""
handle = int(
Library.toolkit().MaaToolkitDesktopWindowGetHandle(window_handle)
)
window_name = (
Library.toolkit()- Ensure
import sysis present near the top ofsource/binding/Python/maa/toolkit.py(alongside the other standard-library imports). - If these new variables (
win32_class_name,macos_pid,macos_bundle_id,macos_application_name,linux_socket_path) are later used to build a dict or object, confirm that the default values (""or0) align with the expected schema/typing for that structure, and adjust as needed (e.g., useNoneinstead if the rest of the code expects nullable fields).
Member
|
有 breaking changes，不太行，下游要遭殃了（ 我觉得直接用同一个接口凑合凑合得了。也可以降低集成方心智，不用给各平台分别写一套代码 |
Contributor
Author
确实 所以我开pr主要是想作为一个提案来讨论下 |
Contributor
Author
|
OBS 上一般是长这样： 以及苹果上有一堆windowLayer不为0的窗口，直接没有WindowName |
Member
这样感觉可以 |
Contributor
Author
行 那我去revert一部分然后再考虑下一些细节 |
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.
继续这里的讨论 #1115
这里开PR主要是想作为一个可能的提案来讨论下
本提交将会在API层面发生一些变动
本提交旨在提升相关的API的通用性和可扩展性，同时更语意化。
以下为具体变更：
将DesktopWindow划分为通用字段和平台专有用段。
原有的class_name是win专有，现API层已冠以平台前缀。
GetHandle返回值类型从指针换成uint64整数以适应多种平台，同时保证了不同语言binding的一致性。 添加了MacOS的专有字段
Linux的专有字段进行重命名
Summary by Sourcery
重构跨平台的 DesktopWindow API，引入与平台无关的句柄和窗口名称访问器以及平台特定字段，并更新绑定、CLI 工具、示例和测试以使用新的接口。
新特性：
改进：
uint64表示，以改进跨语言绑定的一致性。Original summary in English
Summary by Sourcery
Refactor the cross-platform DesktopWindow API to introduce platform-agnostic handle and window name accessors plus platform-specific fields, and update bindings, CLI tools, samples, and tests to use the new interface.
New Features:
Enhancements: