higan-emu / higan

higan is a multi-system emulator focused on accuracy, preservation, and configurability.
Other
1.19k stars 112 forks source link

Mapping XBox triggers to a button causes that button to always be held #108

Closed ghost closed 4 years ago

ghost commented 4 years ago

BBkidLy reported an issue on Discord of the Chrono Trigger and Secret of Mana opening loops ending as soon as they started. It was narrowed down to the Xbox 360 left and right triggers being mapped. Something in the code is falsely deciding these buttons are pressed down when they are not. This happens on Windows 10, other OSes not tested yet.

ghost commented 4 years ago

We found the root of the bug. https://github.com/higan-emu/higan/blob/master/ruby/input/joypad/xinput.cpp#L65 At least on Windows, the emulator is mapping the shoulder buttons as ".Lo" which means active when not pressed. It needs to be changed to ".Hi" somehow for triggers.

Screwtapello commented 4 years ago

My recollection is that gamepad analogue inputs are represented as a number from 0..65535. Normally an analogue axis is centred on about 32767, but since triggers are spring-loaded, they're actually "centred" at one extreme, and code that tries to map an analogue input to a digital input by checking if it's less than 16383 or greater than 49152 will get confused.

Microsoft worked around this in the XBox pad's DirectInput drivers by mapping the triggers to each end of a single analogue axis, that was centred on 32767 and one trigger pulled it to 0 and the other trigger pulled it to 65535, but then you can't press both triggers at once which is weird.

The Linux XBox drivers have the same behaviour, but I don't recall having the same problem there. This bears further investigation.

ghost commented 4 years ago

My proposals for hotfixing:

  1. when assigning the mapping, check if the string contains "Trigger" and force the direction to "Hi", this should work since only the XInput driver supports triggers at all, and the higher value should always be the button pressed value.

  2. update the XInput driver to expose the triggers as buttons, and set the value to 0-1 based on the reading of the analog stick value (0 to 255), where 128 or higher = pressed. This works since higan doesn't emulate any systems that need pressure-sensitive buttons (yet?)

A much more comprehensive fix would be to remove the code that makes the triggers range from -32768 to +32767 and instead have it be 0 to +32767 (since the type is int16_t), and update the GUI to treat triggers differently than axes and not have a concept of Lo/Hi for them. Like you suggested, when converting an analog input to digital, a thumbstick "axis" has three states, (left/center/right) or (up/middle/down), but a trigger has only two (pressed/not pressed). Or in other words, the axis has two trigger points (-32768 and +32767) with a resting state (0), but a trigger has only one action state (+255/+32767/65535) and a resting state (0/-32768/0)

I'd imagine you want the comprehensive fix, but if a hotfix is okay for now let me know and I can send a pull request.