feat: add macOS PlayCover/MaaTools support - #1720
Draft
WenSilvia wants to merge 1 commit into
Draft
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了两个问题，并留下了一些整体性的反馈：
- 在
ConnectionAttr.__init__和Connection.__init__中，针对 PlayCover 的提前return分支会跳过常规的 ADB 相关初始化；请再次确认，当is_playcover为 true 时，这些类上其他代码所依赖的所有属性和不变式（例如adb_client、is_over_http、package_name的处理）要么确实不会被使用，要么都被显式初始化为安全值，以避免属性缺失错误或者细微的行为差异。 - 新增的截图模式中，在 PlayCover 专用的
MacBGR和MacSCK之外，还包含了一个通用的RGBA值；建议重命名或限定RGBA的作用范围，使其清晰地表明是 PlayCover 专用（或者仅在is_playcover为 true 时进行校验），以避免在使用非 PlayCover 控制方式时引起混淆或被误选。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- 在 `ConnectionAttr.__init__` 和 `Connection.__init__` 中，针对 PlayCover 的提前 `return` 分支会跳过常规的 ADB 相关初始化；请再次确认，当 `is_playcover` 为 true 时，这些类上其他代码所依赖的所有属性和不变式（例如 `adb_client`、`is_over_http`、`package_name` 的处理）要么确实不会被使用，要么都被显式初始化为安全值，以避免属性缺失错误或者细微的行为差异。
- 新增的截图模式中，在 PlayCover 专用的 `MacBGR` 和 `MacSCK` 之外，还包含了一个通用的 `RGBA` 值；建议重命名或限定 `RGBA` 的作用范围，使其清晰地表明是 PlayCover 专用（或者仅在 `is_playcover` 为 true 时进行校验），以避免在使用非 PlayCover 控制方式时引起混淆或被误选。
## Individual Comments
### Comment 1
<location path="module/device/app_control.py" line_range="14-18" />
<code_context>
_app_u2_family = ['uiautomator2', 'minitouch', 'scrcpy']
def app_is_running(self) -> bool:
+ if self.is_playcover:
+ return True
method = self.config.script.device.control_method
</code_context>
<issue_to_address>
**issue (bug_risk):** 对于 PlayCover，从 `dump_hierarchy` 返回 `None` 可能会破坏那些期望获得 XML 元素的调用方。
函数签名和文档字符串承诺返回 `etree._Element`，但现在 PlayCover 分支返回 `None`，这会在调用方将 `self.hierarchy` 当作元素（例如调用 `.xpath(...)`）时导致运行时错误。如果这是有意为之，要么更新所有调用点以显式处理 `None`，要么返回一个最小的空层级元素，以保持类型契约的一致性。
</issue_to_address>
### Comment 2
<location path="tests/test_playcover_integration.py" line_range="11" />
<code_context>
+ROOT = Path(__file__).resolve().parents[1]
+
+
+class PlayCoverIntegrationTests(unittest.TestCase):
+ def test_template_defaults(self):
+ template = json.loads((ROOT / "config" / "template.json").read_text(encoding="utf-8"))
</code_context>
<issue_to_address>
**suggestion (testing):** 为 AppControl 中 PlayCover 特定行为添加测试覆盖（app_is_running/app_start/app_stop/dump_hierarchy）。
目前有一条 PlayCover 专用路径尚未被覆盖，即当 `is_playcover` 为 `True` 时 `AppControl` 的行为：
- `app_is_running` 应在不调用 ADB/u2 的情况下直接返回 `True`。
- `app_start` 和 `app_stop` 应该是空操作（no-op）。
- `dump_hierarchy` 应返回 `None`。
请添加一个测试，在 `is_playcover=True` 的情况下（通过真实的 `Connection` 或一个简单的桩对象）调用这些方法，并断言预期的返回值，同时确保不会调用任何 ADB/u2 相关方法，类似现有对 `detect_device`/`adb_connect` 的检查。这样可以防止 PlayCover 的提前返回逻辑在未来出现回归。
建议实现：
```python
ROOT = Path(__file__).resolve().parents[1]
class PlayCoverAppControlTests(unittest.TestCase):
def test_playcover_appcontrol_early_return_behavior(self):
# Deferred import to avoid hard dependency if AppControl is not available in some environments
try:
from module.device.control import AppControl
except ImportError:
self.skipTest("AppControl is not available in this environment")
class FakeConnection:
def __init__(self):
# PlayCover-specific flag
self.is_playcover = True
# ADB / u2-like interfaces that we can inspect for calls
self.adb = Mock()
self.u2 = Mock()
conn = FakeConnection()
# Construct AppControl with a PlayCover connection; adjust if your ctor signature differs
app_control = AppControl(conn)
# app_is_running: should return True and not hit ADB/u2
self.assertTrue(app_control.app_is_running())
# app_start/app_stop: should be no-ops and not hit ADB/u2
self.assertIsNone(app_control.app_start())
self.assertIsNone(app_control.app_stop())
# dump_hierarchy: should return None and not hit ADB/u2
self.assertIsNone(app_control.dump_hierarchy())
# Ensure no ADB/u2 calls were made (protect PlayCover early-return paths)
self.assertEqual(conn.adb.method_calls, [])
self.assertEqual(conn.u2.method_calls, [])
```
1. 如果 `AppControl` 不在 `module.device.control` 中，请在 `test_playcover_appcontrol_early_return_behavior` 中更新导入路径，使其匹配 `AppControl` 实际所在的模块（例如 `from module.app_control import AppControl` 等）。
2. 如果 `AppControl` 的构造函数并非只接收一个 `connection` 参数（例如期望关键字参数或不同的参数名），请相应调整 `app_control = AppControl(conn)` 这一行（比如改为 `app_control = AppControl(connection=conn)`）。
3. 如果你的连接对象上 ADB/u2 属性名称不同（例如 `adb_client`、`u2_client` 或 `device`），请更新 `FakeConnection` 桩类以及最后的 `assertEqual(...method_calls, [])` 断言，使其引用正确的属性。
</issue_to_address>帮我变得更有用！请对每条评论点选 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The early
returnpaths for PlayCover inConnectionAttr.__init__andConnection.__init__skip the usual ADB-related initialization; please double-check that all attributes and invariants other parts of the code expect on these classes (e.g.,adb_client,is_over_http,package_namehandling) are either not used whenis_playcoveris true or are explicitly initialized to safe values to avoid attribute errors or subtle behavior differences. - The new screenshot modes include a generic
RGBAvalue alongside the PlayCover-specificMacBGRandMacSCK; consider renaming or scopingRGBAto be clearly PlayCover-specific (or validating it only whenis_playcoveris true) to avoid confusion or accidental selection when using non-PlayCover control methods.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The early `return` paths for PlayCover in `ConnectionAttr.__init__` and `Connection.__init__` skip the usual ADB-related initialization; please double-check that all attributes and invariants other parts of the code expect on these classes (e.g., `adb_client`, `is_over_http`, `package_name` handling) are either not used when `is_playcover` is true or are explicitly initialized to safe values to avoid attribute errors or subtle behavior differences.
- The new screenshot modes include a generic `RGBA` value alongside the PlayCover-specific `MacBGR` and `MacSCK`; consider renaming or scoping `RGBA` to be clearly PlayCover-specific (or validating it only when `is_playcover` is true) to avoid confusion or accidental selection when using non-PlayCover control methods.
## Individual Comments
### Comment 1
<location path="module/device/app_control.py" line_range="14-18" />
<code_context>
_app_u2_family = ['uiautomator2', 'minitouch', 'scrcpy']
def app_is_running(self) -> bool:
+ if self.is_playcover:
+ return True
method = self.config.script.device.control_method
</code_context>
<issue_to_address>
**issue (bug_risk):** Returning `None` from `dump_hierarchy` for PlayCover may break callers expecting an XML element.
The signature and docstring promise an `etree._Element`, but the PlayCover path now returns `None`, which can cause runtime errors when callers use `self.hierarchy` as an element (e.g., `.xpath(...)`). If this is intentional, either update call sites to handle `None` or return a minimal empty hierarchy element to keep the type contract consistent.
</issue_to_address>
### Comment 2
<location path="tests/test_playcover_integration.py" line_range="11" />
<code_context>
+ROOT = Path(__file__).resolve().parents[1]
+
+
+class PlayCoverIntegrationTests(unittest.TestCase):
+ def test_template_defaults(self):
+ template = json.loads((ROOT / "config" / "template.json").read_text(encoding="utf-8"))
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for PlayCover-specific behavior in AppControl (app_is_running/app_start/app_stop/dump_hierarchy).
One PlayCover-specific path that isn’t covered is `AppControl` behavior when `is_playcover` is `True`:
- `app_is_running` should return `True` without calling ADB/u2.
- `app_start` and `app_stop` should be no-ops.
- `dump_hierarchy` should return `None`.
Please add a test that exercises these methods with `is_playcover=True` (via a real `Connection` or a simple stub) and asserts both the expected return values and that ADB/u2-related methods are not invoked, similar to the `detect_device`/`adb_connect` checks. This will protect the PlayCover early-return logic from regressions.
Suggested implementation:
```python
ROOT = Path(__file__).resolve().parents[1]
class PlayCoverAppControlTests(unittest.TestCase):
def test_playcover_appcontrol_early_return_behavior(self):
# Deferred import to avoid hard dependency if AppControl is not available in some environments
try:
from module.device.control import AppControl
except ImportError:
self.skipTest("AppControl is not available in this environment")
class FakeConnection:
def __init__(self):
# PlayCover-specific flag
self.is_playcover = True
# ADB / u2-like interfaces that we can inspect for calls
self.adb = Mock()
self.u2 = Mock()
conn = FakeConnection()
# Construct AppControl with a PlayCover connection; adjust if your ctor signature differs
app_control = AppControl(conn)
# app_is_running: should return True and not hit ADB/u2
self.assertTrue(app_control.app_is_running())
# app_start/app_stop: should be no-ops and not hit ADB/u2
self.assertIsNone(app_control.app_start())
self.assertIsNone(app_control.app_stop())
# dump_hierarchy: should return None and not hit ADB/u2
self.assertIsNone(app_control.dump_hierarchy())
# Ensure no ADB/u2 calls were made (protect PlayCover early-return paths)
self.assertEqual(conn.adb.method_calls, [])
self.assertEqual(conn.u2.method_calls, [])
```
1. If `AppControl` is not located in `module.device.control`, update the import path in `test_playcover_appcontrol_early_return_behavior` to match the actual module where `AppControl` is defined (e.g. `from module.app_control import AppControl` or similar).
2. If `AppControl`'s constructor does not take a single `connection` argument (e.g. it expects keyword arguments or a different parameter name), adjust the `app_control = AppControl(conn)` line accordingly (for example, `app_control = AppControl(connection=conn)`).
3. If your ADB/u2 attributes are named differently on the connection (for example `adb_client`, `u2_client`, or `device`), update the `FakeConnection` stub and the final `assertEqual(...method_calls, [])` checks so they refer to the correct attributes.
</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
+14
to
18
| if self.is_playcover: | ||
| return True | ||
| method = self.config.script.device.control_method | ||
| # if self.is_wsa: | ||
| # package = self.app_current_wsa() |
There was a problem hiding this comment.
issue (bug_risk): 对于 PlayCover，从 dump_hierarchy 返回 None 可能会破坏那些期望获得 XML 元素的调用方。
函数签名和文档字符串承诺返回 etree._Element，但现在 PlayCover 分支返回 None，这会在调用方将 self.hierarchy 当作元素（例如调用 .xpath(...)）时导致运行时错误。如果这是有意为之，要么更新所有调用点以显式处理 None，要么返回一个最小的空层级元素，以保持类型契约的一致性。
Original comment in English
issue (bug_risk): Returning None from dump_hierarchy for PlayCover may break callers expecting an XML element.
The signature and docstring promise an etree._Element, but the PlayCover path now returns None, which can cause runtime errors when callers use self.hierarchy as an element (e.g., .xpath(...)). If this is intentional, either update call sites to handle None or return a minimal empty hierarchy element to keep the type contract consistent.
| ROOT = Path(__file__).resolve().parents[1] | ||
|
|
||
|
|
||
| class PlayCoverIntegrationTests(unittest.TestCase): |
There was a problem hiding this comment.
suggestion (testing): 为 AppControl 中 PlayCover 特定行为添加测试覆盖（app_is_running/app_start/app_stop/dump_hierarchy）。
目前有一条 PlayCover 专用路径尚未被覆盖，即当 is_playcover 为 True 时 AppControl 的行为：
app_is_running应在不调用 ADB/u2 的情况下直接返回True。app_start和app_stop应该是空操作（no-op）。dump_hierarchy应返回None。
请添加一个测试，在 is_playcover=True 的情况下（通过真实的 Connection 或一个简单的桩对象）调用这些方法，并断言预期的返回值，同时确保不会调用任何 ADB/u2 相关方法，类似现有对 detect_device/adb_connect 的检查。这样可以防止 PlayCover 的提前返回逻辑在未来出现回归。
建议实现：
ROOT = Path(__file__).resolve().parents[1]
class PlayCoverAppControlTests(unittest.TestCase):
def test_playcover_appcontrol_early_return_behavior(self):
# Deferred import to avoid hard dependency if AppControl is not available in some environments
try:
from module.device.control import AppControl
except ImportError:
self.skipTest("AppControl is not available in this environment")
class FakeConnection:
def __init__(self):
# PlayCover-specific flag
self.is_playcover = True
# ADB / u2-like interfaces that we can inspect for calls
self.adb = Mock()
self.u2 = Mock()
conn = FakeConnection()
# Construct AppControl with a PlayCover connection; adjust if your ctor signature differs
app_control = AppControl(conn)
# app_is_running: should return True and not hit ADB/u2
self.assertTrue(app_control.app_is_running())
# app_start/app_stop: should be no-ops and not hit ADB/u2
self.assertIsNone(app_control.app_start())
self.assertIsNone(app_control.app_stop())
# dump_hierarchy: should return None and not hit ADB/u2
self.assertIsNone(app_control.dump_hierarchy())
# Ensure no ADB/u2 calls were made (protect PlayCover early-return paths)
self.assertEqual(conn.adb.method_calls, [])
self.assertEqual(conn.u2.method_calls, [])- 如果
AppControl不在module.device.control中，请在test_playcover_appcontrol_early_return_behavior中更新导入路径，使其匹配AppControl实际所在的模块（例如from module.app_control import AppControl等）。 - 如果
AppControl的构造函数并非只接收一个connection参数（例如期望关键字参数或不同的参数名），请相应调整app_control = AppControl(conn)这一行（比如改为app_control = AppControl(connection=conn)）。 - 如果你的连接对象上 ADB/u2 属性名称不同（例如
adb_client、u2_client或device），请更新FakeConnection桩类以及最后的assertEqual(...method_calls, [])断言，使其引用正确的属性。
Original comment in English
suggestion (testing): Add coverage for PlayCover-specific behavior in AppControl (app_is_running/app_start/app_stop/dump_hierarchy).
One PlayCover-specific path that isn’t covered is AppControl behavior when is_playcover is True:
app_is_runningshould returnTruewithout calling ADB/u2.app_startandapp_stopshould be no-ops.dump_hierarchyshould returnNone.
Please add a test that exercises these methods with is_playcover=True (via a real Connection or a simple stub) and asserts both the expected return values and that ADB/u2-related methods are not invoked, similar to the detect_device/adb_connect checks. This will protect the PlayCover early-return logic from regressions.
Suggested implementation:
ROOT = Path(__file__).resolve().parents[1]
class PlayCoverAppControlTests(unittest.TestCase):
def test_playcover_appcontrol_early_return_behavior(self):
# Deferred import to avoid hard dependency if AppControl is not available in some environments
try:
from module.device.control import AppControl
except ImportError:
self.skipTest("AppControl is not available in this environment")
class FakeConnection:
def __init__(self):
# PlayCover-specific flag
self.is_playcover = True
# ADB / u2-like interfaces that we can inspect for calls
self.adb = Mock()
self.u2 = Mock()
conn = FakeConnection()
# Construct AppControl with a PlayCover connection; adjust if your ctor signature differs
app_control = AppControl(conn)
# app_is_running: should return True and not hit ADB/u2
self.assertTrue(app_control.app_is_running())
# app_start/app_stop: should be no-ops and not hit ADB/u2
self.assertIsNone(app_control.app_start())
self.assertIsNone(app_control.app_stop())
# dump_hierarchy: should return None and not hit ADB/u2
self.assertIsNone(app_control.dump_hierarchy())
# Ensure no ADB/u2 calls were made (protect PlayCover early-return paths)
self.assertEqual(conn.adb.method_calls, [])
self.assertEqual(conn.u2.method_calls, [])- If
AppControlis not located inmodule.device.control, update the import path intest_playcover_appcontrol_early_return_behaviorto match the actual module whereAppControlis defined (e.g.from module.app_control import AppControlor similar). - If
AppControl's constructor does not take a singleconnectionargument (e.g. it expects keyword arguments or a different parameter name), adjust theapp_control = AppControl(conn)line accordingly (for example,app_control = AppControl(connection=conn)). - If your ADB/u2 attributes are named differently on the connection (for example
adb_client,u2_client, ordevice), update theFakeConnectionstub and the finalassertEqual(...method_calls, [])checks so they refer to the correct attributes.
Owner
|
有空研究一下，哦对了提到dev分支 |
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.
中文
本 PR 为 OAS 添加 macOS PlayCover/MaaTools 支持，接入思路参考了 MAA 明日方舟 macOS 使用方案。
主要改动
MacPlayTools。MacBGR、RGBA和MacSCK截图方式。requirements-macos-playcover.txt。使用方式
1280×720。OAS 要求使用该分辨率，否则无法正常识别和点击。control=MacPlayTools）。MacBGR、RGBA或MacSCK。serial，例如localhost:1718。adb或minitouch即可。已使用 OASX Flutter 源码编译并验证 macOS 本地 app。本 PR 不上传该 app，也不内置 Python、OCR 或 OAS 服务。Python 及相关依赖需要按照
requirements-macos-playcover.txt手动安装。测试结果：PlayCover 协议、截图、触控及控制方式切换测试共
13/13通过。English
This PR adds macOS PlayCover/MaaTools support to OAS, following the approach described in the MAA macOS guide.
Changes
MacPlayToolsas a selectable control method in OASX.MacBGR,RGBA, andMacSCKscreenshot methods.requirements-macos-playcover.txtfor manual macOS dependency installation.Usage
1280×720. OAS requires this resolution for recognition and tapping.control=MacPlayTools) in OASX.MacBGR,RGBA, orMacSCKas the screenshot method.serial, for examplelocalhost:1718.adborminitouchwhen using an Android emulator.A local macOS app compiled from the OASX Flutter source was used for validation. The app is not included in this PR and does not bundle Python, OCR, or the OAS service. Python and the required packages must be installed manually using
requirements-macos-playcover.txt.Validation result: all
13/13PlayCover protocol, screenshot, touch, and control-selection tests passed.Summary by Sourcery
添加实验性的 macOS PlayCover/MaaTools 集成为现有基于 Android 的方法之外的备用设备控制路径。
新功能：
文档：
测试：
Original summary in English
Summary by Sourcery
Add experimental macOS PlayCover/MaaTools integration as an alternative device control path alongside existing Android-based methods.
New Features:
Documentation:
Tests: