Skip to content

BreakpointViewer doesn't reload breakpoints anymore#104

Merged
m9710797 merged 1 commit intoopenMSX:masterfrom
pvmm:new-widget
Apr 1, 2022
Merged

BreakpointViewer doesn't reload breakpoints anymore#104
m9710797 merged 1 commit intoopenMSX:masterfrom
pvmm:new-widget

Conversation

@pvmm
Copy link
Copy Markdown
Contributor

@pvmm pvmm commented Mar 29, 2022

BreakpointViewer now stores references to Breakpoints (Breakpoint, Watchpoints and Conditions) through 3 std::maps of new data structure BreakpointData because just storing references in a hidden column in QTableWidget is not flexible or powerful enough. The new data structure allows BreakpointViewer to sync with the emulator only when reconnected and not every time a new breakpoint is created or modified, which is more efficient. Also, since the table of breakpoints is not erased every time, items will not move around without the user's consent (they will not be destroyed and recreated, but reused).

minor changes:

  • renamed processXXXX functions to parseXXXX because it is a less generic name;
  • hidden 6th table column stores breakpoint name (id) instead of index;
  • more generic and useful setTextField function instead of item->setText();
  • message box displayed when error received from openMSX;
    image
  • the debugger now waits for openMSX response before performing changes;
  • to avoid rows moving around, editing or adding a breakpoint now automatically turns sorting off.

This is an attempt to fix the remaining bugs on pull request #91.

@pvmm
Copy link
Copy Markdown
Contributor Author

pvmm commented Mar 29, 2022

In draft mode because there are some minor glitches when changing watch points.

Comment thread src/BreakpointViewer.cpp Outdated
Comment thread src/BreakpointViewer.cpp Outdated
Comment thread src/BreakpointViewer.cpp Outdated
Comment thread src/BreakpointViewer.cpp Outdated
Comment thread src/BreakpointViewer.h Outdated
@pvmm pvmm force-pushed the new-widget branch 2 times, most recently from 3b9bcb3 to c89e508 Compare March 29, 2022 15:25
Comment thread src/BreakpointViewer.cpp Outdated
@pvmm pvmm force-pushed the new-widget branch 2 times, most recently from 4ef0ea4 to 57b265a Compare March 29, 2022 19:50
@pvmm
Copy link
Copy Markdown
Contributor Author

pvmm commented Mar 29, 2022

BreakpointViewer seems to be working fine in tandem with assembly view and "Add breakpoint..." from the start.

@pvmm pvmm marked this pull request as ready for review March 29, 2022 19:51
@m9710797
Copy link
Copy Markdown
Contributor

Thanks. I'll try to do a more detailed review tomorrow (or the day after).

Comment thread src/BreakpointViewer.h Outdated
Comment thread src/BreakpointViewer.h Outdated
Comment thread src/BreakpointViewer.h Outdated
Comment thread src/BreakpointViewer.h
Comment thread src/BreakpointViewer.cpp Outdated
Comment thread src/BreakpointViewer.cpp Outdated
Comment thread src/BreakpointViewer.cpp Outdated
Comment thread src/BreakpointViewer.cpp
Comment thread src/BreakpointViewer.cpp
Comment thread src/BreakpointViewer.cpp
@pvmm pvmm mentioned this pull request Mar 30, 2022
@pvmm pvmm force-pushed the new-widget branch 3 times, most recently from 749b16c to e510eb9 Compare March 30, 2022 22:19
Comment thread src/BreakpointViewer.cpp Outdated
Comment thread src/BreakpointViewer.cpp
@m9710797
Copy link
Copy Markdown
Contributor

I did a 2nd pass of review. Just some minor suggestions (not required to change this).
I think it's ready to be merged. But before doing that I first wanted to ask if it's ready from your side (because I see you were still making changes).

I see you recently created PR#108, I'll try to review that tomorrow, I don't have time today anymore.

BreakpointViewer now stores references to Breakpoints (Breakpoint, Watchpoints and Conditions) through 3 maps of new data structure "BreakpointData" because just storing references in a hidden column in QTableWidget is not flexible or powerful enough. The new data structure allows "BreakpointViewer" to sync with the emulator only when reconnected and not every time a new breakpoint is created or modified, which is more efficient. Also, since the table of breakpoints is not erased every time, items will not move around without the user's consent (they will not be destroyed and recreated, but reused).

minor changes:
* renamed processXXXX functions to parseXXXX because it is less of a generic name;
* hidden 6th table column stores breakpoint name (id) instead of index;
* more generic and useful setTextField function instead of item->setText();
* message box displayed when error received from openMSX;
* debugger now waits for openMSX response before performing changes;
* to avoid rows moving around, editing or adding a breakpoint now automatically turns sorting off.
@m9710797 m9710797 merged commit bd853ea into openMSX:master Apr 1, 2022
@m9710797
Copy link
Copy Markdown
Contributor

m9710797 commented Apr 1, 2022

Thanks a lot!

@pvmm pvmm deleted the new-widget branch April 1, 2022 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants