Alaanor / beatlist

Beatlist is an app to manage playlists and beatmaps for the game Beat Saber.
MIT License
202 stars 54 forks source link

Song title sanitizing before folder creation #20

Closed brosibj closed 4 years ago

brosibj commented 4 years ago

If a song title has a special character in it which Windows does not support inside of directory names, the folder for the song fails to be created and therefore the song cannot be downloaded within Beatlist. At least that's what I'm assuming.

As a work around, I can install via ModAssistant by using the suggested workaround for changing the beatsaver:// handler temporarily mentioned here . After doing this, beatlist does recognize the song is installed. It just cant install it on its own.

Workaround may just be doing a quick search of the string for any characters not allowed in Windows directories if you're not already:

< (less than)
> (greater than)
: (colon - sometimes works, but is actually NTFS Alternate Data Streams)
" (double quote)
/ (forward slash)
\ (backslash)
| (vertical bar or pipe)
? (question mark)
* (asterisk)

If this is something you are already doing then I'm not sure why the folder is failing to create, as any other song I've tried so far has been working without issue.

The song in question that I had noticed the issue with is https://beatsaver.com/beatmap/ebb

Edit: Formatting

brosibj commented 4 years ago

Well I went looking and I think I found what needs to be changed, looks like a few characters can be added to the regex just in case the show up in a song name.

https://github.com/Alaanor/beatlist/blob/5e74bda0d030167c0988d97775d3da80211da5ac/src/lib/BeatSaber.ts#L90

Alaanor commented 4 years ago

I was going to say that is it right and I've fixed this on the rewrite branch. But I haven't published the commit yet. I'll check tomorrow. In any case the issue is valid and will be fixed for the 1.2, thanks for the list of invalid characters 👌

b-rad15 commented 4 years ago

You also need to add : as i just opened up in #35

Alaanor commented 4 years ago

To be honest in this issue I prefered to whitelist a set of character instead of blacklisting. That was just easier.

https://github.com/Alaanor/beatlist/blob/4b19b18fd59479df6babce71641f2b9c296e2380/src/libraries/os/beatSaber/BeatSaber.ts#L76-L92

Closing this issue as it now fixed, implemented and released :)