llllllllll / slider

Utilities for working with osu! files and data
https://llllllllll.github.io/slider/index.html
GNU Lesser General Public License v3.0
39 stars 16 forks source link

beatmap saving needs to sanitize file names for windows #10

Closed de-odex closed 7 years ago

de-odex commented 7 years ago
Traceback (most recent call last):
  File "C:\Users\deodex\AppData\Local\Programs\Python\Python36\lib\site-packages\slider\library.py", line 218, in lookup_by_id
    return self._read_beatmap(self, beatmap_id=beatmap_id)
  File "C:\Users\deodex\AppData\Local\Programs\Python\Python36\lib\site-packages\slider\library.py", line 191, in _raw_read_beatmap
    return Beatmap.from_path(self._cache[f'id:{beatmap_id}'])
  File "C:\Users\deodex\AppData\Local\Programs\Python\Python36\lib\site-packages\slider\library.py", line 31, in __getitem__
    return d[key]
  File "C:\Users\deodex\AppData\Local\Programs\Python\Python36\lib\dbm\dumb.py", line 148, in __getitem__
    pos, siz = self._index[key]     # may raise KeyError
KeyError: b'id:969681'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\deodex\Documents\aEverrBot\bot.py", line 209, in sendpp
    beatmap = l.lookup_by_id(beatmap_id=int(beatmap_data[0]["beatmap_id"]), download=True, save=True)
  File "C:\Users\deodex\AppData\Local\Programs\Python\Python36\lib\site-packages\slider\library.py", line 222, in lookup_by_id
    return self.download(beatmap_id, save=save)
  File "C:\Users\deodex\AppData\Local\Programs\Python\Python36\lib\site-packages\slider\library.py", line 338, in download
    self.save(data, beatmap=beatmap)
  File "C:\Users\deodex\AppData\Local\Programs\Python\Python36\lib\site-packages\slider\library.py", line 269, in save
    with open(path, 'wb') as f:
OSError: [Errno 22] Invalid argument: "C:\\Users\\deodex\\Documents\\aEverrBot\\osulib\\Nanamori-chu*Gorakubu - Acchu~ma Seishun! (Pata-Mon)[Fycho's Seishun].osu"

l is l = slider.library.Library(config.librarydir + "/osulib") It says in the docs that lookup_by_id downloads the map if it doesn't exist, though it still gives me a KeyError. Am I doing something wrong?

llllllllll commented 7 years ago

This error means that the beatmap was downloaded correctly; however, we failed to save it to disk. Does this work if you pass save=False?

What I think is happening is that ntfs (the windows filesystem) is much more restrictive in the acceptable names for files. This path has *~!()[], I bet one of those is the problem.

What we need to do is detect what system you are on and encode the name in a safe way for the given system. We already have some stuff that strips out / from the path, but on windows that needs to be \\.

de-odex commented 7 years ago

Why not use os.name to check what system i'm in? in windows it's nt, in linux or macos its posix

no error in save=False

In another project, the devs sanitized *~!()[] to *~!()[], you can do either that or Do what osu! does - flat out cut them out from the file name

de-odex commented 7 years ago

In fact, ~! are okay with ntfs image

llllllllll commented 7 years ago

yeah, switching to unicode alternative glyphs will just confuse people, I will strip them.

de-odex commented 7 years ago

https://github.com/llllllllll/slider/pull/11

llllllllll commented 7 years ago

merged, thanks for the quick fix!