SimpleMobileTools / Simple-Music-Player

A clean music player with a customizable widget, stylish interface and no ads.
https://www.simplemobiletools.com
GNU General Public License v3.0
1.26k stars 410 forks source link

Fix #426 added feature: Play next #550

Closed Yashraj254 closed 11 months ago

Yashraj254 commented 11 months ago

Fix #426 : add a song next to the currently playing song in the queue...

Screenshot 2023-07-16 043451

https://github.com/SimpleMobileTools/Simple-Music-Player/assets/52420957/18caa448-7943-469a-94b7-373fde05131a

tibbi commented 11 months ago

@naveensingh please check if its ok

naveensingh commented 11 months ago

@Yashraj254 can you resolve the conflicts please

naveensingh commented 11 months ago

@Yashraj254 It crashes when I do Play next on Albums, Artists fragments. Do it in the background.

java.lang.IllegalStateException: Cannot access database on the main thread since it may potentially lock the UI for a long period of time.
    at androidx.room.RoomDatabase.assertNotMainThread(RoomDatabase.kt:444)
    at androidx.room.RoomDatabase.query(RoomDatabase.kt:485)
    at androidx.room.util.DBUtil.query(DBUtil.kt:75)
    at com.simplemobiletools.musicplayer.interfaces.AlbumsDao_Impl.getArtistAlbums(AlbumsDao_Impl.java:250)
    at com.simplemobiletools.musicplayer.adapters.ArtistsAdapter.getAllSelectedTracks(ArtistsAdapter.kt:112)
    at com.simplemobiletools.musicplayer.adapters.ArtistsAdapter.playNext(ArtistsAdapter.kt:102)
    at com.simplemobiletools.musicplayer.adapters.ArtistsAdapter.actionItemPressed(ArtistsAdapter.kt:65)
    at com.simplemobiletools.commons.adapters.MyRecyclerViewAdapter$1.onActionItemClicked(MyRecyclerViewAdapter.kt:58)
    ...
naveensingh commented 11 months ago

@tibbi do you think Play next should be visible when albums/artists are selected? We could add all the tracks in/by the selected album/artist to be played next but most players don't seem to allow this (Play next is only visible with tracks).

Yashraj254 commented 11 months ago

@naveensingh should i remove this play next functionality for artists and albums and allow play next functionality for single selected track only..

naveensingh commented 11 months ago

@Yashraj254 probably yes but let's wait for @tibbi's view...

Play next is sort of a valid option for artists, albums if we wanna add it

naveensingh commented 11 months ago

It is working fine for single tracks so far, the option is crashing at artists, albums fragments. I'm looking for code issues now...

tibbi commented 11 months ago

yeah, lets add it to tracks only

naveensingh commented 11 months ago

Edge cases:

Everything else looks good to me :+1:

Yashraj254 commented 11 months ago

so, should i disable the Play Next option for currently playing track

naveensingh commented 11 months ago

yes, it doesn't work anyway. Note that MusicService.mCurrentTrack could be null in some cases

tibbi commented 11 months ago

so it is ready now?

naveensingh commented 11 months ago

Yes, it is ready. There's some minor style issues I commented above.

Yashraj254 commented 11 months ago

Play next wont be visible if MusicService.mCurrentTrack is null

naveensingh commented 11 months ago

That's okay, otherwise, it'll probably crash something.

naveensingh commented 11 months ago

@tibbi this can be merged now, I'll fix any style issues in https://github.com/SimpleMobileTools/Simple-Music-Player/pull/559

tibbi commented 11 months ago

fixed it myself to speed it up, thanks