Fix "Save Main Configuration" saving to wrong file after "Load Configuration"#18771
Conversation
|
How does this work when RetroArch is started with a specific config file via the CLI parameter To be honest, as a user, if I load a configuration file, then modify options and use 'Save Configuration' I'd expect the save to go to the loaded file rather than a default configuration. |
|
I figured you guys would be most likely to have familiarity with this behavior and potential pitfalls. |
|
"Save Main" is working on Windows as expected after loading any other than default, so if it does not do that with all platforms and this fixes it, then good.
That is why there are "Save Current" and "Save Main" options separately. I guess it could be "Save Default" instead for slightly less confusion. Or maybe not, since "default" implies the fresh default config instead of the default config path.. |
the snapshot is taken after args are parsed, so "Save Main" saves back to the argument specified file. I just tested this again on macOS to be sure.
thanks for testing on windows! |
|
I meant it already works on Windows as-is, since the current method was developed on Windows. Would not make sense for the option to exist at all if it would not work anywhere: #18014 I have not tested this PR. |
|
Got it. To clarify, I was testing my changes on macOS to make sure I didn't break the existing |
|
I merged it but if there are issues let me know here and we can return back to it |
|
I did the same test (I think) as @sonninnos, but on Linux and - without this PR - I loaded a config file manually with 'Load Configuration'.
I'm not sure this PR is needed (on Linux and Windows is seems) ? |
|
@cmitu can you test on Android? I think the issue is prominent for android users specifically. edit: just for some context, I did prove the bug myself on android, and again in android development through adb installs. I'll want to confirm again after a full build. Edit 2: I asked some that were affected by the issue to help confirm it was fixed. #16491 |
I don't have a testing system with Android, sorry. |
Description
After using "Load Configuration" to switch to a different
.cfgfile,"Save Main Configuration" writes to the loaded file instead of the
original startup config. This is because
config_replace()updatesRARCH_PATH_CONFIGto point at the newly loaded file.The previous
CMD_EVENT_MENU_SAVE_MAIN_CONFIGhandler calledopen_default_config_file()as a side effect to reset the path, butdiscarded the returned
config_file_t*, leaking it on every save.Fix:
RARCH_PATH_CONFIG_DEFAULTslot right after
config_load()inretroarch_parse_input_and_config()RARCH_PATH_CONFIGto thedefault path before saving, then restore the previous value
open_default_config_file()static (no longer called outsideconfiguration.c) and remove its declaration fromconfiguration.hSteps to reproduce
.cfgTesting
Android (aarch64, Pixel) and macOS (Metal).
config_save_on_exitstill writes to the loaded file afterconfig_replace()Relates to #16491