Skip to content

Fixed resetting finished audiobooks #355

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.

Sign up for GitHub

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

Open
MitjaHenner wants to merge 2 commits into
base: main
Choose a base branch
from
+7 −0
Open

Fixed resetting finished audiobooks #355

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea5986a
Fixed resetting finished audiobooks
MitjaHenner Feb 28, 2026
eb9a6ff
Moved check inside try/catch and added constants for first chapter an…
MitjaHenner Mar 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/src/main/kotlin/org/grakovne/lissen/playback/service/PlaybackSynchronizationService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ class PlaybackSynchronizationService
val overallProgress = getProgress(exoPlayer) ?: return
val currentItem = currentItem ?: return

currentChapterIndex?.let { currentChapterIndex ->
Copy link
Copy Markdown
Owner

@GrakovNe GrakovNe Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic numbers and magic values

currentChapterIndex > 1 is a magic boundary. No one knows why it is > 1 and not >= 0, > 0, etc. without digging into the business rules.

0.0 is a magic value that implicitly means “user hasn’t started yet”, but this meaning is encoded in a raw primitive instead of being expressed via a named constant, a value object, or a domain‑specific type.

This makes the logic tightly coupled to hidden assumptions in your head rather than something the code communicates explicitly to the next reader.

Copy link
Copy Markdown
Author

@MitjaHenner MitjaHenner Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic numbers and magic values

currentChapterIndex > 1 is a magic boundary. No one knows why it is > 1 and not >= 0, > 0, etc. without digging into the business rules.

0.0 is a magic value that implicitly means “user hasn’t started yet”, but this meaning is encoded in a raw primitive instead of being expressed via a named constant, a value object, or a domain‑specific type.

This makes the logic tightly coupled to hidden assumptions in your head rather than something the code communicates explicitly to the next reader.

My bad, I debugged using an audiobook with a super short first chapter, so it looked like the index would start at 1 when debugging. It should be 0.

Is there a reason to ever sync a currentTotalTime of 0.0? (Even if the first chapter is playing)

Copy link
Copy Markdown
Owner

@GrakovNe GrakovNe Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to ever sync a currentTotalTime of 0.0? (Even if the first chapter is playing)

Yes, we do have a real use case for syncing currentTotalTime = 0.0.

A user may explicitly want to reset their progress in a book. The way this is currently done in the app is by seeking to the very beginning (00:00), even if it’s the first chapter. From the client perspective, that is an intentional state change, not just an implicit “not started yet” situation.

If we ignore 0.0, we lose the ability to propagate that reset across devices, and the previous progress may be restored unintentionally.

That said, I’m not 100% sure how critical this is in practice, but the reset-to-zero scenario is a valid and user-driven action on our side.

if (currentChapterIndex > 1 && overallProgress.currentTotalTime == 0.0) {
Timber.d("Don't sync currentTotalTime of 0.0 if chapter is not first")
return
}
}

Timber.d("Trying to sync $overallProgress for ${currentItem.id}")

serviceScope.launch(Dispatchers.IO) {
Expand Down