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

kernel explorer: Make it single-instance tool #9570

Merged
merged 3 commits into from Jan 10, 2021
Merged

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Jan 8, 2021

Enhancements

  • Show only one instance of kernel explorer at maximum, there is no use in multiple instances even when multi-process will be a thing. Make its menu option an enable-disable switch.
  • Whenever emulation is stopped the kernel explorerer used to do nothing and keeps running, now it will close as well as expected.

@Megamouse
Copy link
Contributor

I don't wanna be an ass. But this PR is really really bad.... :)

@elad335
Copy link
Contributor Author

elad335 commented Jan 9, 2021

Okay, is it the code or the purpose of the pr?

@Megamouse
Copy link
Contributor

Mostly the code. But I don't really understand what you're trying to accomplish anyway

@elad335
Copy link
Contributor Author

elad335 commented Jan 9, 2021

Cleaned the code a bit, added some reasoning in description.
If you have further advice on the code side you can comment them on specific cases.

rpcs3/rpcs3qt/kernel_explorer.cpp Outdated Show resolved Hide resolved
rpcs3/rpcs3qt/main_window.ui Outdated Show resolved Hide resolved
@@ -1971,6 +1984,7 @@ void main_window::CreateDockWindows()
m_debugger_frame->setObjectName("debugger");
m_log_frame = new log_frame(m_gui_settings, m_mw);
m_log_frame->setObjectName("logger");
m_kernel_explorer = new kernel_explorer(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

this whole idea of having a persistent window is wrong.
A: there is no need for it to be created on boot
B: 99% of the users won't need it
C: it will always show up as a hidden window if you investigate your open windows

The correct way to do it is to make it non-modal and to use a singleton approach as seen in GalCiv's code (e.g. the skylanders dialog).

This also removes the need of a convoluted signal slot mechanic (which you overcomplicated by a lot, since there are already native signals for this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about now?

rpcs3/rpcs3qt/main_window.cpp Outdated Show resolved Hide resolved
rpcs3/rpcs3qt/kernel_explorer.cpp Outdated Show resolved Hide resolved
rpcs3/rpcs3qt/main_window.cpp Outdated Show resolved Hide resolved
rpcs3/rpcs3qt/main_window.cpp Show resolved Hide resolved
rpcs3/rpcs3qt/kernel_explorer.h Outdated Show resolved Hide resolved
rpcs3/rpcs3qt/kernel_explorer.h Outdated Show resolved Hide resolved
@Megamouse Megamouse merged commit 70804e2 into RPCS3:master Jan 10, 2021
@elad335 elad335 deleted the ke branch March 16, 2021 17:41
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.

None yet

2 participants