Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

增加长按倍速的倍数设置功能 #1487

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

MasoFod
Copy link

@MasoFod MasoFod commented Jan 12, 2025

在播放器设置最下面设置了长按倍速的倍数设置功能。
用了很笨的方法,一步一步传参。

@Him188 Him188 self-requested a review January 12, 2025 12:05
Copy link
Member

@StageGuard StageGuard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以看一下 PlayerControllerState 是怎么从 view model 传递给 UI 的

/**
* 长按倍速的倍数
*/
val fastSkipTimes: Float = 3f,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

参数名不好,建议改为 fastSkipMultiplier

@@ -17,7 +17,7 @@ import me.him188.ani.app.ui.foundation.ProvideCompositionLocalsForPreview
@Composable
@Preview(widthDp = 1080 / 3, heightDp = 2400 / 3, showBackground = true)
@Preview(device = Devices.TABLET, showBackground = true)
internal fun PreviewEpisodePage() {
internal fun PreviewEpisodePage(fastSkipTimes: Float = 3f,) {
Copy link
Member

@StageGuard StageGuard Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

带有 @Preview 的预览函数不能有参数,看一下其他参数怎么传递的

Copy link
Member

@Him188 Him188 Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preview 直接写死 3f 就好了, 在下面 31 行写死 = 3f.

@@ -270,6 +272,12 @@ internal fun EpisodeVideoImpl(
onExitFullscreen = onExitFullscreen,
family = gestureFamily,
indicatorState,
fastSkipState = rememberPlayerFastSkipState(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这种情况下最好直接在 EpisodeViewModel 里构建 FaskSkipState,传递 FastSkipState 给 UI。

Copy link
Member

@Him188 Him188 Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我觉得目前没必要, 两种写法 equally ok

目前你改成 VM 存 state 反而会更麻烦 (因为调用链太长了). 得等重构完播放页的 UI 才比较好改成你说的.

)
},
onValueChangeCompleted = {
fun stringToFloat(str: String): Float? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

使用 toFloatOrNull

},
isErrorProvider = {
try {
it.toFloat()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

使用 toFloatOrNull

@@ -527,6 +528,38 @@ fun SettingsScope.PlayerGroup(
},
title = { Text("播放失败时自动切换资源") },
)
HorizontalDividerItem()
TextFieldItem(
Copy link
Member

@StageGuard StageGuard Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议使用 DropdownMenuItem 让用户从给定的几个值里选择,不超过 4 个预设可选值

Copy link
Member

@Him188 Him188 Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1x 2x 2.5x 3x 就可以了. 目前用户想要的都是 2x 或者 3x.

Comment on lines 28 to 36
fun rememberPlayerFastSkipState(
playerState: PlaybackSpeed,
gestureIndicatorState: GestureIndicatorState,
fastSkipTimes: Float,
): FastSkipState {
return remember(playerState) {
PlayerFastSkipState(playerState, gestureIndicatorState).fastSkipState
PlayerFastSkipState(playerState, gestureIndicatorState, fastSkipTimes).fastSkipState
}
}
Copy link
Member

@Him188 Him188 Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fastSkipTimes 是一个值, 而且没有在 rememebr key 里, 会导致用户修改设置后, 新的设置不会反映到 UI. 建议把 fastSkipTimes 改成 provider:

fun rememberPlayerFastSkipState(
    playerState: PlaybackSpeed,
    gestureIndicatorState: GestureIndicatorState,
    multiplierProvider: () -> Float,
): FastSkipState {
    return remember(playerState, multiplierProvider) {
        PlayerFastSkipState(playerState, gestureIndicatorState, multiplierProvider).fastSkipState
    }
}

(现有代码 gestureIndicatorState 也是没 remember 的, 这是一个 bug, 不过这个 PR 里不需要修复).

PlayerFastSkipState 里面每次使用都从 multiplierProvider 调用一下获取.

@Him188 Him188 added the s: player 子系统: 视频播放器 label Jan 12, 2025
@Him188 Him188 linked an issue Jan 12, 2025 that may be closed by this pull request
2 tasks
@@ -267,7 +267,7 @@ private fun AniAppContentImpl(
context,
)
}
EpisodeScene(vm, Modifier.fillMaxSize(), windowInsets)
EpisodeScene(vm, Modifier.fillMaxSize(), windowInsets, fastSkipTimes = vm.videoScaffoldConfig.fastSkipTimes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不要在这里增加这个参数,因为这个参数是包含在 vm 里的,已经传递了

@Him188
Copy link
Member

Him188 commented Jan 25, 2025

因为有段时间没有更新了, 我先改成 draft 了, 解决问题后欢迎重新标记为 ready

@Him188 Him188 marked this pull request as draft January 25, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: player 子系统: 视频播放器
Projects
None yet
Development

Successfully merging this pull request may close these issues.

增加可設置的播放速度
3 participants