Skip to content
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

DolphinQt/Config/ARCodeWidget: Avoid unnecessary disk operations #8277

Merged
merged 8 commits into from Aug 9, 2019

Conversation

@lioncash
Copy link
Member

commented Jul 31, 2019

In a few cases we were writing out to disk, even when we didn't need to. e.g.

  • If a user wanted to clone a code and then clicked cancel on the following edit dialog, we'd still create the code and write out to disk.
  • If a user wanted to edit a code and then did nothing and hit cancel, we'd write that code out to disk (even though nothing changed).

This amends our code so that we're not unnecessarily writing out to the disk. While we're in this area, we can do a little bit of cleanup and also apply it to the Gecko code widget, as the code is somewhat similar to the AR code handling.

lioncash added some commits Jul 31, 2019

DolphinQt/Config/ARCodeWidget: Deduplicate ini path
We can just store this to a const local and use it to avoid doing the
same work twice.
DolphinQt/Config/ARCodeWidget: Avoid unnecessary disk operations
If a user indicates that they want to clone and edit an AR code, then
click cancel on the following dialog, we shouldn't actually clone the
code.

We also shouldn't resave the codes if the edit dialog is opened and then
closed again via cancel, as there's nothing that actually changed. This
way we don't perform disk accesses unless they're actually necessary.
DolphinQt/Config/ARCodeWidget: Call LoadDefaultGameIni() directly
This is a static function, so we don't need to go through the instance
of SConfig in order to call it.
DolphinQt/Config/ARCodeWidget: Use forward declarations where applicable
Avoids propagating headers into scopes where they're not necessary.

Also uncovered reliance on an indirect inclusion within
CheatsManager.cpp, which is now fixed.
DolphinQt/Config/GeckoCodeWidget: Deduplicate ini path
We can store this to a local variable to avoid duplicating the same
string creation twice.
DolphinQt/Config/GeckoCodeWidget: Make exec() outcomes explicit
Makes it a little more explicit which dialog outcomes we're expecting.
While we're at it, we can invert them into guard clauses to unindent
code a little bit.
DolphinQt/Config/GeckoCodeWidget: Call LoadDefaultGameIni() directly
This is a static class function, so we don't need to go through the
SConfig instance in order to call it.
DolphinQt/Config/GeckoCodeWidget: Use forward declarations where appl…
…icable

Avoids propagating headers into scopes where they're not needed.
@BhaaLseN
Copy link
Member

left a comment

I kinda had a dejavu while looking at those changes, but for the intents and purposes of this PR they seem fine. Untested though.

@stenzek

stenzek approved these changes Aug 9, 2019

Copy link
Contributor

left a comment

Also seems fine to me

@stenzek stenzek merged commit 8be5ee9 into dolphin-emu:master Aug 9, 2019

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details

@lioncash lioncash deleted the lioncash:code branch Aug 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.