Conversation
Summary of ChangesHello @kazukokawagawa, 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 introduces a user-configurable 'Fuck AI Mode' feature, providing users with the option to streamline their audio quality choices by removing certain advanced, AI-enhanced audio formats from various selection interfaces. The primary goal is to offer a cleaner, more focused audio experience for those who prefer to exclude these specific high-fidelity options, while still allowing for Dolby Atmos. This change impacts how audio quality options are presented and handled throughout the application, from download selections to playback settings. 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
This pull request introduces a feature to filter out certain AI-enhanced audio quality options. While the functionality is a good addition, the implementation has some issues with code duplication and maintainability. Specifically, the list of 'AI' audio levels is hardcoded in multiple files, and some logic is redundant or overly complex. I've left specific comments with suggestions for improvement. Also, the user-facing name for this feature should be made more professional.
| <n-text class="name">Fuck AI Mode</n-text> | ||
| <n-text class="tip" :depth="3"> | ||
| 开启后在所有的地方都不显示 Hi-res以上的音质选项（高清环绕声、沉浸环绕声、超清母带）保留杜比全景声 | ||
| </n-text> |
There was a problem hiding this comment.
The feature name 'Fuck AI Mode' is unprofessional and not suitable for user-facing text. Please consider renaming it to something more descriptive and professional, for example:
<n-text class="name">过滤 AI 音质</n-text>
<n-text class="tip" :depth="3">
开启后将隐藏部分 AI 增强音质选项（如超清母带、沉浸环绕声等），但会保留杜比全景声。
</n-text>
| if (disableAiAudio) { | ||
| if (typeof song === "object" && song) { | ||
| // 如果已启用 Fuck AI，且包含 level | ||
| if ("level" in song) { | ||
| const level = song.level; | ||
| // 如果是 AI 音质，且不是 杜比，则返回 HiRe 或 SQ/HQ | ||
| if (["jymaster", "sky", "jyeffect", "vivid"].includes(level)) { | ||
| // 尝试降级显示 | ||
| return QualityType.HiRes; | ||
| } | ||
| // 其他音质正常显示 | ||
| } | ||
| // 云盘歌曲适配 | ||
| if ("privilege" in song) { | ||
| const privilege = song.privilege; | ||
| let level = privilege?.playMaxBrLevel ?? privilege?.plLevel; | ||
|
|
||
| if (["jymaster", "sky", "jyeffect", "vivid"].includes(level)) { | ||
| level = "hires"; | ||
| } | ||
| const quality = levelQualityMap[level]; | ||
| if (quality) return quality; | ||
| } | ||
| } | ||
| // 数组遍历检查时也跳过 AI 音质 key | ||
| } | ||
|
|
||
|
|
||
| if (typeof song === "object" && song) { | ||
| // 含有 level 特殊处理 | ||
| if ("level" in song) { | ||
| const quality = levelQualityMap[song.level]; | ||
| // 再次检查 (如果上面没拦截住或者不想太复杂) | ||
| let quality = levelQualityMap[song.level]; | ||
| if (disableAiAudio && ["Master", "Spatial", "Surround"].includes(quality)) { | ||
| // 强制降级显示或者不返回? 这里返回 HiRes 比较合理，因为文件可能还是好的，只是标变了 | ||
| return QualityType.HiRes; | ||
| } | ||
| if (quality) return quality; | ||
| } | ||
| // 云盘歌曲适配 | ||
| if ("privilege" in song) { | ||
| const privilege = song.privilege; | ||
| const quality = levelQualityMap[privilege?.playMaxBrLevel] | ||
| ?? levelQualityMap[privilege?.plLevel]; | ||
| if (disableAiAudio && ["Master", "Spatial", "Surround"].includes(quality)) { | ||
| return QualityType.HiRes; | ||
| } | ||
| if (quality) return quality; | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for filtering 'AI' qualities is spread out and contains redundancies. For example, the check on lines 459-461 is redundant with the 'Fuck AI Filter' block on lines 426-451. This makes the code hard to follow and maintain.
Please consolidate all the filtering logic into a single, clear block at the beginning of the function. This will improve readability and prevent potential bugs from inconsistent checks.
Additionally, the hiddenLevels array is hardcoded in multiple places within this function. It should be imported from a shared constant.
| if (settingStore.disableAiAudio) { | ||
| return allData.filter((item) => { | ||
| // item.level is "dolby", "jymaster" etc. | ||
| // item.value is "db", "jm" etc. | ||
| if (item.level === "dolby") return true; | ||
| const hiddenLevels = ["jymaster", "sky", "jyeffect", "vivid"]; | ||
| return !hiddenLevels.includes(item.level); | ||
| }).map((item) => ({ | ||
| label: item.name, | ||
| value: item.value, | ||
| size: undefined, | ||
| })); | ||
| } |
There was a problem hiding this comment.
The .map() call is duplicated for both cases (when disableAiAudio is true and false). You can simplify this by first determining the list of items to be mapped, and then calling .map() once.
Also, the hiddenLevels array is hardcoded here. This list is used across multiple files and should be extracted into a shared constant to avoid duplication and improve maintainability.
| if (settingStore.disableAiAudio) { | ||
| availableQualities.value = levels.filter(q => { | ||
| if (q.level === "dolby") return true; | ||
| return !["jymaster", "sky", "jyeffect", "vivid"].includes(q.level); |
There was a problem hiding this comment.
The list of hidden levels ['jymaster', 'sky', 'jyeffect', 'vivid'] is hardcoded here. This list is used in multiple files. To improve maintainability, please define it as a constant in a shared file (e.g., src/utils/meta.ts) and import it here.
| if (settingStore.disableAiAudio) { | ||
| return allData.filter((item) => { | ||
| if (item.level === "dolby") return true; | ||
| const hiddenLevels = ["jymaster", "sky", "jyeffect", "vivid"]; | ||
| return !hiddenLevels.includes(item.level); | ||
| }).map((item) => ({ | ||
| label: item.name, | ||
| value: item.value, | ||
| })); | ||
| } |
There was a problem hiding this comment.
Similar to DownloadModal.vue, the .map() call is duplicated here. This can be refactored to avoid repetition. The hiddenLevels array is also hardcoded and should be imported from a shared constant.
| const hiddenLevels = ["jymaster", "sky", "jyeffect", "vivid"]; | ||
| return !hiddenLevels.includes(option.value); |
There was a problem hiding this comment.
The hiddenLevels array is hardcoded here. It's also duplicated on line 586 and in several other files. This should be extracted to a shared constant (e.g., in src/utils/meta.ts) to improve maintainability.
| if (disableAiAudio && ["jm", "sk", "je"].includes(itemKey.key)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The list of keys ['jm', 'sk', 'je'] is hardcoded here. This list is directly related to the AI-enhanced audio levels that are being filtered. Instead of hardcoding it, this list should be derived from the shared constant for hidden levels and songLevelData to ensure consistency. For example, you could create a AI_AUDIO_LEVEL_KEYS constant alongside AI_AUDIO_LEVELS.
2 participants
No description provided.