Closed eternaldensity closed 4 years ago
Thank you; I will give it a closer look later when I have more time. I'm not sure I fully understood the need for the 'downloads' section in the Playlist; is this necessary to specify "arbitrary" locations for songs? I think we could just require this parameter.
From a user perspective I think we could require to have on the quest users to create a folder ""/sdcard/BeepSaber" and put the songs in there. Your auto-parsing code could than just create a full playlist from all songs in /sdcard/BeepSaber/Songs" and we also require another folder /sdcard/BeepSaber/PlayLists" maybe?
Regardint the permission request in-app: I have code for this here https://github.com/NeoSpark314/VoxelWorksQuest/blob/25d562773f7f104dc5b99513c9876dc975e8dec1/ui/GameplaySettings.gd#L205
I can add it later or you could include it in your PR for the auto-parsing
I added the extra section to the playlists only for backwards compatibility, as I was unsure whether the existing format you were using was something that needed to be preserved. If you're fine with changing the format and requiring that parameter, then you can just rename the 'downloads' section to 'songs' and remove the old 'songs' code.
I haven't tested other folders than the Downloads folder yet. It was simple for testing for me since I keep my zips in a folder in my Downloads. And I figured it needed to be able to look in the Quest Download folder anyway for being able to process or open zips downloaded on Quest, since it's really convenient to be able to do that without needing to connect to PC to move files around. (though sideloading a file manager should also let that be done in-headset)
But yeah having a /sdcard/BeepSaber folder structure does make sense. Do you want to allow multiple files containing playlists, or have them all in one Playlists.json? (plus presumably a builtin playlist with any builtin songs. that would be read only if stored in res://)
I think I tried out the permission request and it didn't work cos I had something set up wrong with the android templates. I'll have another go at it.
I shall make a new version of this PR using /sdcard/BeepSaber/Songs and with just a 'songs' section in the playlist file.
On Thu, Oct 22, 2020 at 7:54 PM Holger Dammertz notifications@github.com wrote:
Thank you; I will give it a closer look later when I have more time. I'm not sure I fully understood the need for the 'downloads' section in the Playlist; is this necessary to specify "arbitrary" locations for songs? I think we could just require this parameter.
From a user perspective I think we could require to have on the quest users to create a folder ""/sdcard/BeepSaber" and put the songs in there. Your auto-parsing code could than just create a full playlist from all songs in /sdcard/BeepSaber/Songs" and we also require another folder /sdcard/BeepSaber/PlayLists" maybe?
Regardint the permission request in-app: I have code for this here
I can add it later or you could include it in your PR for the auto-parsing
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NeoSpark314/BeepSaber/pull/2#issuecomment-714340821, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJQFGILCDKR7UXZM3GFNGTSL7XMNANCNFSM4S23CRSQ .
Thanks for the clarification; at the moment backwards compatibility is not needed; it is just the start of the standalone version so we can discuss and define how it should look and also can change anything.
Regarding the ability to download from quest: this is a good point; if unzip works it will allow to get and play new songs without leaving the headset; if possible we should make it for users as easy as possible so we should probably just keep it in addition that the Downloads folder is supported and checked
Multiple playlists (loaded from /sdcard/BeepSaber/Playlists) are probably nice + an autogenerated playlist with all songs in /sdcard/BeepSaber/Songs folder.
regarding the permission: I will try this afternoon; I remember that there was a bug some time ago that prevented the permission request working correctly; maybe it came back and is included in the bin/android_debug.apk template
I think I'd had the wrong template since no permissions stuff was working at all until I fixed that.
I've got it loading multiple playlists now. well I've only tested it on one playlist but it no longer hardcodes the name. (though it does still load the one from res:// too)
it's simpler codewise to just load songs from one main folder (BeepSaber/Songs/) and have all playlists point to there (plus whatever is hardcoded into res://). Downloads folder support would probably be just unzipping zips in there and putting them into BeepSaber/Songs.
On Thu, Oct 22, 2020 at 9:06 PM Holger Dammertz notifications@github.com wrote:
Thanks for the clarification; at the moment backwards compatibility is not needed; it is just the start of the standalone version so we can discuss and define how it should look and also can change anything.
Regarding the ability to download from quest: this is a good point; if unzip works it will allow to get and play new songs without leaving the headset; if possible we should make it for users as easy as possible so we should probably just keep it in addition that the Downloads folder is supported and checked
Multiple playlists (loaded from /sdcard/BeepSaber/Playlists) are probably nice + an autogenerated playlist with all songs in /sdcard/BeepSaber/Songs folder.
regarding the permission: I will try this afternoon; I remember that there was a bug some time ago that prevented the permission request working correctly; maybe it came back and is included in the bin/android_debug.apk template
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NeoSpark314/BeepSaber/pull/2#issuecomment-714384286, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJQFGMQ3ZPC46I334FDJILSL77ZJANCNFSM4S23CRSQ .
I've now updated it to look in BeepSaber/Playlists (multiple) and load songs from BeepSaber/Songs. It also autogenerates playlists, though practically that should be on a button. Also we'll probably want it to only add songs which aren't already in other playlists. and it currently doesn't save playlists, just generates them.
Wow you are quick! Thanks; I will merge this now and we can continue from there.
Loads additional playlists from a Playlists.json in the download directory as specified by OS.get_system_dir(3) Loads old playlist format as before, unchanged. If a playlist has a "Downloads" section it will treat those songs a little differently. Instead of just an id for each song it now also optionally tracks a 'source'. If the key 'source' is present then it is treated as a subfolder name in the download directory. Otherwise it's just loaded from res:// (if source is an empty string then it will load from the download directory, not a subfolder)
make sure you have the right Android Build Template you'll also need to run the following adb command once (in SideQuest): adb shell pm grant org.dammertz.vr.godot_oculus_quest_toolkit_demo android.permission.READ_EXTERNAL_STORAGE
Then it will be able to successfully open files in the Download directory on Quest.
Here's an example of a Playlists file which contains a playlist with a song which is the downloads directory:
the id still needs to be equal to name of the song's individual folder. source is what makes it look in the platform-specific downloads folder if source is not empty then it will look for the song folder in a subfolder so if source was "beep saber songs in here" then you'd need to put the song folder inside a folder with that name.
here it is working:
I also wrote a thing for scanning the downloads directory for folders with songs and automatically generating playlists from them but I'll PR that separately.
I also did a thing for loading songs in zip files but 1. it's rather slow and 2. it keeps hanging when trying to play the song on Quest. In retrospect making gdscript do an unzip of the whole archive for every file it reads was a bad idea on my part.