Skip to content
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

Config search #114

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Config search #114

wants to merge 11 commits into from

Conversation

flure
Copy link

@flure flure commented May 27, 2019

This is to give bonzomatic, when on Linux, the Linux way of looking for configuration and data.
It searches the config.json file first in the working directory, then in the user directory (~/.config/bonzomatic), and lastly in the system config directory (/etc/bonzomatic).

flure added 11 commits May 26, 2019 19:14
This work is handled by the ApplicationSettings class. On linux, it searches the config.json file first in working directory (directory level), then in ~/.config/bonzomatic (user level),  then in /etc/bonzomatic (system level).
It also infers the data directory (where are stored textures and other files) based on that location. In directory level and user level it is the same directory, in system level it's /usr/share/bonzomatic.
The platform_common (Windows and OSX) implementation always searches for config.json in current working directory.
Default shader is now loaded from data directory according to where config.json was found.
@Gargaj
Copy link
Owner

Gargaj commented Jun 1, 2019

I don't know how I feel about this, it feels like a ton of code for one specific platform that isn't necessarily needed because it's trying to compensate for a misconfiguration.

Thoughts, @PoroCYon ?

@PoroCYon
Copy link
Contributor

PoroCYon commented Jun 10, 2019

The diff is a bit large for what it actually does. While it could've been added in an #ifdef, now it'll need testing on all platforms...

And even then, you're not actually adhering to the XDG standard :P: it's $XDG_CONFIG_HOME/file first (which defaults to, and usually is, $HOME/.config/file), then $HOME/.file, and then /etc/file. Also, in a few (very strange) environments, HOME mightn't be defined.

Secondly, you might want to put the relative search path in the config file instead of kludging it in like this, but idk what Gargaj would think of this.

EDIT: sorry for the late reply, but somehow I missed this. I blame exams :p

@Gargaj
Copy link
Owner

Gargaj commented Jun 10, 2019

Secondly, you might want to put the relative search path in the config file instead of kludging it in like this, but idk what Gargaj would think of this.

You can already pass a path as parameter for an explicit config file path.

@flure
Copy link
Author

flure commented Jun 13, 2019

Thank you for the reply

And even then, you're not actually adhering to the XDG standard :P: it's $XDG_CONFIG_HOME/file first (which defaults to, and usually is, $HOME/.config/file), then $HOME/.file, and then /etc/file. Also, in a few (very strange) environments, HOME mightn't be defined.

Actually there is no $XDG_CONFIG_HOME out of the box on my system (Debian stretch). I have to install the package xdg-user-dirs to have it.
I've never heard of a case where $HOME is not defined, so I'm a bit puzzled...

The diff is a bit large for what it actually does. While it could've been added in an #ifdef, now it'll need testing on all platforms...

Mmmmh I'm sorry about that. I suppose I could make a shorter version and enclose it with an #ifdef.

EDIT: sorry for the late reply, but somehow I missed this. I blame exams :p

Same, sorry as well for the late reply, but I blame life :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants