Hello again everyone,
I've run into an issue that requires sweeping changes, and I thought I'd post here to get everyone's thoughts on my proposed solution. The issue: "\" on Linux messes up things like "file_exists()" and "sound_add()" (and a LOT of other functions). Although using "/" will work on both Windows and Linux, many developers forget or aren't aware. At the very least, both Project Mario and Iji are affected.
My proposed fix is outlined
in this commit, which is a prototype fix that I will expand to cover all the other file functions if everyone thinks it makes sense.
Basically, I've changed the following:
//Every instance of "fname", such as :
int file_exists(std::string fname);
//...becomes:
int file_exists(const filestr& fname);
The "filestr" class is defined in PFfilemanip.h, and takes a "const char*" in a single-argument constructor. This allows you do the following in your script:
file_exists("somefile.txt")
...in other words, no changes are required to user scripts. The "filestr" class also exports "c_str()", so the functions themselves (e.g., "file_exists") only need to change the parameter type of "fname". The constructor of "filestr" is platform-dependent:
//On Linux:
filestr::filestr(const char* fname) : data(fname) { std::replace(data.begin(), data.end(), '\\', '/');}
//On Windows:
filestr::filestr(const char* fname) : data(fname) {}
In other words, Windows filenames are unaffected, and Linux filenames should "just work". (There are still potential problems with case sensitivity, but by far the most common problem I've seen is with backslashes.)
Please let me know what your thoughts are on my solution. In particular:
- Is this something generic enough to replace *every* occurrence of "string fname" with "const filestr& fname"?
- Is the "filestr" class defined in the right place?
- Are there any other concerns I should be aware of, or reasons this is not considered a good solution?
I'm happy to scour the code base and make these changes, but I want to make sure this is a good approach first.