Support for multi deleting downloads and other major improvements/fixes

[Luna712]

TODO:

• Support deleting all episodes in a series
• Support multi deleting in child fragment (multi-deleting episodes)
• Update DownloadButtonSetup long click stuff to remove the unnecessary toasts and instead just handle multi delete
• If currently in 'multi-delete mode', clicking on an item should not play the media but toggle its deletion state
• Fix a UI bug with the AppBarLayout's always having one or the other never be visible until you start scrolling down
• Fix a bug where if you scroll down about 25 items some of the items at the top start getting deselected
• Fix a bug that only happens sometimes where if you long press to select an item, then click the cancel button, then long click the same item it will select and immediately deselect again
• Add a 'select all' button
• Make the back button close 'multi-delete mode' if it is currently active
• A few more UI/UX improvements, maybe utilizing some snack bars or toasts
• Improve formatting of confirmation dialog, to better differentiate series and others
• Add total bytes selected for deletion to delete app bar
• Make setOnLongClickListener toggle, so if it is already selected, another long click should deselect
• Don't unexpectedly exit 'multi-delete mode' if you manually deselect all items, instead add a message in app bar to select items, and make the 'Select All' possible to become a 'Deselect All'
• Move updating child lists to the view model
• Fix a bug where the download storage app bar is hidden when there are no downloads
• Fix a bug where the download storage app bar and delete app bar both overlap each other after actually deleting and it updates the storage data in view model which reshows that, after deleting 'multi-delete' mode should deactivate
• Fix bug where if you select all (manually or with the button) then manually deselect any, the deselect all button still appears and doesn't go back to select all, unless you actually click the deselect all button
• Improve multi-delete dialog for child downloads, mention season and episode, and make dialog mention episodes — we should use context.getNameFull() here
• Unset downloadDeleteEvent if we are multi-deleting so we don't end up running update on the lists for every single download
• Add better support for single deletions in dialog message
• Use view model for single deletions
• Faster update view model lists
• Further performance improvements in view model
• Fix updating child list on deletion
• Fix layout crash on Emulator and TV
• Fix focus issues on TV
• Fix potential race condition
• Fix paused download UI bug when adapter is refreshed
• Fix completely random inconsistent sorting in downloads
• Fix incorrect usedBytes
• Fix bug where if you delete a child episode with multi-delete, and re long click, the checkbox does not get checked properly
• Fix deprecations in Context.isNetworkAvailable()
• Fix a pre-existing bug where if you delete an item, a random item somewhere below it has it's bytes disappear
• Fix UI delay with download icons
• Make sure matching subtitles are also deleted (closes Feature Request : Delete the subtitles along with movie file #185)

Code-cleanup:

• Rename selectedChangedCallback in adapter to the cleaner and more descriptive onItemSelectionChanged — also should rename other callbacks to this format as well
• Remove unneeded multiDeleteStateCallback (we can just use selectedChangedCallback/onItemSelectionChanged)
• Don't use notifyDataSetChanged()
• Add a BackPressedCallbackHelper to unify back pressed logic
• Add a SubtitleUtils for more shared code to reduce duplication
• Remove usage of selectedIds in adapter in favor of using view model
• Only use IDs for storing in view model
• Massive cleanup to DownloadViewModel
• Remove use of requireContext()
• Rename updateList() to updateHeaderList() to match updateChildList() format
• Use Set() for ids since they should be unique
• Pass UiText in SnackbarHelper
• Remove unneeded TODO in DownloadChildFragment
• Fix comments for isMatchingSubtitle

I have tested this with up to 30 random videos from invidious and two different series with 4 episodes each, all at once, and it does appear to delete them all properly, along in their subtitles, and in about the same amount of time as just deleting one with the pre release version takes for me.

Follow-up patches

Collapsed for cleaner description here since these don't necessarily have anything to do with this patch.

• Make sure series sub-folders themselves are completely deleted if all child downloads are deleted
• Check network connection before trying to load online results
• Navigate to downloads by default if there is no network connection, and you have downloads
• Maybe remove the old delete method eventually when this method becomes more known, or some sort of edit button added to UI, similar to Netflix
• Add an animation when an item is selected, similar to Netflix — I would do this in this PR but I don't know how yet, I am working on it and this will likely be a follow-up patch instead
• Add support for the watch progress bar on header cards
• Should the UI for this be approved here, follow-ups will include multi select (using similar UI) for batch download, and batch mark as watched
  • Mark as watched will be first as it's easier than batch downloads due to how the download queuing system works and is in need of further improvement first
  • Batch downloads will mean a major update to the performance and functionality of downloads, improving how they work, and applying some of Better download feedback / queue view #1166 (which even before that issue went up was part of my plans)
  • Basically all those in queue will need to be made clear that they are and for failed downloads to work better
  • I would also probably apply Expand duplicate checking to resume watching and downloads as well #1185 for Downloads before batch download because it can cause a significant issue if you duplicate download due to naming conventions of Downloads. If you duplicate download and the name is the same, the old download is automatically deleted without confirmation. For an even large batch download this is a bigger issue
  • The queue needs fixed so 3 can simultaneously be started regardless in the order they were started in. As it stands after you start 3 downloads, when you queue the fourth, you have to wait for whatever one you started third to finish, then the fourth has to finish before the fifth starts, etc... this needs fixed before we go to true batch downloading as well

[fire-light42]

I believe this is best done in the viewmodel as otherwise the views will recycle when scrolling or switching the episode ranges ect.

[Luna712]

I believe this is best done in the viewmodel as otherwise the views will recycle when scrolling or switching the episode ranges ect.

@fire-light42

I thought about that, but I figured this would work to do through the adapter, and I wasn't sure how best to actually implement something through the view model. But I agree that would be best also. The whole reason I actually passed scope to the delete method was to pass in a view model scope, which would be best. But then I couldn't figure out how to actually implement it through the view model. But I can definitely give another shot at it. Thanks!

Also, one more note, the reason I even switched single deletes to use the new method passing one ID as a listOf() is because it seemed to actually have better performance. But maybe that was just in my mind. The old method, when monitoring some old logcat, seems to have some sort of OOM when used as a standalone, and this seemed to be less likely to OOM, but I'm also not certain about this 100%. But I did test that part that it works, which also confirms the actual function works, but I will attempt to move the multi delete activation code to the view model.

Also, just an FYI, I will do the actually multi-delete functionality in this PR and UI in another PR when I build it. I'd rather split it because that way, it's easier to review and less likely to be buggy, in my mind anyway. But if you'd rather me do it differently, I can try as well.

[Luna712]

Since I have a working UI that just needs more improvement I have decided to just do this all at once.

[Luna712]

@fire-light42 for the most part this works other than those TODOs however for the life of me I can not figure out that UI bug mentioned above and was wondering if you have any ideas/suggestions on that...

EDIT: I FINALLY figured the UI bug out, turns out it is some sort of Android bug, but there is a workaround the seemed to work...

[Luna712]

@fire-light42 other than deleting folders (which I am not sure what the best path for that is) I think this is ready for review. I do still plan to finish folders somehow, and you may want me to tweak UI which is fine if so as well, and I can do that.

EDIT: I found a few more things that still need to be worked on.

[Luna712]

@fire-light42 I partially fixed the UI updates so it at least does not show a whole separate icon now but progress still has to repopulate. I think that might be good enough for now?

[fire-light42]

The previous issues has been resolved from my test, however deleting a movie does not remove it from the list. You have to manually switch tabs for it to register.

[Luna712]

The previous issues has been resolved from my test, however deleting a movie does not remove it from the list. You have to manually switch tabs for it to register.

Hmm, it removes it for me... odd...

[Luna712]

It won't remove it if it marks it as failed to delete, is there any indication in logs that it thinks the file failed to delete?

[fire-light42]

@Luna712 After some debugging I found out that it is not a safefile/security issue this time, but a race condition. As deleteFile calls some events, those events can delete the file, resulting in toFile=null or delete=false. The fix for this is to also check for exists and moving the toFile to before the event.

    private fun deleteFile(context: Context, id: Int): Boolean {
        val info =
            context.getKey<DownloadedFileInfo>(KEY_DOWNLOAD_INFO, id.toString()) ?: return false
        val file = info.toFile(context)

        downloadEvent.invoke(id to DownloadActionType.Stop)
        downloadProgressEvent.invoke(Triple(id, 0, 0))
        downloadStatusEvent.invoke(id to DownloadType.IsStopped)
        downloadDeleteEvent.invoke(id)

        val isFileDeleted = file?.delete() == true || file?.exists() == false
        if (isFileDeleted) deleteMatchingSubtitles(context, info)

        return isFileDeleted
    }

[Luna712]

@Luna712 After some debugging I found out that it is not a safefile/security issue this time, but a race condition. As deleteFile calls some events, those events can delete the file, resulting in toFile=null or delete=false. The fix for this is to also check for exists and moving the toFile to before the event.

    private fun deleteFile(context: Context, id: Int): Boolean {
        val info =
            context.getKey<DownloadedFileInfo>(KEY_DOWNLOAD_INFO, id.toString()) ?: return false
        val file = info.toFile(context)

        downloadEvent.invoke(id to DownloadActionType.Stop)
        downloadProgressEvent.invoke(Triple(id, 0, 0))
        downloadStatusEvent.invoke(id to DownloadType.IsStopped)
        downloadDeleteEvent.invoke(id)

        val isFileDeleted = file?.delete() == true || file?.exists() == false
        if (isFileDeleted) deleteMatchingSubtitles(context, info)

        return isFileDeleted
    }

@fire-light42

Ah that makes sense, thanks... fixed!

[fire-light42]

Was about to merge this, however I found one final bug in my final round of testing. 😔

Download an episode of a series, select that episode, then switch to home and back. Then you wont be able to select that episode on the first try. You need to release before holding again or it may also get unchecked even when it tells you 1 episode is selected. Please double check your viewmodel+adapter logic 👍

[Luna712]

Was about to merge this, however I found one final bug in my final round of testing. 😔

Download an episode of a series, select that episode, then switch to home and back. Then you wont be able to select that episode on the first try. You need to release before holding again or it may also get unchecked even when it tells you 1 episode is selected. Please double check your viewmodel+adapter logic 👍

@fire-light42

Whoops, sorry about that, should be fixed.

[Luna712]

I found and fixed one last bug caused by this.

[fire-light42]

This will be merged after testing it for a final final time, if I do not find anything new. I will take a look tomorrow. 👍

[fire-light42]

I found another bug 😔 However it is sorta already in prerelease, so I dont think I can block this merge, but a fix would be appreciated.

If you cancel a movie or episode from notification during the download, that item wont get removed from the list. This was already the case for movies in prerelease, however this PR also makes episodes have the same bug.

[Luna712]

I found another bug 😔 However it is sorta already in prerelease, so I dont think I can block this merge, but a fix would be appreciated.

If you cancel a movie or episode from notification during the download, that item wont get removed from the list. This was already the case for movies in prerelease, however this PR also makes episodes have the same bug.

@fire-light42

This PR is already so large if it is another unrelated bug I would prefer to fix in a follow-up if it would be okay with you. If you really want I can try to fix it in this PR.

[fire-light42]

Lets make the prerelease people stress test this.