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

DolphinQt: Add "Reset Ignore Panic Handler" MenuBar Option #10174

Merged

Conversation

dreamsyntax
Copy link
Member

@dreamsyntax dreamsyntax commented Oct 17, 2021

Overview:

  • Adds a "Reset Ignore Panic Handler" button to the Options subgroup in the MenuBar; It will only appear if Debug mode is enabled
  • The button will do nothing if Config -> Interface -> Use Panic Handlers is disabled

Originally, I was going to make this a toggle like Automatic Start and Boot to Pause. Since Main.cpp calls SetEnabledAlert directly and there is no way to call MenuBar qActions from Main.cpp, I opted to go with a stateless button. User usage of the button also made little sense to have a toggle, since Config -> Interface already has a "Use Panic Handlers".

Why this is useful:

When a person debugging runs into an exception like this, sometimes there are 5000+ messages and the only choice is to choose "Ignore for this session". There really should be a "pause" option for those scenarios, but that's coming in another PR. This prevents any further pop-up messages about exceptions. The person debugging then has no way to re-enable the Ignore flag[*note1] without completely restarting Dolphin. With this change now there is a clear way to do it under the Options sub-menu.

mem-exception

reset-ignore-button-fix

[*note1] - You could disable panic handlers under Interface and re-enable them, but the state of the config at this point is desynced under that scenario.

@dreamsyntax dreamsyntax changed the title DolphinQt/MenuBar - add "Reset Ignore Panic Handles" button DolphinQt/MenuBar: add "Reset Ignore Panic Handles" button Oct 17, 2021
@dreamsyntax dreamsyntax changed the title DolphinQt/MenuBar: add "Reset Ignore Panic Handles" button DolphinQt/MenuBar: Add "Reset Ignore Panic Handles" Button Oct 17, 2021
@dreamsyntax dreamsyntax changed the title DolphinQt/MenuBar: Add "Reset Ignore Panic Handles" Button DolphinQt: Add "Reset Ignore Panic Handles" MenuBar Option Oct 17, 2021
@dreamsyntax dreamsyntax changed the title DolphinQt: Add "Reset Ignore Panic Handles" MenuBar Option DolphinQt: Add "Reset Ignore Panic Handler" MenuBar Option Oct 18, 2021
@MayImilae
Copy link
Contributor

MayImilae commented Oct 19, 2021

Hmm. To be direct, I don't understand why this should exist when creating a pause for the panic handler does the same thing but better.

EDIT: OH, I think I misunderstood. Still an idea though.

So Dolphin already has the ability to turn panic handling off and on at demand. So why not turn panic handling off until you reach what you need, then turn it back again? Wouldn't that achieve the same result? To make it easier, you could add an option in the drop down for debug mode to make it more accessible, and/or add a hotkey.

@phire
Copy link
Member

phire commented Oct 19, 2021

They do completely different things.

Pause allows you to pause (and potentially debug).

This mention option allows you to hit ignore all, and then later come back and restore the original behaviour of asking on a Panic Alert (which then allows you to hit pause again)

@dreamsyntax
Copy link
Member Author

dreamsyntax commented Oct 19, 2021

Hmm. To be direct, I don't understand why this should exist when creating a pause for the panic handler does the same thing but better.

EDIT: OH, I think I misunderstood. Still an idea though.

So Dolphin already has the ability to turn panic handling off and on at demand. So why not turn panic handling off until you reach what you need, then turn it back again? Wouldn't that achieve the same result?

We cannot programmatically 'reach what you need' as every game/context is different.
If a panic appears and you hit ignore, it does not matter if Use Panic Handlers is checked. The state is not updated.

To make it easier, you could add an option in the drop down for debug mode to make it more accessible, and/or add a hotkey.

I'm not opposed to this, but I don't think it would be widely used as a hotkey. The use cases I see this being used is validating/tracking down buffer overflows, memory leaks, or validating unstable mods/codes easier.
I do plan to rewrite how panic alerts are handled entirely though, maybe ignoring types rather than ignoring all could be a thing I'll consider as well.

@dreamsyntax
Copy link
Member Author

dreamsyntax commented Oct 19, 2021

This mention option allows you to hit ignore all, and then later come back and restore the original behaviour of asking on a Panic Alert (which then allows you to hit pause again)

Yes, which even without the Pause On Break PR this is still useful. It allows the scenario where an ignore recovers. The user could then Reset Ignore to see if further exceptions happen later on within the same session.

@dreamsyntax
Copy link
Member Author

@phire is there any other requirements to get this through? Or are we good to merge this?
I've been using this in my integration test for my regular workflows and find it useful.

@dreamsyntax
Copy link
Member Author

Bumping this, this is a minor change that adds value to those using debugging

@AdmiralCurtiss
Copy link
Contributor

This is a very simple change that's only visible in the debugging UI and potentially useful, so I'm for it.

@AdmiralCurtiss AdmiralCurtiss merged commit f5657d0 into dolphin-emu:master Mar 19, 2022
@iljatanger

This comment was marked as off-topic.

@dreamsyntax dreamsyntax deleted the panic-handles-restore-messages branch March 20, 2022 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants