libretro / scummvm

ScummVM main repository
https://www.scummvm.org
GNU General Public License v3.0
4 stars 6 forks source link

scummvm core backend tab default playlists path is wrong #48

Closed i30817 closed 6 months ago

i30817 commented 7 months ago

It shows the system dir instead. And when I opened it to change, it opened in my scummvm games directory, which was a bit strange too, but may be a scummvm behavior of opening the last directory of any filechooser (I opened mass add by accident when I was trying to find this new function)

spleen1981 commented 7 months ago

That is the indended behaviour, there is no way to get retroarch playlist folder path from libretro API afaik, so user needs to choice it manually. Once one path is chosen, it will be saved as a specific setting. If no setting is available (e.g. cleared from GUI or first run) it should default to s_homeDir which depends on platform.

i30817 commented 7 months ago

Playlist directory it's in the cfg though, it can be parsed from there.

Even if that's too troublesome because of overrides, dependencies of finding which runtime cfg is being used (I tend I agree now that I listed these), id argue that setting the default to the correct value 99% of the time (RA main dir + 'playlists') then fail with a informative message in the 'status' label you have if it doesn't exist or is not writable, is much better than potentially dumping a playlist in system?!

Would that even do something?

spleen1981 commented 7 months ago

Playlist directory it's in the cfg though, it can be parsed from there.

I would like to avoid any dirty solution parsing directly configuration files, it's hacky and not guaranteed to work properly. Playlist path is not provided by the API, though support would be very easy to implement. I've submitted a PR to add this feature. If it is merged, once libretro-common will be resynced, I will add the API provided path as default playlist path in the core (if available from the frontend, otherwise it will fall back to current behaviour). Anyways user is supposed to check if the target path shown in the generator is correct or not, eventually...

informative message in the 'status' label you have if it doesn't exist or is not writable

That is already included

i30817 commented 7 months ago

Yes, but instead of the cfg you can use the RetroArch home + playlists fixed directory, which usually exists and usually is the playlist dir.

I think that might be a better default than the same + system which also usually exists but is completely wrong.

I actually don't know how you can use that as default and get a status error, because it usually exists.

spleen1981 commented 6 months ago

Default playlist path through the brand new API functionality is now implemented 2927be02805bd7de9ac6900a1804f768a4f254c4