-
Notifications
You must be signed in to change notification settings - Fork 414
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
Fix autoplay infinite loop #5711
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost perfect.
!history.some((h) => { | ||
const directMatch = h.uri === recommendedForUri[i]; | ||
const claimNameMatch = h.uri.includes(recommendedUriInfo.claimName); | ||
const idMatch = claimsByUri[h.uri] && claimsByUri[h.uri].claim_id === recommendedUriInfo.claimId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this check work in all cases? If so do we need to check for a name match?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No change
claimsByUri
doesn't exist unless we resolve the entire history at startup (don't want to do that) -- it was added as a "try if possible" effort, primarily to handle odd URLs like
lbry://vacuum-tube-computer-p.09-–-building#5212bc8bc63c373e2bf1ebc5b765595ed7b6514d
lbry://vacuum-tube-computer-p-09-–-building#5212bc8bc63c373e2bf1ebc5b765595ed7b6514d
const recommendedUriInfo = parseURI(recommendedUri); | ||
|
||
if (claimsByUri[uri] && claimsByUri[uri].claim_id === recommendedUriInfo.claimId) { | ||
// Skip myself (same claim ID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can potentially see someone complaining and thinking their videos are never recommended because they never their own videos autoplay. While it's likely you don't want to autoplay watch your own videos, I think a user would rather notice their video being suggested over thinking it's never suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated?
This is not checking against "claims that I own", but solving the "video A recommended for video A" problem.
There's also a weird case where the URL differs by just a little (p.09 vs p-09), but with the same claim ID:
lbry://vacuum-tube-computer-p.09-–-building#5212bc8bc63c373e2bf1ebc5b765595ed7b6514d
lbry://vacuum-tube-computer-p-09-–-building#5212bc8bc63c373e2bf1ebc5b765595ed7b6514d
## Issue Closes 3661: Autoplay + Related go into loops ( infinite ) sometimes ## GUI Push the actual "next" item into the top of the list. ## History search 1. Skip if the next item is itself. 2. The URL stored in the history comes in various forms, so a direct comparison won't work. - There's also a weird case where the URL differs by just a little (p.09 vs p-09), but with the same claim ID: lbry://vacuum-tube-computer-p.09-–-building#5212bc8bc63c373e2bf1ebc5b765595ed7b6514d lbry://vacuum-tube-computer-p-09-–-building#5212bc8bc63c373e2bf1ebc5b765595ed7b6514d Check the claim_id as well to cover cases like these.
493db02
to
93aba59
Compare
|
Issue
Closes #3661: Autoplay + Related go into loops ( infinite ) sometimes
GUI
Push the actual "next" item into the top of the list.
History search
lbry://vacuum-tube-computer-p.09-–-building#5212bc8bc63c373e2bf1ebc5b765595ed7b6514d
lbry://vacuum-tube-computer-p-09-–-building#5212bc8bc63c373e2bf1ebc5b765595ed7b6514d
Check the claim_id as well to cover cases like these.