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: Add conditional breakpoints to memory BPs #11274

Merged

Conversation

TryTwo
Copy link
Contributor

@TryTwo TryTwo commented Nov 13, 2022

I think it's fairly straightforward. I copied the conditional instruction BP changes.

breakpointdialog

@TryTwo TryTwo force-pushed the PR_Conditional_BP_Add_Memory branch 2 times, most recently from 3f24ae4 to 487f4fc Compare November 30, 2022 20:44
@JMC47
Copy link
Contributor

JMC47 commented Dec 1, 2022

This would have been so useful in my various debugging adventures. Had to use Visual Studio's breakpoints instead.

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

Beside these, tested and the condition seems evaluated properly.

Source/Core/DolphinQt/Debugger/BreakpointWidget.h Outdated Show resolved Hide resolved
Source/Core/Core/PowerPC/BreakPoints.cpp Show resolved Hide resolved
Source/Core/Core/PowerPC/BreakPoints.cpp Outdated Show resolved Hide resolved
@TryTwo TryTwo force-pushed the PR_Conditional_BP_Add_Memory branch from 487f4fc to 60e868d Compare December 2, 2022 05:54
@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 2, 2022

Looks like I fixed the breakpoint strings with $.

Source/Core/Core/PowerPC/BreakPoints.cpp Outdated Show resolved Hide resolved
Source/Core/Core/PowerPC/BreakPoints.cpp Outdated Show resolved Hide resolved
@TryTwo TryTwo force-pushed the PR_Conditional_BP_Add_Memory branch from 60e868d to 9df282f Compare December 4, 2022 00:58
@TryTwo TryTwo force-pushed the PR_Conditional_BP_Add_Memory branch from 9df282f to a17fbe7 Compare December 4, 2022 18:28
@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 6, 2022

Retested now that the other conditional PR was merged. Seems to be working the same, no problems. This should be the final conditional PR.

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

Tested and LGTM.

@AdmiralCurtiss AdmiralCurtiss merged commit b207611 into dolphin-emu:master Dec 7, 2022
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants