Repository navigation
feat(player): 编辑弹幕时暂停播放 - #2711
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| required bool playing, | ||
| }) { | ||
| if (hasFocus) { | ||
| if (playing && _pausedByEdit) { |
There was a problem hiding this comment.
WARNING: Re-claim branch pauses even when the setting is disabled or a SyncPlay session exists
This branch returns pause before the shouldPause(enabled:, hasSession:) check below, so stale ownership overrides both gates. Example: the composer pauses, the sleep timer expires mid-edit, and the blur-time resume is vetoed. _pausedByEdit stays true because resumeHandled() never runs. The user then presses play and either joins a SyncPlay room or turns the setting off. Focusing the composer now reaches this branch, and the panel calls pause(enableSync: false). That pauses playback locally with the setting off, or fights the room's playback, which the class doc says must never happen.
When shouldPause is false, drop the stale ownership (_pausedByEdit = false; _resumeAfterSubmit = false;) and return none. Only re-claim when shouldPause(...) is true.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // feature allows. | ||
| return ListenableBuilder( | ||
| listenable: textFieldFocus, | ||
| builder: (context, child) => TapRegion( |
There was a problem hiding this comment.
WARNING: consumeOutsideTaps also eats taps on the text field's own selection toolbar and handles
This TapRegion has no groupId, so it forms a group of its own. Flutter draws the TextField's context menu (Paste/Copy/Cut/Select all) and its selection handles in the Overlay, wrapped in TextFieldTapRegion (groupId: EditableText). A tap on any of them is therefore "outside" this region. While hasFocus && pausedByEdit is true, the surface adds an accepting arena member and the toolbar button never receives the tap. On mobile, where the setting is on by default, long-press → Paste stops working while editing.
Put the region in the EditableText group so the field's own overlays count as inside, the same way MenuAnchor scopes its consumer to its own group:
| builder: (context, child) => TapRegion( | |
| builder: (context, child) => TapRegion( | |
| groupId: EditableText, |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge This incremental review covers
The revert also brings back the uncommon edge case noted in the Files Reviewed (1 file)
Previous Review Summaries (5 snapshots, latest commit 36f8d24)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 36f8d24)Status: 1 Issue Found | Recommendation: Address before merge This incremental review covers Overview
Issue Details (click to expand)WARNING
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous review (commit 1270922)Status: No Issues Found | Recommendation: Merge The branch was force-pushed, so the previous review commit Both earlier findings are fixed:
One edge case, not posted inline because it needs an unusual sequence of events: if a resume is vetoed because a route is on top when the composer loses focus, Files Reviewed (4 files)
Previous review (commit 73fe7e0)Status: 2 Issues Found | Recommendation: Address before merge The branch was force-pushed and now has a 3-file implementation, so the previous review commit is no longer in history. This review covers the full diff at Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit 3e5c680)Status: No Issues Found | Recommendation: Merge The branch was force-pushed, so the previous review commit is no longer in history. This review covers the full diff at Both earlier findings are fixed:
Files Reviewed (7 files)
Note (not posted inline because it takes an uncommon sequence of events): if a resume is vetoed, Previous review (commit 3afd740)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Note: a vetoed resume (for example, a dialog on top when the composer loses focus) leaves Reviewed by claude-opus-5.5 · Input: 264 · Output: 3.8K · Cached: 302.8K |
3afd740 to
3e5c680
Compare
|
@Predidit 想请教一下：现在这个提交看起来有点臃肿（7 个文件 +1046 行），需要瘦身吗？ 需要的话希望你能给点建议，我按你的意见改；要是觉得没必要，我就不动了。谢谢！ |
|
我还没来得及看这个pr，现在有些太晚了，我会在明天给出review 大概是需要进行重构的，为这个小功能增加1000行代码是无法接受的，deepseek应该犯了一些架构上的错误，小参数模型总是这样 此外如果可以的话，尽可能不要把ai的回复原封不动贴到pr里，毕竟已经勾选了 我对生成的代码充分理解。稍微用几句话解释一下，而不是上千字的ai输出比较好 |
|
ok我这边明天应该也会整体重新看一下架构的问题，最初版架构基本上是我提出并且自己审查，完全没有臃肿到这么严重的程度。后面的确由于我本人一直在让ds跑导致出现的问题。 我最初整体的思路其实是复用类似于倍速弹窗那种给整体加一个遮罩+暂停然后就能实现了。也麻烦您帮忙review一下了 这边我指的是之前关闭的PR #2706 |
3e5c680 to
73fe7e0
Compare
|
作者大大您好，我个人稍微重构了一下：现在的diff情况是 3 个文件 +75 行，我把ds自己迭代中产生的独立 controller、独立 widget 和一大堆新增测试全部删掉了，但是“一起看”的机制可能会有点复杂，我做了一个比较简单的决策，也想问一下您的意见。麻烦您有空时可以直接review这版。 |
| return ListenableBuilder( | ||
| listenable: textFieldFocus, | ||
| builder: (context, _) => TextFieldTapRegion( | ||
| consumeOutsideTaps: _pausedByDanmakuEdit, |
There was a problem hiding this comment.
WARNING: The barrier stays active after the composer loses focus
consumeOutsideTaps only checks _pausedByDanmakuEdit and ignores focus. That flag stays true while the field is unfocused in two cases:
- The destination sheet is open (
_holdResumeForDestinationSheet). - A resume was vetoed because another route is on top, such as the sleep timer dialog.
Both the sheet and the dialog are outside this region, so taps on their buttons get consumed.
The flag also goes stale. When the sheet closes, line 142 sets _pausedByDanmakuEdit = false without a focus change, so this ListenableBuilder does not rebuild and the region keeps consumeOutsideTaps: true. Until the composer gains focus again, taps on the video and the player controls keep being consumed.
Arm the barrier only while the field has focus, as the previous revision did. Focus is still true at pointer-down for the tap that dismisses the composer, so that tap is still consumed:
| consumeOutsideTaps: _pausedByDanmakuEdit, | |
| consumeOutsideTaps: textFieldFocus.hasFocus && _pausedByDanmakuEdit, |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| final destination = await showDanmakuDestinationSheet(context); | ||
| _holdResumeForDestinationSheet = false; | ||
| _onDanmakuEditFocusChanged(); |
There was a problem hiding this comment.
WARNING: _onDanmakuEditFocusChanged() runs after await with no mounted check
If the panel is disposed while the destination sheet is open, this call still runs. With _pausedByDanmakuEdit set and the field unfocused, it reaches ModalRoute.of(context). On an unmounted State, context throws, so the result is an unhandled async error. The !mounted check on the next line comes too late to prevent it.
| _onDanmakuEditFocusChanged(); | |
| if (mounted) _onDanmakuEditFocusChanged(); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
Hi, 这个PR现在还在你的视野范围内吗，在review之前，需要先解决 kilo bot 自动审查问题，或是为其找到的问题给出明确的异议 |
|
如果你陷入了某种令人讨厌的修改循环，可以尝试一下不同的模型，例如 Gemini 会是低成本的绝佳选择 |
|
作者大大您好，感谢您的建议，这个PR目前还在我的视野范围内，我这边借用了朋友的gpt 6.1 sol xhigh进行了review和建议，同时也进行了人工复核，有几件事情需要告知您：
|
- 聚焦弹幕输入框时暂停、失焦后只恢复自己造成的暂停；一起看房间内不介入 - 复用的 TextFieldTapRegion 屏障只在编辑中且持有暂停时消费那一次点击 - 发送目标弹窗期间保持暂停，关闭后才恢复 - 补齐 mounted、应用前台、睡眠定时到期三处守卫；定时到期由真实事件回收恢复资格
73fe7e0 to
1270922
Compare
|
Kilo 这轮已经是 No Issues Found / Merge，上面提到的两条也都修正到最新提交了，方便的话麻烦您 review 这一版。 |
| _danmakuPauseTick = TimedShutdownService().expiryTicks.value; | ||
| unawaited(playerController.pause(enableSync: false)); | ||
| } else { | ||
| _danmakuPauseTick = null; |
There was a problem hiding this comment.
WARNING: Sending an empty danmaku now leaves the video paused
FocusNode.unfocus() doesn't change focus right away. Flutter applies the change in a microtask, so hasFocus is still true when the empty-message branch of _submitDanmakuText calls _onDanmakuEditFocusChanged() synchronously (line 142). Playback is already paused because the editor paused it, so this call lands in the new else and clears _danmakuPauseTick. When the real blur arrives a moment later, _canResumeAfterDanmakuEdit is false and playback never resumes. Before this change, the focused branch did nothing in that state, so the later blur still resumed.
Remove the synchronous call at line 142 and let the focus listener handle the blur once _holdResumeForDestinationSheet is false. Another fix is to skip this else when the call doesn't come from a real focus gain.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
This reverts commit 36f8d24.
…redidit#2711) * feat(player): 编辑弹幕时暂停播放 - 聚焦弹幕输入框时暂停、失焦后只恢复自己造成的暂停；一起看房间内不介入 - 复用的 TextFieldTapRegion 屏障只在编辑中且持有暂停时消费那一次点击 - 发送目标弹窗期间保持暂停，关闭后才恢复 - 补齐 mounted、应用前台、睡眠定时到期三处守卫；定时到期由真实事件回收恢复资格 * update * Revert "update" This reverts commit 36f8d24. --------- Co-authored-by: Predidit <34627277+Predidit@users.noreply.github.com> (cherry picked from commit 9a20a01)
描述
在播放器底部的弹幕输入框里打字时，番剧仍在继续播放：等弹幕打完，想和小伙伴们吐槽的那一幕往往已经过去，只好先手动暂停、发完再手动继续。这个 PR 让聚焦输入框时自动暂停播放、输入框失焦后自动恢复，和 B 站发弹幕逻辑类似，同时也参考了 animeko 的
pauseVideoOnEditDanmaku的行为。默认值与 B 站客户端一致：桌面端不暂停，移动端暂停（移动端行为已在 Android / 鸿蒙 / iOS 上确认，见 #2708）。设置项位于 设置 → 弹幕 → 弹幕显示 → 编辑弹幕时暂停播放，可随时开关。
开启后的行为：
实现要点
桌面端和移动端不同默认值的实现：在
settings_keys里面我做了三处修改，一共是 15 行。第一处是用来判断是否是桌面端供默认值使用的，第二处是加了个设置项，也就是我们这个 PR 的本体，即编辑弹幕时暂停，同时做了一个刚刚提到的第一处的逆运算：如果是桌面端那就为关闭，不是就开启，这两处合起来就实现了桌面端默认关闭，移动端默认开启的功能。第三处就是把该设置项加进了SettingsKeys.all，作用是调整回默认的时候能把这个也一并重置回到平台默认，这个和其他的设置项是保持一致的。定时关闭服务的小改动：在
timed_shutdown_service.dart里加了 5 行 —— 定时真正到期时把expiryTicks序号 +1；面板用“暂停时记下的序号”与当前序号比较来判断恢复资格是否仍然有效，这样即使中途面板被重建（切集、画中画）也不会用旧状态把视频播起来。用户在图形交互层界面设置的实现：在
danmaku_settings里面我也做了三处修改，其中前两个是为了和其他设置项保持一致，即存放设置的本地状态、打开设置页读一次存储，其他的弹幕设置都有这样的字段，第三处就是加了图形界面上的开关本体，可以供用户自己选择开闭。功能层的实现（即点击输入框聚焦暂停、发送或点击输入框外取消机制的实现）：这个功能主要是对
player_item_panel.dart做出了修改（面板+63/-3），机制如下：当我们点击输入框时，函数会监听一个输入框焦点变化信号textFieldFocus，如果监听到了输入框得到了聚焦，此时做两个条件判断：如果在一起看房间内，直接return不参加功能（和操作一览中提到的一样）；接下来看如果用户开启了输入暂停且视频正在播放，如果符合该条件，则记录“这次暂停属于本次编辑”（_danmakuPauseTick快照），然后通过原有的pause接口函数暂停弹幕画布和播放，同时播放键图标也随之切换。到这里聚焦暂停工作就实现了。而失焦有 A 和 B 两条路：A 路径调用了原有
main里面的onTapOutside（即点击输入框以外任何地方），此时会传出一个焦点丢失的信号，这个信号会通知播放器和弹幕画布等恢复播放；同时为了防止用户点击播放键造成「失焦恢复 → 点了播放键又暂停了」的负负得正的情况出现，我加入了一个_danmakuEditBarrier屏障，在这个屏障作用的过程中，用户即使点到了播放键也不会触发，从而避免了负负得正的情况。B 路径则是回车后点击了发送，但是我们还不能恢复播放，因为此时 Kazumi 会弹出一个弹窗要求选择发送到聊天室还是弹幕墙，故而就做了一个_holdResumeForDestinationSheet的true手动标记继续暂停，等到弹窗消失后变为false，手动补一次失焦，播放恢复。关联的 Issues
PR 类型
AI 辅助开发
AI 辅助模型: deepseek-v4.1-flash
提交前检查
附注
pauseVideoOnEditDanmaku提供了同样的能力，见 VideoScaffoldConfig.kt。flutter test213 项通过）；flutter analyze --no-fatal-infos --fatal-warnings退出码 0，未引入新的提示。player_item.dart:808-850）补齐了三处守卫：mounted（await 返回后先判断）、应用前台状态（AppLifecycleState.resumed）、睡眠定时到期；并保留发送目标弹窗期间“保持暂停”的等待逻辑。