New path stuff for package distribuition#23
Conversation
|
i see there are issues with windows |
|
I noticed, but I don't thin this is an issue caused by me. I thing raylib is included before the windows API somewhere and it conficts |
well windows builds are always shitty in terms of failing |
|
I realized, I can probably set up the toolchain for it, so I'll try to do that |
| char fullPath[1024]; | ||
| const char *homeDir = getenv("XDG_CONFIG_HOME"); | ||
| if (!homeDir) homeDir = getenv("HOME"); | ||
| snprintf(fullPath,sizeof(fullPath),"%s/.config/TLEscope/%s", homeDir,tlefile); |
There was a problem hiding this comment.
XDG_CONFIG_HOME is already the full base directory (e.g. /home/user/.config) per the XDG basedir spec. Appending /.config/ unconditionally means that when the var is set, the path becomes /home/user/.config/.config/TLEscope/....
Correct handling:
const char *configHome = getenv("XDG_CONFIG_HOME");
if (configHome && configHome[0] == '/')
snprintf(path, sizeof(path), "%s/TLEscope/%s", configHome, file);
else
snprintf(path, sizeof(path), "%s/.config/TLEscope/%s", getenv("HOME"), file);The [0] == '/' check: spec says relative paths must be treated as invalid.
Same bug in every copy of this block across config.c and ui.c.
| printf("The $HOME env variable is not set. Exiting\n"); | ||
| return 1; | ||
| } | ||
| snprintf(configPath,sizeof(configPath),"%s/.config/TLEscope", homeDir); |
There was a problem hiding this comment.
Same XDG issue — hardcodes $HOME/.config/TLEscope without checking XDG_CONFIG_HOME.
mkdir also won't create parent dirs, so this fails if ~/.config/ doesn't exist.
| const char* get_resource_path(const char* resource) { | ||
| static char fullPath[512]; | ||
| #if defined (BUILD_FOR_DIST) | ||
| snprintf(fullPath, sizeof(fullPath), "/usr/share/TLEscope/%s", resource); |
There was a problem hiding this comment.
Consider a compile-time define (-DDATA_DIR="/usr/share/TLEscope") so packagers targeting other prefixes can override without patching.
| #include <sys/types.h> | ||
|
|
||
| //This is toggled by make options. Only here for convinience. | ||
| //#define BUILD_FOR_DIST |
There was a problem hiding this comment.
Commented-out #define probably shouldn't be commited. Doing make BUILD_FOR_DIST=1 would be enough for quick dev.
| if (!homeDir) homeDir = getenv("HOME"); | ||
| snprintf(fullPath,sizeof(fullPath),"%s/.config/TLEscope/%s", homeDir,tlefile); | ||
|
|
||
| printf("DEBUG: %s\n", fullPath); |
There was a problem hiding this comment.
Debug printfs should be removed or gated behind #ifdef DEBUG. Same across all copies.
| fprintf(file, " \"show_scattering\": %s,\n", config->show_scattering ? "true" : "false"); | ||
| fprintf(file, " \"show_skybox\": %s,\n", config->show_skybox ? "true" : "false"); | ||
| fprintf(file, " \"hint_vsync\": %s,\n", config->hint_vsync ? "true" : "false"); | ||
| fprintf(file, " \"show_first_run_dialog\": %s,\n", config->show_first_run_dialog ? "true" : "false"); |
There was a problem hiding this comment.
show_first_run_dialog was removed from the saved JSON output — the setting will be lost on every save. Intentional?
| #if defined (BUILD_FOR_DIST) | ||
| //The full path, with /settings.json at the end | ||
| char fullPath[1024]; | ||
| const char *homeDir = getenv("HOME"); |
There was a problem hiding this comment.
LoadAppConfig checks XDG_CONFIG_HOME before HOME, but SaveAppConfig only checks HOME. If a user has XDG_CONFIG_HOME set (Nix, some Arch setups, etc.), the app reads config from the XDG path but writes it back to $HOME/.config/ — changes never persist because next launch it reads from the XDG location again where the old file still lives.
| char fullPath[1024]; | ||
| const char *homeDir = getenv("XDG_CONFIG_HOME"); | ||
| if (!homeDir) homeDir = getenv("HOME"); | ||
| snprintf(fullPath,sizeof(fullPath),"%s/.config/TLEscope/%s", homeDir,tlefile); |
There was a problem hiding this comment.
All of these copies of the same path-resolution block, all with the same XDG bug.
A shared helper would replace all of them and make things much cleaner:
const char* get_config_path(const char* filename) {
static char path[1024];
#ifdef BUILD_FOR_DIST
const char *xdg = getenv("XDG_CONFIG_HOME");
if (xdg && xdg[0] == '/')
snprintf(path, sizeof(path), "%s/TLEscope/%s", xdg, filename);
else
snprintf(path, sizeof(path), "%s/.config/TLEscope/%s", getenv("HOME"), filename);
#else
snprintf(path, sizeof(path), "%s", filename);
#endif
return path;
}Every call site becomes fopen(get_config_path("data.tle"), "r").
|
guys, what do I do with these even? build breaking issues remain |
Just forger it and go on with life :nald2: ;) |
|
Besides addressing the code issues from my review, this PR should drop 31122a0 and rebase onto current main, where the Windows build issues are now resolved. |
4 participants
Now if you build with
make BUILD_FOR_DIST=yesinstead of using the resources in the current directory, they are loaded form/usr/share/TLEscopeand user config and downloaded TLEs are saved to.config/TLEscope. If the config directory is not found, it is made, then populated with defaults.