-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Settings: Allow user to manually set RTC per-game #12208
base: master
Are you sure you want to change the base?
Conversation
ac57d4c
to
15fe65c
Compare
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.
Quick check, been a while since I have done a review on the emulator, so feel free to correct me if I'm being a dumbass :)
pcsx2-qt/SettingWidgetBinder.h
Outdated
int DEFAULT_MINUTE = 0; | ||
int DEFAULT_SECOND = 0; | ||
|
||
std::string SECTION = "EmuCore"; |
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.
SECTION.c_str()
is 15 characters whereas "EmuCore" is 9 with quotes, making it not only clearer if we use it directly but also shorter.
Is there any reason that we define all those entries manually? It would be handy if we wanted to rename them, but besides that unsure of whether the added verbosity is worth it.
Also, if we keep them this way, a define might be worth it to avoid the cast to c_str
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.
This wrapper already does a lot of hard-coding, but I'm thinking I'm going to un-hardcode SECTION
in case this ever becomes an option for input recording specifically, since I intend to expand the capabilities of input recording in the future (including making a separate menu for them instead of them just being under 'Tools', thus it would have its own section). As for the keys, I think giving them variable names just makes the code more readable overall. For example, someone coming into this might see "RtcYear", "RtcMonth", etc. scattered a couple dozen times in this wrapper and have no idea what's going on, but when I see things like YEAR_KEY
, I understand intuitively what's happening (for example, "from the settings interface, get the integer value at this section and key and then write it into a value"). I imagine compiler optimizations completely smooth this over performance-wise, so I'm okay with the tradeoff of verbosity versus readability (as someone who currently thinks the codebase is already a bit impenetrable in places and who was only able to make this PR because the rest of SettingWidgetBinder.h
self-documents so nicely). If, however, you think the compiler isn't clevering this conversion out of the code, I can set it to a char *
, since I think the self-documentation is still really valuable.
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.
The compiler is probably clever-ing it out, but it adds some burden to the human reading said code ("wait, is this cast done at runtime? wait no it's defined above").
Regardless, don't see the need for it but won't die on this hill either.
15fe65c
to
dff774b
Compare
dff774b
to
7a723f3
Compare
Description of Changes
Allows the user the option to manually input a date and time for the PlayStation 2's real-time clock rather than 2020-04-03 00:00:00 for input recording and the OS' system clock otherwise. Only works if the BIOS is set to a region which is GMT+0 and DST is Summer Time (which is the default PCSX2 uses).
Rationale behind Changes
Multiple games have mechanics that play with the real-time clock.
Suggested Testing Steps
Screenshots
Tested on 2005-03-09 03:04:05