My xinput implementation please test, plus I am getting segfault with spore

[Andrew Fenn]

Hi Henri,

Thanks for the input, could you take a look at my patches I sent on
wine patches instead as I have fixed all these issues and updated my
diffs.

http://www.winehq.org/pipermail/wine-patches/2008-October/063800.html
http://www.winehq.org/pipermail/wine-patches/2008-October/063801.html
http://www.winehq.org/pipermail/wine-patches/2008-October/063802.html

Also I managed to find out why spore was crashing too. Thank you for
replying (I thought the email never arrived). I didn't put any
information into the Stub. I was thinking about it but I wasn't sure
how to format it correctly in "wine style".

If you find and error with those patches please let me know.

Regards,
Andrew

On Tue, Oct 28, 2008 at 9:24 PM, Henri Verbeet <hverbeet at gmail.com> wrote:
> 2008/10/25 Andrew Fenn <andrewfenn at gmail.com>:
>> Hi all,
>>
>> I am getting a segfault with the release version of Spore (I didn't
>> test 1.1) right after my code implementation of xinput is handled by
>> the game. Is there something I am doing wrong here or is it the game?
> ...
>> +DWORD XInputGetState(DWORD dwUserIndex, XINPUT_STATE* pState)
> As Stefan already mentioned, this function should be WINAPI. Keeping
> it like this will screw up the stack after calls to XInputGetState(),
> which could be the cause for the problem with Spore.
>
>> +        // Get the state
> Don't use C++ comments.
>
>> +        ok(result != ERROR_BAD_ARGUMENTS, "XInputGetState failed with (%d)\n", result);
> Writing the test like means the test will succeed if the call fails
> with a return code other than ERROR_BAD_ARGUMENTS . It would be better
> to use "result == ERROR_SUCCESS || result ==
> ERROR_DEVICE_NOT_CONNECTED" instead.
>
>> +            printf("-- Results for controller %d --\n", controllerNum);
>> +            printf("XInputGetState: %d\n", result);
>> +            printf("State->dwPacketNumber: %d\n", controllerState.dwPacketNumber);
>> +            printf("Gamepad Variables --\n");
>> +            printf("Gamepad.wButtons: %#x\n", controllerState.Gamepad.wButtons);
>> +            printf("Gamepad.bLeftTrigger: 0x%08x\n", controllerState.Gamepad.bLeftTrigger);
>> +            printf("Gamepad.bRightTrigger: 0x%08x\n", controllerState.Gamepad.bRightTrigger);
>> +            printf("Gamepad.sThumbLX: %d\n", controllerState.Gamepad.sThumbLX);
>> +            printf("Gamepad.sThumbLY: %d\n", controllerState.Gamepad.sThumbLY);
>> +            printf("Gamepad.sThumbRX: %d\n", controllerState.Gamepad.sThumbRX);
>> +            printf("Gamepad.sThumbRY: %d\n", controllerState.Gamepad.sThumbRY);
> If you really need this information, you should use trace() instead of
> printf(). I suspect you don't need the information though.
>
>> +    FIXME("Stub \n");
> You don't need the space there. Also, it's often useful to print the
> function's arguments in FIXMEs like these, although it's less of an
> issue for regular function than for COM methods.
>
>> -#define COBJMACROS
>> -
> If you don't want this, you shouldn't add it in your first patch,
> rather than removing it in the second patch.
>
>  - I don't think you should duplicate the test for each dll. If you
> find differences between different versions of the dll it might be
> worth it to add tests for those specific differences, but if the
> behaviour is the same it'll just make it harder to maintain the tests.
>  - You should add a prototype for XInputGetState() to xinput.h
>  - Try to be a bit more consistent with in your style. eg.:
>> +    for(controllerNum=0; controllerNum < 4; controllerNum++) {
> vs.
>> +    if (hXinput)
>> +    {
>