Skip to content

feat: 實作 Win32 的啟動與關閉應用 / Impliment start app and stop app for Win32#1376

Open
kaihuang1122 wants to merge 8 commits into
MaaXYZ:mainfrom
kaihuang1122:main
Open

feat: 實作 Win32 的啟動與關閉應用 / Impliment start app and stop app for Win32#1376
kaihuang1122 wants to merge 8 commits into
MaaXYZ:mainfrom
kaihuang1122:main

Conversation

@kaihuang1122

@kaihuang1122 kaihuang1122 commented Jun 19, 2026

Copy link
Copy Markdown

Summary by Sourcery

实现 Win32 控制器对应用启动和停止的支持,并记录新的行为。

新功能:

  • 添加 Win32 控制器实现,通过可执行文件路径或命令启动应用程序。
  • 添加 Win32 控制器实现,通过关联窗口或可执行文件名停止应用程序。

文档:

  • 记录 Win32 控制器在启动应用时,将可执行文件路径或命令作为 intent 格式的用法。
  • 记录 Win32 控制器在 Win32 上的停止行为,允许在 intent 为空时,使用可选的基于进程名的终止方式或基于窗口的终止方式。
Original summary in English

Summary by Sourcery

Implement Win32 controller support for starting and stopping applications and document the new behavior.

New Features:

  • Add Win32 controller implementation for starting applications via executable path or command.
  • Add Win32 controller implementation for stopping applications either by associated window or by executable name.

Documentation:

  • Document Win32 controller intent format for starting apps as executable path or command.
  • Document Win32 controller stop behavior for Win32, allowing optional process-name based termination or window-based termination when intent is empty.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - 我发现了两个问题,并留下了一些高层次的反馈:

  • stop_app 中,直接将 intentPROCESSENTRY32W::szExeFile 匹配会在 intent 是完整路径(正如文档中建议的那样)而不仅仅是可执行文件名时失败;建议将 intent 规范化为文件名,或者显式限制为基础文件名(basename),以使行为更可预测。
  • 当前的 stop_app 逻辑始终使用 TerminateProcess,这相当强硬;当 hwnd_ 可用时,可以先尝试优雅关闭(例如发送 WM_CLOSE/WM_QUERYENDSESSION),再在必要时回退到强制终止,从而降低数据丢失或应用状态不一致的风险。
给 AI Agent 的提示
Please address the comments from this code review:

## Overall Comments
- In `stop_app`, matching `intent` directly against `PROCESSENTRY32W::szExeFile` will fail when `intent` is a full path (as suggested by the docs) rather than just the bare executable name; consider normalizing `intent` to a filename or explicitly constraining it to a basename to make behavior predictable.
- The current `stop_app` logic always uses `TerminateProcess`, which is quite forceful; when `hwnd_` is available you may want to attempt a graceful shutdown first (e.g., sending `WM_CLOSE`/`WM_QUERYENDSESSION`) before falling back to termination to reduce the risk of data loss or inconsistent app state.

## Individual Comments

### Comment 1
<location path="source/MaaWin32ControlUnit/Manager/Win32ControlUnitMgr.cpp" line_range="260-269" />
<code_context>
+        return false;
+    }
+
+    std::wstring cmd = MAA_NS::to_u16(intent);
+
+    STARTUPINFOW si;
+    PROCESS_INFORMATION pi;
+    ZeroMemory(&si, sizeof(si));
+    si.cb = sizeof(si);
+    ZeroMemory(&pi, sizeof(pi));
+
+    if (!CreateProcessW(nullptr, cmd.data(), nullptr, nullptr, FALSE, 0, nullptr, nullptr, &si, &pi)) {
+        DWORD err = GetLastError();
+        if (err == ERROR_ELEVATION_REQUIRED) {
+            LogInfo << "Elevation required. Falling back to ShellExecuteExW...";
+            SHELLEXECUTEINFOW sei = { sizeof(sei) };
+            sei.fMask = SEE_MASK_NOCLOSEPROCESS;
+            sei.lpVerb = L"runas";
+            sei.lpFile = cmd.c_str();
+            sei.nShow = SW_SHOWNORMAL;
+            if (ShellExecuteExW(&sei)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** 将完整命令行作为 `ShellExecuteExW``lpFile` 使用时,可能会把参数误解为可执行文件路径的一部分。

这里的 `cmd` 包含完整命令行(可执行文件 + 参数),但 `ShellExecuteExW` 期望 `lpFile` 只包含可执行文件,剩余部分放在 `lpParameters` 中。如果 `intent` 包含参数,这里可能会失败,或者指向错误的可执行文件。请将 `intent` 拆分为可执行文件和参数(例如使用类似 `CommandLineToArgvW` 的解析方式),把可执行文件赋值给 `sei.lpFile`,其余参数赋值给 `sei.lpParameters`。
</issue_to_address>

### Comment 2
<location path="source/MaaWin32ControlUnit/Manager/Win32ControlUnitMgr.cpp" line_range="331-340" />
<code_context>
+        return true;
+    }
+
+    std::wstring target_exe = MAA_NS::to_u16(intent);
+    HANDLE hSnap = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
+    if (hSnap == INVALID_HANDLE_VALUE) {
+        LogError << "CreateToolhelp32Snapshot failed, error:" << GetLastError();
+        return false;
+    }

-    return false;
+    PROCESSENTRY32W pe;
+    pe.dwSize = sizeof(pe);
+    bool terminated_any = false;
+
+    if (Process32FirstW(hSnap, &pe)) {
+        do {
+            if (target_exe == pe.szExeFile) {
+                HANDLE hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, pe.th32ProcessID);
+                if (hProcess) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 直接将 `target_exe``pe.szExeFile` 比较,在处理完整路径和大小写时比较脆弱。

在 Windows 上,`PROCESSENTRY32W::szExeFile` 通常只是可执行文件名(例如 `foo.exe`),而 `intent` 可能是完整路径,且/或使用不同的大小写。直接用 `std::wstring` 做相等比较是区分大小写的,在这些情况下会失败。建议先从 `target_exe` 中提取基础文件名(例如使用 `PathFindFileNameW` 或类似方法),再与 `szExeFile` 进行不区分大小写的比较。

建议实现:

```cpp
    std::wstring target_exe = MAA_NS::to_u16(intent);

    // Extract executable basename (without path) for robust comparison with PROCESSENTRY32W::szExeFile
    std::wstring target_exe_name = target_exe;
    const auto last_sep_pos = target_exe_name.find_last_of(L"\\/");
    if (last_sep_pos != std::wstring::npos && last_sep_pos + 1 < target_exe_name.size()) {
        target_exe_name = target_exe_name.substr(last_sep_pos + 1);
    }

    HANDLE hSnap = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);

```

```cpp
    if (Process32FirstW(hSnap, &pe)) {
        do {
            // Compare executable names case-insensitively to handle differing casing
            if (_wcsicmp(target_exe_name.c_str(), pe.szExeFile) == 0) {

```

如果该翻译单元中尚未包含对应声明,你可能需要:
1. 在文件顶部添加 `#include <cwchar>`(或其它提供 `_wcsicmp` 的头文件)。
2. 确保构建设置允许使用 `_wcsicmp`(它是常见的 MSVC 扩展;在 Windows 项目中通常已经可用)。
</issue_to_address>

Sourcery 对开源项目是免费的——如果你觉得我们的评审有帮助,请考虑分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进评审质量。
Original comment in English

Hey - I've found 2 issues, and left some high level feedback:

  • In stop_app, matching intent directly against PROCESSENTRY32W::szExeFile will fail when intent is a full path (as suggested by the docs) rather than just the bare executable name; consider normalizing intent to a filename or explicitly constraining it to a basename to make behavior predictable.
  • The current stop_app logic always uses TerminateProcess, which is quite forceful; when hwnd_ is available you may want to attempt a graceful shutdown first (e.g., sending WM_CLOSE/WM_QUERYENDSESSION) before falling back to termination to reduce the risk of data loss or inconsistent app state.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `stop_app`, matching `intent` directly against `PROCESSENTRY32W::szExeFile` will fail when `intent` is a full path (as suggested by the docs) rather than just the bare executable name; consider normalizing `intent` to a filename or explicitly constraining it to a basename to make behavior predictable.
- The current `stop_app` logic always uses `TerminateProcess`, which is quite forceful; when `hwnd_` is available you may want to attempt a graceful shutdown first (e.g., sending `WM_CLOSE`/`WM_QUERYENDSESSION`) before falling back to termination to reduce the risk of data loss or inconsistent app state.

## Individual Comments

### Comment 1
<location path="source/MaaWin32ControlUnit/Manager/Win32ControlUnitMgr.cpp" line_range="260-269" />
<code_context>
+        return false;
+    }
+
+    std::wstring cmd = MAA_NS::to_u16(intent);
+
+    STARTUPINFOW si;
+    PROCESS_INFORMATION pi;
+    ZeroMemory(&si, sizeof(si));
+    si.cb = sizeof(si);
+    ZeroMemory(&pi, sizeof(pi));
+
+    if (!CreateProcessW(nullptr, cmd.data(), nullptr, nullptr, FALSE, 0, nullptr, nullptr, &si, &pi)) {
+        DWORD err = GetLastError();
+        if (err == ERROR_ELEVATION_REQUIRED) {
+            LogInfo << "Elevation required. Falling back to ShellExecuteExW...";
+            SHELLEXECUTEINFOW sei = { sizeof(sei) };
+            sei.fMask = SEE_MASK_NOCLOSEPROCESS;
+            sei.lpVerb = L"runas";
+            sei.lpFile = cmd.c_str();
+            sei.nShow = SW_SHOWNORMAL;
+            if (ShellExecuteExW(&sei)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Using the full command line as `lpFile` for `ShellExecuteExW` can misinterpret arguments as part of the executable path.

Here `cmd` contains the full command line (exe + args), but `ShellExecuteExW` expects only the executable in `lpFile` and the rest in `lpParameters`. If `intent` includes arguments, this can fail or target the wrong executable. Please split `intent` into executable and arguments (e.g., via `CommandLineToArgvW`-style parsing), assign the executable to `sei.lpFile` and the remaining arguments to `sei.lpParameters`.
</issue_to_address>

### Comment 2
<location path="source/MaaWin32ControlUnit/Manager/Win32ControlUnitMgr.cpp" line_range="331-340" />
<code_context>
+        return true;
+    }
+
+    std::wstring target_exe = MAA_NS::to_u16(intent);
+    HANDLE hSnap = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
+    if (hSnap == INVALID_HANDLE_VALUE) {
+        LogError << "CreateToolhelp32Snapshot failed, error:" << GetLastError();
+        return false;
+    }

-    return false;
+    PROCESSENTRY32W pe;
+    pe.dwSize = sizeof(pe);
+    bool terminated_any = false;
+
+    if (Process32FirstW(hSnap, &pe)) {
+        do {
+            if (target_exe == pe.szExeFile) {
+                HANDLE hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, pe.th32ProcessID);
+                if (hProcess) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Directly comparing `target_exe` to `pe.szExeFile` is brittle regarding full paths and case sensitivity.

On Windows, `PROCESSENTRY32W::szExeFile` is typically just the executable name (e.g., `foo.exe`), while `intent` may be a full path and/or use different casing. A direct `std::wstring` equality is case-sensitive and will fail in those cases. Consider normalizing by extracting the basename from `target_exe` (e.g., via `PathFindFileNameW` or similar) and comparing it to `szExeFile` case-insensitively.

Suggested implementation:

```cpp
    std::wstring target_exe = MAA_NS::to_u16(intent);

    // Extract executable basename (without path) for robust comparison with PROCESSENTRY32W::szExeFile
    std::wstring target_exe_name = target_exe;
    const auto last_sep_pos = target_exe_name.find_last_of(L"\\/");
    if (last_sep_pos != std::wstring::npos && last_sep_pos + 1 < target_exe_name.size()) {
        target_exe_name = target_exe_name.substr(last_sep_pos + 1);
    }

    HANDLE hSnap = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);

```

```cpp
    if (Process32FirstW(hSnap, &pe)) {
        do {
            // Compare executable names case-insensitively to handle differing casing
            if (_wcsicmp(target_exe_name.c_str(), pe.szExeFile) == 0) {

```

If not already present in this translation unit, you may need to:
1. Add `#include <cwchar>` (or another header that provides `_wcsicmp`) near the top of the file.
2. Ensure your build settings allow use of `_wcsicmp` (it's a common MSVC extension; typically already available in Windows projects).
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread source/MaaWin32ControlUnit/Manager/Win32ControlUnitMgr.cpp Outdated
Comment thread source/MaaWin32ControlUnit/Manager/Win32ControlUnitMgr.cpp
@kaihuang1122

kaihuang1122 commented Jun 19, 2026

Copy link
Copy Markdown
Author

針對 sourcery-ai 提出的反饋:

  1. 作為底層引擎的API,使用者應當知道並使用精確的進程名,非嚴格比對可能導致誤殺其他進程。
  2. 大多數遊戲收到WM_CLOSE只會彈出類似「是否退出遊戲」的對話框,不利於自動化場景的構建。

因此,我認為應該拒絕 sourcery-ai 的建議。

@MistEO

MistEO commented Jun 20, 2026

Copy link
Copy Markdown
Member

嵌套太多了,尽量用守卫模式。然后不要用 do while(0)

@kaihuang1122

Copy link
Copy Markdown
Author

嵌套太多了,尽量用守卫模式。然后不要用 do while(0)

Fixed,多謝指教,僅存的 do while 是 微軟寫法 且我認為具有必要性

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants