Android: Add cheat GUI

[JosJuice]

This is one of the most requested features from Android users.

I would appreciate if this could be merged in mid-October at the latest, since that's when I'm planning to merge PR #9696. The new location for the User folder which that PR introduces is relatively hard to access on Android 11, and cheats are currently the main reason for the typical Android user to go digging around in the User folder.

---

This change is 

[JMC47]

Tested and verified everything to work correctly.

Creating new cheats works

Downloading Gecko Codes works

Enabling/Disabling cheats works, as well as the shortcut to the config panel to re-enable cheats globally if they are disabled. There is one strange thing in my opinion. There's no checkmark to "save" (even though I'm guessing INIs save as soon as I select an option?) you just have to hit back, which isn't really a problem as the other menus also do this. It's just weird there isn't some checkmark/save icon in the upper right to save just like in the other menus I guess.

LGTM otherwise, all functionality works, and I'm not too hung up on this one tiny thing that probably has a reason behind it.

[BlueSwordM]

Everything seems to work correctly as well, including the creation of a new cheat, and downloading them if they exist.

I have the same "complaint" as JMC: a UI toggle switch to save or indicate "OK" to the user should be present to not be confusing.

https://slow.pics/c/OTKlcPWr

Really nice to see an RGB trail code working on Android(don't mind the missing model, this is a mod bug, not a Dolphin bug).

[JosJuice]

It saves as soon as you leave the activity (more specifically in onStop). I've felt that the save icon we have in the settings activity feels rather un-Android-like (it's actually a holdover from back when Dolphin didn't save the settings automatically and if you didn't hit the save button all your changes would be lost with no warning, which was pretty terrible) and was thinking of removing it, but if people would prefer to have one in the cheats activity, I suppose I could add one there instead.

For what it's worth, the game properties on desktop don't have a Save/OK button either, just a close button. Does it make sense anyway?

[JMC47]

Even just a checkmark to close to the menu in the upper right would be enough if you don't think the save is appropriate. I don't care that much about it, obviously. It's just one of the things I noticed while using it.

[JosJuice]

I've added a save button for now (like in the settings activity).

[JMC47]

Everything works as expected now.

[leoetlino]

Reviewed 4 of 4 files at r1, 19 of 19 files at r2, 3 of 3 files at r3, 12 of 12 files at r4, 9 of 9 files at r5, 4 of 4 files at r6, 14 of 14 files at r7, 5 of 5 files at r8, 12 of 12 files at r9, 11 of 11 files at r10, 1 of 1 files at r11, 8 of 8 files at r12, 14 of 14 files at r13, 5 of 5 files at r14, 6 of 6 files at r15, 7 of 7 files at r16, 13 of 13 files at r17, 1 of 1 files at r18, 1 of 1 files at r19, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @JosJuice)

---

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/cheats/model/ARCheat.java, line 25 at r4 (raw file):

  public native String getName();

  public native boolean getEnabled();

Not that it really matters, but would it make sense for this to be part of the AbstractCheat interface?

