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

Require frontend to initialize controllers #10783

Merged
merged 2 commits into from Jul 18, 2022

Conversation

JosJuice
Copy link
Member

We currently have two different code paths for initializing controllers: Either the frontend (DolphinQt) can do it, or if the frontend doesn't do it, the core will do it automatically when booting. Having these two paths has caused problems in the past due to only one frontend being tested (see PR #9086). I would like to get rid of the latter path to avoid further problems like this.

@AdmiralCurtiss
Copy link
Contributor

I think nogui never deinitializes controllers so it now crashes on program termination because some parts of that are still running. At least I'm getting a failure in some std::thread destructor on Windows.

@AdmiralCurtiss
Copy link
Contributor

Yeah, adding a call to UICommon::ShutdownControllers(); before UICommon::Shutdown(); fixes it, but only if BootManager::BootCore() succeeds. Can you put the shutdown stuff in nogui in a ScopeGuard?

@JosJuice JosJuice marked this pull request as draft July 3, 2022 10:01
@JosJuice
Copy link
Member Author

JosJuice commented Jul 3, 2022

I've marked this as a draft for now because of the FifoCI failures. Neither I or AdmiralCurtiss can reproduce the problem on Windows, so if there's someone who could try to reproduce this on Linux (maybe headless is needed?), that would be appreciated. Alternatively, if there's some way to get logs from FifoCI that I'm not aware of...

@AdmiralCurtiss
Copy link
Contributor

FifoCI logs are at eg. https://dolphin.ci/#/builders/16/builds/7127/steps/7/logs/stdio but it doesn't really help here, it just says segmentation fault.

We currently have two different code paths for initializing controllers:
Either the frontend (DolphinQt) can do it, or if the frontend doesn't do
it, the core will do it automatically when booting. Having these two
paths has caused problems in the past due to only one frontend being
tested (see de7ef47). I would like to get rid of the latter path to
avoid further problems like this.
@JosJuice
Copy link
Member Author

FifoCI is no longer reporting any failures. Well, I'm not quite sure what happened, but since there seem to be no problems right now and I don't know of anything that's wrong in the code, I'll unmark this as a draft.

@JosJuice JosJuice marked this pull request as ready for review July 17, 2022 12:24
@AdmiralCurtiss
Copy link
Contributor

Yeah I don't get it either, but it works and the code looks correct to me so shrug.

@AdmiralCurtiss AdmiralCurtiss merged commit fa30ba1 into dolphin-emu:master Jul 18, 2022
@golivax
Copy link

golivax commented Jul 19, 2022

Hi. Sorry to hijack this PR. However, I believe I have information that might be useful to you folks. This PR broke Dolphin on Android using Real WiiMotes and a DolphinBar. Steps to reproduce:

  • Install Dolphin 5.0.16940
  • Go to Settings - Wii Input - Wii Remote 1
  • Change from Emulated to Real Wii Remote
  • Go to Config - Wii
  • Enable Wii Remote Continuous Scanning
  • Enable Wii Remote Speaker
  • Go to game selection screen and confirm that INI file has been updated
  • Close Dolphin
  • Try to open Dolphin

Dolphin will try to launch, but will crash immediately. Phone is a Galaxy S10 with Snapdragon CPU running Android 12. Logs here

@JosJuice
Copy link
Member Author

Thanks for quickly noticing and reporting this! Since you've already created a report on the issue tracker, let's continue the conversation there: https://bugs.dolphin-emu.org/issues/12980

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