Fetch and play previous episode#8141
Conversation
287a626 to
2561c45
Compare
Cloudflare Pages deployment
|
anagnorisis2peripeteia
left a comment
There was a problem hiding this comment.
I dont think this updates the actual queue. getPlaylist() returns a copy, so unshift(newItem) only changes that local array and the queue manager still has the old playlist. The episode from getEpisodes() also doesnt have a PlaylistItemId, so setPlaylistState() ends up pointing at nothing. I think this needs to add the item through the queue manager and have a test that checks the real playlist and current item afterwards.
Just for the sake of this working, it sure does gets the job done. It is quite slow to fetch episodes on that preview URL 🤔 If you know how to properly use the queue I will happily update the PR. |
|
Ah, I see why it looks like it works — There isn't a prepend API currently. I'd add one to For the slowness, A small queue-manager test checking |
Inline changes would help and please with your own words, not Claude's. I don´t think you even got the preview URL opened before posting a review. |
|
Pressing the previous button whilst on Pioneer One S1E2 does start S1E1, but afterwards the queue still contains S1E2–S1E6, S1E1 is absent, and the current item’s PlaylistItemId is null. Playback works, but the queue state is still wrong for the reason identified in the review. There ya go. owuld it help if I jsut dpoknt even clean up typos so you feel super confident I typed it"? Do you want the inline changes or not? |
anagnorisis2peripeteia
left a comment
There was a problem hiding this comment.
The review on the diff stands.
2561c45 to
2b3248e
Compare
|
Sorry for the sharp tongue. I've been dealing with AI PRs and code during work for quite sometime and it gets into my nerves. It is an amazing tool but not everyone uses it properly... Now I see the difference, the unshift wasn´t actually being used only the item was being played. I thought i had tested moving forward and backward but missed the request being made everytime for the same episode. |
anagnorisis2peripeteia
left a comment
There was a problem hiding this comment.
could you add a regression test please? I think it should check the real playlist and current item afterwards, starting on E2 at index 0 then going back to E1. The queue should end up as E1, E2, E3, with E1 having a PlaylistItemId and being the current item.\n\nalso, separate from the review decision, could you include me as coauthor on the commit with the changes from my suggestions? thats not conditional on the review, I just think the attribution should be kept.
I appreciate the concession. I've been trying to use it to help me get to the bottom of a restart bug on flaky connection on mobile and I've spent days and days trying to recreate it - so the hostility is quite frustrating to be honest. Perhaps you could help me with that? |
If there was already a test file for this class I wouldn´t mind but only to test this change I would say it is overenginnering.
tbh, no idea where to start. I would likely think it as a wifi or the server's hardware bottleneck. |
My PR 8200 was insta closed. I am about to raise another issue but I suspect the same will happen there without some help from current contributors. On this one, I think the justification to not include a regression test here is fine enough. That said I have made one and reference it in the follow up comment. |
There was a problem hiding this comment.
fair enough, . im happy for that test coverage to be a follow up once theres a sensible harness for this class. The connection failure bug I mentioned is now #8206: #8206. thats separate from this PR, but the reproduction and proof are attached there.also, my coauthor request from the previous review is separate from this approval.
I have included one anyway if you want to use it. It could be a start.
https://gist.github.com/anagnorisis2peripeteia/60609e71f54ead6d20ee50d6b8091575
|
@will-blk, @thornbill closed my issue without looking at it again. This is such a hostile repo lmao |



Enable previous track button for episode based media. The player will look for the latest episode available before the current playing one.
Changes
Issues
Fix #8089
Code assistance
None