Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a new system tray icon feature using Rust, replacing the C-based tray implementation. The Rust library handles menu structure, internationalization (Chinese, English, Japanese), event loops, and icon management, while C++ provides a thin wrapper for application-specific callbacks and platform-specific operations. The implementation uses the tray-icon and muda Rust crates with a C FFI boundary for cross-language integration.
Key changes:
- Migrates tray icon logic from C to Rust with multi-language support
- Introduces new C FFI API (
tray_init_ex,tray_loop,tray_exit) for tray lifecycle management - Integrates Rust build into CMake workflow with platform-specific target detection
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/system_tray_rust.cpp | C++ wrapper providing callbacks for Rust tray events and platform-specific handlers (Windows process termination) |
| rust_tray/src/lib.rs | Core Rust tray implementation with menu building, event handling, and FFI exports |
| rust_tray/src/i18n.rs | Internationalization module with translation tables for Chinese, English, and Japanese |
| rust_tray/src/actions.rs | Menu action definitions and URL handling for browser integration |
| rust_tray/include/rust_tray.h | C API header defining FFI interface between Rust and C++ |
| rust_tray/build.rs | Cargo build script for platform-specific linker configuration |
| rust_tray/Cargo.toml | Rust package manifest with dependencies and build configuration |
| rust_tray/README.md | Documentation for Rust tray library (contains outdated information) |
| rust_tray/plan.md | Implementation plan and architecture documentation (in Chinese) |
| cmake/targets/rust_tray.cmake | CMake configuration for building and linking Rust static library |
| cmake/compile_definitions/*.cmake | Platform-specific CMake changes to integrate Rust tray library |
| .github/workflows/main.yml | CI workflow update to add MinGW Rust target for Windows builds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Get locale from config | ||
| std::string locale = "zh"; // Default to Chinese |
There was a problem hiding this comment.
The default locale is hardcoded to Chinese ("zh"), but the default locale in the Rust i18n module is English. This inconsistency could cause unexpected behavior. Consider defaulting to English here as well, or make the default configurable.
| std::string locale = "zh"; // Default to Chinese | |
| std::string locale = "en"; // Default to English |
| void update_tray_playing(std::string app_name) { | ||
| if (!tray_initialized) return; | ||
|
|
||
| tray_set_icon(TRAY_ICON_TYPE_PLAYING); | ||
|
|
||
| std::string tooltip = "Sunshine - Playing: " + app_name; | ||
| tray_set_tooltip(tooltip.c_str()); | ||
| } | ||
|
|
||
| void update_tray_pausing(std::string app_name) { | ||
| if (!tray_initialized) return; | ||
|
|
||
| tray_set_icon(TRAY_ICON_TYPE_PAUSING); | ||
|
|
||
| std::string tooltip = "Sunshine - Paused: " + app_name; | ||
| tray_set_tooltip(tooltip.c_str()); | ||
| } | ||
|
|
||
| void update_tray_stopped(std::string app_name) { | ||
| if (!tray_initialized) return; | ||
|
|
||
| tray_set_icon(TRAY_ICON_TYPE_NORMAL); | ||
|
|
||
| std::string tooltip = "Sunshine "s + PROJECT_VER; | ||
| tray_set_tooltip(tooltip.c_str()); | ||
| } |
There was a problem hiding this comment.
The functions update_tray_playing, update_tray_pausing, and update_tray_stopped take std::string by value, which causes unnecessary string copies. Consider using const std::string& instead for better performance, especially since these functions may be called frequently during streaming.
src/system_tray_rust.cpp
Outdated
| BOOST_LOG(info) << "Successfully terminated sunshine-gui.exe"sv; | ||
| } | ||
| CloseHandle(process_handle); |
There was a problem hiding this comment.
Error handling for Windows API calls is incomplete. If OpenProcess fails (returns NULL), the code silently continues without logging the error. Similarly, TerminateProcess failure is not logged. Consider adding error logging to aid in debugging permission or access issues.
| BOOST_LOG(info) << "Successfully terminated sunshine-gui.exe"sv; | |
| } | |
| CloseHandle(process_handle); | |
| BOOST_LOG(info) << "Successfully terminated sunshine-gui.exe"sv; | |
| } else { | |
| DWORD err = GetLastError(); | |
| BOOST_LOG(error) << "Failed to terminate sunshine-gui.exe (PID: " << pe32.th32ProcessID << "), error: " << err; | |
| } | |
| CloseHandle(process_handle); | |
| } else { | |
| DWORD err = GetLastError(); | |
| BOOST_LOG(error) << "Failed to open sunshine-gui.exe (PID: " << pe32.th32ProcessID << ") for termination, error: " << err; |
| void terminate_gui_processes() { | ||
| #ifdef _WIN32 | ||
| BOOST_LOG(info) << "Terminating sunshine-gui.exe processes..."sv; | ||
|
|
||
| HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); | ||
| if (snapshot != INVALID_HANDLE_VALUE) { | ||
| PROCESSENTRY32W pe32; | ||
| pe32.dwSize = sizeof(PROCESSENTRY32W); | ||
|
|
||
| if (Process32FirstW(snapshot, &pe32)) { | ||
| do { | ||
| if (wcscmp(pe32.szExeFile, L"sunshine-gui.exe") == 0) { | ||
| BOOST_LOG(info) << "Found sunshine-gui.exe (PID: " << pe32.th32ProcessID << "), terminating..."sv; | ||
| HANDLE process_handle = OpenProcess(PROCESS_TERMINATE, FALSE, pe32.th32ProcessID); | ||
| if (process_handle != NULL) { | ||
| if (TerminateProcess(process_handle, 0)) { | ||
| BOOST_LOG(info) << "Successfully terminated sunshine-gui.exe"sv; | ||
| } | ||
| CloseHandle(process_handle); | ||
| } | ||
| } | ||
| } while (Process32NextW(snapshot, &pe32)); | ||
| } | ||
| CloseHandle(snapshot); | ||
| } |
There was a problem hiding this comment.
The terminate_gui_processes function terminates all sunshine-gui.exe processes without checking if they belong to the current user session or if they're associated with this specific Sunshine instance. In a multi-user environment or when multiple Sunshine instances are running, this could terminate GUI processes belonging to other users or instances, which is a security and usability issue.
src/system_tray_rust.cpp
Outdated
| std::thread([]() { | ||
| std::this_thread::sleep_for(10s); | ||
| tray_set_vdd_enabled(1); | ||
| }).detach(); |
There was a problem hiding this comment.
The detached thread created here is not tracked or joined, which could lead to a race condition during shutdown. If the application exits before the 10-second sleep completes, the thread may attempt to call tray_set_vdd_enabled on an already-destroyed tray. Consider using a cancellable timer or tracking the thread to ensure proper cleanup.
ShadowLemoon
force-pushed
the
feat/rust-tray-icon
branch
from
4baf35c to
3bfa5fa
Compare
ShadowLemoon
force-pushed
the
feat/rust-tray-icon
branch
from
3bfa5fa to
8f7c9e9
Compare
- 将菜单事件处理拆分为 identify_menu_action 和 execute_action 两步 - 先释放 mutex 锁再执行实际操作，避免切换语言、重启、退出时死锁
ShadowLemoon
force-pushed
the
feat/rust-tray-icon
branch
from
fe452f0 to
3dadc7a
Compare
qiin2333
force-pushed
the
master
branch
2 times, most recently
from
728df07 to
33745c6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //! Windows Toast Notification module | ||
| //! | ||
| //! Provides native Windows toast notifications using WinRT. | ||
|
|
||
| use winrt_notification::{Duration, Sound, Toast}; |
There was a problem hiding this comment.
Similar to dialogs.rs, this notification module uses Windows-specific APIs (winrt_notification) but lacks explicit platform guards. While lib.rs has the guard, add #![cfg(target_os = "windows")] at the module level for clarity and to prevent misuse.
| ├── Cargo.toml # Rust package manifest | ||
| ├── build.rs # Build script (platform-specific setup) | ||
| ├── src/ | ||
| │ ├── lib.rs # Main library implementation | ||
| │ └── ffi.rs # FFI type definitions | ||
| └── README.md # This file |
There was a problem hiding this comment.
The architecture documentation shows src/ffi.rs which doesn't exist in the actual implementation. The actual structure includes i18n.rs, actions.rs, dialogs.rs, menu.rs, menu_items.rs, and notification.rs. Update the architecture diagram to reflect the actual file structure.
| ├── Cargo.toml # Rust package manifest | |
| ├── build.rs # Build script (platform-specific setup) | |
| ├── src/ | |
| │ ├── lib.rs # Main library implementation | |
| │ └── ffi.rs # FFI type definitions | |
| └── README.md # This file | |
| ├── Cargo.toml # Rust package manifest | |
| ├── build.rs # Build script (platform-specific setup) | |
| ├── src/ | |
| │ ├── lib.rs # Main library implementation and C FFI | |
| │ ├── i18n.rs # Internationalization and localized strings | |
| │ ├── actions.rs # Actions and callbacks triggered from the tray | |
| │ ├── dialogs.rs # Platform-specific dialog helpers | |
| │ ├── menu.rs # Tray menu construction and management | |
| │ ├── menu_items.rs # Individual tray menu item definitions | |
| │ └── notification.rs # User notifications from the tray | |
| └── README.md # This file |
| - No changes are required to `src/system_tray.cpp` | ||
| - The original `third-party/tray/src/tray.h` header is still used for the C++ code |
There was a problem hiding this comment.
The README claims "No changes are required to src/system_tray.cpp" but this is incorrect - the PR actually replaces system_tray.cpp with system_tray_rust.cpp (as shown in common.cmake line 120). This statement is misleading and should be removed or updated to reflect the actual changes made.
| - No changes are required to `src/system_tray.cpp` | |
| - The original `third-party/tray/src/tray.h` header is still used for the C++ code | |
| - When `SUNSHINE_USE_RUST_TRAY=ON` is enabled, the Rust-based implementation is used in place of the original `src/system_tray.cpp` implementation via CMake | |
| - The original `third-party/tray/src/tray.h` header is still used as the public API header for the C++ code |
| version = "0.1.0" | ||
| edition = "2021" | ||
| build = "build.rs" | ||
| description = "Windows-only system tray implementation for Sunshine" |
There was a problem hiding this comment.
The package description claims "Windows-only system tray implementation" but the CMake configuration (rust_tray.cmake) includes platform-specific support for macOS (lines 102-108) and Linux (lines 110-118). Additionally, the README states "Cross-platform support (Windows, Linux, macOS)". Either the description is outdated, or the non-Windows support is not actually functional (since lib.rs has #[cfg(target_os = "windows")]). Clarify the actual platform support or update the description and documentation to be consistent.
| description = "Windows-only system tray implementation for Sunshine" | |
| description = "System tray implementation for Sunshine (currently targeting Windows)" |
| // Safety: TrayState is only accessed from the main thread | ||
| unsafe impl Send for TrayState {} | ||
| unsafe impl Sync for TrayState {} |
There was a problem hiding this comment.
The unsafe Send and Sync implementations for TrayState (lines 48-49) are justified with a comment that "TrayState is only accessed from the main thread". However, the TrayState is wrapped in a Mutex (line 52) which suggests it might be accessed from multiple threads. If it's truly only accessed from the main thread, the Mutex is unnecessary. If it can be accessed from multiple threads, the unsafe Send/Sync impls need more careful justification, especially since TrayIcon and Menu from the tray-icon and muda crates may not be thread-safe. Review whether the Mutex is needed and whether the unsafe impls are truly safe.
| // Safety: TrayState is only accessed from the main thread | |
| unsafe impl Send for TrayState {} | |
| unsafe impl Sync for TrayState {} | |
| // Safety: TrayState is created and used only on the main thread. | |
| // It is never moved out of the global Mutex/OnceCell, and all access | |
| // must remain confined to the main thread. The Send bound is required | |
| // only so that `Mutex<Option<TrayState>>` can be used in a global. | |
| unsafe impl Send for TrayState {} |
| std::thread([]() { | ||
| std::this_thread::sleep_for(10s); | ||
| s_vdd_in_cooldown = false; | ||
| update_vdd_menu_state(); | ||
| }).detach(); |
There was a problem hiding this comment.
The detached thread used for VDD cooldown creates a fire-and-forget timer that cannot be cancelled or properly cleaned up. If end_tray() is called while a cooldown is in progress, the detached thread will continue to run and attempt to call update_vdd_menu_state() after the tray may have been destroyed. Consider using a cancellable timer mechanism or tracking detached threads to ensure proper cleanup on shutdown.
| //! Dialog utilities module (Windows only) | ||
| //! | ||
| //! 提供系统对话框功能。 | ||
|
|
||
| use std::ffi::OsStr; | ||
| use std::os::windows::ffi::OsStrExt; |
There was a problem hiding this comment.
This module uses Windows-specific APIs (std::os::windows::ffi::OsStrExt and windows_sys) but lacks a #[cfg(target_os = "windows")] attribute. While the parent lib.rs has #[cfg(target_os = "windows")] on line 14, it's a best practice to also annotate platform-specific modules to prevent accidental usage on other platforms. Add #![cfg(target_os = "windows")] at the top of this module.
| ${RUST_TRAY_SOURCE_DIR}/build.rs | ||
| ${RUST_TRAY_SOURCE_DIR}/src/lib.rs | ||
| ${RUST_TRAY_SOURCE_DIR}/src/i18n.rs | ||
| ${RUST_TRAY_SOURCE_DIR}/src/actions.rs |
There was a problem hiding this comment.
The CMake DEPENDS list for the Rust build doesn't include all Rust source files. It only lists lib.rs, i18n.rs, and actions.rs, but is missing dialogs.rs, menu.rs, menu_items.rs, and notification.rs. This means changes to these files won't trigger a rebuild. Add all source files to the DEPENDS list to ensure proper incremental builds.
| ${RUST_TRAY_SOURCE_DIR}/src/actions.rs | |
| ${RUST_TRAY_SOURCE_DIR}/src/actions.rs | |
| ${RUST_TRAY_SOURCE_DIR}/src/dialogs.rs | |
| ${RUST_TRAY_SOURCE_DIR}/src/menu.rs | |
| ${RUST_TRAY_SOURCE_DIR}/src/menu_items.rs | |
| ${RUST_TRAY_SOURCE_DIR}/src/notification.rs |
| assert_eq!(get_string(StringKey::OpenSunshine), "打开 Sunshine"); | ||
|
|
||
| set_locale(Locale::Japanese); | ||
| assert_eq!(get_string(StringKey::OpenSunshine), "Sunshineを開く"); |
There was a problem hiding this comment.
The test expects "打开 Sunshine" for Chinese translation, but the actual translation on line 129 is "打开基地面板" (which means "Open Base Panel"). Similarly, the test expects "Sunshineを開く" for Japanese but the actual translation on line 168 is "GUIを開く" (which means "Open GUI"). Update the tests to match the actual translations or update the translations to match the tests.
| assert_eq!(get_string(StringKey::OpenSunshine), "打开 Sunshine"); | |
| set_locale(Locale::Japanese); | |
| assert_eq!(get_string(StringKey::OpenSunshine), "Sunshineを開く"); | |
| assert_eq!(get_string(StringKey::OpenSunshine), "打开基地面板"); | |
| set_locale(Locale::Japanese); | |
| assert_eq!(get_string(StringKey::OpenSunshine), "GUIを開く"); |
| fn test_get_string() { | ||
| set_locale(Locale::English); | ||
| assert_eq!(get_string(StringKey::OpenSunshine), "Open Sunshine"); | ||
|
|
||
| set_locale(Locale::Chinese); | ||
| assert_eq!(get_string(StringKey::OpenSunshine), "打开 Sunshine"); | ||
|
|
||
| set_locale(Locale::Japanese); | ||
| assert_eq!(get_string(StringKey::OpenSunshine), "Sunshineを開く"); |
There was a problem hiding this comment.
The test expects "Open Sunshine" for English translation, but the actual translation defined on line 90 is "Open GUI". This test will fail. Either update the test expectation to match the actual translation or update the translation to match the test.
qiin2333
force-pushed
the
master
branch
2 times, most recently
from
029ba99 to
eba4145
Compare
qiin2333
force-pushed
the
master
branch
2 times, most recently
from
e6bbe21 to
d74f9ed
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 Walkthrough概述本变更将系统托盘功能从 C 实现迁移至 Rust，新增 变更
时序图sequenceDiagram
participant CppApp as C++ 应用
participant RustTray as Rust 托盘库
participant WinAPI as Windows API
participant MenuSys as 菜单系统
CppApp->>RustTray: tray_init_ex(图标, 本地化, 回调)
activate RustTray
RustTray->>MenuSys: 构建本地化菜单
RustTray->>WinAPI: 创建 TrayIcon
RustTray->>WinAPI: 注册 TrayIcon 到系统托盘
deactivate RustTray
Note over WinAPI: 用户交互...
CppApp->>RustTray: tray_loop(非阻塞)
activate RustTray
RustTray->>WinAPI: 处理 Windows 消息
WinAPI-->>RustTray: 菜单事件
RustTray->>MenuSys: 识别菜单项 ID
RustTray->>RustTray: 执行本地处理器
RustTray-->>CppApp: 回调(动作 ID)
deactivate RustTray
activate CppApp
CppApp->>CppApp: 路由至业务逻辑(启动 UI/VDD/重启/退出)
CppApp->>RustTray: tray_update_vdd_menu(...)
CppApp->>RustTray: tray_set_locale(新语言)
deactivate CppApp
activate RustTray
RustTray->>MenuSys: 使用新语言重建菜单
RustTray->>WinAPI: 刷新 TrayIcon
deactivate RustTray
代码审查工作量评估🎯 4 (复杂) | ⏱️ ~75 分钟 评估理由：本变更涉及多个维度的复杂性——（1）新增 541 行 Rust FFI 实现与多个子模块（菜单、i18n、通知），每个模块引入新的数据结构与交互；（2）跨语言集成：Rust ↔ C++ ↔ Windows API，需验证 FFI 边界的内存安全与类型正确性；（3）菜单状态管理与多线程访问（RwLock、Lazy）；（4）CMake 构建逻辑新增（平台检测、Cargo 调用、目标三元组映射）；（5）动作回调链路涉及字符串转换、本地化与冷却时间管理；（6）354 行 C++ 包装层涉及原子变量、全局状态与 Windows API 使用。代码异构性较高（Rust 模块多样、CMake 新逻辑、C++ 中介），需针对各部分分别理解与验证。 诗歌
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (5)
rust_tray/.gitignore (1)
1-2: 考虑是否应该提交Cargo.lock对于作为可执行程序一部分的 Rust 库，通常建议提交
Cargo.lock以确保构建的可复现性。虽然对于纯 Rust 库通常会忽略此文件，但由于rust_tray是集成到 C++ 应用程序中的静态库，提交Cargo.lock可以确保所有开发者和 CI 使用相同的依赖版本。🔧 建议的修改
/target/ -/Cargo.lock🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust_tray/.gitignore` around lines 1 - 2, 当前 .gitignore 忽略了 Cargo.lock，但因为 rust_tray 是作为静态库/可执行程序的一部分被集成到 C++ 应用中，应提交 Cargo.lock 以保证依赖可复现；请在 rust_tray 的 .gitignore 中移除 Cargo.lock（或直接将 Cargo.lock 添加到仓库并提交），并在以后更新依赖后保持同步提交更新的 Cargo.lock（定位到 .gitignore 和 Cargo.lock 文件以修改/提交）。rust_tray/src/i18n.rs (1)
237-240: 缺失翻译时考虑添加日志记录当翻译键在所有语言中都找不到时，函数静默返回空字符串。这可能导致 UI 中出现空白文本而难以调试。建议在此处添加日志警告，便于开发阶段发现遗漏的翻译。
♻️ 建议的修改
// Fallback to English if let Some(s) = TRANSLATIONS.get(&(Locale::English, key)) { return s; } // Return empty string if not found + // Consider: log::warn!("Missing translation for key: {:?}", key); "" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust_tray/src/i18n.rs` around lines 237 - 240, The function in rust_tray/src/i18n.rs that currently ends by returning an empty string (the final `""`) should log a warning when a translation key is missing instead of silently returning empty; update the function (e.g., the translate/t or equivalent lookup function that contains the `return ""`) to call your logging facility (warn! or tracing::warn!) with the missing key and context (requested locale(s) and/or the list of available languages) before returning the empty string so missing translations are discoverable during development.rust_tray/src/lib.rs (1)
531-538: 配对请求通知的格式化模式与其他类型不同，但逻辑正确第 3 种通知类型（配对请求）的格式化模式与前三种相反：标题使用格式化字符串，正文使用普通字符串。这是因为配对请求需要在标题中显示设备名称。代码逻辑正确。
默认分支
_ => return静默返回，建议添加日志记录无效的通知类型以便调试。♻️ 建议添加日志
3 => { // Pairing request let title = i18n::get_string_fmt(i18n::StringKey::IncomingPairingRequest, &app_name_str); let text = i18n::get_string(i18n::StringKey::ClickToCompletePairing); (title, text.to_string(), notification::NotificationIcon::Normal) }, - _ => return, + _ => { + eprintln!("Unknown notification type: {}", notification_type); + return; + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust_tray/src/lib.rs` around lines 531 - 538, The match's default arm silently returns on unknown notification types; change `_ => return` to emit a warning log including the unexpected notification type so failures are discoverable. Locate the match that builds (title, text, notification::NotificationIcon::Normal) (the arm handling `3` uses i18n::get_string_fmt and i18n::get_string with app_name_str) and replace the silent return with a log call (e.g., log::warn! or tracing::warn!) that includes the unmatched value or enum variant, then return or handle as before.rust_tray/src/menu_items.rs (1)
269-291: 建议：考虑缓存菜单项定义以避免重复分配
execute_handler每次调用时都会通过get_all_items()分配新的Vec。虽然菜单点击频率不高，但如果追求更好的性能，可以考虑使用OnceCell或lazy_static缓存菜单项定义。♻️ 可选的优化方案
use once_cell::sync::Lazy; static ALL_ITEMS: Lazy<Vec<MenuItemInfo>> = Lazy::new(get_all_items); pub fn execute_handler(item_id: &str) -> (bool, bool) { if let Some(item) = ALL_ITEMS.iter().find(|i| i.id == item_id) { // ... existing logic } (false, false) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust_tray/src/menu_items.rs` around lines 269 - 291, execute_handler currently calls get_all_items() on every invocation which allocates a new Vec each time; change to cache the menu definitions in a static lazy variable (e.g., use once_cell::sync::Lazy or OnceCell) so execute_handler looks up items from a static ALL_ITEMS: Lazy<Vec<MenuItemInfo>> (initialized with get_all_items) and then performs the same find/handler/trigger_action_for_id logic against ALL_ITEMS instead of calling get_all_items() repeatedly.cmake/targets/rust_tray.cmake (1)
8-11: Windows 目标平台硬编码为 x86_64，缺少 ARM64 支持macOS 和 Linux 都有 ARM64 架构检测逻辑，但 Windows 部分硬编码为
x86_64-pc-windows-gnu。如果需要支持 Windows ARM64 设备，建议添加相应的架构检测：♻️ 建议的修复方案
if(WIN32) # Windows uses MinGW/UCRT toolchain - must use gnu target - set(RUST_TARGET "x86_64-pc-windows-gnu") + if(CMAKE_SYSTEM_PROCESSOR MATCHES "ARM64" OR CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64") + set(RUST_TARGET "aarch64-pc-windows-gnu") + else() + set(RUST_TARGET "x86_64-pc-windows-gnu") + endif() set(RUST_LIB_NAME "libtray.a")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmake/targets/rust_tray.cmake` around lines 8 - 11, The Windows block hardcodes RUST_TARGET to x86_64 and omits ARM64 support; update the WIN32 branch to detect the host/process architecture (using CMAKE_HOST_SYSTEM_PROCESSOR or CMAKE_SYSTEM_PROCESSOR) and set RUST_TARGET accordingly (e.g., "x86_64-pc-windows-gnu" for x86_64/AMD64 and "aarch64-pc-windows-msvc" or the appropriate aarch64-pc-windows-gnu variant for ARM64), leaving RUST_LIB_NAME as before; implement a simple conditional on the processor string (match "AMD64"/"x86_64" vs "ARM64"/"aarch64") to select the correct target triple so Windows ARM64 devices are supported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmake/targets/rust_tray.cmake`:
- Around line 63-69: The DEPENDS list for the rust tray target is incomplete:
files referenced by lib.rs (dialogs.rs, menu.rs, menu_items.rs, notification.rs)
are missing so CMake won't re-run when they change; update the DEPENDS block
that references ${RUST_TRAY_SOURCE_DIR}/Cargo.toml, build.rs, src/lib.rs,
src/i18n.rs, src/actions.rs to also include
${RUST_TRAY_SOURCE_DIR}/src/dialogs.rs, ${RUST_TRAY_SOURCE_DIR}/src/menu.rs,
${RUST_TRAY_SOURCE_DIR}/src/menu_items.rs and
${RUST_TRAY_SOURCE_DIR}/src/notification.rs (or replace the explicit list by a
file(GLOB ...) over ${RUST_TRAY_SOURCE_DIR}/src/*.rs if you prefer
auto-collecting files, noting GLOB caveats) so changes to those modules trigger
incremental rebuilds.
In `@rust_tray/build.rs`:
- Around line 14-22: 在 build.rs 的 Windows 平台块（#[cfg(target_os = "windows")] 内使用
println!("cargo:rustc-link-lib=...") 的地方，补上 CMake 中缺失的库：添加 println! 行分别为
oleaut32、uuid、bcrypt、ntdll、userenv、ws2_32 和 pthread（同时保留已存在的
user32、gdi32、shell32、ole32、comctl32，避免重复新增相同项）；如果这些差异是有意为之，请改为在该模块或仓库 README
中明确记录何时使用 build.rs 链接而何时依赖 CMake，以避免构建时缺少依赖。
In `@rust_tray/include/rust_tray.h`:
- Around line 142-159:
头文件声明的三个深色模式函数（tray_enable_dark_mode、tray_force_dark_mode、tray_force_light_mode）在
lib.rs 中未导出，导致链接错误；为修复，在 lib.rs 中为每个函数添加对应的导出实现（使用 #[no_mangle] pub extern "C"
fn tray_enable_dark_mode(), #[no_mangle] pub extern "C" fn
tray_force_dark_mode(), #[no_mangle] pub extern "C" fn
tray_force_light_mode()），实现内部逻辑（可以调用现有初始化或平台函数以切换主题，或暂时做空实现以避免崩溃），并确保命名与头文件完全一致以正确导出符号。
In `@rust_tray/plan.md`:
- Line 29: The doc lists non-existent APIs tray_set_vdd_checked and
tray_set_vdd_enabled; update the runtime-updates line to match the real header
API by replacing those entries with the actual function name and signature
tray_update_vdd_menu(int can_create, int can_close, int is_persistent, int
is_active), and ensure the runtime update list includes tray_set_icon,
tray_set_tooltip, tray_update_vdd_menu, tray_set_locale, and
tray_show_notification so the documentation matches
rust_tray/include/rust_tray.h.
In `@rust_tray/README.md`:
- Around line 54-68: The README's architecture section is outdated: update the
directory tree to list the new Rust files (actions.rs, menu.rs, notification.rs,
dialogs.rs) under src and replace the note that mentions src/system_tray.cpp
with the current entry point src/system_tray_rust.cpp; also add a language tag
to the code fence (e.g., ```text or ```bash) to satisfy markdownlint MD040 so
the tree block is properly marked. Ensure the paragraph referencing
compatibility/third-party headers still applies or update it if the integration
changed, and keep all edits inside README.md to reflect the new filenames and
entry point.
- Around line 7-10: The README currently claims cross-platform support but the
implementation files rust_tray/src/dialogs.rs and rust_tray/src/notification.rs
and the Cargo.toml only enable Windows-specific APIs/dependencies; update
README.md to explicitly state this crate is Windows-only (or clearly document
that Linux/macOS are not yet implemented and list required preconditions for
those platforms), remove or mark as Windows-only any Linux/macOS artifact paths,
and update the corresponding lines referenced (including lines 28-31) to avoid
implying full cross-platform support; reference rust_tray/src/dialogs.rs,
rust_tray/src/notification.rs, and rust_tray/Cargo.toml when making the README
changes.
In `@rust_tray/src/i18n.rs`:
- Around line 262-271: The test_get_string assertions for
StringKey::OpenSunshine don't match the translation table values used by
get_string; update the test or the translations so they align: either change the
expected values in test_get_string to "Open GUI", "打开基地面板", and "GUIを開く" to
match the current entries, or modify the translation entries referenced by
get_string/StringKey::OpenSunshine to "Open Sunshine", "打开 Sunshine", and
"Sunshineを開く"; ensure you update the set_locale calls and assertions in
test_get_string accordingly so the test and get_string behavior are consistent.
In `@rust_tray/src/lib.rs`:
- Around line 47-49: The unsafe impls for TrayState rely on the comment "only
accessed from the main thread" but offer no enforcement; update the code to
either document this constraint in the C header and FFI docs (clearly state that
all FFI calls touching TrayState must be made from the main thread) and/or add a
runtime guard: record std::thread::ThreadId (e.g., main_thread_id) in TrayState
at initialization and validate it at the start of every extern "C" function that
accesses TrayState (return an error or abort if the current thread id differs).
Reference the unsafe impl Send/Sync for TrayState and the FFI entry points that
use TrayState to locate where to add the ThreadId field and per-call checks.
In `@rust_tray/src/menu.rs`:
- Around line 24-28: CREATED_ITEMS 是线程局部 (thread_local!)，导致在非创建线程（如
tray_update_vdd_menu() 被分离线程调用时）看不到状态更新并 no-op；将状态修改封送回 tray/UI 线程。改法：替换或补充
CREATED_ITEMS 的 TLS 存储，改用全局线程安全结构（例如 lazy_static/once_cell +
Arc<Mutex<HashMap<...>>> 或者锁/并发 map）或在所有 setter 中通过一个跨线程 channel 将“变更事件/闭包”发送到
UI/tray 线程并在 tray_update_vdd_menu() 或 UI 事件循环处消费并应用这些变更；确保所有读取/写入点（涉及
CREATED_ITEMS、相关 setter）改为使用该线程安全通道/全局结构以保证跨线程更新生效。
In `@src/system_tray_rust.cpp`:
- Around line 340-344: 函数 update_tray_vmonitor_checked 的 checked
参数被忽略：你要么把它改为真正使用传入的 checked 值去设置对应的菜单项（在函数内调用更新单个 VMonitor 复选框的逻辑，避免仅调用
update_vdd_menu_state），要么将函数签名去掉 checked 并重命名为 refresh_tray_vmonitor 或 similar
表示仅刷新当前状态；更新调用处以匹配新签名并确保唯一定位符为 update_tray_vmonitor_checked /
update_vdd_menu_state。
- Around line 252-257: process_tray_events currently calls tray_loop(0) which
busy-polls; change it so that if the tray runs on its own thread you call the
blocking form of the loop (use tray_loop with the blocking argument, e.g.,
tray_loop(-1) or the API's blocking mode) inside process_tray_events when
tray_initialized indicates a dedicated tray thread, otherwise keep non-blocking
tray_loop(0) but detect the "no events" return value and call
std::this_thread::sleep_for(std::chrono::milliseconds(n)) or
std::this_thread::yield() before returning to avoid busy-wait; update callers
that repeatedly call process_tray_events (they use process_tray_events in tight
loops) to rely on the blocking mode when appropriate or accept the short
sleep/yield when using non-blocking mode.
- Around line 186-205: terminate_gui_processes currently force-kills every
process named "sunshine-gui.exe"; instead restrict termination to only the GUI
instances our process actually launched or those in the same session. Update
terminate_gui_processes to prefer using a known GUI PID (e.g., stored gui_pid)
and only call OpenProcess/TerminateProcess for that PID; if the PID is not
available, filter process entries by SessionId (use ProcessIdToSessionId and
compare to the current session), and further verify the process image path
matches the expected executable (use QueryFullProcessImageNameW) and/or that the
parent PID matches our process (via CreateToolhelp32Snapshot/PROCESSENTRY32W
parent check) before calling OpenProcess/TerminateProcess; keep the current
CreateToolhelp32Snapshot/PROCESSENTRY32W loop but add these checks to avoid
killing unrelated instances.
---
Nitpick comments:
In `@cmake/targets/rust_tray.cmake`:
- Around line 8-11: The Windows block hardcodes RUST_TARGET to x86_64 and omits
ARM64 support; update the WIN32 branch to detect the host/process architecture
(using CMAKE_HOST_SYSTEM_PROCESSOR or CMAKE_SYSTEM_PROCESSOR) and set
RUST_TARGET accordingly (e.g., "x86_64-pc-windows-gnu" for x86_64/AMD64 and
"aarch64-pc-windows-msvc" or the appropriate aarch64-pc-windows-gnu variant for
ARM64), leaving RUST_LIB_NAME as before; implement a simple conditional on the
processor string (match "AMD64"/"x86_64" vs "ARM64"/"aarch64") to select the
correct target triple so Windows ARM64 devices are supported.
In `@rust_tray/.gitignore`:
- Around line 1-2: 当前 .gitignore 忽略了 Cargo.lock，但因为 rust_tray
是作为静态库/可执行程序的一部分被集成到 C++ 应用中，应提交 Cargo.lock 以保证依赖可复现；请在 rust_tray 的 .gitignore
中移除 Cargo.lock（或直接将 Cargo.lock 添加到仓库并提交），并在以后更新依赖后保持同步提交更新的 Cargo.lock（定位到
.gitignore 和 Cargo.lock 文件以修改/提交）。
In `@rust_tray/src/i18n.rs`:
- Around line 237-240: The function in rust_tray/src/i18n.rs that currently ends
by returning an empty string (the final `""`) should log a warning when a
translation key is missing instead of silently returning empty; update the
function (e.g., the translate/t or equivalent lookup function that contains the
`return ""`) to call your logging facility (warn! or tracing::warn!) with the
missing key and context (requested locale(s) and/or the list of available
languages) before returning the empty string so missing translations are
discoverable during development.
In `@rust_tray/src/lib.rs`:
- Around line 531-538: The match's default arm silently returns on unknown
notification types; change `_ => return` to emit a warning log including the
unexpected notification type so failures are discoverable. Locate the match that
builds (title, text, notification::NotificationIcon::Normal) (the arm handling
`3` uses i18n::get_string_fmt and i18n::get_string with app_name_str) and
replace the silent return with a log call (e.g., log::warn! or tracing::warn!)
that includes the unmatched value or enum variant, then return or handle as
before.
In `@rust_tray/src/menu_items.rs`:
- Around line 269-291: execute_handler currently calls get_all_items() on every
invocation which allocates a new Vec each time; change to cache the menu
definitions in a static lazy variable (e.g., use once_cell::sync::Lazy or
OnceCell) so execute_handler looks up items from a static ALL_ITEMS:
Lazy<Vec<MenuItemInfo>> (initialized with get_all_items) and then performs the
same find/handler/trigger_action_for_id logic against ALL_ITEMS instead of
calling get_all_items() repeatedly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38de3456-3cbb-49fc-8c9c-4250e84b204e
📒 Files selected for processing (20)
.github/workflows/main.ymlcmake/compile_definitions/common.cmakecmake/compile_definitions/windows.cmakecmake/targets/rust_tray.cmakerust_tray/.cargo/config.tomlrust_tray/.gitignorerust_tray/Cargo.tomlrust_tray/README.mdrust_tray/build.rsrust_tray/include/rust_tray.hrust_tray/plan.mdrust_tray/src/actions.rsrust_tray/src/dialogs.rsrust_tray/src/i18n.rsrust_tray/src/lib.rsrust_tray/src/menu.rsrust_tray/src/menu_items.rsrust_tray/src/notification.rssrc/system_tray.hsrc/system_tray_rust.cpp
| DEPENDS | ||
| ${RUST_TRAY_SOURCE_DIR}/Cargo.toml | ||
| ${RUST_TRAY_SOURCE_DIR}/build.rs | ||
| ${RUST_TRAY_SOURCE_DIR}/src/lib.rs | ||
| ${RUST_TRAY_SOURCE_DIR}/src/i18n.rs | ||
| ${RUST_TRAY_SOURCE_DIR}/src/actions.rs | ||
| VERBATIM |
There was a problem hiding this comment.
DEPENDS 列表不完整，可能导致增量构建失效
根据 lib.rs 中声明的模块，src/ 目录下还有 dialogs.rs、menu.rs、menu_items.rs 和 notification.rs，但这些文件未包含在依赖列表中。当这些文件修改时，CMake 不会触发重新构建。
🔧 建议的修复方案
DEPENDS
${RUST_TRAY_SOURCE_DIR}/Cargo.toml
${RUST_TRAY_SOURCE_DIR}/build.rs
${RUST_TRAY_SOURCE_DIR}/src/lib.rs
${RUST_TRAY_SOURCE_DIR}/src/i18n.rs
${RUST_TRAY_SOURCE_DIR}/src/actions.rs
+ ${RUST_TRAY_SOURCE_DIR}/src/dialogs.rs
+ ${RUST_TRAY_SOURCE_DIR}/src/menu.rs
+ ${RUST_TRAY_SOURCE_DIR}/src/menu_items.rs
+ ${RUST_TRAY_SOURCE_DIR}/src/notification.rs
VERBATIM或者使用 file(GLOB ...) 自动收集所有 .rs 文件（但需注意 GLOB 在新增文件时的限制）。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmake/targets/rust_tray.cmake` around lines 63 - 69, The DEPENDS list for the
rust tray target is incomplete: files referenced by lib.rs (dialogs.rs, menu.rs,
menu_items.rs, notification.rs) are missing so CMake won't re-run when they
change; update the DEPENDS block that references
${RUST_TRAY_SOURCE_DIR}/Cargo.toml, build.rs, src/lib.rs, src/i18n.rs,
src/actions.rs to also include ${RUST_TRAY_SOURCE_DIR}/src/dialogs.rs,
${RUST_TRAY_SOURCE_DIR}/src/menu.rs, ${RUST_TRAY_SOURCE_DIR}/src/menu_items.rs
and ${RUST_TRAY_SOURCE_DIR}/src/notification.rs (or replace the explicit list by
a file(GLOB ...) over ${RUST_TRAY_SOURCE_DIR}/src/*.rs if you prefer
auto-collecting files, noting GLOB caveats) so changes to those modules trigger
incremental rebuilds.
| // Platform-specific linker flags | ||
| #[cfg(target_os = "windows")] | ||
| { | ||
| println!("cargo:rustc-link-lib=user32"); | ||
| println!("cargo:rustc-link-lib=gdi32"); | ||
| println!("cargo:rustc-link-lib=shell32"); | ||
| println!("cargo:rustc-link-lib=ole32"); | ||
| println!("cargo:rustc-link-lib=comctl32"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 验证 CMake 中定义的平台库是否与 build.rs 重复
echo "=== CMake RUST_TRAY_PLATFORM_LIBS ==="
rg -A 20 'RUST_TRAY_PLATFORM_LIBS' cmake/targets/rust_tray.cmake
echo ""
echo "=== build.rs link-lib ==="
cat rust_tray/build.rsRepository: AlkaidLab/foundation-sunshine
Length of output: 1734
build.rs 中的 Windows 库链接不完整且存在重复
build.rs 中声明的 Windows 库集合与 CMake 配置不一致。虽然 build.rs 中的 5 个库（user32、gdi32、shell32、ole32、comctl32）确实在 CMake 中也被链接，形成重复，但更重要的问题是 build.rs 缺少 CMake 中指定的 7 个库：oleaut32、uuid、bcrypt、ntdll、userenv、ws2_32、pthread。
当通过 cargo 独立构建此 Rust 库时，将缺少这些关键依赖，可能导致链接或运行时失败。建议：
- 更新
build.rs以包含 CMake 中定义的所有库，或 - 明确记录两者适用的场景（如果设计上有意区分）
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust_tray/build.rs` around lines 14 - 22, 在 build.rs 的 Windows
平台块（#[cfg(target_os = "windows")] 内使用 println!("cargo:rustc-link-lib=...")
的地方，补上 CMake 中缺失的库：添加 println! 行分别为 oleaut32、uuid、bcrypt、ntdll、userenv、ws2_32 和
pthread（同时保留已存在的
user32、gdi32、shell32、ole32、comctl32，避免重复新增相同项）；如果这些差异是有意为之，请改为在该模块或仓库 README
中明确记录何时使用 build.rs 链接而何时依赖 CMake，以避免构建时缺少依赖。
| /** | ||
| * @brief Enable dark mode for context menus (follow system setting) | ||
| * | ||
| * Call this before creating menus. The menu will automatically | ||
| * follow the system's dark/light mode setting. | ||
| * Note: Only effective on Windows 10 1903+ and Windows 11. | ||
| */ | ||
| void tray_enable_dark_mode(void); | ||
|
|
||
| /** | ||
| * @brief Force dark mode for context menus | ||
| */ | ||
| void tray_force_dark_mode(void); | ||
|
|
||
| /** | ||
| * @brief Force light mode for context menus | ||
| */ | ||
| void tray_force_light_mode(void); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 搜索深色模式函数的实现
echo "=== Searching for dark mode function implementations ==="
rg -n "tray_enable_dark_mode|tray_force_dark_mode|tray_force_light_mode" --type rust
echo ""
echo "=== Checking for #[no_mangle] exports ==="
rg -n "#\[no_mangle\]" rust_tray/src/ -A 2 | grep -E "(dark_mode|light_mode)"Repository: AlkaidLab/foundation-sunshine
Length of output: 172
🏁 Script executed:
# Check project structure
find rust_tray -type f -name "*.c" -o -name "*.cpp" -o -name "*.h" | head -20
# Search more broadly for function names (including C files)
echo "=== Searching all source files ==="
rg -n "enable_dark_mode|force_dark_mode|force_light_mode" --type c --type cpp
# Check if header file references these from elsewhere
echo ""
echo "=== Checking if functions are mentioned anywhere ==="
rg -n "tray_enable_dark_mode|tray_force_dark_mode|tray_force_light_mode" -iRepository: AlkaidLab/foundation-sunshine
Length of output: 597
🏁 Script executed:
# List all files in rust_tray/src to understand the structure
echo "=== Files in rust_tray/src ==="
find rust_tray/src -type f -name "*.rs"
# Check all #[no_mangle] pub extern "C" exports to see what's implemented
echo ""
echo "=== All #[no_mangle] exports in Rust ==="
rg -n "#\[no_mangle\]" -A 1 rust_tray/src/
# Check the full lib.rs to see if there are any mode-related functions
echo ""
echo "=== Checking for any mode-related functions (case-insensitive) ==="
rg -i "mode|theme|dark|light" rust_tray/src/lib.rs -nRepository: AlkaidLab/foundation-sunshine
Length of output: 1438
🏁 Script executed:
# Check the total count of #[no_mangle] exports to be comprehensive
echo "=== Total count of #[no_mangle] functions ==="
rg -c "#\[no_mangle\]" rust_tray/src/lib.rs
# Get a list of all exported function names
echo ""
echo "=== All exported C function names ==="
rg "#\[no_mangle\]" -A 2 rust_tray/src/lib.rs | grep "pub.*extern" | sed 's/.*fn //' | sed 's/(.*//'Repository: AlkaidLab/foundation-sunshine
Length of output: 316
rust_tray.h 中声明的深色模式函数缺少实现
头文件在第 149、154、159 行声明了三个函数，但它们在 lib.rs 中均未实现：
tray_enable_dark_modetray_force_dark_modetray_force_light_mode
实际的导出函数（通过 #[no_mangle] pub extern "C"）仅包含 9 个函数：tray_init_ex、tray_loop、tray_exit、tray_set_icon、tray_set_tooltip、tray_update_vdd_menu、tray_set_locale、tray_show_notification、tray_show_localized_notification。这些声明的函数需要实现以避免链接错误。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust_tray/include/rust_tray.h` around lines 142 - 159,
头文件声明的三个深色模式函数（tray_enable_dark_mode、tray_force_dark_mode、tray_force_light_mode）在
lib.rs 中未导出，导致链接错误；为修复，在 lib.rs 中为每个函数添加对应的导出实现（使用 #[no_mangle] pub extern "C"
fn tray_enable_dark_mode(), #[no_mangle] pub extern "C" fn
tray_force_dark_mode(), #[no_mangle] pub extern "C" fn
tray_force_light_mode()），实现内部逻辑（可以调用现有初始化或平台函数以切换主题，或暂时做空实现以避免崩溃），并确保命名与头文件完全一致以正确导出符号。
| 运行时与 API 变化： | ||
| - 初始化：推荐使用 `tray_init_ex(icon_normal, icon_playing, icon_pausing, icon_locked, tooltip, locale, callback)`。 | ||
| - 事件循环：使用 `tray_loop(blocking)` 驱动；返回 -1 表示要求退出。可在单线程或分线程中调用（包装器提供线程化入口）。 | ||
| - 运行时更新：`tray_set_icon`、`tray_set_tooltip`、`tray_set_vdd_checked`、`tray_set_vdd_enabled`、`tray_set_locale`、`tray_show_notification`。 |
There was a problem hiding this comment.
API 文档与实际接口不一致
文档中提到的 tray_set_vdd_checked 和 tray_set_vdd_enabled 函数在 rust_tray/include/rust_tray.h 中并不存在。根据头文件，实际的 API 是 tray_update_vdd_menu(int can_create, int can_close, int is_persistent, int is_active)。
建议更新文档以反映实际的 API 接口。
📝 建议的修改
-- 运行时更新：`tray_set_icon`、`tray_set_tooltip`、`tray_set_vdd_checked`、`tray_set_vdd_enabled`、`tray_set_locale`、`tray_show_notification`。
+- 运行时更新：`tray_set_icon`、`tray_set_tooltip`、`tray_update_vdd_menu`、`tray_set_locale`、`tray_show_notification`、`tray_show_localized_notification`。📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - 运行时更新：`tray_set_icon`、`tray_set_tooltip`、`tray_set_vdd_checked`、`tray_set_vdd_enabled`、`tray_set_locale`、`tray_show_notification`。 | |
| - 运行时更新：`tray_set_icon`、`tray_set_tooltip`、`tray_update_vdd_menu`、`tray_set_locale`、`tray_show_notification`、`tray_show_localized_notification`。 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust_tray/plan.md` at line 29, The doc lists non-existent APIs
tray_set_vdd_checked and tray_set_vdd_enabled; update the runtime-updates line
to match the real header API by replacing those entries with the actual function
name and signature tray_update_vdd_menu(int can_create, int can_close, int
is_persistent, int is_active), and ensure the runtime update list includes
tray_set_icon, tray_set_tooltip, tray_update_vdd_menu, tray_set_locale, and
tray_show_notification so the documentation matches
rust_tray/include/rust_tray.h.
| - Cross-platform support (Windows, Linux, macOS) | ||
| - Compatible C API matching the original `tray.h` header | ||
| - Uses the `tray-icon` Rust crate for the underlying implementation | ||
| - Better maintainability and reduced cross-platform adaptation code |
There was a problem hiding this comment.
README 仍把当前实现写成跨平台。
rust_tray/src/dialogs.rs 和 rust_tray/src/notification.rs 已经直接依赖 Windows API / WinRT，rust_tray/Cargo.toml 也只在 cfg(windows) 下声明了这部分依赖，但这里仍写着 Linux/macOS 支持并列出对应产物路径。建议改成明确的 Windows-only 说明，或者把尚未实现的前提条件写清楚，避免误导手动构建和集成。
Also applies to: 28-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust_tray/README.md` around lines 7 - 10, The README currently claims
cross-platform support but the implementation files rust_tray/src/dialogs.rs and
rust_tray/src/notification.rs and the Cargo.toml only enable Windows-specific
APIs/dependencies; update README.md to explicitly state this crate is
Windows-only (or clearly document that Linux/macOS are not yet implemented and
list required preconditions for those platforms), remove or mark as Windows-only
any Linux/macOS artifact paths, and update the corresponding lines referenced
(including lines 28-31) to avoid implying full cross-platform support; reference
rust_tray/src/dialogs.rs, rust_tray/src/notification.rs, and
rust_tray/Cargo.toml when making the README changes.
| // Safety: TrayState is only accessed from the main thread | ||
| unsafe impl Send for TrayState {} | ||
| unsafe impl Sync for TrayState {} |
There was a problem hiding this comment.
线程安全假设需要文档说明或运行时保证
TrayState 的 unsafe impl Send 和 unsafe impl Sync 依赖于"仅从主线程访问"的假设。然而，这个约束仅通过注释说明，没有编译时或运行时保证。
如果 C++ 端从多个线程调用这些 FFI 函数，可能会导致未定义行为。建议：
- 在 C 头文件中明确记录线程安全约束
- 或者考虑添加运行时检查（如
std::thread::current().id()检查）
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust_tray/src/lib.rs` around lines 47 - 49, The unsafe impls for TrayState
rely on the comment "only accessed from the main thread" but offer no
enforcement; update the code to either document this constraint in the C header
and FFI docs (clearly state that all FFI calls touching TrayState must be made
from the main thread) and/or add a runtime guard: record std::thread::ThreadId
(e.g., main_thread_id) in TrayState at initialization and validate it at the
start of every extern "C" function that accesses TrayState (return an error or
abort if the current thread id differs). Reference the unsafe impl Send/Sync for
TrayState and the FFI entry points that use TrayState to locate where to add the
ThreadId field and per-call checks.
| // Thread-local storage for actual muda items (not thread-safe) | ||
| thread_local! { | ||
| static CREATED_ITEMS: std::cell::RefCell<HashMap<String, CreatedItem>> = | ||
| std::cell::RefCell::new(HashMap::new()); | ||
| } |
There was a problem hiding this comment.
CREATED_ITEMS 会让跨线程菜单更新静默失效。
这些 setter 只能看到创建菜单那个线程里的 item，但 src/system_tray_rust.cpp 的 Line 93-105 会在分离线程里再次调用 tray_update_vdd_menu()。10 秒冷却结束后的那次更新会落在空的 TLS 上直接 no-op，VDD 菜单就可能一直停留在 cooldown 状态。这里需要把状态变更封送回 tray/UI 线程再应用。
Also applies to: 64-96, 202-226
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust_tray/src/menu.rs` around lines 24 - 28, CREATED_ITEMS 是线程局部
(thread_local!)，导致在非创建线程（如 tray_update_vdd_menu() 被分离线程调用时）看不到状态更新并
no-op；将状态修改封送回 tray/UI 线程。改法：替换或补充 CREATED_ITEMS 的 TLS 存储，改用全局线程安全结构（例如
lazy_static/once_cell + Arc<Mutex<HashMap<...>>> 或者锁/并发 map）或在所有 setter 中通过一个跨线程
channel 将“变更事件/闭包”发送到 UI/tray 线程并在 tray_update_vdd_menu() 或 UI
事件循环处消费并应用这些变更；确保所有读取/写入点（涉及 CREATED_ITEMS、相关 setter）改为使用该线程安全通道/全局结构以保证跨线程更新生效。
| void terminate_gui_processes() { | ||
| #ifdef _WIN32 | ||
| HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); | ||
| if (snapshot != INVALID_HANDLE_VALUE) { | ||
| PROCESSENTRY32W pe32; | ||
| pe32.dwSize = sizeof(PROCESSENTRY32W); | ||
|
|
||
| if (Process32FirstW(snapshot, &pe32)) { | ||
| do { | ||
| if (wcscmp(pe32.szExeFile, L"sunshine-gui.exe") == 0) { | ||
| HANDLE process_handle = OpenProcess(PROCESS_TERMINATE, FALSE, pe32.th32ProcessID); | ||
| if (process_handle != NULL) { | ||
| TerminateProcess(process_handle, 0); | ||
| CloseHandle(process_handle); | ||
| } | ||
| } | ||
| } while (Process32NextW(snapshot, &pe32)); | ||
| } | ||
| CloseHandle(snapshot); | ||
| } |
There was a problem hiding this comment.
不要按进程名全局强杀 sunshine-gui.exe。
这里会终止系统里所有同名进程，包含其他用户会话、手动启动的实例，甚至未来可能的第二个 Sunshine 实例。退出托盘时最好只结束当前进程实际拉起的 GUI PID，至少再按 SessionId、父进程或可执行路径做过滤。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/system_tray_rust.cpp` around lines 186 - 205, terminate_gui_processes
currently force-kills every process named "sunshine-gui.exe"; instead restrict
termination to only the GUI instances our process actually launched or those in
the same session. Update terminate_gui_processes to prefer using a known GUI PID
(e.g., stored gui_pid) and only call OpenProcess/TerminateProcess for that PID;
if the PID is not available, filter process entries by SessionId (use
ProcessIdToSessionId and compare to the current session), and further verify the
process image path matches the expected executable (use
QueryFullProcessImageNameW) and/or that the parent PID matches our process (via
CreateToolhelp32Snapshot/PROCESSENTRY32W parent check) before calling
OpenProcess/TerminateProcess; keep the current
CreateToolhelp32Snapshot/PROCESSENTRY32W loop but add these checks to avoid
killing unrelated instances.
| int process_tray_events() { | ||
| if (!tray_initialized) { | ||
| return -1; | ||
| } | ||
| return tray_loop(0); // Non-blocking | ||
| } |
There was a problem hiding this comment.
非阻塞 tray_loop(0) 会把事件循环变成忙轮询。
process_tray_events() 在无消息时仍返回 0，而本文件的 Line 284-285 和 src/main.cpp 的 Line 110-117 都在无休眠的 while 里反复调用它。空闲时会持续占用 CPU。若已经有独立 tray 线程，建议改成阻塞模式；否则至少在无消息时 sleep_for / yield。
Also applies to: 279-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/system_tray_rust.cpp` around lines 252 - 257, process_tray_events
currently calls tray_loop(0) which busy-polls; change it so that if the tray
runs on its own thread you call the blocking form of the loop (use tray_loop
with the blocking argument, e.g., tray_loop(-1) or the API's blocking mode)
inside process_tray_events when tray_initialized indicates a dedicated tray
thread, otherwise keep non-blocking tray_loop(0) but detect the "no events"
return value and call std::this_thread::sleep_for(std::chrono::milliseconds(n))
or std::this_thread::yield() before returning to avoid busy-wait; update callers
that repeatedly call process_tray_events (they use process_tray_events in tight
loops) to rely on the blocking mode when appropriate or accept the short
sleep/yield when using non-blocking mode.
| void update_tray_vmonitor_checked(int checked) { | ||
| if (!tray_initialized) return; | ||
| // Use the unified VDD menu update function | ||
| update_vdd_menu_state(); | ||
| } |
There was a problem hiding this comment.
checked 参数在这里被完全忽略了。
这个接口名和签名都表示调用方可以显式更新复选框状态，但实现只是重新计算整套 VDD 状态并无视传入值。若这里本意只是“刷新当前状态”，建议删除这个参数并重命名接口；否则应该按参数更新对应菜单项。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/system_tray_rust.cpp` around lines 340 - 344, 函数
update_tray_vmonitor_checked 的 checked 参数被忽略：你要么把它改为真正使用传入的 checked
值去设置对应的菜单项（在函数内调用更新单个 VMonitor 复选框的逻辑，避免仅调用 update_vdd_menu_state），要么将函数签名去掉
checked 并重命名为 refresh_tray_vmonitor 或 similar 表示仅刷新当前状态；更新调用处以匹配新签名并确保唯一定位符为
update_tray_vmonitor_checked / update_vdd_menu_state。
Summary by CodeRabbit
发行说明
新功能
改进