fix: show: inconsistent shuffle on long lists
Massimo Melina committed
Dec 10, 2024 at 20:42 UTC
840d7316d8c2e2905ff0dd5dec819eca3b171206
2 files changed
+28
-15
frontend/src/show.ts
+27
-14
@@ -1,5 +1,5 @@
1
import { DirEntry, DirList, ext2type, state, useSnapState } from './state'
2
-import { createElement as h, Fragment, useEffect, useMemo, useRef, useState } from 'react'
2
+import { createElement as h, Fragment, useEffect, useRef, useState } from 'react'
3
import {
4
basename, dirname, domOn, hfsEvent, hIcon, isMac, newDialog, pathEncode, restartAnimation, useStateMounted,
5
isNumeric,
@@ -11,7 +11,6 @@ import { openFileMenu } from './fileMenu'
11
import { alertDialog, toast } from './dialog'
12
import _ from 'lodash'
13
import { getId3Tags } from './id3'
14
-import { subscribeKey } from 'valtio/utils'
14
import i18n from './i18n'
15
const { t, useI18N } = i18n
16
@@ -40,19 +39,15 @@ export function fileShow(entry: DirEntry, { startPlaying=false, startShuffle=fal
39
if (firstUri !== uri) // user must have clicked the folder link inside file-menu (which happens only for search results)
40
close()
41
}, [uri])
43
- const [cur, setCur] = useState(entry)
42
+ const [cur, setCur, getCur] = useStateMounted(entry)
43
const moving = useRef(0)
44
const lastGood = useRef(entry)
45
const [mode, setMode] = useState(ZoomMode.contain)
46
const [shuffle, setShuffle] = useState<undefined | DirList>()
47
useEffect(() => toggleShuffle(startShuffle), [])
49
- // shuffle the rest of the list as we continue getting entries, leaving intact the part we've already played/being through
50
- const shuffleIdx = useMemo(() => shuffle?.findIndex(x => x.n === cur.n), [cur])
51
- useEffect(() => subscribeKey(state, 'list', list => {
52
- const n = 1 + (shuffleIdx ?? Infinity)
53
- setShuffle(x => list && x?.slice(0, n).concat(_.shuffle(list.slice(n))))
54
- }), [shuffleIdx])
55
- const [repeat, setRepeat, { get: getRepeat }] = useStateMounted(false)
48
+ const shufflePlayed = useRef(0) // keep track of how many entries of the shuffle list we played
49
+ if (!shuffle) shufflePlayed.current = 0
50
+ const [repeat, setRepeat, getRepeat] = useStateMounted(false)
51
const [cover, setCover] = useState('')
52
useEffect(() => {
53
if (shuffle)
@@ -266,18 +261,32 @@ export function fileShow(entry: DirEntry, { startPlaying=false, startShuffle=fal
261
setFailed(cur.n)
262
}
263
269
- function go(dir=1, from=cur) {
264
+ function go(dir=1) {
265
+ if (getCur() !== cur) return // this was fired with a stale state (closure), cancel. To reproduce: hold right-arrow on the keyboard
266
+ const { list } = state
267
+ /* this is a lazy approach to shuffling: since list is not fully available from the start, it's best to wait,
268
+ or the shuffle will be limited to a few entries. Benchmark: _.shuffle of 1M entries takes 10ms on a M1 pro. */
269
+ let workingShuffle = shuffle // in case we setShuffle, we need to do the rest of job with fresh data
270
+ // if playing shuffle, and going forward, where never played before, and got new entries since last time, then shuffle again
271
+ if (shuffle && dir > 0 && shuffle.length < list.length) {
272
+ const shuffleIdx = _.findIndex(shuffle, { n: cur.n })
273
+ if (shuffleIdx >= shufflePlayed.current) {
274
+ const ofs = 1 + shuffleIdx
275
+ const played = shuffle.slice(0, ofs) // keep the part already played, shuffle the rest
276
+ setShuffle(workingShuffle = played.concat(_.shuffle(_.difference(list, played)))) // list has unstable order (when searching), so we use difference
277
+ }
278
+ }
279
if (dir)
280
moving.current = dir
272
- let e = from
281
+ let e = cur
282
while (1) {
274
- e = e.getSibling(moving.current, shuffle)
283
+ e = e.getSibling(moving.current, workingShuffle)
284
if (anyGood()) break
285
if (e) continue // try next
286
// reached last/first
287
if (dir! > 0) {
288
if (getRepeat()) {
280
- e = shuffle?.[0] || state.list[0]
289
+ e = workingShuffle?.[0] || list[0]
290
if (anyGood()) break
291
continue
292
}
@@ -287,6 +296,10 @@ export function fileShow(entry: DirEntry, { startPlaying=false, startShuffle=fal
296
return restartAnimation(document.body, '.2s blink')
297
}
298
goTo(e)
299
+ if (shuffle) {
300
+ const playingIdx = _.findIndex(workingShuffle, { n: e.n })
301
+ shufflePlayed.current = Math.max(shufflePlayed.current, playingIdx)
302
+ }
303
304
function anyGood() {
305
return e && !e.isFolder && getShowComponent(e)
shared/react.ts
+1
-1
@@ -17,7 +17,7 @@ export function useStateMounted<T>(init: T) {
17
if (isMounted())
18
set(newValue)
19
}, [isMounted, set])
20
- return [v, setIfMounted, { isMounted, get: () => ref.current }] as const
20
+ return [v, setIfMounted, () => ref.current] as const
21
}
22
23
export function reactFilter(elements: any[]) {