Re: winejoystick.drv: Add axis remapping support

[Vitaliy Margolen]

Adolfo R. Brandes wrote:
> Greetings!
>
> The purpose of this patch is to add registry axis remapping and POV
> hat support to winejoystick.drv, much like what's already in place for
> dinput/joystick_linux.c.  I needed this for X-Wing Alliance
> (http://appdb.winehq.org/objectManager.php?sClass=application&iId=2110),
> which recognizes my Thrustmaster Top Gun Fox 2 Pro USB joystick, but
> not the POV hat and with the Rudder and Throttle axes switched around.
>
> It uses the following conventions:
>
> HKCU\Software\Wine\Joystick<joyname> = <axeslist>
>
> or
>
> HKCU\Software\Wine\AppDefaults\app.exe\Joystick<joyname> = <axeslist>
>
> Where <joyname> is the device name (as shown in jstest), and
> <axeslist> is a comma-separated list of axis names.  However, in
> contrast to joystick_linux.c, the axis names are the same as in
> jstest.  For example, in my case jstest reports:

Don't do that. Keep it uniform. As-is it's a pain to configure. You want to 
introduce even more?

First few general notes about your patch - do not mix tabs and spaces! Set 
tab width to be _always_ 8 spaces! For best results just don't use tabs. Do 
not change formating, of the function unless you changing big part of it and 
you changing it to default Wine formating (4 spaces, curly brackets on their 
own line).
> +static char *axis_name(char *str, unsigned char type, int len) {
> -
>   switch(type) {
> -
>           case ABS_X:
> -
>                   memcpy(str,"X",len - 1);
> -
>                   break;
> -
>           case ABS_Y:

The way you using this function it makes much more sense to return static 
text ex:
static const char *axis_name(unsigned char type)
{
     switch (type)
     {
         case ABS_X: return "X";
         case ABS_Y: return "Y";
         case ABS_Z: return "Z";
         default: return "?";
     }
}

BTW what will that "?" do? I don't see you comparing it against anything.
> -
>                   else
> -
>                           ERR("Not all joystick axes mapped: %d axes, %d arguments\n", nrOfAxes, j);

Disabling axis might not be a bad thing, especially for testing. I don't 
think you need to worry about extra axis on the device that are not mapped 
to anything.
> -
>   for (i = 0; i < nrOfAxes; i++)
> -
>           switch(axis_map[i]) {
> -
>           case ABS_HAT0X:
> -
>           case ABS_HAT0Y:
> -
>                   lpCaps->wCaps |= JOYCAPS_HASPOV;
> -
>                   break;
> -
>           }

You need both X and Y axis for it to work. You have to check that both are 
present. Also what about multiple POVs? Here you checking for only one.
>     switch(nrOfAxes) {
> -
>   case 6: lpCaps->wCaps |= JOYCAPS_HASV;
> -
>   case 5: lpCaps->wCaps |= JOYCAPS_HASU;
> -
>   case 4: lpCaps->wCaps |= JOYCAPS_HASR;
> -
>   case 3: lpCaps->wCaps |= JOYCAPS_HASZ;
> -
>       /* FIXME: don't know how to detect for
> -
>          JOYCAPS_HASPOV, JOYCAPS_POV4DIR, JOYCAPS_POVCTS */
> -
>   case 6:
> -
>           lpCaps->wCaps |= JOYCAPS_HASV;
> -
>           lpCaps->wVmax = 0xFFFF;
> -
>   case 5:
> -
>           lpCaps->wCaps |= JOYCAPS_HASU;
> -
>           lpCaps->wUmax = 0xFFFF;
> -
>   case 4:
> -
>           lpCaps->wCaps |= JOYCAPS_HASR;
> -
>           lpCaps->wRmax = 0xFFFF;
> -
>   case 3:
> -
>           lpCaps->wCaps |= JOYCAPS_HASZ;
> -
>           lpCaps->wZmax = 0xFFFF;

Now that you can remap axis it's not guaranteed that 6th axes is the speed 
and so on.

Vitaliy.