Win32 debugger tweaks and fixes

[warmCabin]

This was originally a lazy Notepad++ branch to fix a pet peeve of mine, but it's become my hobby over the last couple of weeks. Thanks to @bbbradsmith for the toolbar layout plan!

• Consolidate a bunch of existing options and features into a toolbar, including:

  • Symbolic Debug toggle
  • Default register names toggle
  • Reset to default window size
  • ROM patcher (not to be confused with the weirdly secret inline assembler)
  • Use ROM offsets

• Add some new options and features:

  • Toggle for showing addresses alongside symbol names (the original pet peeve 😏)
  • A code dumper (currently only a skeleton)
  • Add the RTS ---------- thing to RTI

• Bug fixes

  • comments ending in a colon no longer get colored as a label
  • Default reg name replacement no longer gets called multiple times per line
  • X and Y coordinates no longer interpreted as a window handle in the scroll wheel callback...

• Refactor the thousand-line megalithic WindProc into a bunch of callbacks with the stupid wParam and lParam values parsed

What if we also showed symbol addresses as a tooltip? Should I add dashes to BRK?

Shoutouts to that lazy guy

[warmCabin]

I need to get a proper development environment going before I do something significant like that.

[ClusterM]

Get rid of label/offset disassembly

Not sure that it's a good idea. At least it should be optional, IMHO.

[warmCabin]

[warmCabin]

I found this great bug where someone interpreted the lParam of a WM_MOUSEWHEEL event as a window handle instead of packed X and Y coords. I don't have a firm grasp on handles, but Windows somehow knows to silently do nothing when you give it a garbage one, which is the only reason scrolling ever didn't crash.

[warmCabin]

retest this please

[warmCabin]

Merge was a simple res.rc conflict

[warmCabin]

Appveryor literally marked it as failing because this is a draft PR. What a jerk

[warmCabin]

Actually, the build error is that I declared my dialog process as a BOOL and not an INT_PTR. What fun!

[warmCabin]

Here is an example of something the new code dumper output:

 0D:9EE2: 30 0F     BMI $9EF3
 0D:9EE4: 0F        UNDEFINED
 0D:9EE5: 12        UNDEFINED
 0D:9EE6: 2C        INTERRUPTED
do_title_screen:
; Set nametable addresses
 0D:9EE7: A9 10     LDA #$10
 0D:9EE9: 85 F7     STA ppu_ctrl_mirror
 0D:9EEB: 8D 00 20  STA PPU_CTRL
; Disable rendering
; (But leave things visible on the left 8 pixels?)
 0D:9EEE: A9 06     LDA #$06
 0D:9EF0: 85 F8     STA ppu_mask_mirror
 0D:9EF2: 8D 01 20  STA PPU_MASK
; Set horizontal mirroring
 0D:9EF5: A9 0F     LDA #$0F
 0D:9EF7: 20 5D C0  JSR set_mmc1_control

Look at $9EE6. It really wants to interpret that as BIT $10A9, but if it sees a label on one of the operand bytes, it's marked as "INTERRUPTED" and disassembly continues from the label. Is this something we want to add to the InstructionUp() and InstructionDown() logic for the actual debugger window?

[warmCabin]

I don't like that appveyor won't build this unless I resolve the merge conflicts. I'll probably keep force pushing over them.

[warmCabin]

@mjbudd77 Have you seen this branch? Looks like we're doing some parallel feature work here

[mjbudd77]

@warmCabin i check in on this branch every now and then for ideas. But my changes are only for the Qt front end and I try not to change the core.

[g0me3]

feature request: debugger step/trace hotkeys like in any other debugger. the one fceux does not have any so this makes me sad ;(

[vadosnaprimer]

Debugger - Step Into hotkey has been added years ago. https://github.com/TASVideos/fceux/commit/4eb00bdca6e4a46d26201b9786d94072bd14a8c9

[g0me3]

this is quick and dirty kostyl which works only with backgroung input. i need more useful solution as well as more commands to be binded to keys. when you debug something you can push step button with mouse. but most frequently i need to hit run button instead lol for example

[vadosnaprimer]

Agreed.

[warmCabin]

Currently if you click it, you can spam enter to re-click it. Hardly a solution, but hey! I'll see if I can figure out how to add some real hotkeys.

[warmCabin]

Any opinions on the scroll up code respecting labels?

[warmCabin]

I assume you'd want it mappable, but what's a good standard set of default shortcuts?

[ClusterM]

BTW, about this line: https://github.com/TASVideos/fceux/blob/6e0a3a81998078c1e29b31de5233f360a036afe5/src/drivers/win/debugger.cpp#L2447

We can edit ROM in HEX editor while emulation is not paused. We can browse code in debugger while emulation is not paused. So why in the hell do we need this FCEUI_EmulationPaused() check here?

[ClusterM]

I assume you'd want it mappable, but what's a good standard set of default shortcuts?

I prefer Visual Studio defaults

[warmCabin]

Ok, wow, remind me never to use hotkeys again. They are operating system-level events that get consumed by your application, and no one else gets to have them. So the little Ctrl+A shortcut for goto address intefered with my ability to select all in Notepad++. Pure jank.

I've implemented "hotkeys" properly as accelerators. I had to touch main.cpp, but it's in a spot where zeromus wrote, "other accelerator capable dialogs could be added here." This was back in '08 when you guys invented the fm2 format and I was in 6th grade. lol

[ClusterM]

Ok, wow, remind me never to use hotkeys again. They are operating system-level events that get consumed by your application, and no one else gets to have them. So the little Ctrl+A shortcut for goto address intefered with my ability to select all in Notepad++. Pure jank.

I've implemented "hotkeys" properly as accelerators. I had to touch main.cpp, but it's in a spot where zeromus wrote, "other accelerator capable dialogs could be added here." This was back in '08 when you guys invented the fm2 format and I was in 6th grade. lol

We should disable background input for emulator's system functions, I think. Or add an option for it.

[warmCabin]

I think the Mac build failure is the result of some missing configuration from upstream. 5a483e315783ace333f47f3711e8110822647905 could not possibly have caused it. A rebase should fix that.

[warmCabin]

We should disable background input for emulator's system functions, I think. Or add an option for it.

What other system functions are there? Everything else seems to use accelerators or manually process WM_KEYDOWN events.

[mjbudd77]

I think the Mac build failure is the result of some missing configuration from upstream. 5a483e3 could not possibly have caused it. A rebase should fix that.

Don’t worry, sometimes the build environment fails to pull in the proper dependencies from the package manager. In this case it looks like homebrew failed to pull in Qt5. I bet it will succeed if it runs again

[zeromus]

what's going on in this PR? does someone need to make a test build for us to evaluate? it seems like a lot of work has been put into this.

[warmCabin]

Damn, has it really been a year since I touched this? Have people been working on the debugger core in the time since? This merge conflict doesn't seem too bad... Looks like a lot of the stuff I pulled out of debugger.cpp is trying to shove its way back in.

Anyway, this branch builds on my machine. I think it's failing on AppVeyor because of a pipeline glitch. I haven't made a new commit since then to test that.

There are 4 things I want to fix before we call this good enough.

1. Debugger window hogs keyboard shortcuts, making Ctrl+A in hex editor unusable.
2. Certain opcodes, like UNDEFINED or BRK, disassemble as 1 byte but are specified to have a length of 2 in asm.cpp, which makes scrolling really confusing and annoying in many cases.
3. Add the label-respecting logic of the code dumper window to the realtime disassembly view. See this comment.
4. '@' character in label names gets highlighted as trace data

I had some other plans, but we could always merge this in and do those in a new branch.

1. Check code data logger to stop disassembling known data blocks. 1a. Mark blocks as known data via NL?
2. Configurable keyboard shortcuts (right now they are hardcoded)
3. Render literal values in decimal or binary. As a config flag, or part of each NL entry?
4. Resizable breakpoints panel so the names don't get cut off
5. Add a search window for symbol names.
6. Option to not follow PC during debug step

[zeromus]

yes, it looks like a pipeline glitch. I hate to let so much good work bitrot. Are you going to do the other stuff you wanted any time soon? Do I need to lean on people to test it? It's hard to coordinate testing in a branch, I'd just rather do it all in master, test and debug it, and only in case we decide to shitcan it, revert it.

[warmCabin]

Thanks man, I'm glad you want to incorporate this. This build still thinks it's 2.4.0-dev-adfkjasdkfj, so it's getting quite rotten indeed.

First I think we should tackle the merge conflicts. If the changes on master are minor enough, maybe you could revert them and I could work to incorporate them into my big refactor? I'm pretty sure git wants to create two copies of all the debugger code right now.

I can tackle the "good enough" requirements this week. Apparently I already got started on Future Plan 1 (07c81eabb21df9ca4564cef8e8407d387ceb21b2), so I can work on that as well.