dinput - Allow the use of the standard Joystick GUID when calling CreateDevice

[Vitaliy Margolen]

Peter Dons Tychsen wrote:
> On Sun, 2007-07-29 at 11:15 -0600, Vitaliy Margolen wrote:
>> Peter Dons Tychsen wrote:
>>> Hello again Wine!
>>>
>>> Here is a small fix for dinput which fixes application which call
>>> CreateDevice with the standard Joystick GUID instead of the GUID
>>> supplied in the enumeration callback. 
>> The patch looks good, and I see what you trying to fix. It would be
>> nice if you add some tests for this. MSDN is pretty explicit about
>> what GUIDs could be used there.
>>
>> Few nitpicks about your patch:
>>
>>> -static BOOL IsJoystickGUID(REFGUID guid)
>>> +/******************************************************************************
>>> +  *     Acquire : gets exclusive control of the joystick
>>> +  */
>>> +static CHAR IsJoystickGUID(REFGUID guid)
>>>  {
>>>
>> Comment are good, but only when they are correct. Also you probably should
>> rename this function to something like get_joystick_index.
>>
>>> +  return -1;
>>>  }
>>>
>>>  static HRESULT joydev_create_deviceA(IDirectInputImpl *dinput, REFGUID rguid, REFIID riid, LPDIRECTINPUTDEVICEA* pdev)
>>>  {
>>> -  if ((IsEqualGUID(&GUID_Joystick,rguid)) ||
>>> -      (IsJoystickGUID(rguid))) {
>>> +  CHAR index;
>> Why CHAR? the Data3 defined as "unsigned short Data3;". That is what
>> you really should use. BTW be careful with that "return -1;" it won't
>> be < 0 for unsigned anymore.
>>
>>> -    GUID wine_joystick = DInput_Wine_Joystick_GUID;
>>> -    GUID dev_guid = *guid;
>>> +  GUID wine_joystick = DInput_Wine_Joystick_GUID;
>>> +  GUID dev_guid = *guid;
>> Please do not reformat the code. It should be 4-spaces indentation, not 2.
>>
>> And while you changing this, don't forget to change joystick_linuxinput.c as well.
>> They should be kept in sync.
>>
>>
>> Vitaliy.
> 

> Hi Vitaliy.
> 
> OK, thanks for the input:

First of all, please bottom post on this list. We are not business.

> 
> I have re-sent the patch with all your remarks fixed, except for the one
> regarding joystick_linuxinput.c, which cannot be done with the current
> code as alloc_device() is too different in the two versions for the
> patch to work. I have however made sure that the version in
> joystick_linuxinput.c does not have this problem.
> 
Why do you think they are so different? Looks almost the same to me.
Few more things to fix:

> -#undef MAX_JOYSTICKS
You removed this, but you haven't added it back.

> GetJoystickIndex
All the functions around that place named properly. Don't use up-casing names.
I suggested using "get_joystick_index" - all lover case, with underscores.

> -static HRESULT alloc_device(REFGUID rguid, const void *jvt, IDirectInputImpl *dinput, LPDIRECTINPUTDEVICEA* pdev)
> +static HRESULT alloc_device(REFGUID rguid, const void *jvt, IDirectInputImpl *dinput, LPDIRECTINPUTDEVICEA* pdev, unsigned short index)
This is a really long line, please make it shorter.