funkyg / funkytunes

A streaming music player for Android, using torrents.
GNU General Public License v3.0
122 stars 24 forks source link

Show loading notification #35

Closed gjedeer closed 7 years ago

gjedeer commented 7 years ago

Closes https://github.com/funkyg/funkytunes/issues/19

funkyg commented 7 years ago

some things i noticed:

gjedeer commented 7 years ago

Makes sense, I'll take care of it. Thanks for reviewing.

play/pause button in the notification and in the bottom bar are not consistent, eg if you pause it from the bottom bar. pretty sure this worked correctly before

Do you have any clues why, maybe?

the loading circle is sometimes interrupted and restarted, same thing you noticed on the loading notificatoin

You mean the spinning circle ("indeterminate progress") not the progress bar? Interesting that i did not notice that.

gjedeer commented 7 years ago

Re. crash: do you happen to remember which torrent caused it? I think I know what the problem is but just want to confirm.

funkyg commented 7 years ago

regarding play/pause state, are you setting the bottom bar button directly anywhere? it should always go through MusicService.pause(), MusicService.resume() etc.

Yes the spinning circle. i was testing how the loading works, and clicking on different songs all the time. About half of the time it worked fine, but the other half the circle was reset every second or so. i guess getAdapter().notifyDataSetChanged() is called too often for some reason.

the album was "bruno mars - 24k magic". Like i said i was testing the loading, so i was clicking on different songs all the time, maybe that had something to do with it.

funkyg commented 7 years ago

i think in PlayingQueueActivity.onProgress, you only need to call notifyDataSetChanged() if the indeterminate progress bar is invisible on that item (so not every time). i just tested it again, and for some reason its showing me a normal progress bar (not indeterminate). screenshot

funkyg commented 7 years ago

pressing the close button in the notification also crashes the app:

FATAL EXCEPTION: main
Process: com.github.funkyg.funkytunes, PID: 6785
Theme: themes:{}
java.lang.IndexOutOfBoundsException: Invalid index 0, size is 0
    at java.util.ArrayList.throwIndexOutOfBoundsException(ArrayList.java:255)
    at java.util.ArrayList.get(ArrayList.java:308)
    at com.github.funkyg.funkytunes.activities.PlayingQueueActivity.onProgress(PlayingQueueActivity.kt:65)
    at com.github.funkyg.funkytunes.service.MusicService$playTrack$3$1.run(MusicService.kt:165)
    at android.os.Handler.handleCallback(Handler.java:739)
    at android.os.Handler.dispatchMessage(Handler.java:95)
    at android.os.Looper.loop(Looper.java:148)
    at android.app.ActivityThread.main(ActivityThread.java:5461)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616) 

edit: while a torrent is loading

gjedeer commented 7 years ago

regarding play/pause state, are you setting the bottom bar button directly anywhere? it should always go through MusicService.pause(), MusicService.resume() etc.

Not touching the bottom bar.

i think in PlayingQueueActivity.onProgress, you only need to call notifyDataSetChanged() if the indeterminate progress bar is invisible on that item (so not every time)

but the moment onProgress() is called I change the indeterminate progress bar to a real one. And every time onProgress() is called means that more parts of the file have been downloaded and progress bar needs to be updated.

Crash: I know why this is happening (using index of file in torrent as song number), i have a fix ready for this but no time to test

funkyg commented 7 years ago

okay i will try to fix these 2 bugs later

edit: i also noticed the play/pause thing on master, so its not caused by your code

gjedeer commented 7 years ago

I think i removed all problems you mentioned except for displaying the pause icon when paused.

gjedeer commented 7 years ago

Aaaaand the pause icon is there too

gjedeer commented 7 years ago

The latest commit should fix the problem. I was able to reproduce it with that Bruno Mars album and it's gone now

funkyg commented 7 years ago

great, i will make a new release :)