search

araghon007 / X1nput

Xinput hook for Impulse Trigger emulation
MIT License
344 stars 37 forks source link

Plugin assumes XInputGetState will be called first #3

Closed CookiePLMonster closed 5 years ago

CookiePLMonster commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Sorry about that, the vcxproj should be fixed now

araghon007 commented 5 years ago

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 commented 5 years ago

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

araghon007 commented 5 years ago

Alright, thanks a lot for the help