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

[Debugger] Memory View search box improvements #13470

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Nitch2024
Copy link
Contributor

@Nitch2024 Nitch2024 commented Mar 29, 2025

  • Added alignment option for memory search. Useful to find only valid code patterns or valid float values for example
  • Added auto breakpoint setting when value found

Helpful when trying to find all accessed instances of a value for example 0x41f00000 (30.0) in a game. Put 0x41f0000 in the search box, set alignment to 4, set auto bp memory and spam the search button. Then run the game and examine the code using those values to see if any is relevant. Similar use case for code with lis r0, 0x41f0 for example.

@TryTwo
Copy link
Contributor

TryTwo commented Mar 30, 2025

How much overlap does this have with cheat search? Other than not being able to set a BP from cheat search, it seems to allow the same thing without spamming a button. I don't use this technique, so I could be wrong.

We can add a way to turn a selection in cheat search into memory BPs, if everything else is fine.

@Nitch2024
Copy link
Contributor Author

How much overlap does this have with cheat search? Other than not being able to set a BP from cheat search, it seems to allow the same thing without spamming a button. I don't use this technique, so I could be wrong.

We can add a way to turn a selection in cheat search into memory BPs, if everything else is fine.

The workflow is very different and debugger related. For example I search for 0x41F00000, set a breakpoint on the first 20 instances. Run the game. Check which ones were hit or not. Check if the ones hit matter with debugger or not, delete everything. Rinse and repeat until found. The level of control and relatedness to the code matters. It would not be as useful to me in the cheat search, this is less about values changing but about tracking static values usage and location and impact.

Alignment filter in search is very important too.

@TryTwo
Copy link
Contributor

TryTwo commented Mar 30, 2025

I agree aligning search is important. I want to make sure you're giving cheat search a fair consideration and that auto-BPs isn't overly specific to your workflow.

I popped a context item to make BPs into cheat search (this should probably have been in CS already), and this seems to do what you want. Cheat search works fine with handling static values, specific address ranges, and is organized by address.

GIF

Dolphin_jC2zB9xAuU

If it's still a No, I won't insist further.

@Nitch2024
Copy link
Contributor Author

Yes I use the Cheat Search extensively and it could be used in some cases as you shown in the video. It is just a quality of life thing I am proposing, when you are in the memory viewer, less clicks, mouse movements, you can eyeball the surrounding variables, skip places in memory where the values don't make sense, etc. Both Cheat Search and Memory View are very useful even if they share common functionality, they do it in different ways. For example technically we could remove the search in Memory View and use the Cheat Search for all searches, that would work, it would just not be as convenient to use.

@TryTwo
Copy link
Contributor

TryTwo commented Mar 30, 2025

Alright, adding it isn't a big deal, just making sure.

Should the BP size obey the alignment option? Except for code of course. -- Though this seems a little difficult to add. Do we create the breakpoint in MemoryWidget and avoid MemoryView? Or add an optional size variable to ToggleBreakpoint? Or just expect people to use the correct alignment Display?

@@ -60,6 +60,7 @@ class MemoryWidget : public QDockWidget
void OnBPTypeChanged();

void OnSearchAddress();
void OnBreakOnAll();
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry yes code went into the branch. Good catch.

@Nitch2024
Copy link
Contributor Author

Removed all the code that was not necessary or duplicative.

@dolphin-ci
Copy link

dolphin-ci bot commented Mar 30, 2025

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

automated-fifoci-reporter

@TryTwo
Copy link
Contributor

TryTwo commented Mar 30, 2025

Code looks good to me, if using the BP size derived from the display alignment makes sense, then I have nothing else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants