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 updates #2359

Merged
merged 4 commits into from May 8, 2015
Merged

DolphinQt updates #2359

merged 4 commits into from May 8, 2015

Conversation

s-mcf
Copy link
Contributor

@s-mcf s-mcf commented May 2, 2015

Since wxWidgets is terrible and the QT frontend hasn't been touched in a while, I've been working on bringing DolphinQT up to speed with the wx frontend.

In the interest of proper code review, I've been told it's probably better to send smaller PRs more often vs massive PRs less often.

@degasus
Copy link
Member

degasus commented May 3, 2015

lgtm

1 similar comment
@Tilka
Copy link
Member

Tilka commented May 3, 2015

lgtm

std::string path = ShowFolderDialog().toStdString();
if (!path.empty()){
std::vector<std::string>::iterator itResult = std::find(
SConfig::GetInstance().m_ISOFolder.begin(),

This comment was marked as off-topic.

This comment was marked as off-topic.

@waddlesplash
Copy link
Contributor

It's Qt not QT.


}

bool DMainWindow::Stop(){

This comment was marked as off-topic.

@waddlesplash
Copy link
Contributor

FWIW: This is appreciated, but next time, please ask before doing something. I've got a lot of stuff in half-finished state offline, so at the very least I might have something to give you before you start working.

@s-mcf s-mcf changed the title DolphinQT updates DolphinQt updates May 3, 2015
@s-mcf
Copy link
Contributor Author

s-mcf commented May 3, 2015

@waddlesplash got it, thanks for the heads up. In that vein you should know I've got auto-resizing for the main window on another branch, and have started working on rendering to a separate window.

@waddlesplash
Copy link
Contributor

@darkengine-io Render-to-separate-window used to work but it was removed on purpose after some significant discussion, IIRC. I don't recall when / where but there was a PR about it and it was approved.

@s-mcf
Copy link
Contributor Author

s-mcf commented May 3, 2015

Found it, #1688. I don't see any explanation in that PR as to why render-to-separate-window was removed, maybe it was had in IRC?

@lioncash
Copy link
Member

lioncash commented May 3, 2015

Is there an argument for a separate rendering window anyways? With correct UI design, you could simply hide the rest of the UI (treating each segment as a detachable widget), which obsoletes the need for this. For reference, citra sort of does this.

@s-mcf
Copy link
Contributor Author

s-mcf commented May 3, 2015

Yes, we could give the option to the user to hide the UI elements or not, effectively replacing rendering outside the main window. However this would prevent the use of the UI while the game is running if this is enabled.

@lioncash
Copy link
Member

lioncash commented May 3, 2015

The user would have the capability to dynamically enable/disable the UI on the fly, which alleviates that concern.

@s-mcf
Copy link
Contributor Author

s-mcf commented May 3, 2015

Sounds fair, I'll try taking a crack at that on my other branch.

Is there anything left to be done for this PR?

@shuffle2
Copy link
Contributor

shuffle2 commented May 3, 2015

@waddlesplash No, people do not need to ask you in order to do something. ffs...

@waddlesplash
Copy link
Contributor

@shuffle2 Of course not, it's just that it's duplication of work in some instances. Just saying.

@s-mcf
Copy link
Contributor Author

s-mcf commented May 5, 2015

Anyone want to give a review for the latest iteration? I've got another PR's worth of commits ready locally once this gets merged.

return QFileDialog::getExistingDirectory(this, tr("Browse for a directory to add"),
QDir::homePath(),
QFileDialog::ShowDirsOnly);
}

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

{
auto itResult = std::find(
iso_folder.begin(),
iso_folder.end(), path);

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

shuffle2 added a commit that referenced this pull request May 8, 2015
@shuffle2 shuffle2 merged commit 6fe2ab7 into dolphin-emu:master May 8, 2015
@KAMiKAZOW
Copy link
Contributor

Btw "half-finished stuff" should be in a WIP branch, not offline.
Thanks for picking the Qt version up again, @darkengine-io.

It appears that building wx or Qt versions is still an exclusive choice (at least CMakeLists.txt is untouched). From a packager's point of view, it would be nice to be able to build all versions at once and then group the resulting binaries into sub-packages (I already did that for wx and nogui).

@degasus
Copy link
Member

degasus commented May 8, 2015

I've thought this is possible, at least "make" build dolphin-emu (wx), dolphin-emu-qt and dolphin-emu-nogui here at once.

@waddlesplash
Copy link
Contributor

Yes, you can make all three of them at the same time, I use that in my development setup.

@KAMiKAZOW
Copy link
Contributor

Huh, it once didn't work and after that I just watched CMakeLists.txt for changes regarding Qt. It was probably my fault then…

@leoetlino leoetlino modified the milestone: Qt Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet