feat: wlroots use_win32_vk_code support#40
Merged
Conversation
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出了一些总体反馈:
- 将
NewWlRootsController和MaaWlRootsControllerCreate的函数签名修改以添加useWin32VkCode属于破坏性 API 变更;建议考虑新增一个构造函数,或者使用 options 模式,以保持现有调用端的兼容性。 - 由于
useWin32VkCode只是一个简单的布尔标志,从长期来看,将控制器配置归纳到一个结构体或使用函数式 options 可能更清晰,从而在后续增加更多标志时避免函数签名不断膨胀。
面向 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- Changing `NewWlRootsController` and `MaaWlRootsControllerCreate` signatures to add `useWin32VkCode` is a breaking API change; consider introducing a new constructor or using an options pattern to keep existing call sites compatible.
- Since `useWin32VkCode` is a simple boolean flag, it may be clearer long term to group controller configuration into a struct or functional options to avoid signature creep as more flags are added.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- Changing
NewWlRootsControllerandMaaWlRootsControllerCreatesignatures to adduseWin32VkCodeis a breaking API change; consider introducing a new constructor or using an options pattern to keep existing call sites compatible. - Since
useWin32VkCodeis a simple boolean flag, it may be clearer long term to group controller configuration into a struct or functional options to avoid signature creep as more flags are added.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Changing `NewWlRootsController` and `MaaWlRootsControllerCreate` signatures to add `useWin32VkCode` is a breaking API change; consider introducing a new constructor or using an options pattern to keep existing call sites compatible.
- Since `useWin32VkCode` is a simple boolean flag, it may be clearer long term to group controller configuration into a struct or functional options to avoid signature creep as more flags are added.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
Adds an optional useWin32VkCode flag to the WlRoots controller constructor and propagates it through the Go native binding layer, with accompanying documentation/CI updates to reflect and validate the new API against MaaFramework releases.
Changes:
- Extend the native binding signature for
MaaWlRootsControllerCreateto acceptuseWin32VkCode. - Update
NewWlRootsControllerto take and forward the new flag. - Document the constructor signature change in
CHANGELOG.mdand adjust CI to download a resolved MaaFramework release tag.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/native/framework.go |
Updates the registered native function signature for WlRoots controller creation to include useWin32VkCode. |
controller.go |
Extends NewWlRootsController API and forwards the new flag to the native constructor. |
CHANGELOG.md |
Documents the WlRoots constructor signature update and the new Win32 VK keycode behavior option. |
.github/workflows/test.yml |
Changes MaaFramework download to use a resolved release tag (via github-script) rather than latest/preRelease flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
MaaXYZ/MaaFramework#1283
Summary by Sourcery
为 WlRoots 控制器新增可选的 Win32 虚拟键码支持,并更新构造函数接口文档。
New Features:
Enhancements:
useWin32VkCode标志。Original summary in English
Summary by Sourcery
Add optional Win32 virtual key code support to the WlRoots controller and document the updated constructor interface.
New Features:
Enhancements:
新功能:
useWin32VkCode标志,以便可选地将按键事件视作 Win32 虚拟键码。增强内容:
useWin32VkCode选项。Original summary in English
Summary by Sourcery
为 WlRoots 控制器新增可选的 Win32 虚拟键码支持,并更新构造函数接口文档。
New Features:
Enhancements:
useWin32VkCode标志。Original summary in English
Summary by Sourcery
Add optional Win32 virtual key code support to the WlRoots controller and document the updated constructor interface.
New Features:
Enhancements: