Resume a figure after a library rescan
play_figure resumed only when the player's playlist *was* the figure's playlist object. rescan_library rebuilds every Playlist, so after the refresh button - or the podcast poller landing a new episode, every few hours on its own - putting a figure back started it over. Playlist is a frozen dataclass: compare by value. The album picker's "keep what is already playing" check had the same identity comparison and restarted the album after a rescan; it compares by value too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -316,9 +316,6 @@ def _build_app(
|
|||||||
mouse=mouse,
|
mouse=mouse,
|
||||||
player=player,
|
player=player,
|
||||||
library=library,
|
library=library,
|
||||||
# One source of truth: the figure path and the web path must hand the player the
|
|
||||||
# same Playlist object for the same folder, because `play_figure` resumes on an
|
|
||||||
# identity check.
|
|
||||||
playlists=library.figure_playlists(),
|
playlists=library.figure_playlists(),
|
||||||
clock=clock,
|
clock=clock,
|
||||||
tippen=tippen,
|
tippen=tippen,
|
||||||
|
|||||||
@@ -132,9 +132,6 @@ class MusicLibrary:
|
|||||||
one per subfolder, depth-first and alphabetical, which is the order the LED ring
|
one per subfolder, depth-first and alphabetical, which is the order the LED ring
|
||||||
offers them in - and the ring has room for no more than
|
offers them in - and the ring has room for no more than
|
||||||
:data:`MAX_FIGURE_ALBUMS`.
|
:data:`MAX_FIGURE_ALBUMS`.
|
||||||
|
|
||||||
The figure path and the web path must hand the player the *same* object for the
|
|
||||||
same album: ``reactions.playback.play_figure`` resumes on an identity check.
|
|
||||||
"""
|
"""
|
||||||
by_figure: dict[str, list[Album]] = {}
|
by_figure: dict[str, list[Album]] = {}
|
||||||
for album, _ in self._entries.values():
|
for album, _ in self._entries.values():
|
||||||
|
|||||||
@@ -75,7 +75,11 @@ def play_figure(event: PlayFigureRequested, app: App) -> None:
|
|||||||
playlist = choices[app.default_choice(event.figure)]
|
playlist = choices[app.default_choice(event.figure)]
|
||||||
remember_figure_album(app, event.figure, playlist)
|
remember_figure_album(app, event.figure, playlist)
|
||||||
|
|
||||||
if not event.restart and app.player.playlist is playlist:
|
# Equality, not identity: a library rescan (the refresh button, or the podcast
|
||||||
|
# poller landing an episode) rebuilds every Playlist object, and an `is` check made
|
||||||
|
# the figure start over after that. Equal means the same album with the same files;
|
||||||
|
# if the files changed, starting over is the honest thing to do.
|
||||||
|
if not event.restart and app.player.playlist == playlist:
|
||||||
_log.info("Resuming %r", event.figure)
|
_log.info("Resuming %r", event.figure)
|
||||||
app.player.play()
|
app.player.play()
|
||||||
return
|
return
|
||||||
@@ -92,8 +96,7 @@ def remember_figure_album(app: App, figure: str, playlist: Playlist) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def _start_album(app: App, album: Album, track_index: int) -> None:
|
def _start_album(app: App, album: Album, track_index: int) -> None:
|
||||||
# A figure album keeps the figure's own resume bookkeeping honest: it is the same
|
# A figure album started from anywhere becomes the album that figure picks next.
|
||||||
# Playlist object either way, because both come from the library index.
|
|
||||||
playlist = None
|
playlist = None
|
||||||
if album.figure is not None:
|
if album.figure is not None:
|
||||||
playlist = next(
|
playlist = next(
|
||||||
|
|||||||
@@ -130,7 +130,8 @@ def selector_pressed(_event: SelectorPressed, app: App) -> None:
|
|||||||
playlist = selector.choices[selector.selected]
|
playlist = selector.choices[selector.selected]
|
||||||
_close(app, fade=True)
|
_close(app, fade=True)
|
||||||
_play_sound(app, confirm_wav, CONFIRM_GAIN)
|
_play_sound(app, confirm_wav, CONFIRM_GAIN)
|
||||||
if app.player.playlist is playlist:
|
# By value: a rescan rebuilds the Playlist objects (see play_figure).
|
||||||
|
if app.player.playlist == playlist:
|
||||||
_log.info("Album picker: keeping %s", playlist.album_id)
|
_log.info("Album picker: keeping %s", playlist.album_id)
|
||||||
return
|
return
|
||||||
_log.info("Album picker: %r -> %s", selector.figure, playlist.album_id)
|
_log.info("Album picker: %r -> %s", selector.figure, playlist.album_id)
|
||||||
|
|||||||
@@ -126,6 +126,22 @@ async def test_putting_the_same_figure_back_resumes_where_it_left_off(
|
|||||||
mouse.check("track", "1")
|
mouse.check("track", "1")
|
||||||
|
|
||||||
|
|
||||||
|
async def test_resuming_survives_a_library_rescan(
|
||||||
|
mouse: SimulatorDriver, sim: Simulation
|
||||||
|
) -> None:
|
||||||
|
"""A rescan rebuilds every Playlist object; resuming must not depend on identity."""
|
||||||
|
await mouse.place("fuchs")
|
||||||
|
await mouse.wait(TRACK_SECONDS + 1)
|
||||||
|
mouse.check("track", "1")
|
||||||
|
|
||||||
|
await mouse.remove()
|
||||||
|
await sim.app.rescan_library()
|
||||||
|
await mouse.place("fuchs")
|
||||||
|
|
||||||
|
mouse.check("playing", "true")
|
||||||
|
mouse.check("track", "1")
|
||||||
|
|
||||||
|
|
||||||
async def test_a_different_figure_starts_from_the_top(mouse: SimulatorDriver) -> None:
|
async def test_a_different_figure_starts_from_the_top(mouse: SimulatorDriver) -> None:
|
||||||
await mouse.place("fuchs")
|
await mouse.place("fuchs")
|
||||||
await mouse.wait(TRACK_SECONDS + 1)
|
await mouse.wait(TRACK_SECONDS + 1)
|
||||||
|
|||||||
@@ -199,15 +199,15 @@ async def test_play_can_start_at_a_track(client: Client, sim: Simulation) -> Non
|
|||||||
assert (await client.get("/api/state")).json()["track_index"] == 2
|
assert (await client.get("/api/state")).json()["track_index"] == 2
|
||||||
|
|
||||||
|
|
||||||
async def test_playing_a_figure_album_reuses_the_figure_playlist(
|
async def test_playing_a_figure_album_plays_the_figure_playlist(
|
||||||
client: Client, sim: Simulation
|
client: Client, sim: Simulation
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Identity matters: ``play_figure`` resumes on an ``is`` check."""
|
"""Equal to the figure's own playlist, so placing the figure afterwards resumes it."""
|
||||||
album = await album_by_title(client, "Fuchs")
|
album = await album_by_title(client, "Fuchs")
|
||||||
await client.post("/api/play", json={"album_id": album["id"]})
|
await client.post("/api/play", json={"album_id": album["id"]})
|
||||||
await sim.bus.drain()
|
await sim.bus.drain()
|
||||||
|
|
||||||
assert sim.player.playlist is sim.app.playlists["fuchs"][0]
|
assert sim.player.playlist == sim.app.playlists["fuchs"][0]
|
||||||
|
|
||||||
|
|
||||||
async def test_playing_an_unknown_album_is_a_404(client: Client) -> None:
|
async def test_playing_an_unknown_album_is_a_404(client: Client) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user