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

Fix default input config default device not being loaded/found #10248

Merged

Conversation

Filoppi
Copy link
Contributor

@Filoppi Filoppi commented Nov 27, 2021

Fixes bug: https://bugs.dolphin-emu.org/issues/12744

Before e1e3db1 the ControllerInterface m_devices_mutex was "wrongfully" locked for the whole Initialize() call, which included the first device population refresh, this has the unwanted (accidental) consequence of often preventing the different pads (GC Pad, Wii Contollers, ...) input configs from loading until that mutex was released (the input config defaults loading was blocked in EmulatedController::LoadDefaults()), which meant that the devices population would often have the time to finish adding its first device, which would then be selected as default device (by design, the first device added to the CI is the default default device, usually the "Keyboard and Mouse" device).

After the commit mentioned above removed the unnecessary m_devices_mutex calls, the default default device would fail to load (be found) causing the default input mappings, which are specifically written for the default default device on every platform, to not be bound to any physical device input, breaking input on new dolphin installations (until a user tried to customize the default device manually).

Default devices are now always added synchronously to avoid the problem, and so they should in the future (I added comments and warnings to help with that).
The only one which was async was the Windows "Keyboard and Mouse" device, and I took care of that. I also changed devices sorting order to be clearer (and avoid other devices accidental ending up as default) (the sorting order only affect the Qt devices list order and nothing else).

@Filoppi Filoppi marked this pull request as draft November 27, 2021 21:16
@Filoppi Filoppi force-pushed the fix_input_config_default_device_load branch from dc1c6cd to a0e77be Compare November 27, 2021 21:29
@Filoppi Filoppi marked this pull request as ready for review November 27, 2021 21:50
Copy link
Member

@JosJuice JosJuice left a comment

Choose a reason for hiding this comment

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

I suppose this solution works too. Personally I like to use a raw condition variable in cases like this, but I know there are others that prefer Common::Event, so let's not make that a blocker.

@Filoppi Filoppi force-pushed the fix_input_config_default_device_load branch from a0e77be to 5af35cc Compare November 28, 2021 01:08
@PatrickFerry
Copy link
Contributor

PatrickFerry commented Nov 28, 2021

I had to build myself since the buildbot seems to be having trouble with building for Windows,

It works to resolve the issue.

@Filoppi Filoppi force-pushed the fix_input_config_default_device_load branch 4 times, most recently from 7220993 to 925c3de Compare November 29, 2021 22:34
@dezraj
Copy link

dezraj commented Dec 4, 2021

Hello! I've downloaded Dolphin 5.0-15611 (most recent version for Windows) from build-bot and the issue is still there.

Is there a build already available with this fix or should I wait?

@JosJuice
Copy link
Member

JosJuice commented Dec 4, 2021

Once this pull request says "Merged" instead of "Open", the change will be available in a build at https://dolphin-emu.org/. For now, either download a test build from this page, or wait.

…/found

Fixes bug: https://bugs.dolphin-emu.org/issues/12744

Before dolphin-emu@e1e3db1
the ControllerInterface m_devices_mutex was "wrongfully" locked for the whole Initialize() call, which included the first device population refresh,
this has the unwanted (accidental) consequence of often preventing the different pads (GC Pad, Wii Contollers, ...) input configs from loading
until that mutex was released (the input config defaults loading was blocked in EmulatedController::LoadDefaults()), which meant that the devices
population would often have the time to finish adding its first device, which would then be selected as default device (by design, the first device
added to the CI is the default default device, usually the "Keyboard and Mouse" device).

After the commit mentioned above removed the unnecessary m_devices_mutex calls, the default default device would fail to load (be found)
causing the default input mappings, which are specifically written for the default default device on every platform, to not be bound to any
physical device input, breaking input on new dolphin installations (until a user tried to customize the default device manually).

Default devices are now always added synchronously to avoid the problem, and so they should in the future (I added comments and warnings to help with that)
@Filoppi Filoppi force-pushed the fix_input_config_default_device_load branch from 925c3de to 125971d Compare December 5, 2021 21:36
@JosJuice JosJuice merged commit e0a61ed into dolphin-emu:master Dec 9, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants