修复在线/离线播放历史记录#2261
Conversation
1. 为历史记录增加在线/离线身份并按 scoped key 存储。 2. 同步 WebDAV 历史事件、播放恢复和离线入口，避免记录互相覆盖。 3. 增加历史仓储、同步合并和集数解析回归测试。
liangyuR
changed the title
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| String urlItem = resolvedEpisode.episodePageUrl; | ||
| if (urlItem.contains(currentPlugin.baseUrl) || | ||
| urlItem.contains(currentPlugin.baseUrl.replaceAll('https', 'http'))) { | ||
| urlItem = urlItem; |
There was a problem hiding this comment.
SUGGESTION: Dead no-op self-assignment.
When the original condition if (!contains && !contains) was inverted, the if branch was left with urlItem = urlItem;, which does nothing. The logic is still correct, but the empty branch is dead code. Consider collapsing this into a single guard:
if (!urlItem.contains(currentPlugin.baseUrl) &&
!urlItem.contains(currentPlugin.baseUrl.replaceAll('https', 'http'))) {
urlItem = currentPlugin.baseUrl + urlItem;
}Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The SyncPlay episode round-trip flagged in the prior review is now resolved: the broadcast file name and the Files Reviewed (4 files)
Previous Review Summaries (4 snapshots, latest commit a2f06be)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a2f06be)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (17 files)
Fix these issues in Kilo Cloud Previous review (commit ba1dd0c)Status: No Issues Found | Recommendation: Merge Overview
The latest commits refactor the history-card playback flow in Files Reviewed (2 files changed since last review)
Previous review (commit 08ea8ee)Status: No Issues Found | Recommendation: Merge Overview
The latest commit resolves the previously reported SUGGESTION in Files Reviewed (1 file changed since last review)
Previous review (commit 19fceed)Status: 1 Issue Found | Recommendation: Optional cleanup; safe to merge Overview
Issue Details (click to expand)SUGGESTION
The history online/offline split is implemented cleanly: scoped history keys ( Files Reviewed (17 files)
Reviewed by claude-4.8-opus-20260528 · Input: 5.1K · Output: 8.2K · Cached: 583.8K |
1. 将在线播放 URL 补全逻辑折回单 guard。 2. 移除 robot 指出的 urlItem 自赋值死代码。
|
可以考虑在 history_card 上新增一个 badget 表示此条观看记录来自在线还是缓存，此外当在线源不可用时，不要试图 fallback 到离线源，直接弹出 toast 报错就可以了 |
|
|
不要给 widget 做测试，我了解这是最佳实践，但是此工程一直以来只测试逻辑，不测试UI，和之前保持一致，删除 history_card_test |
|
好的，但是为什么呢？这样做有什么风险？还是 test rule ？ |
- 1. 新增 `openHistoryPlaybackForEntry`，统一在线与缓存观看记录的播放入口处理。 - 2. 新增 `HistoryPlaybackOpenResult`，封装播放结果与失败提示。 - 3. 新增 `_HistorySourceBadge`，在历史卡片上标识在线或缓存来源。 - 4. 更新 `BangumiHistoryCardV`，使用新的播放处理并在标题旁展示来源标识。
liangyuR
force-pushed
the
codex/history-online-offline-sync
branch
from
f10c9fd to
a2f06be
Compare
|
没有风险，也不是明确的 test rule ，但这个工程从来没有考虑过 UI 可测试性的问题，大量的 widget 是不可测试的 现在添加 widget 测试很古怪，容易脆弱 |
| late final PlayerSyncPlayController syncplay = PlayerSyncPlayController( | ||
| bangumiId: () => bangumiId, | ||
| currentEpisode: () => currentEpisode, | ||
| currentEpisode: () => currentDanmakuEpisodeNumber, |
There was a problem hiding this comment.
WARNING: SyncPlay episode round-trip is now inconsistent.
The SyncPlay file name is broadcast as "$bangumiId[$currentDanmakuEpisodeNumber]" (see player_controller.dart:184 and player_syncplay_controller.dart:242), so this callback now exposes the danmaku episode number (parsed from the episode title via extractEpisodeNumber). However, the receiver parses that number out of the file name and feeds it straight into changeEpisode(episode, ...) (player_syncplay_controller.dart:159), where the first argument is treated as the 1-based playlist index (_resolveOnlineEpisode does index = episode - 1 into roadData.data).
Before this rename the broadcast value and changeEpisode both used the playlist index, so they matched. Now, whenever the title's parsed number differs from the list position (e.g. a split-cour road where 第13话 sits at position 1), peers will jump to the wrong position or hit 集数解析失败. Consider broadcasting/consuming the playlist index consistently, or mapping the received danmaku number back to a list index before calling changeEpisode.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
已确认。这确实是一个 SyncPlay 往返回归问题，但不属于此 PR 的范围。我会在单独的 PR 中修复它，恢复 SyncPlay 中一致的播放列表索引处理。 @Predidit
|
也许应该建一个 AGENTS.md 以及 docs 说明，比如在 test-rule.md 中将 “不考虑 UI 可测试性“ 这项规则加入 |
|
有少量代码清洁性问题，部分死代码没删，例如
|
|
上面bot的评论是需要更正的，syncplay 有些问题，虽然之前也有问题，但是现在语义上特别糟糕 发送的是 标题解析集数，接收端当成 播放列表位置 要么我们和之前一样两边均使用播放列表位置，并把问题留到下个PR 要么在此PR里进行更大的变更 |
- 恢复 PlaybackInitParams 的播放列表位置字段，用于 SyncPlay 往返协议。 - 保留 danmakuEpisodeNumber 仅用于弹幕加载与历史身份。 - 补充测试覆盖播放列表位置和弹幕集数并存。
* ### fix: 分离在线与离线观看历史记录 1. 为历史记录增加在线/离线身份并按 scoped key 存储。 2. 同步 WebDAV 历史事件、播放恢复和离线入口，避免记录互相覆盖。 3. 增加历史仓储、同步合并和集数解析回归测试。 * ### fix: 清理视频 URL 构造 no-op 分支 1. 将在线播放 URL 补全逻辑折回单 guard。 2. 移除 robot 指出的 urlItem 自赋值死代码。 * ### Feat：增强观看历史卡片播放处理与来源标识 - 1. 新增 `openHistoryPlaybackForEntry`，统一在线与缓存观看记录的播放入口处理。 - 2. 新增 `HistoryPlaybackOpenResult`，封装播放结果与失败提示。 - 3. 新增 `_HistorySourceBadge`，在历史卡片上标识在线或缓存来源。 - 4. 更新 `BangumiHistoryCardV`，使用新的播放处理并在标题旁展示来源标识。 * Remove obsolete episode selection helpers * ### Fix：恢复 SyncPlay 播放列表位置语义 - 恢复 PlaybackInitParams 的播放列表位置字段，用于 SyncPlay 往返协议。 - 保留 danmakuEpisodeNumber 仅用于弹幕加载与历史身份。 - 补充测试覆盖播放列表位置和弹幕集数并存。 --------- Co-authored-by: liangyu Ran <ranliangyu@enneagon.com> (cherry picked from commit 07a2c64)
2 participants
#2261 的mini版本，仅针对 修复在线/离线播放历史记录 做的最小修复，同时考虑了后续统一数据模型的考量
后续约 3 ~ 4 个 PR
摘要
entryKind、剧集页面 URL，以及每集的播放进度时间戳。根因
在线播放和已下载播放使用了相同的历史身份结构，因此本地存储和同步时，可能会将同一个 Bangumi 条目和插件下的记录合并或覆盖。
这也导致在在线历史和离线历史同时存在时，旧版 WebDAV 历史快照和删除标记变得含义不明确。
验证
git diff --cached --checkgit diff --checkflutter test --no-pub test/history_repository_test.dart test/history_sync_test.dart test/resolved_episode_test.dartflutter test test/history_repository_test.dart test/history_sync_test.dart test/resolved_episode_test.dart手动后续验证