[Linux] Predictable paths for configuration, nvram, etc

[casasfernando]

When running Supermodel on Linux the program expects to find the configuration files and folders (e.g. Config, NVRAM, Saves) in the same path where the binary was called from.

For example if the user is sitting on /directory_a and he executes supermodel, the program expects to find Config directory in /directory_a/Config, but then if the user moves to say /directory_b and executes supermodel again, the program will look for the Config directory in /directory_b/Config and it will fail. This forces the user to always execute the program binary while sitting in the same directory where the Config and other program folders are otherwise it will not find them.

In order to solve this the program folders should reside in a standard and predictable place where the program will always look for them instead of current directory. E.g.: the user home directory (/home/<username>/.config/supermodel). As a plus, this would also allow for different users in the same Linux machine to have separate configuration files, nvram, save states, screenshots, etc.

You can find a sample, outdated, patch that reflects what I'm talking about, here: https://aur.archlinux.org/cgit/aur.git/tree/multiuser.patch?h=supermodel-git

I can work on a PR for this if you are interested.

Looking forward your thoughts. Thanks

[casasfernando]

Worked on my fork and created the following commit to address this issue: https://github.com/casasfernando/Supermodel/commit/310276b88dd143eae063e70be3b63634851fce66

Any feedback or thoughts are welcome. Thanks.

[trzy]

Thanks. Here are my thoughts:

• We should limit the use of #ifdef. Instead, we should have a source file in OSD/UNIX or something that only gets compiled on UNIX and MacOS platforms providing functions that return the various folder paths. A default implementation can be provided in OSD/SDL and that can continue to be used for Windows.
• I kind of like having Config/ in the current Supermodel directory but understand why not everyone prefers this. So how about: 1) first look for local Config/ and if present, go with that. 2) If not present, try ~/.supermodel/Config
• ~/.config seems wrong. Don't most UNIX programs use ~/.programname? If so, then we should have ~/.supermodel with the folder structure inside replicated: ~/.supermodel/Config, ~/.supermodel/NVRAM, ~/.supermodel/Saves, ~/.supermodel/Screenshots

If you can make these changes and submit a PR that would be great and we'll nit-pick it from there :)

[casasfernando]

Thanks for looking into this @trzy I really appreciate the feedback. Will add my comments below:

1. This sounds great, but in all honesty I don't have a clue on how to implement it that way. That's why I went with the simple (and probably not sophisticated/elegant) solution. My programming skills (specially in C++) are very limited. :(
2. Makes sense although I would invert the search order, yet is not a big deal.
3. The reason I used ~/.config/supermodel is because ideally (at least in Linux) one should follow the XDG base directory specs (https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html). Moreover if following the specification only Config should go in ~/.config/supermodel, while NVRAM, Saves, Screenshots and probably also the log file should go into ~/.local/share/supermodel. Having said that while this is not mandatory and different apps do different things (unfortunately). From having everything under ~/.config/appname to as you suggest ~/.appname. This is why I decided to go with something in the middle and use ~/.config but keep it simple and put everything under it. If I get to choose I would try to follow the XDG spec, but any of the other options mentioned would work.

About doing the changes and submitting a PR, I will do some research and see how it goes, but I'm not holding my breath. As I said in 1. I'm not really a developer and my programming skills are very basic.

Thanks.

[trzy]

Number 1 isn't as hard as it sounds. Maybe we can try to get it started with providing the Config/ directory for the ini and xml files first. Note the current directory structure:

Src/OSD/ <-- "OS-dependent" stuff
Src/OSD/Windows/ <-- Windows specific. Only Makefile.Win32 includes files in here.
Src/OSD/SDL/ <-- the "default" case common to all systems, since we are using SDL. Should have a minimum (ideally none) of platform-dependent code. Note that right now, Linux and Mac OS don't have any additional OS-dependent code of their own.

So, begin by creating: Src/OSD/UNIX/

Then, create Src/OSD/SDL/FileSystem.cpp to begin with. In that file, create a function that gets the path to the Config folder. For now, this will just be "Config/" (the local folder we currently use). Since I'm not looking at the code right now, I don't know what it should be called but it should be used in the places you had modified to get the home directory. Probably it should be something like:

std::string GetPersistentDirectoryBase() { ... }

Or GetPersistentDirectory().

Create Src/OSD/FileSystem.h -- the header file defining this function should be there.

Once you modify the Makefile to include FileSystem.cpp, you should have a build of Supermodel that does nothing different. The Windows build will end up using this version for now.

Now you can create a FileSystem.cpp in Src/OSD/UNIX/ and implement the same function. This time, however, it will return something different: Config/ if it exists otherwise ~/.supermodel/ or ~/.config/supermodel. I still kind of prefer ~/.supermodel/ for everything but we can debate that later. Now you have two FileSystem.cpp in the source tree, only one FileSystem.h (since all versions of FileSystem.cpp implement the same function), and the different Makefiles will decide which FileSystem.cpp to include.

In Makefile.UNIX and Makefile.Mac you would use this FileSystem.cpp (Src/OSD/UNIX/FileSystem.cpp) whereas in Makefile.Win32, you would instead have Src/OSD/SDL/FileSystem.cpp, our default one, instead.

Does that make sense? If not, you can make a PR for what you have and we can go from there in more detail.

[casasfernando]

Makes sense. Thanks for the guidance. I will prepare something in the coming days and share it for review since I expect a few iterations before having some good code for a PR.

[casasfernando]

@trzy I followed your guidance.

Take a look at the code here: https://github.com/casasfernando/Supermodel/commit/6eab89b715b9f2a45d0eab3af986ab8df0339b45

Notes:

1. I had to create a FileSystemPath.cpp for Windows under Src/OSD/Windows, instead of leaving it under Src/OSD/SDL because for some reason that I still don't understand, make was ignoring the one under Src/OSD/Unix (as configured in Makefile.UNIX) and used the one under Src/OSD/SDL instead.
2. I went a little bit further and will check for Config in current path, ~/.supermodel and ~/.config/supermodel in that order. Depending on where Config is found, the other directories will follow.
3. Also in case the directories don't exist, they will be created, so users don't need to learn or guess their names/locations from the error output or the source code.

Looking forward your thoughts and corrections since it's my first C++ coding exercise.

Thanks.

[trzy]

This looks pretty good -- I will definitely have some stylistic comments, though. I'll try to get to it this evening!

[trzy]

Apologies but I haven't been able to test this out yet and will try to do so by end of week or Saturday!

[trzy]

Hey I just left a bunch of comments. Feel free to discuss there or here! It looks really good and is exactly what I was thinking in terms of file structure. The API is nice, too! I have a few easy fixes to make (formatting stuff) and one that is a little trickier and for which I want your input as a UNIX user (whether directories should be created or not).

[casasfernando]

Thanks for reviewing the PR @trzy. Indeed in my experience it's typical for applications to create their default files (e.g.: default config file) and directories automatically on first run, if they don't exist.

I went through your comments in the PR, acknowledged some of them and replied to others with some thoughts.

Looking forward your feedback to apply the required changes to the PR.

[trzy]

Can you submit this as a PR? I don't see it in the PR list.

[casasfernando]

Ah sorry for the confusion @trzy I didn't create the PR yet because I was waiting to finalize the discussion on the changes needed based on the feedback you already shared. Take a look at the comments: https://github.com/casasfernando/Supermodel/commit/6eab89b715b9f2a45d0eab3af986ab8df0339b45

Do you prefer me to change the code already agreed on those comments, create a PR and continue discussion on the PR?

[trzy]

Let's make a PR and continue discussion there -- I think it's pretty much almost there just wanted to have another close look. Unfortunately it may take me until next weekend to get to it but I'll try to find some time this week to look at this and the other outstanding PR!

[casasfernando]

No worries. Will create the PR before the weekend, including the changes already agreed on and we can take it from there.

[casasfernando]

@trzy PR created: https://github.com/trzy/Supermodel/pull/44 I already included some obvious changes but others I wanted to wait for further discussion to confirm them and change the code.

Thanks!

[casasfernando]

Added another commit to change pathType from string var to enum as suggested in the comments. That made much more sense so it's implemented now.