This is a pretty simple PR - it looks good and that's a good environment variable name - but I had a question about this:

it just that due to the argument parsing order the wrong pref_path is printed on "-get_flags json_v1",

Can you elaborate on this? If this is a bug, we should fix it.

-get_flags is not intended to be used along other flags, it just happens that the portable mode flag actually changes the output, so i dont know if this should be considered a bug or not. It does not affect the functionality of the -portable_mode function itself.

Also i dont know what this means exactly "// DO THIS FIRST to avoid unrecognized flag warnings when just getting flag file"
How checking if a flag is present can give you a unrecognized flag warning?

For the record, before anyone do any changes here, i belive printing "./" as the value of the pref_path in the flags json for when FSO is in legacy mode or while using -portable_mode is not completely ok here, as that is a relative path, and the SDL path printed in normal operation is an absolute path. So any launcher reading the json flags will have to interpret "./" as the executable path instead. But changing this is risky as it might break old launchers (or do nothing)

Also i dont know what this means exactly "// DO THIS FIRST to avoid unrecognized flag warnings when just getting flag file"
How checking if a flag is present can give you a unrecognized flag warning?

This comment may date from a time when the command-line was parsed in that function, rather than just checked. The intent of -get_flags is that the engine should create the flags file and then immediately exit. It should not throw any Warning or Error dialogs, otherwise these could interfere with wxLauncher and now Knossos. So, anything that produces a dialog should be avoided, whether it is applying a command-line parameter or even parsing a command-line parameter.

-get_flags is not intended to be used along other flags, it just happens that the portable mode flag actually changes the output, so i dont know if this should be considered a bug or not. It does not affect the functionality of the -portable_mode function itself.

After some consideration, I think that yes, this is a bug. The portable mode flag is relevant to the core functionality of the engine, and it is also relevant to -get_flags. So, I think -portable_mode should be moved above -get_flags and given an appropriate comment, something like "DO THIS FIRST FIRST" or "DO THIS ZEROETH". You can make this change as part of this PR.

For the record, before anyone do any changes here, i belive printing "./" as the value of the pref_path in the flags json for when FSO is in legacy mode or while using -portable_mode is not completely ok here, as that is a relative path, and the SDL path printed in normal operation is an absolute path. So any launcher reading the json flags will have to interpret "./" as the executable path instead. But changing this is risky as it might break old launchers (or do nothing)

Hmm. Good question.

Well, first we should double check our assumptions. Find an old build of FSO, from before the SDL upgrade, and check the output of -get_flags. See if the pref_path was "./" back then.

ill try with old builds, but there is a limit of how much i can go back, i think json flags is around 3.8.0.
mmmm
thinking about it, i think only knossos and knet uses the json flags...

ok, this is impossible to test directly on old builds. But ive looked at the code and 3.8.0 (before json flags) should report "./" for legacy/portable/fallback and the full sdl path for normal operation
https://github.com/scp-fs2open/fs2open.github.com/blob/release_3_8_0/code/osapi/osapi.cpp#L562

Anything before json flags is irrelevant anyway as pref_path is not exported to flags.ich
https://github.com/scp-fs2open/fs2open.github.com/blob/release_3_8_0/code/cmdline/cmdline.cpp#L1250

So, what i think i could do is to modify a build to export the executable path when "./" is set as the current preferences path. (This should only be for the json export and not to be used internally)
And then test if this breaks old-knossos normal operation (not sure if it supported portable executable mode) but i could try.
Knet is irrelevant if something breaks i can fix it, but im petty sure it should work.

And then report back, what im not sure if that should be done on this pr or not, or how soon should i try it.

Alternatively, i could define a "json_v2" for the get_flags that will now change "./" to current executable path, leaving v1 alone and any potential to break stuff on old launcher (i still petty sure knossos is the only to use json flags).
What im not sure if making a "-get_flags json_v2" is worth it for such change, it is small but it does have a small chance to break something, somewhere.

So, what i think i could do is to modify a build to export the executable path when "./" is set as the current preferences path. (This should only be for the json export and not to be used internally)
And then test if this breaks old-knossos normal operation (not sure if it supported portable executable mode) but i could try.
Knet is irrelevant if something breaks i can fix it, but im petty sure it should work.

And then report back, what im not sure if that should be done on this pr or not, or how soon should i try it.

Yes this is a good plan. Short, quick, and if it works that will be a very good thing.

This should be done before we close out this PR. If it works, we can make that change right now. If it doesn't work, we can tie that change to the json_v2 format.

EDIT: I'm also almost certain that the json format is only used for old-Knossos and Knossos.NET. I can confirm that Launcher 5.5g and wxLauncher both used flags.lch.

i havent realised that getting the path to the current executable directory (or executable) is non standart in C++ and needs a utility function with a OS specific implementation. This feels like a bit complicated, it is worth it? And i have no idea of how to do it for mac.

Hmm. It's an important issue that you pointed out, so I think it should be fixed, but it sounds like it's outside the scope of this PR. So I'll approve this one, and then that can be done separately. I'll also ask taylor what the best strategy is.

Thank you goober. Ill give it a try anyway when i can to test if the knossos and knet have a problem with that change, in case that is needed later.

I tested this on Linux.

export FSO_PREFERENCES_PATH=/home/user/fs/config
./fs2_open

This creates an unexpected file: ./configfs2_open.ini.

It works as expected with a trailing slash (/):

export FSO_PREFERENCES_PATH=/home/user/fs/config/
./fs2_open

This creates an expected file: ./config/fs2_open.ini.

I tested this on Linux.

export FSO_PREFERENCES_PATH=/home/user/fs/config
./fs2_open

This creates an unexpected file: ./configfs2_open.ini.

It works as expected with a trailing slash (/):

export FSO_PREFERENCES_PATH=/home/user/fs/config/
./fs2_open

This creates an expected file: ./config/fs2_open.ini.

this is correct, it is because the normal SDL path also have a trailing directory separator character, so if you set your own to override it, it should have it.

EDIT: That said, os_get_config_path() should be able to deal with that. Is the config system getting the path directly from getPreferencesPath()?