Skip to content

fix(player): 记忆桌面端播放器音量#2278

Open
liangyuR wants to merge 3 commits into
Predidit:mainfrom
liangyuR:fix/persist-desktop-volume
Open

fix(player): 记忆桌面端播放器音量#2278
liangyuR wants to merge 3 commits into
Predidit:mainfrom
liangyuR:fix/persist-desktop-volume

Conversation

@liangyuR

Copy link
Copy Markdown
Contributor

问题

桌面端每次打开视频时,播放器音量都会重置为最大(100),不会记住上次的设置。

方案

  • 新增设置项 SettingsKeys.defaultVolumedouble,默认 100.0,归属 SettingGroup.player),用于持久化桌面端音量。
  • PlayerController 中新增 _persistDesktopVolume(),在 setVolume 及音量初始化时把当前音量写入设置(仅桌面端,做范围裁剪与去抖,避免重复写入)。
  • init() 中桌面端默认音量由硬编码 100 改为读取 SettingsKeys.defaultVolume,从而在会话之间记忆音量。

测试

  • 新增 test/settings_default_volume_test.dart 单元测试,覆盖默认值、分组、全局注册与持久化键名。
  • flutter analyze --no-fatal-infos --fatal-warnings:无问题。
  • flutter test test/settings_default_volume_test.dart:全部通过。

@liangyuR liangyuR marked this pull request as ready for review June 27, 2026 06:03
@liangyuR liangyuR marked this pull request as draft June 27, 2026 06:04
Co-authored-by: Cursor <cursoragent@cursor.com>
@kilo-code-bot

kilo-code-bot Bot commented Jun 27, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

This incremental review covers the new desktop mute feature added since the previous review. toggleMute sets muted = true before setVolume(0), so _persistDesktopVolume correctly skips writing 0 to defaultVolume; _clearMuteIfNeeded only clears mute on a positive volume, so muting does not self-cancel. On a fresh desktop start, init() restores muted and seeds _preMuteVolume from the remembered defaultVolume, which is preserved because writes are skipped while muted. The removed lastVolume field has no remaining references, and the new SettingsKeys.playerMuted is registered in both all and the player group. No issues found in the changed code.

Files Reviewed (3 files)
  • lib/pages/player/player_controller.dart
  • lib/pages/player/player_item.dart
  • lib/services/storage/settings_keys.dart
Previous Review Summaries (2 snapshots, latest commit b787532)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit b787532)

Status: No Issues Found | Recommendation: Merge

The change persists the desktop player volume via the new SettingsKeys.defaultVolume setting. The setting is registered in both all and the player group, init() seeds the volume from storage, and the isDesktop() + rounded-equality guards correctly debounce writes. The previously added test/settings_default_volume_test.dart was removed in the latest commit; this deletion-only change introduces no issues.

Files Reviewed (2 files)
  • lib/pages/player/player_controller.dart
  • lib/services/storage/settings_keys.dart

Previous review (commit f0ef8f9)

Status: No Issues Found | Recommendation: Merge

The change cleanly persists the desktop player volume via a new SettingsKeys.defaultVolume setting. Verified that double.clamp(0.0, 100.0) statically returns double, getSetting<double> returns the typed value so .round() is valid, and the isDesktop() + rounded-equality guards correctly debounce writes. The setting is registered in both all and the player group, and init() now seeds the volume from storage.

Files Reviewed (3 files)
  • lib/pages/player/player_controller.dart
  • lib/services/storage/settings_keys.dart
  • test/settings_default_volume_test.dart

Reviewed by claude-4.8-opus-20260528 · Input: 3.3K · Output: 7.8K · Cached: 327.8K

@liangyuR liangyuR marked this pull request as ready for review June 27, 2026 06:06
@Predidit

Copy link
Copy Markdown
Owner

这个功能叠加静音快捷键 M 使用时的行为有些奇怪

静音状态没有持久化,这样在重启之后,静音前的音量丢失,使用 M 将不能恢复静音,只能重新调剂音量

@liangyuR

Copy link
Copy Markdown
Contributor Author

那么 保存静音 状态来解决这个问题? 我看到静音的实现是存了 lastX,然后将音量设置为 0

@Predidit

Copy link
Copy Markdown
Owner

除了持久化静音状态,也应该持久化静音前的音量

Co-authored-by: 文质彬彬的大锤 <liangyuR@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants