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

Add CheatEngine and support for Gateway cheats #4406

Merged
merged 16 commits into from Nov 17, 2018

Conversation

Projects
None yet
7 participants
@B3n30
Copy link
Contributor

B3n30 commented Nov 7, 2018

Since we now have accurate Memory I decided to add cheat support again.

I updated the old cheat PR and fixed a lot of stuff that PR had.
For now I also removed the possibility to add/edit/delete cheats from the PR. It had multiple race conditions in the old PR and is something that can be properly added by a follow up PR.

Cheats that are currently supported are Gateway cheats. Other formats like ntr plugins could follow. Thus there is a CheatBase class and all other cheat formats will get derived from that base.

The documentation of the Gateway format was taken from https://gbatemp.net/threads/guide-how-to-create-gateway-cheat-codes.410926/ and https://www.maxconsole.com/threads/gateway-arcode-cheat-sheet.40381/

I tested it with some games and some cheats. Not all of them worked but I would guess that is due to a wrong version of the game or faulty cheat codes in general. It would probably a good idea if we host our own cheat db. Maybe in the future we could also add a way that cheats will get downloaded from our web-services.

There are many websites that host cheat codes for games e.g. https://www.max-cheats.com
To use those cheats a .txt file with the title_id as name needs to be placed inside the user_folder/cheats

I hope I removed all design flaws the old PR had. For reference the old PR was #2063

Edit: Picture of the ui change

bildschirmfoto 2018-11-07 um 11 32 38


This change is Reviewable

@B3n30

This comment has been minimized.

Copy link
Contributor Author

B3n30 commented Nov 7, 2018

One issue @wwylele noticed: Cheats uses Write32 etc which accesses the memory of the current process. This could lead to issues if the current process is e.g. an applet/home-menu. I didn't observe any issues yet, but the fix would be to add Write functions that get the process as an argument. we don't have those functions yet and I would prefer to add them in a separate PR if necessary.

@B3n30 B3n30 force-pushed the B3n30:cheat branch from 4e25553 to 3f5791f Nov 7, 2018

@zhaowenlan1779
Copy link
Member

zhaowenlan1779 left a comment

UI nits. Didn't check the impl yet

Show resolved Hide resolved src/citra_qt/cheats.cpp
Show resolved Hide resolved src/citra_qt/cheats.cpp Outdated
Show resolved Hide resolved src/citra_qt/cheats.cpp Outdated
Show resolved Hide resolved src/citra_qt/cheats.cpp
Show resolved Hide resolved src/citra_qt/cheats.cpp Outdated
Show resolved Hide resolved src/citra_qt/cheats.ui Outdated
Show resolved Hide resolved src/citra_qt/cheats.ui Outdated
Show resolved Hide resolved src/citra_qt/main.ui Outdated
@Subv
Copy link
Member

Subv left a comment

All cheats are executed every tick frame?

@B3n30

This comment has been minimized.

Copy link
Contributor Author

B3n30 commented Nov 7, 2018

@Subv every 1/60s so basically every frame for a 60fps game

@wwylele

This comment has been minimized.

Copy link
Member

wwylele commented Nov 7, 2018

I tested it with some games and some cheats. Not all of them worked but I would guess that is due to a wrong version of the game or faulty cheat codes in general.

Note that it wasn't guaranteed that my last PR resolved all memory inconsistency issue (it was rather a by-product that it happens to fix some). Specifically, when debugging the PR I noticed that some games (mistakenly?) use svcGetProcessInfo to determine the used memory amount and allocate memory according to it. As svcGetProcessInforeturns a slightly larger number than occupied APP region memory, which we doesn't accurately emulate, game would allocate a different amount of memory in citra and every client-side allocation growing from the end would get an offset. Example: super smash bros.

@B3n30 B3n30 force-pushed the B3n30:cheat branch from 237eb2f to 5a10a90 Nov 7, 2018

@wwylele wwylele added the canary-merge label Nov 7, 2018

@FearlessTobi
Copy link
Contributor

FearlessTobi left a comment

Just one question

Show resolved Hide resolved src/citra_qt/cheats.cpp Outdated
Show resolved Hide resolved src/core/cheats/cheat_base.h
Show resolved Hide resolved src/core/cheats/cheats.cpp Outdated
Show resolved Hide resolved src/core/cheats/cheats.cpp Outdated
Show resolved Hide resolved src/core/cheats/cheats.h Outdated
Show resolved Hide resolved src/core/cheats/cheats.h Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/core.h
Show resolved Hide resolved src/core/core.h Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/cheat_base.h Outdated
Show resolved Hide resolved src/core/cheats/cheat_base.h Outdated
@B3n30

This comment has been minimized.

Copy link
Contributor Author

B3n30 commented Nov 9, 2018

Probably fixed the crash reported in #4416 . But I'm not sure if that was the issue, since I wasn't able to reproduce the crash on my system and the crash is probably os based

@B3n30 B3n30 force-pushed the B3n30:cheat branch from 90a25d0 to c63d7ec Nov 9, 2018

@jroweboy
Copy link
Member

jroweboy left a comment

quick review. didn't want to let this get merged before you fixed the cheat interpreter tho so i figured a small review would be fine for now. the UI could use a lot of work, and i'd be much happier getting this merged if i knew for sure you had the code ready for it so we don't end up with another controller config ui issue 👍

Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp
@zhaowenlan1779
Copy link
Member

zhaowenlan1779 left a comment

