为 SettingsV2 的各个开关添加快捷键绑定#114
为 SettingsV2 的各个开关添加快捷键绑定#114Wind-DeterMination-backup wants to merge 8 commits intoTinyLake:mainfrom
Conversation
|
/gemini review |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the user experience by introducing customizable keyboard shortcuts for toggle settings in SettingsV2. Users can now assign keys to quickly switch settings or hold a key to temporarily activate them, streamlining gameplay and interaction with the game's features. The implementation carefully integrates with existing keybinding systems while keeping setting-specific shortcuts separate for better organization and conflict avoidance. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for adding shortcut bindings to toggleable settings, complete with 'toggle' and 'hold' modes. The implementation is robust, particularly the refactoring of KeybindDialog to support reusable key capture and the thoughtful design of the 'hold' functionality. The UI for managing these shortcuts is also user-friendly. I've identified a couple of minor areas for improvement in SettingsV2.kt related to performance and code simplification. Overall, this is a well-executed feature addition.
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for adding shortcut bindings to toggleable settings. The implementation is well-thought-out, particularly the refactoring of KeybindDialog for reusability and the comprehensive shortcut logic within SettingsV2.kt. The support for both 'toggle' and 'hold' modes is a great touch. I have a few suggestions to enhance code quality and performance.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a great new feature for binding shortcuts to toggle settings, including both 'toggle' and 'hold' modes. The implementation is well-structured, with a thoughtful refactoring of KeybindDialog to make the key capture logic reusable and robust. I have one suggestion to make the 'hold' mode more versatile, but overall this is a high-quality contribution.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to bind shortcuts to toggleable settings, with support for both "toggle" and "hold" modes. The implementation is well-structured, with keybinding logic for settings encapsulated within the CheckPref class. The changes to KeybindDialog are a significant improvement, refactoring the key capture logic into a reusable, self-contained RebindSession which is then leveraged for the new settings shortcuts. The overall implementation is solid, but I've identified a couple of UI reactivity issues in the new shortcut configuration panel where the UI state doesn't update automatically after changes are made. My review includes suggestions to fix these issues using update listeners to ensure a smoother user experience.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature allowing users to bind shortcuts to toggleable settings in SettingsV2. The implementation includes support for 'toggle' and 'hold' modes for shortcuts, with persistence for these settings. The changes also involve a significant and beneficial refactoring of the KeybindDialog to better encapsulate the key rebinding logic and to support hidden keybinds that don't appear in the main controls list. The overall implementation is robust and well-designed. I have one suggestion to improve the user experience during key capture.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-implemented feature for binding shortcuts to toggle settings. The changes are comprehensive, including UI for configuration, persistence, and the core logic for handling 'toggle' and 'hold' modes. The refactoring of KeybindDialog is a significant improvement, making the keybinding functionality more modular and reusable. I have a couple of minor suggestions for code simplification and idiomatic style, but overall this is a great addition.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-designed feature for binding shortcuts to toggleable settings. The implementation is robust, covering both 'toggle' and 'hold' modes with a thoughtful UI/UX for configuration. A significant part of this change involves refactoring the core KeybindDialog to be more extensible and reusable, which is a great improvement. The integration with the existing settings system is clean and correctly handles potential conflicts during key capture. I have one suggestion to improve the structure of the CheckPref class for better long-term maintainability.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature allowing users to bind keyboard shortcuts to toggleable settings in SettingsV2. It adds support for both 'toggle' and 'hold' modes for these shortcuts, persisting them per-setting. The implementation is well-designed, encapsulating the shortcut logic within the CheckPref class and its inner ShortcutHandler. A significant part of this PR is the refactoring of the core KeybindDialog to expose its key-capturing functionality in a reusable way, which is done cleanly through patch files. This refactoring also introduces a welcome improvement by clearing conflicting keybinds automatically. The code quality is high, and the feature is robustly implemented. I did not find any issues of medium or higher severity.
|
根据这几轮 Gemini review，这个 PR 已经做过一轮针对性清理，结论总结如下。 已采纳并已修改的建议：
有意未继续修改的点：
当前 PR 的状态是：功能问题、交互问题和中等级别以上 review 问题都已经处理完成，剩余主要是可选的代码风格层面讨论。 |
Wind-DeterMination-backup
changed the title
1 participant
功能概述
这个 PR 为
SettingsV2的布尔开关设置项新增了快捷键绑定能力，目标是让用户可以直接在设置界面中为开关项配置快捷键，而不需要再把这类状态切换逻辑硬编码到独立的全局按键里。本次实现覆盖了以下用户侧行为：
SettingsV2设置项右侧都显示一个快捷键 iconCheckPref（布尔开关）支持真正的快捷键绑定toggle和hold设计与实现说明
1. SettingsV2 层的实现
核心逻辑落在
src/mindustryX/features/SettingsV2.kt：CheckPref增加了内部快捷键处理器ShortcutHandlerKeyBindCore.settings，键名格式为settingV2-shortcut-mode-<name>SettingsV2.pollShortcuts()轮询这些绑定，并驱动对应设置项状态变化hold模式当前语义是“按住期间临时反转当前布尔值，松开后恢复原值”，而不是单纯“强制置 true”。这样对默认开启和默认关闭的开关都可用。2. UI 层的实现
SettingsV2.Data.addTools()现在统一插入快捷键工具位：Data默认提供禁用的快捷键 icon，提示“仅开关设置项支持快捷键”CheckPref重写addShortcutTool()，提供可交互的快捷键按钮这样可以保证：
3. KeybindDialog / 底层绑定复用
为了避免重新发明一套快捷键捕获逻辑，本次直接复用了
work/core/src/mindustry/ui/dialogs/KeybindDialog.java的按键捕获流程，并做了几项底层扩展：showRebindDialog(...)，让 SettingsV2 可以直接调用 Controls 的捕获逻辑isCapturing()，在捕获期间暂停常规快捷键轮询，避免误触发功能Escape被保留为取消捕获，不允许作为可绑定快捷键，和提示文案保持一致这些底层改动已经同步导出为 patch：
patches/client/0074-FC-SettingsV2-shortcut-capture-integration.patchpatches/client/0075-fix-reserve-escape-for-key-capture-cancel.patchpatches/client/0076-refactor-simplify-rebind-session-flow.patch4. 主循环接入
src/mindustryX/Hooks.java中新增了：SettingsV2.INSTANCE.pollShortcuts();这样设置项快捷键与原有
BindingExt.pollKeys()一样，运行在客户端 update 流程中。同时
src/mindustryX/features/BindingExt.kt也增加了捕获期间短路：KeybindDialog.isCapturing()为true时不处理常规功能快捷键这保证了 SettingsV2 快捷键捕获与原有功能键系统不会互相干扰。
与 Gemini Review 相关的修正
这个 PR 在开发过程中多轮触发了 Gemini review，并已经把其中有价值的建议逐步落实到实现里，包括：
shortcutBind.load()ImageButtonStylehold模式调整为“临时反转当前值”Escape作为捕获取消键，不允许绑定RebindSession.rebind(...)的参数结构CheckPref的快捷键逻辑内聚到ShortcutHandler验证
已执行的验证：
work\\gradlew.bat :core:compileJava补充说明：当前仓库的
patchGeneratedSources/kapt在增量构建下偶发出现生成目录脏状态，处理方式是清理work/core/build后重跑；这属于工作树构建环境问题，不是本次功能逻辑本身的回归。