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

Plugin assumes XInputGetState will be called first #3

Closed
CookiePLMonster opened this issue Mar 18, 2019 · 10 comments
Closed

Plugin assumes XInputGetState will be called first #3

CookiePLMonster opened this issue Mar 18, 2019 · 10 comments

Comments

@CookiePLMonster
Copy link
Contributor

CookiePLMonster commented Mar 18, 2019

I can totally see games call XInputGetCapabilities before XInputGetState, and that will cause a crash.

Current m_gamepad initialization is also not thread safe, so in an unlikely event of two thread racing unexpected results may occur.

@araghon007
Copy link
Owner

Well, the only issue I found with XInputGetCapabilities is games not accepting controller input.

I was able to debug an app that crashed using this DLL and I found out the crash is caused by a module not being loaded, so I'm assuming it's an issue with WinRT, which Windows.Gaming.Input is using.

Also, I will look into thread safety.

@CookiePLMonster
Copy link
Contributor Author

That doesn't really matter. m_gamepad is being initialized only in GetState so any other function being called before that will crash. That includes GetStateEx, which some apps are using instead of GetState.

@araghon007
Copy link
Owner

Oh, right. I'll figure out a better way ASAP. The reason I did it this way, because initializing the game pad on DLL injection caused crashes, and I didn't figure out to delay the initialization.
Any suggestions?

@CookiePLMonster
Copy link
Contributor Author

InitOnceExecuteOnce on top of each function using m_gamepad where you initialize this pointer. It also handles thread safety for you, so it solves two problems at once!

@araghon007
Copy link
Owner

Alright, I've updated the configurable branch. I know I should have asked sooner, but if you know a proper way to implement InitOnceExecuteOnce, please let me know.

@CookiePLMonster
Copy link
Contributor Author

CookiePLMonster commented Mar 18, 2019

I wanted to take a look and potentially submit a PR back, but looks like the project cannot be opened. Did you commit your .vcxproj changes only partially or something?

Anyway, in your case InitOnceExecuteOnce is super easy - in callback function you just initialize m_gamepad and return TRUE - then if InitOnceExecuteOnce returns true you can assume everything initialized correctly, otherwise just do bail out of functions IMO.

@araghon007
Copy link
Owner

Sorry about that, the vcxproj should be fixed now

@araghon007
Copy link
Owner

Ah, I just used the example by Microsoft. So I can safely remove all stuff related to creating an event object in the callback function?

@CookiePLMonster
Copy link
Contributor Author

Sure thing - just initialize m_gamepad there and don't worry about other stuff.

@araghon007
Copy link
Owner

Alright, thanks a lot for the help

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

No branches or pull requests

2 participants