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 MemoryWidget: Navigate to memory from breakpoint widget and cheat search. #10701

Merged

Conversation

TryTwo
Copy link
Contributor

@TryTwo TryTwo commented May 28, 2022

When an address is sent to memory widget, it doesn't go in the memory address target box and must still be copied there before being able to get edited. This is a bit annoying, but forcibly changing the target address could be annoying as well. This wouldn't be a big issue if we could edit cells directly. I think we need another PR to improve this some way.

@TryTwo TryTwo force-pushed the PR_Memory_Widget_Connections branch 2 times, most recently from 4abe8d7 to ff01445 Compare May 31, 2022 00:51
@TryTwo
Copy link
Contributor Author

TryTwo commented May 31, 2022

Rebase and added WatchWidget a context menu option too.

// i18n: This kind of "watch" is used for watching emulated memory.
// It's not related to timekeeping devices.
menu->addAction(tr("Show in Memory"), this, [this, row] { ShowInMemory(row); });
Copy link
Contributor

Choose a reason for hiding this comment

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

This line needs to be before the i18n comment.

Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss left a comment

Choose a reason for hiding this comment

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

Rest seems fine. I always get confused by the exact logic signals and slots are supposed to use, but if it works it's alright to me.

QMenu* menu = new QMenu(this);

menu->addAction(tr("Go to memory"), [this, &address] { emit ShowMemory(address); });
Copy link
Contributor

Choose a reason for hiding this comment

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

[this, &address] -> [this, address], it's just an u32.

@@ -361,7 +362,7 @@ void BreakpointWidget::OnContextMenu()
emit BreakpointsChanged();
Update();
});
menu->addAction(tr("Go to"), [this, bp_address] { emit SelectedBreakpoint(bp_address); });
menu->addAction(tr("Go to"), [this, &bp_address] { emit ShowCode(bp_address); });
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add the & here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's just how other connection around it were passing the variable.

@@ -372,6 +373,7 @@ void BreakpointWidget::OnContextMenu()
if (mb_iter == memory_breakpoints.end())
return;

menu->addAction(tr("Go to"), [this, &bp_address] { emit ShowMemory(bp_address); });
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't need the by-reference either.

@@ -395,6 +396,11 @@ void WatchWidget::AddWatchBreakpoint(int row)
emit RequestMemoryBreakpoint(PowerPC::debug_interface.GetWatch(row).address);
}

void WatchWidget::ShowInMemory(int row)
{
emit(ShowMemory(PowerPC::debug_interface.GetWatch(row).address));
Copy link
Contributor

Choose a reason for hiding this comment

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

emit is not a function call, this should just be
emit ShowMemory(PowerPC::debug_interface.GetWatch(row).address);

Copy link
Contributor Author

@TryTwo TryTwo Jun 1, 2022

Choose a reason for hiding this comment

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

nvm got it

@TryTwo TryTwo force-pushed the PR_Memory_Widget_Connections branch from ff01445 to d1055ac Compare June 1, 2022 05:57
@TryTwo TryTwo force-pushed the PR_Memory_Widget_Connections branch from d1055ac to 177dae6 Compare June 1, 2022 08:53
@AdmiralCurtiss AdmiralCurtiss merged commit a58bb2a into dolphin-emu:master Jun 2, 2022
10 checks passed
@TryTwo TryTwo deleted the PR_Memory_Widget_Connections branch June 2, 2022 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants