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 MemoryViewWidget: always highlight target address #11232

Merged

Conversation

TryTwo
Copy link
Contributor

@TryTwo TryTwo commented Oct 30, 2022

memviewselect

Highlighting wasn't doing much before. Now it lets you know what address is being targeted. The colors can be changed, I just set it to normal. A stylesheet needed to be used, or else the selected item coloring is removed when focus is lost, which isn't good.

There is also a thin outline box on whatever cell you click, without changing the highlighting on the target.

I'm not sure if set focus is needed anymore, but it seemed a little broken when the table was given the slider navbar, so I fixed that. I think it was mostly a highlighting thing, but then I overrode the highlighting altogether.

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. The behaviour on this PR is more intuitive than the current Dolphin's one. The selected cell stays highlighted during scrolls. The right click options seem to still work as intended.

@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 1, 2022

I noticed if you put a misaligned address in, nothing gets highlighted. I'm not sure if that's good behavior or if it should align down.

@sepalani
Copy link
Contributor

sepalani commented Dec 3, 2022

I noticed if you put a misaligned address in, nothing gets highlighted. I'm not sure if that's good behavior or if it should align down.

IMO, I think that's fine. The issue doesn't appear when no alignment is set. If the current behaviour isn't considered as fine, you should check the highlight address boundary with the display type size or align down m_address_highlight. For the latter, you should make sure you don't "loose" precision when changing back and forth from display type with the right/wrong alignment.

@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 3, 2022

Ok, I'll just leave it, so it doesn't give the wrong idea.

@AdmiralCurtiss AdmiralCurtiss merged commit 2bd47d1 into dolphin-emu:master Dec 4, 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