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

Add auto-hooking mechanism. #82

Merged
merged 5 commits into from
Nov 4, 2023
Merged

Conversation

cristian64
Copy link
Collaborator

  • The Hook and Unhook buttons have been moved to the new Dolphin top-bar menu.
  • A new Auto-hook checkable menu action (checked by default) has been added to the menu too.

Dolphin Memory Engine - Auto-hooking Mechanism

Bonus:

  • The Open memory viewer button has been moved to the existing View top-bar menu.
  • The Copy Memory Range dialog is now created only once on startup, and its enable state is synced with hooked state. Its associated menu action in the View menu is now enabled/disabled too.

The dialog is now disabled/enabled according to the hook state. Its
associated menu action in the **View** top-bar menu is also synced.

Bonus: Also the following build warning has been addressed:
```
/w/Dolphin-memory-engine/Source/GUI/MemCopy/DlgCopy.cpp:62:27: warning: comparison between 'enum QDialogButtonBox::ButtonRole' and 'enum QDialogButtonBox::StandardButton' [-Wenum-compare]
   62 |             else if (role == QDialogButtonBox::Close)
      |                      ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
```
Copy link
Collaborator

@dreamsyntax dreamsyntax left a comment

Choose a reason for hiding this comment

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

I'm not exactly liking the removal of the single click flow for hook/unhook, but I also acknowledge for most users it's redundant.

I suppose this is fine for now, I think we should consider redesigning the general view anyway. The info about where the address is hooked to could maybe just be under an info dialog popup. Hook status should definitely still be displayed, but I suppose that's for another PR.

@dreamsyntax dreamsyntax merged commit 3acda02 into aldelaro5:master Nov 4, 2023
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