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

Jump to bottom

add Break On Hit and Log On Hit for instruction breakpoints #8940

Merged
merged 1 commit into from Aug 8, 2020
Merged

add Break On Hit and Log On Hit for instruction breakpoints #8940

merged 1 commit into from Aug 8, 2020
+71 −19

Conversation

RenaKunisaki
Copy link
Contributor

No description provided.

sepalani
sepalani suggested changes Jul 7, 2020
View reviewed changes
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.

clang-format should be used to fix the formatting issues.

This PR reminds me of #6057

Source/Core/Core/PowerPC/PowerPC.cpp Outdated Show resolved Hide resolved
Source/Core/Core/PowerPC/PowerPC.cpp Outdated Show resolved Hide resolved
@BhaaLseN
Copy link
Member

BhaaLseN commented Jul 7, 2020

Feel free to combine those two commits; there is very little value in having that second commit stick around as an individual one.

@RenaKunisaki
Copy link
Contributor Author

How do I do that?

@BhaaLseN
Copy link
Member

BhaaLseN commented Jul 7, 2020

Assuming command line (like git bash or similar), you run git rebase -i upstream/master master (assuming that upstream is the name of the Dolphin remote, use git remote -v to check and replace as necessary. It also helps to git fetch upstream first, just to be sure.) That should bring up a text editor that has two lines that start with the word pick. Change the second one (that should also say "run clang-format" next to a hash) to fixup; then save and quit the editor. Git should do a few things, and drop you back into a console when its done. Assuming it went without error, you have to force-push this to your repository (depending on how you set up your branch, git push -f might be enough; otherwise try git push -f origin master - again, check git remote -v for your own repository here on GitHub and replace as necessary).
Let us know if you need any help (or drop by on IRC if thats easier)

@RenaKunisaki
Copy link
Contributor Author

Alright, that should do it.

@lioncash
Copy link
Member

lioncash commented Jul 8, 2020

Commit message seems to be missing an A at the beginning of it.

sepalani
sepalani suggested changes Jul 8, 2020
View reviewed changes
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.

I tested the PR, it works as expected when adding new breakpoints. LGTM once @lioncash comment is addressed.

@booto Do you have any comment regarding this PR since you did one for reorganizing breakpoints?

Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp Outdated Show resolved Hide resolved
@RenaKunisaki RenaKunisaki force-pushed the master branch 2 times, most recently from c5ada79 to 7f6a429 Compare July 8, 2020 20:31
@RenaKunisaki
Copy link
Contributor Author

Fixed commit message and changed 'p' to 'b'.

sepalani
sepalani suggested changes Jul 9, 2020
View reviewed changes
Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp
@@ -308,7 +316,11 @@ void BreakpointWidget::OnSave()

void BreakpointWidget::AddBP(u32 addr)
{
PowerPC::breakpoints.Add(addr);
AddBP(addr, false, true, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason it's logging by default? Is this method still used? If not, it should be removed.

There is also a missing newline after the function. Not sure why the lint bot didn't catch 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.

I believe "write to log and break" was always the default? The log function does still work; I've been using it to monitor function calls.

@Tilka Tilka merged commit 76b955e into dolphin-emu:master Aug 8, 2020
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sepalani sepalani sepalani requested changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
5 participants
@RenaKunisaki @BhaaLseN @lioncash @sepalani @Tilka