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: Add Reset button #12082

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Debugger: Add Reset button #12082

wants to merge 1 commit into from

Conversation

allkern
Copy link

@allkern allkern commented Dec 13, 2024

Description of Changes

Add a handy 'Reset' button next to the 'Run' button.

Rationale behind Changes

As an emulator developer, resetting while working with the debugger is a pretty common occurrence. Having to reach out to the main window, then clicking on System -> Reset is somewhat cumbersome.

Suggested Testing Steps

Open debugger, click on the 'Reset' button, voila!

Screenshot

image

Add a handy 'Reset' button next to the 'Run' button
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for submitting a contribution to PCSX2

As this is your first pull request, please be aware of the contributing guidelines.

Additionally, as per recent changes in GitHub Actions, your pull request will need to be approved by a maintainer before GitHub Actions can run against it. You can find more information about this change here.

Please be patient until this happens. In the meantime if you'd like to confirm the builds are passing, you have the option of opening a PR on your own fork, just make sure your fork's master branch is up to date!

Copy link
Contributor

@TheTechnician27 TheTechnician27 left a comment

Choose a reason for hiding this comment

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

PR works as intended, and the code to my untrained eye looks good. However (and I think fobes might agree with me here), I think that it works too well as intended. The button, directly adjacent to the pause button, resets the game immediately on one click. This is, in my opinion, an accident waiting to happen, especially because people using the debugger are the type spend a lot of time trying to meticulously set something up in-game. I think there at least needs to be a prompt confirming to the end user that they want to reset the game. I know this defeats some of the convenience if you're repeatedly reseting the game for some reason, but this is a trade-off for what I imagine is a very small portion of users compared to the much higher likelihood that a user generally doesn't want to reset the game from within the debugger.

@chaoticgd
Copy link
Contributor

chaoticgd commented Dec 13, 2024

Just as a heads up, I'm currently working on an overhaul of the debugger's UI. If I end up finishing it I'll probably do another pass on how the toolbars work anyway e.g. I could add options to show/hide them and move them around anywhere. I think that would solve the problem nicely, so for now maybe just put the Reset button further away from the Pause button. Don't worry about merge conflicts with said overhaul, they should be easy to fix in this case.

@F0bes
Copy link
Member

F0bes commented Dec 13, 2024

image
What if something like this is done (But fix the parent QAction and remove the top level reset action)

@allkern
Copy link
Author

allkern commented Dec 15, 2024

PR works as intended, and the code to my untrained eye looks good. However (and I think fobes might agree with me here), I think that it works too well as intended. The button, directly adjacent to the pause button, resets the game immediately on one click. This is, in my opinion, an accident waiting to happen, especially because people using the debugger are the type spend a lot of time trying to meticulously set something up in-game. I think there at least needs to be a prompt confirming to the end user that they want to reset the game. I know this defeats some of the convenience if you're repeatedly reseting the game for some reason, but this is a trade-off for what I imagine is a very small portion of users compared to the much higher likelihood that a user generally doesn't want to reset the game from within the debugger.

I agree, I definitely wouldn't mind moving the reset button elsewhere as long as it's there, maybe to the right of 'Step Out'? F0bes suggestion also looks nice. I don't really know Qt that much so I wouldn't know how to pull it off, but you're welcome to change the code however you like

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants