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

Allowing Xbox via DInput, increasing max Xinput Count #10132

Closed
wants to merge 11 commits into from

Conversation

CollinCodez
Copy link

@CollinCodez CollinCodez commented Sep 28, 2021

My Change Goals

  • Give users the option to let Dolphin see xbox controllers via DInput (for uses with over 4 xbox controllers)
  • Make dolphin able to work with over 4 controllers Via XInput
    • This also requires either an OpenXInput DLL that is compiled to support the at least the same number of controllers as you would like dolphin to via XInput, or for Dolphin to be launched via Steam and used with Steam input
    • Moved to Separate PR

Why?

Uses like 16 player Mario Kart Double Dash (with 4 dolphin instances on the same PC), or 8 player Mario Party 7, can take advantage of being able to access more xbox controllers.

Things I still need to do or Need Assistance/Advice with

  • Connecting the value from the Allow Accessing Xbox Controllers via DInput checkbox to the DinputJoystick.cpp file
  • Making the XInput device count adjustable via the UI (Should this be a set of values for the user to pick from? How high of values?) Moved to Separate PR
  • How should I deal with the OpenXinput dll?

@MayImilae
Copy link
Contributor

MayImilae commented Sep 28, 2021

Wouldn't Microsoft new Windows.Gaming.Input (gods that's a horrible name) API be a better choice for this goal? Especially considering the joined triggers issue that Xbox controllers still have under DInput.

There is already an open pull request for windows.gaming.input but it's stalled/abandoned. #7614

@CollinCodez
Copy link
Author

Wouldn't Microsoft new Windows.Gaming.Input (gods that's a horrible name) API be a better choice for this goal? Especially considering the joined triggers issue that Xbox controllers still have under DInput.

I have not yet dug into Windows Gaming Input, but OpenXInput is extremely easy to implement and fixes that issue. On my machine I have it working with up to 16 controllers via XInput by basically just putting in the OpenXInput DLL into the folder and making dolphin try to initialize more than 4 controllers.
DInput is there more of as a backup option.

@MayImilae
Copy link
Contributor

MayImilae commented Sep 29, 2021

Hmm. Well, before we debate which route is best, my main current issue with this pull request is that it has two ideas - adding both OpenXInput and DInput. Personally I am much less in favour of one idea versus the other, and that makes this PR hard to swallow for me. So IMO, it would be best to split this into two PRs so we can evaluate these ideas separately.

@CollinCodez
Copy link
Author

Alright, I can do that.

@jordan-woyak
Copy link
Member

I dislike the idea of optionally allowing Xbox controllers to appear over DirectInput. They are purposely hidden for lack of trigger and rumble support. Users will think they want this enabled, but they really don't. The combined trigger situation makes Xbox controllers effectively completely unusable over DirectInput. It's just going to ultimately end in confusion.

I think Windows.Gaming.Input is a better solution than OpenXInput. The former supports the Xbox One trigger rumble motors and is endorsed by Microsoft.

I should get around to cleaning up #7614 (Windows.Gaming.Input support) in the near future.

@phire
Copy link
Member

phire commented Sep 29, 2021

I agree that allowing xbox controllers over DirectInput is non-desirable, especially when other solutions exist.

But I personally have no opinion over integrated OpenXInput vs Windows.Gaming.Input

@CollinCodez
Copy link
Author

I just removed OpenXinput from this branch, will be making another PR for that separately

@CollinCodez
Copy link
Author

@mirh Yuzu uses SDL. This PR is to add a toggle for DInput access to xbox controllers, and I am working on another to add OpenXInput to Dolpin for more Xbox controller support via XInput. I have just been busy over the last few weeks and haven't had much time to work on them.

@vrs99-hub
Copy link

vrs99-hub commented Jan 7, 2022

Hello @CollinCodez, would you be willing to release your (binaries) build of Dolphin that has the Xinput limit removed? It sounds awesome to be able to play Mario Kart, Bomberman Blast, Driift Mania and the like with more than 4 players!

@mbc07
Copy link
Member

mbc07 commented Jan 7, 2022

On the bottom of the PR discussion on GitHub, click on "Show all checks", then on "Details" of the matching OS (e.g. pr-win-x64). You'll be directed to the buildbot, where you can download a pre-compiled binary...

@vrs99-hub
Copy link

Thanks for the tip mbc07, that's certainly good to know!

Unfortunately, this is not the pull request for the specific XInput build CollinCodez mentioned (the lines with strikethrough from the first post). Nor does there seem to be a pull request for that build.

@Nemirtingas
Copy link

Thanks for the tip mbc07, that's certainly good to know!

Unfortunately, this is not the pull request for the specific XInput build CollinCodez mentioned (the lines with strikethrough from the first post). Nor does there seem to be a pull request for that build.

Looks easy to me, download Dolphin sources, download OpenXinput with branch OpenXinput1_4, setup OpenXinput project with cmake and build a library.
Using OpenXinput as a .dll (Shared Library):
Modify Core/InputCommon/ControllerInterface/XInput/XInput.cpp:
Replace

static HMODULE hXInput = nullptr;

by

static HMODULE hXInput = nullptr;

static DWORD DeviceCount = XUSER_MAX_COUNT;
typedef DWORD(WINAPI* OpenXInputGetMaxControllerCount_t)();

Then replace:

PXInputGetState = (XInputGetState_t)::GetProcAddress(hXInput, (LPCSTR)100);

by

PXInputGetState = (XInputGetState_t)::GetProcAddress(hXInput, (LPCSTR)100);
OpenXInputGetMaxControllerCount_t pf = (OpenXInputGetMaxControllerCount_t)::GetProcAddress(hXInput, "OpenXInputGetMaxControllerCount");
if( pf != nullptr )
  DeviceCount = pf();

And finally:

for (int i = 0; i != 4; ++i)

by

for (DWORD i = 0; i != DeviceCount; ++i)

Build Dolphin and copy OpenXinput's Xinput1_4.dll next to Dolphin.exe.

image

I used a modified x360ce to setup a lot of Xinput devices.

@CollinCodez
Copy link
Author

That's a much better way than I did it, I just compiled OpenXInput with the increased controller count, and increased the number of times that FOR loop ran.

I have indeed not made the openxinput PR of dolphin yet, I've just been busy with school and such and not had much time to work on it (at least when I am thinking well enough to work on code like this).

@Nemirtingas
Copy link

That's a much better way than I did it, I just compiled OpenXInput with the increased controller count, and increased the number of times that FOR loop ran.

I have indeed not made the openxinput PR of dolphin yet, I've just been busy with school and such and not had much time to work on it (at least when I am thinking well enough to work on code like this).

Dolphin's maintainers might prefer to use a static library and add an option to use OpenXinput vs Xinput. I just the wrote the most straight way to do it with the fewer modifications.

@CollinCodez
Copy link
Author

Yeah. I learned that after I initially did it, and they also mentioned that they would prefer it to be compiled just into the dolphin.exe rather than a separate dll

@vrs99-hub
Copy link

Thanks for the explanation Nemirtingas, and your work on OpenXinput of course. I didn't know how to build anything honestly. But with your code and the explanation from the Dolphin Wiki it wasn't too complicated. Still, it was about an hours work so hopefully functionality that will be incorporated into standard Dolphin.

@jordan-woyak
Copy link
Member

PR #7614 was merged which should allow for more than four Xbox controllers. Unfortunately Xbox controllers over DirectInput isn't a practical solution because of the combined triggers so I think this PR can be closed.

@AdmiralCurtiss
Copy link
Contributor

Yeah, I'm gonna close this, since WGI should solve the >4 controller issue. If you still think this is worthwhile to have just write a comment or reopen the PR.

@mirh
Copy link

mirh commented Apr 6, 2022

Would that work even with old controllers?
Otherwise you'd still have a point into trying to add OpenXInput (putting aside also supporting older OSs)

@AdmiralCurtiss
Copy link
Contributor

Are there any controllers that work over XInput but not over WGI? I don't think so, but feel free to prove me wrong.

@CollinCodez CollinCodez deleted the master branch June 16, 2022 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

9 participants