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

[Linux - GTK3/GTK2] Action Replay code menu (+ other cheat menu improvements) #847

Merged
merged 11 commits into from
Oct 25, 2024

Conversation

En-En-Code
Copy link
Contributor

Summary:
The Linux GTK frontend's cheat menu now reads Action Replay codes from a game's dct file and adds them to a seperate section where users can add, remove, toggle, and modify them correctly and conveniently.
A couple changes were also made to the internal cheats' section of the menu to make it harder to misuse:

  • The "Offset" field has been changed to an "Address" field, and now expects 7 nibbles of hexadecimal data (fixes Cheat offsets are decimal in Linux Gtk frontend #725).
  • The value can no longer be outside of its logical range (for example, a 1-byte cheat is capped at 255).

Notes:
Action Replay code entry still leaves something to be desired. The scroll window sometimes doesn't scroll all of the way down and there is no size cut-offs, so long codes can consume far too much space. Coding two new GTK objects to get multiline editable text was easily the hardest part of this coding adventure though.
I still need to backport much to the GTK2 frontend, but with a working GTK2 build, I expect that part to be merely tedious, not actually hard.
The formatting was fairly scattered throughout the sections I worked on, (noticable really early on, when Neovim picked up the .editorconfig and inserted tabs, which I usually set to 2 spaces). I wrote an opinionated .clang-format that kept at least some of the formatting decisions I gleaned (but not tabs, screw tabs). I'll share it if you want it.
I wanted to cheat something for a fun project, which led me to realizing the shortcomings of the internal cheat menu. After converting to decimal and discovering they were not capable of doing what I wanted, I found the file cheats were stored, added Action Replay codes to another file on a Windows version, then copied them back to the dct file on Linux. Seeing they worked flawlessly meant everything in the backend was there, so it was just interface programming. Found the culprit atoi messing with internal cheat offset in like 2 minutes, and though the rest was not as easy, its still really nice at the end to tangibly improve a project because it's open source. <3

The decision to change the value column to store G_TYPE_UINT is purely pragmatic--having all data be treated and displayed as unsized is simpler than writing a custom display to handle signedness and cleaner than 1-3 byte ints appearing unsigned and 4 byte int appearing signed.
My implementation plan is to use a GtkTreeModelFilter to create separate section for internal and ActionReplay cheats, but I was not convinced the indices in the tree path would correspond to the indices in , hence the additional column.
…date/delete

Filters were a little more cumbersome than I was expecting, but managed to get raw cheats operational again.

From now on, I'm focusing purely on GTK3 and will backport the changes to GTK2 right before the PR
Goodness, was messing around with GTK to get it to all work out such a pain. Not completely happy with the scroll not expanding to fit text as is it added, but at least everything is functional.
+ a few comments and a few edits for clarification
+ some additional clean-up comments and small bug patches
@@ -120,6 +120,9 @@ class CHEATS

bool remove(const size_t pos);

void toggle(bool enabled, const size_t pos);
void toggle(u8 enablbed, const size_t pos);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo. do we really need an overloaded func for this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not, but the entire cheatSystem overloaded enabled this way: add L99-100, update L103-104, add_AR L109-110, etc. I followed suit, assuming there was a design reason behind it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then it's probably fine.

@rofl0r
Copy link
Collaborator

rofl0r commented Oct 11, 2024

this is marked as a draft. does this mean that the GTK2 version isn't completed yet ?
i also only see the new TU being used by the GTK3 code.

please also provide some examples of cheat code/game name so this can be tested.

@En-En-Code
Copy link
Contributor Author

this is marked as a draft. does this mean that the GTK2 version isn't completed yet ? i also only see the new TU being used by the GTK3 code.

Yes, I realized part way through that repeatedly backporting everything before every commit would be a waste when I commit changes on top of one another, so I decided I could save backporting for the end and make a draft PR with most of the progress.

please also provide some examples of cheat code/game name so this can be tested.

I like my test suites big, so I will claim anything on DeSmuMe's wiki page on cheats will work.

At reviewer request
@rofl0r
Copy link
Collaborator

rofl0r commented Oct 12, 2024

cool. give me a heads-up when it's ready so i can test the gtk2 version.

Upset with the amount of procrastinating I did, though I am going to tinker with the custom CellRenderer because I think I am close to a more aesthetic solution.
Hopefully passes CI now. Make systems are hard.
@En-En-Code En-En-Code marked this pull request as ready for review October 15, 2024 15:24
@En-En-Code
Copy link
Contributor Author

Alright, GTK2 should be on par with GTK3 functionality. I am still disappointed with the UI with the cheat cell, though it is usable for cheats of reasonable length. There should be a way to bound CellRenderer's maximum height and use a scroll bar for display, but for starting with no GTK knowledge, I cannot complain. It would be an improvement of marginal enough value that I think it should not delay the feature merge.

@En-En-Code En-En-Code requested a review from rofl0r October 15, 2024 16:10
@En-En-Code En-En-Code changed the title [Linux - GTK3/GTK2 (WIP)] Action Replay code menu (+ other cheat menu improvements) [Linux - GTK3/GTK2] Action Replay code menu (+ other cheat menu improvements) Oct 15, 2024
@rofl0r
Copy link
Collaborator

rofl0r commented Oct 25, 2024

finally got around to test the PR, it's a huge improvement - thanks. action replay codes finally work and the input is hex as expected.

however i had to make the following changes:

--- a/desmume/src/frontend/posix/gtk/utilsGTK.cpp
+++ b/desmume/src/frontend/posix/gtk/utilsGTK.cpp
@@ -19,6 +19,7 @@
  */
 
 #include "utilsGTK.h"
+#include <string.h>
 
 /*
     A C++ implementation of a GtkCellRendererText subclass which handles
diff --git a/desmume/src/frontend/posix/gtk2/utilsGTK.cpp b/desmume/src/frontend/posix/gtk2/utilsGTK.cpp
index 831340e1..f7f893c5 100644
--- a/desmume/src/frontend/posix/gtk2/utilsGTK.cpp
+++ b/desmume/src/frontend/posix/gtk2/utilsGTK.cpp
@@ -20,7 +20,7 @@
 
 #include "utilsGTK.h"
 #include <gdk/gdkkeysyms.h>
-
+#include <string.h>
 /*
     A C++ implementation of a GtkCellRendererText subclass which handles
     newline-delimited text and allows for editing that text with the ability
@@ -56,11 +56,13 @@ static void desmume_entry_nd_cell_editable_init(GtkCellEditableIface *iface);
 
 // As defined in GObject 2.38, which is past the last release of GTK2.
 // https://gitlab.gnome.org/GNOME/glib/-/blob/main/gobject/gtype.h#L2188
+#ifndef G_ADD_PRIVATE
 #define G_ADD_PRIVATE(TypeName)                                                \
     {                                                                          \
         TypeName##_private_offset = g_type_add_instance_private(               \
             g_define_type_id, sizeof(TypeName##Private));                      \
     }
+#endif
 
 G_DEFINE_TYPE_WITH_CODE(DesmumeEntryNd, desmume_entry_nd, GTK_TYPE_EVENT_BOX,
                         G_ADD_PRIVATE(DesmumeEntryNd) G_IMPLEMENT_INTERFACE(

this is because you used strlen but didn't include string.h. also the G_blah macro is already defined - glib and gtk2 are independent and it's possible to use a newer glib with gtk2.
please add that patches changes to your fork, then it can be merged.

@En-En-Code
Copy link
Contributor Author

Thanks for getting around to the review. I know how long things can take in open source.

Can't believe I missed the string.h includes. Can believe missing the #define guards. Arch Linux appears to bundle the last GObject release before GTK3 was released as part of its GTK2 package. I think this was beneficial, actually, since it caught the GObject macros not every GTK2 would have (and was the hardest part of the backport imo).

@rofl0r rofl0r merged commit efdd938 into TASEmulators:master Oct 25, 2024
9 checks passed
@En-En-Code En-En-Code deleted the linux-cheat-improvements branch November 1, 2024 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cheat offsets are decimal in Linux Gtk frontend
2 participants