✨ feat: Discord RPC 插件分离 #682
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Summary of ChangesHello @imsyy, 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! 此拉取请求对 Discord Rich Presence (RPC) 的实现进行了重大重构，将其核心逻辑从 Rust 原生模块迁移到 Electron 主进程中的 TypeScript。这一改变旨在简化 Discord RPC 的管理，使其能够更灵活地直接从 JavaScript 端控制，同时原生模块现在专注于 Windows 上的系统媒体传输控件 (SMTC) 功能。此外，还引入了一个新的设置项，允许用户独立控制原生 SMTC 的启用状态。 Highlights
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
此次 Pull Request 成功地将 Discord RPC 功能从 Rust 重构到了 TypeScript，这简化了原生模块，并将相关逻辑集中到了 Electron 主进程中。新的 DiscordRpcManager 结构清晰，很好地处理了客户端生命周期和活动状态更新。
我发现了一些可以改进的地方：
- 在
MediaSessionManager中存在一个高严重性缺陷，它导致在 Windows 上禁用原生 SMTC 后，无法回退使用标准的navigator.mediaSession。 ipc-smtc.ts中存在一些重复代码，可以通过重构来提高可维护性。- 新的 Discord RPC 实现中有一个小逻辑问题，可能导致按钮链接不正确。
- 通过为 Discord RPC 元数据定义一个专用类型，可以提高类型安全性。
总的来说，这是一个很棒的重构。解决这些问题将使实现更加健壮和易于维护。
electron/main/utils/discord-rpc.ts
Outdated
| * @returns 歌曲链接 | ||
| */ | ||
| private processSongUrl(ncmId?: number): string | null { | ||
| if (!ncmId) return "https://music.163.com/"; |
There was a problem hiding this comment.
当 ncmId 不存在时，返回网易云音乐首页链接可能不是预期的行为。这会导致即使用户正在收听本地音乐或来自其他来源的音乐，Discord 上也会显示一个指向网易云音乐首页的按钮。更好的做法是在没有 ncmId 时返回 null，这样就不会显示“收听”按钮，从而提供更准确的用户体验。
| if (!ncmId) return "https://music.163.com/"; | |
| if (!ncmId) return null; |
src/types/global.d.ts
Outdated
| "discord-update-metadata": MetadataParam; | ||
| "discord-update-play-state": PlayStatePayload; | ||
| "discord-update-timeline": TimelinePayload; |
There was a problem hiding this comment.
为 discord-update-metadata 等 Discord 相关事件重用来自原生模块的 MetadataParam 类型可能会引起混淆，因为这两个接口的用途和所需字段不同。例如，Discord RPC 实现不需要 coverData，但原生 SMTC 需要。
为了提高类型安全性和代码清晰度，建议为 Discord RPC 的元数据、播放状态和时间线定义专门的类型（例如 DiscordMetadataParam），并在 IpcChannelMap 中使用它们。这将使接口意图更加明确，并与刚刚分离的 Discord RPC 逻辑保持一致。
|
/gemini review |
There was a problem hiding this comment.
Code Review
这次的 PR 主要将 Discord RPC 功能从原生 Rust 模块重构到了 Electron 主进程的 TypeScript 模块中。这是一个很好的架构改进，它简化了原生插件，使得 Discord 集成功能更容易维护。相关的改动，包括 IPC 处理、状态管理和构建脚本的更新，都组织得很好。
我发现了一个与新的 Discord RPC 配置逻辑相关的严重 bug，它会导致类型不匹配和运行时错误。另外，我还提出了一个小的改进建议，以便在重构后清理原生模块中不再需要的数据。
总的来说，这是一个不错的 PR，提升了代码库的质量。在修复了那个严重问题后，应该就可以合并了。
|
/gemini review |
There was a problem hiding this comment.
Code Review
这个 PR 很好地将 Discord RPC 功能从 SMTC 模块中分离出来，创建了一个新的原生模块。这对于代码的模块化和长期维护性来说是一个很棒的改进。代码结构更清晰，职责更分明。
我发现了一些可以改进的地方，主要集中在代码一致性和可读性上。例如，建议统一 IPC 通道命名，并对一些较长的函数进行重构。此外，我还发现了一个关于 Rust Edition 的配置问题，可能会导致编译失败，需要注意。
总的来说，这是一个高质量的重构，我的建议旨在让代码库更加健壮和易于维护。
|
/gemini review |
There was a problem hiding this comment.
Code Review
本次 PR 成功地将 Discord RPC 功能从 SMTC 模块中分离出来，创建了一个独立的 discord-rpc-for-splayer 原生模块。这次重构使得代码结构更清晰，权责更分明。同时，前端代码也进行了相应的适配，引入了更通用的 IPC 处理函数，并调整了相关设置项。
我发现了一些可以改进的地方，主要集中在新的 Rust 原生模块中，包括使用了不稳定的 Rust edition、一个潜在的逻辑 bug 和资源管理方面的问题。请查看具体的 review comments。
| if let Some(last_end) = last_sent_end_timestamp { | ||
| let diff = (*last_end - end).abs(); | ||
| if diff < TIMESTAMP_UPDATE_THRESHOLD_MS { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
时间戳更新节流的逻辑似乎存在问题。diff 是以秒为单位计算的，但它与 TIMESTAMP_UPDATE_THRESHOLD_MS（值为100，且从命名上看单位应为毫秒）进行比较。这导致了一个100秒的阈值，太大了，会妨碍在拖动进度条后及时更新状态。
建议将阈值调整为一个较小的值（例如2秒），以正确检测跳转操作。
| if let Some(last_end) = last_sent_end_timestamp { | |
| let diff = (*last_end - end).abs(); | |
| if diff < TIMESTAMP_UPDATE_THRESHOLD_MS { | |
| return true; | |
| } | |
| if let Some(last_end) = last_sent_end_timestamp { | |
| let diff = (*last_end - end).abs(); | |
| // Using a smaller threshold (e.g., 2 seconds) to correctly detect seeks. | |
| if diff < 2 { | |
| return true; | |
| } | |
| pub fn shutdown() { | ||
| discord_core::disable(); | ||
| } |
There was a problem hiding this comment.
shutdown 函数目前调用 discord_core::disable()，但这仅向后台线程发送一条消息，并不会终止它，可能导致资源泄漏。建议实现一个真正的关闭逻辑来停止线程。
这可以通过在 discord_core.rs 中添加一个 shutdown 函数来销毁 Sender 实现，这将导致后台线程的 recv() 调用失败并退出循环。
例如，在 discord_core.rs 中添加：
pub fn shutdown() {
if let Ok(mut guard) = SENDER.lock() {
if guard.take().is_some() {
info!("Shutting down Discord RPC thread.");
}
}
}然后在 lib.rs 中调用这个新函数。
| pub fn shutdown() { | |
| discord_core::disable(); | |
| } | |
| pub fn shutdown() { | |
| discord_core::shutdown(); | |
| } | |
| } | ||
|
|
||
| fn build_base_activity(data: &ActivityData, display_mode: DiscordDisplayMode) -> Activity<'_> { | ||
| fn build_base_activity(data: &ActivityData, _display_mode: DiscordDisplayMode) -> Activity<'_> { |
There was a problem hiding this comment.
参数 _display_mode 被标记为未使用。由于 StatusDisplayType 在当前版本的 discord-rich-presence 中已不再是 Activity 的一部分，这个参数不再需要，可以从函数签名中移除，以提高代码的清晰度。
| fn build_base_activity(data: &ActivityData, _display_mode: DiscordDisplayMode) -> Activity<'_> { | |
| fn build_base_activity(data: &ActivityData) -> Activity<'_> { | |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Discord Rich Presence (RPC) functionality into a new, dedicated native Rust module named discord-rpc-for-splayer, separating it from the existing smtc-for-splayer module. The Cargo.lock and Cargo.toml files are updated to reflect the new module and its dependencies, including a version downgrade for discord-rich-presence and the addition of numerous icu_* related packages. The electron/main/ipc/ipc-smtc.ts file is modified to load and manage both native modules independently, introducing a generic handler for IPC communication and distinct channels for Discord RPC. The new discord-rpc-for-splayer module includes its own NAPI bindings, build scripts, TypeScript definitions, and core Rust logic for managing Discord activity, with improvements to cover asset handling (specifically for Netease Cloud Music) and timestamp calculations. The smtc-for-splayer module is streamlined by removing its direct Discord RPC dependencies and related code. On the frontend, the PlaySetting.vue component's SMTC toggle is moved to ThirdSetting.vue, and a new setting enableNativeSmtc is introduced for Windows users to control native SMTC support. The MediaSessionManager.ts and PlayerController.ts files are updated to conditionally send Discord RPC updates via the new module, and the PlayerIpc.ts file now includes new functions for Discord RPC communication. The src/types/global.d.ts and src/types/smtc.ts files are adjusted to reflect the new Discord RPC types and enum changes. A review comment suggests changing the Rust edition from 2024 to 2021 in the new discord-rpc-for-splayer/Cargo.toml due to the instability of Rust 2024.
| [package] | ||
| name = "discord-rpc-for-splayer" | ||
| version = "0.1.0" | ||
| edition = "2024" |
There was a problem hiding this comment.
edition = "2024" 的使用存在风险。Rust 2024 版本目前还不稳定，只在 nightly 工具链中可用。这可能会导致使用 stable Rust 的用户在编译时遇到问题。建议在 2024 版本稳定之前，继续使用最新的稳定版本 "2021"。
| edition = "2024" | |
| edition = "2021" |
感觉是有点问题，还是放上来和原来的比较一下