---

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/cheats/model/CheatsViewModel.java, line 255 at r17 (raw file):

    for (GeckoCheat cheat : cheats)
    {
      if (!mGeckoCheats.contains(cheat))

How many cheats will mGeckoCheats typically contain? If cheats and mGeckoCheats have about the same size (or if they're equal because the user tried to re-download cheats and mGeckoCheats already contains all downloaded cheats), this has quadratic time complexity.

---

Source/Android/jni/Cheats/ARCheat.cpp, line 95 at r4 (raw file):

  const std::string game_id = GetJString(env, jGameID);
  const std::string ini_path = File::GetUserPath(D_GAMESETTINGS_IDX) + game_id + ".ini";

Not really an issue with your PR, but at some point we should probably add a "get game INI path" helper — this is duplicated dozens of times across the codebase :(

---

Source/Android/jni/Cheats/Cheats.h, line 6 at r7 (raw file):

#pragma once

constexpr int TRY_SET_FAIL_NO_NAME = -1;

Could we add a name prefix or put this in some namespace?

---

Source/Android/jni/Cheats/GeckoCheat.cpp, line 92 at r9 (raw file):

    if (std::optional<Gecko::GeckoCode::Code> c = Gecko::DeserializeLine(line))
      entries.push_back(*c);

Consider moving the code into entries: entries.emplace_back(*std::move(c))

---

Source/Android/jni/Cheats/GeckoCheat.cpp, line 197 at r17 (raw file):

  const std::string gametdb_id = GetJString(env, jGameTdbId);

  bool success;

Could we set success to a sane value (false), just in case DownloadCodes does not do it (for whatever reason, including bad refactoring in the future)?

---

Source/Android/jni/Cheats/PatchCheat.cpp, line 91 at r9 (raw file):

    if (std::optional<PatchEngine::PatchEntry> entry = PatchEngine::DeserializeLine(line))
      entries.push_back(*entry);

entries.emplace_back(*std::move(entry))

[JosJuice]

---

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/cheats/model/ARCheat.java, line 25 at r4 (raw file):

Previously, leoetlino (Léo Lam) wrote…

Not that it really matters, but would it make sense for this to be part of the AbstractCheat interface?

This is already part of the Cheat interface, which AbstractCheat implements.

[JosJuice]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @leoetlino)

---

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/cheats/model/CheatsViewModel.java, line 255 at r17 (raw file):

Previously, leoetlino (Léo Lam) wrote…

How many cheats will mGeckoCheats typically contain? If cheats and mGeckoCheats have about the same size (or if they're equal because the user tried to re-download cheats and mGeckoCheats already contains all downloaded cheats), this has quadratic time complexity.

Perhaps a few hundred at worst, if the user has downloaded codes for a popular game. This is the same algorithm as in DolphinQt (GeckoCodeWidget.cpp).

---

Source/Android/jni/Cheats/Cheats.h, line 6 at r7 (raw file):

Previously, leoetlino (Léo Lam) wrote…

Could we add a name prefix or put this in some namespace?

Do you have a suggestion for a namespace name?

---

Source/Android/jni/Cheats/GeckoCheat.cpp, line 92 at r9 (raw file):

Previously, leoetlino (Léo Lam) wrote…

Consider moving the code into entries: entries.emplace_back(*std::move(c))

Done (and I did it for ARCheat as well).

---

Source/Android/jni/Cheats/GeckoCheat.cpp, line 197 at r17 (raw file):

Previously, leoetlino (Léo Lam) wrote…

Could we set success to a sane value (false), just in case DownloadCodes does not do it (for whatever reason, including bad refactoring in the future)?

Done.

---

Source/Android/jni/Cheats/PatchCheat.cpp, line 91 at r9 (raw file):

Previously, leoetlino (Léo Lam) wrote…

entries.emplace_back(*std::move(entry))

Done.

[leoetlino]

Reviewed 1 of 40 files at r20, 2 of 19 files at r21, 2 of 3 files at r22, 2 of 9 files at r24, 1 of 4 files at r25, 1 of 14 files at r26, 1 of 12 files at r28, 1 of 11 files at r29, 1 of 8 files at r31, 7 of 14 files at r32, 1 of 5 files at r33, 2 of 6 files at r34, 5 of 7 files at r35, 12 of 13 files at r36, 1 of 1 files at r37, 1 of 1 files at r38, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @JosJuice)

---

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/cheats/model/ARCheat.java, line 25 at r4 (raw file):

Previously, JosJuice wrote…

This is already part of the Cheat interface, which AbstractCheat implements.

Oh, my bad.

---

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/cheats/model/CheatsViewModel.java, line 255 at r17 (raw file):

Previously, JosJuice wrote…

Perhaps a few hundred at worst, if the user has downloaded codes for a popular game. This is the same algorithm as in DolphinQt (GeckoCodeWidget.cpp).

OK, that's probably fine then.

---

Source/Android/jni/Cheats/ARCheat.cpp, line 104 at r38 (raw file):

    if (std::holds_alternative<ActionReplay::AREntry>(parse_result))
      entries.emplace_back(std::get<ActionReplay::AREntry>(std::move(parse_result)));

The move won't do anything because parse_result is const.

---

Source/Android/jni/Cheats/Cheats.h, line 6 at r7 (raw file):

Previously, JosJuice wrote…

Do you have a suggestion for a namespace name?

Cheats, perhaps? Or Cheat since that's what the class is called (though that would be inconsistent with this file's name).

[JosJuice]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @leoetlino)

---

Source/Android/jni/Cheats/ARCheat.cpp, line 104 at r38 (raw file):

Previously, leoetlino (Léo Lam) wrote…

The move won't do anything because parse_result is const.

Done.

---

Source/Android/jni/Cheats/Cheats.h, line 6 at r7 (raw file):

Previously, leoetlino (Léo Lam) wrote…

Cheats, perhaps? Or Cheat since that's what the class is called (though that would be inconsistent with this file's name).

Done.

[leoetlino]

Reviewed 3 of 32 files at r39, 1 of 11 files at r40, 1 of 8 files at r42, 7 of 14 files at r43, 1 of 5 files at r44, 2 of 6 files at r45, 5 of 7 files at r46, 12 of 13 files at r47, 1 of 1 files at r48, 1 of 1 files at r49, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JosJuice)