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

[Henri Verbeet]

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)
> +    {