Bookplayer: Add `completed` boolean state for a book

[pichfl]

Tracking via the duration has caused issues while working on 3.0 which are solved but maybe it's better to have this around.

[ryantstone]

Is there a visual element that is going to be displayed or are we just wanting to have a boolean internally to denote completion?

[GianniCarlo]

the 'pie' progress should be replaced with a checkmark, it sometimes does and sometimes doesn't. Currently we have a computed-value for isCompleted, and although we round off the currentTime vs duration it obviously isn't perfect, having a boolean as a core data property should be the better option

[GianniCarlo]

we could set it to true when the AVAudioPlayer delegate calls the audioPlayerDidFinishPlaying

[GianniCarlo]

also when it's explicitly marked by the user inside the player 'more' options

[pichfl]

The reason the checkmark rarely shows up is that I'm still struggling with typed languages and comparing two floats is a bad idea. I put a preliminary fix into #266, casting the values to Int before comparing them:
https://github.com/TortugaPower/BookPlayer/blob/aa50c2ed3759bcae604b09b106055dd008987ed2/BookPlayer/Models/Book%2BCoreDataClass.swift#L51-L55

[bryanrezende]

I've been away from this project for ages now, but for what it's worth - you may not have to compare floats for equivalency to mark a book as complete. A book that's within (let's say) a minute of completion is likely complete due to the legal fluff that fills the end of a lot of books. The audible app counts things as complete as long as they are close enough to the end if I remember correctly, because at that point the book is functionally finished for the listener.

A completed flag might still be nice to have though - if you wanted to record a completion date for whatever reason (stat/listening tracking?), the presence of a nonnull completion date could also function as a flag?

[pichfl]

@bryanrezende welcome back 🙂 your remark is true for Audible as they strictly use M4B containers, but it doesn't work for MP3 based books which can be split into parts of 5 minutes or less and we have to account for those as well, as not all our users are willing to convert their books.

[ryantstone]

I've got this started but have a few questions. We have a few ways to handle completion.

1. Detect when a book is close to done and when a user pauses present a dialog asking if they are done within 1-2 minutes (if it's a book that's split we can write some logic surround this functionality to overcome the 5 minute files)
2. Add a UITableViewRowAction for marking complete
3. When time is complete mark complete

Also, when adding this I went ahead and added isComplete to LibraryItem so this will encompass playlists too and we can have a callback when a book within a playlist completes and once all books are complete in a playlist update it's state too.

[GianniCarlo]

• Not sure about the dialog 🤔 we could test it out and see how it feels.
• Totally agree with the UITableViewRowAction, some time ago we were discussing with @pichfl , that all the options inside the player 'more' button, should be available as row(s) action.
• Good call on the property on LibraryItem, when setting the property the book could call on it's playlist property and notify it if it exists.

I'd like to expand on when the book should be marked as completed, besides the callback of audioPlayerDidFinishPlaying, we should add something to jumpTo too so it would cover if either the user jumps back via chapter selection or by scrubbing, and would let us mark/unmark the isComplete if necessary. EDIT - (currentTime no longers need to be tied to isCompleted)

[pichfl]

I don't think the dialog is necessary. I agree with everything else.

[ryantstone]

I'm working on wrapping up this PR but I've encountered a few things that need some further discussion. If a user has finished a book by listening or scrubbing to the end progress will make it appear complete by showing the check mark, but if that same book is then marked Incomplete it will show as completed because the progress amount 1.0.

The question then is, if a user marks a book complete should we leave any progress intact knowing the side effect will be that if they go back and mark as incomplete we will still display it as complete. Or do we reset progress on marking complete knowing the user will lose their place?

[pichfl]

Good point. For me, marking a book as complete should reset the progress to 0 and scrubbing never marks as complete, only active playback or manually doing so sets the isComplete flag. I think that especially when paused scrubbing to the wrong position should not result in an unwanted action. Does that make sense?

[GianniCarlo]

Agreed, when I proposed the scrubbing thing, I was still thinking inside the box of the constraints we currently have without the isComplete flag.

We could go the Audible way 🤔: marking finished/unfinished doesn't alter in any way the currentTime and besides marking it manually, the only way it changes 'automatically' would be when the event audioPlayerDidFinishPlaying is called

[GianniCarlo]

I would argue that the complete state should be completely independent of the progress (ala Audible), if we do that, it won't have any side effect when marking as incomplete, and it won't (possibly) annoy the user since it won't lose their place when marking as complete.

[pichfl]

Works for me, we should just make sure that the "complete" state overrides the progress badge.