I have some questions below. It would be helpful of you to answer them. Thanks!

Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.h Outdated
// half[XXXXXXX])
addr = line.address + offset;
val = Memory::Read16(addr);
if (static_cast<u16>(line.value) > ~(static_cast<u16>(~line.value >> 16) & val)) {

This comment has been minimized.

Copy link
@zhaowenlan1779

zhaowenlan1779 Nov 10, 2018

Member

I think it might be better to do line.value & 0xFFFF for the static_cast<u16>(line.value), just to be clear.

This comment has been minimized.

Copy link
@B3n30

B3n30 Nov 10, 2018

Author Contributor

the static_cast would still be necessary to avoid warnings. I think it's fine with just the static_casts but if you insist i will add the mask, too

Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
u32 val = 0;
int if_flag = 0;
int loop_count = 0;
s32 loopbackline = 0;

This comment has been minimized.

Copy link
@zhaowenlan1779

zhaowenlan1779 Nov 10, 2018

Member
  1. what about nested loops? I think your code isn't handling this correctly as you are only holding a single loopbackline.
  2. Ultra nit, name it loop_back_line or something similar

This comment has been minimized.

Copy link
@B3n30

B3n30 Nov 10, 2018

Author Contributor

Nested loops aren't supported atm. Not sure if this is necessary though

@zhaowenlan1779
Copy link
Member

zhaowenlan1779 left a comment

nitpicking

Show resolved Hide resolved src/core/core.h Outdated
Show resolved Hide resolved src/citra_qt/cheats.h Outdated
Show resolved Hide resolved src/citra_qt/cheats.h Outdated
Show resolved Hide resolved src/citra_qt/cheats.h Outdated
Show resolved Hide resolved src/citra_qt/cheats.cpp Outdated
@zhaowenlan1779
Copy link
Member

zhaowenlan1779 left a comment

Just found an additional (and probably rather critical) issue when I was testing some cheats.

Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated

@B3n30 B3n30 force-pushed the B3n30:cheat branch from 82432c4 to cd84def Nov 10, 2018

@zhaowenlan1779
Copy link
Member

zhaowenlan1779 left a comment

This is looking much cleaner! However there still seem to be some issues with cheat code interpretation. Hope you can look at them.

Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp
Show resolved Hide resolved src/core/cheats/gateway_cheat.h Outdated
Show resolved Hide resolved src/core/core.h Outdated

@B3n30 B3n30 force-pushed the B3n30:cheat branch from 3d04f8d to affebc2 Nov 10, 2018

Show resolved Hide resolved src/citra_qt/cheats.h Outdated
Show resolved Hide resolved src/core/cheats/gateway_cheat.cpp Outdated

@B3n30 B3n30 force-pushed the B3n30:cheat branch 2 times, most recently from 446e625 to cdb61d0 Nov 11, 2018

@B3n30 B3n30 force-pushed the B3n30:cheat branch from cdb61d0 to d9b90db Nov 11, 2018

@zhaowenlan1779
Copy link
Member

zhaowenlan1779 left a comment

LGTM
@jroweboy might have some comments regarding the UI though.

@@ -317,6 +319,8 @@ class Module final {
Kernel::SharedPtr<Kernel::Event> event_gyroscope;
Kernel::SharedPtr<Kernel::Event> event_debug_pad;

PadState state;

This comment has been minimized.

Copy link
@wwylele

wwylele Nov 12, 2018

Member

As real HID shouldn't need to maintain an extra state and this can be surprising to a first-time code reader, please add a comment stating that this is made specifically for cheat engine to read, and is not intended for other purpose.

@B3n30 B3n30 force-pushed the B3n30:cheat branch from 9cac3dd to 2595e08 Nov 15, 2018

@wwylele

This comment has been minimized.

Copy link
Member

wwylele commented Nov 16, 2018

Can I have a list of current state/TODO/roadmap? For example, how many games are tested, what outstanding issues are there, and what is the general idea of further UI design?

And do you expect this to be merge soon?

@B3n30

This comment has been minimized.

Copy link
Contributor Author

B3n30 commented Nov 16, 2018

@wwylele Sure

State: Imo ready to merge, We tested a lot of cheats for a lot of games. Most worked. Some didn't (but the same happened on a real 3DS) and some games (like Smash) just don't work at all. Probably caused by still inaccurate memory.

TODO: For this PR nothing

Readmap:

  • Next PR will include a proper UI with possibility to add/edit/delete cheats as well as a way to download (and upload) cheats from (to) our web api. The api is already prepared for that thanks to @chris062689
  • A follow up PR will include a way to search the memory, edit the memory from the ui, and create cheats based of those edits
  • Dolphin has something similar, so if you are interested in how it might look, just take a look at dolphins cheat manager

I didn't include any of the UI features in that PR, so reviewers can focus on the CheatEngine itself and just added a pretty simple and underdeveloped UI so that at least testing of Cheats is possible

template <typename T>
static inline std::enable_if_t<std::is_integral_v<T>> WriteOp(
const GatewayCheat::CheatLine& line, const State& state,
std::function<void(VAddr, T)> write_func, Core::System& system) {

This comment has been minimized.

Copy link
@wwylele

wwylele Nov 16, 2018

Member

As this is already a function template, you could parametrize the type of callable write_func for less overhead, just like how standard library does for those function param in <algorithm>. Same for other similar functions below.

@B3n30 B3n30 force-pushed the B3n30:cheat branch from e04e0d6 to 93f067c Nov 16, 2018

@jroweboy jroweboy merged commit b90ff73 into citra-emu:master Nov 17, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@B3n30 B3n30 deleted the B3n30:cheat branch Apr 11, 2019

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