-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix VFS regression #10914
Fix VFS regression #10914
Conversation
2756a7a
to
22d483d
Compare
@@ -101,6 +101,24 @@ void fmt_class_string<game_boot_result>::format(std::string& out, u64 arg) | |||
}); | |||
} | |||
|
|||
template<> | |||
void fmt_class_string<cfg_mode>::format(std::string& out, u64 arg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong with keeping all logic in m_config_path? Because std::string is usually defined everywhere it can be used in headers which do not include System.h very cleany.
Formatting is implicitly declated and it is one less argument passed to BootGame.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the flip side, this is cleaner. Encoding settings in a config path and having special interpretation of strings is not the best approach. This approach is more explicit about what is going on which is easier to maintain when the team inevitably changes in the future.
I think this pr can be rewritten to remove ability to control VFS from manully selected comfig. I do not see a good reason behind this rewrite except maybe renaming some modes and other stuff. |
Everything you asked or recommended was addressed in the description |
22d483d
to
5c3d55d
Compare
e5bae48
to
539f641
Compare
A recent PR introduced a new config path emu member.
In the future we should detach VFS and similar global settings from the regular config file to prevent this issue from happening.
But for now this shall act as an alternative (and hotfix as is).
Hopefully fixes #10862
Fixes #10867 (at least the idea behind it)