Add the configuration of how a memory check is added via the memory window #4131
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aldelaro5
force-pushed
the
memoryViewer-memChecks-improvements
branch
3 times, most recently
from
a3b99ea
to
b32c6d2
Compare
|
@dolphin-emu-bot rebuild |
1 similar comment
|
@dolphin-emu-bot rebuild |
These are needed for the next commit. I had to modify the implementation of the DSP one too, but since it basically isn`t used, I don`t think it matters much. These options only matters when adding one.
aldelaro5
force-pushed
the
memoryViewer-memChecks-improvements
branch
from
b32c6d2
to
72c9201
Compare
|
Now that I actually got this to pass linter (me being lazy), @lioncash care to review this for me? pretty old pr lol |
| @@ -47,7 +48,9 @@ enum | |||
| IDM_U32, | |||
| IDM_SEARCH, | |||
| IDM_ASCII, | |||
| IDM_HEX | |||
| IDM_HEX, | |||
| IDM_READ, | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
…y window There might be a better way to highlight the options when adding the memory check, but for now, this works and the breakpoint list reports the right settings anyway.
aldelaro5
force-pushed
the
memoryViewer-memChecks-improvements
branch
from
72c9201
to
72f1d99
Compare
|
Review status: 0 of 9 files reviewed at latest revision, 1 unresolved discussion. Source/Core/DolphinWX/Debugger/MemoryWindow.cpp, line 52 at r1 (raw file):
|
|
Reviewed 8 of 9 files at r1, 1 of 1 files at r2. Comments from Reviewable |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am adding this feature because not only it allows the same options as when adding via the breakpoint list, but it actually is quicker when adding a single address instead of a region as memory check and you don't want some options (like read for example). It's basically still a quick add, but now it has the same options as when using the list.
The toggleMemCheck in debugInterface assumed that the memCheck would have read, write and log at on so to get thsi to work, I had to add the oiptions in the interface. The other solution would have been to add or remove them manually, but I felt this would have been too complicated. Plus, the method still toggles, it just can now set the options.
As for the UI, I kept the default behaviour like it used to be, but my only concern is it might be confusing when looking at the blue square which option was chosen. Thankfully, the breakpoint list shows them (you do have to workaround a bug when it doesn't update by adding a breakpoint or apply pr #4115 ), but it might be possible to simply change the color depending on the settings (or perhapd do the same in the breakpoint list by putting the letter flags?).
Here's how it would look for the GUI part:
This change is