New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
When loading save file, state, and screenshot directories, attempt to… #16421
base: master
Are you sure you want to change the base?
Conversation
… create them if they do not exist.
I'm not a fan of implicitly creating directories like this though. I think that should never be forced like this in this way, or at least not with mkdir function invocations littered all over the place. |
Thank you for the feedback. Would you prefer I back out the mkdir changes and just submit the warning message changes, or is this whole PR not needed at all? |
Yeah, just the warnings should be good. It's likely the installer that should set up those directories/configurations rather than the application itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made inline suggestions for just the message updates.
Just notify user of the fall back directory. Co-authored-by: Rob Loach <robloach@gmail.com>
Thank you! I have committed your changes (I hope, this is the first time I used the Github suggestions feature which is pretty cool). I agree it makes sense for the installer to handle directory creation, but users can change it later in the configs which is why I initially wanted to try to create the path if it does not exist. To give a little more context on why I initially wanted to make a folder if it didn't exist, it was due to me noticing that if you use setup to sort screenshots (or saves) by content directory, you need to go and create each folder yourself. Like if you put your screenshots in ~/Emulators/screenshots/, you need to create yourself. I wound up writing a python script to do that by scanning subfolders in my ROMs directory, but it'd be nice to avoid that step! |
… create them if they do not exist.
Guidelines
Description
In configuration.c, when checking the directory of the save state, save file, or screenshot paths either at initial program load or when applying overrides, we just fall back to a default if the directory is invalid or does not exist. This change makes it so we first attempt to create the directory with path_mkdir first. If that fails then we continue to log a warning and fall back to defaults. From my testing, path_mkdir respects filesystem permissions so we will not create a unwritable directory (it will fail and we go back to the default just like current behavior).
This will make the directory behavior more consistent since currently we do automatically create a Core or Content Directory parent folder when using the options to sort by content directory or core name.
I also updated the warning messages to tell the user where we are falling back to.
Related Issues
Could not find any
Related Pull Requests
N/A
Reviewers
[If possible @mention all the people that should review your pull request]