Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Digital Trigger vs. Analog Trigger with Button Conflicts #15

Closed
jgeumlek opened this issue Oct 8, 2016 · 15 comments
Closed

Digital Trigger vs. Analog Trigger with Button Conflicts #15

jgeumlek opened this issue Oct 8, 2016 · 15 comments

Comments

@jgeumlek
Copy link
Owner

jgeumlek commented Oct 8, 2016

Some gamepads have digital triggers, like the Wii U Pro controller. The triggers are just buttons.

Some have analog triggers, like the Xbox 360 controller.

Some have both, like the Gamecube controller or DS4. For the former, the digital button fires when the trigger is pressed fully. For the later, it fires on even the slightest trigger pull.

Current configs have mappings like

gamepad.tr2 = tr2
gamepad.tr2_axis = tr2_axis

When simulating a 360 controller, the tr2 event sends a full trigger pull. (Allowing a device like the Wii U Pro to fake the analog trigger as best it can.)

This leads to a problem for the DS4: It's trigger button fires on the slightest pull, which then sends a full pull events. For the Gamecube controller, the problem happens when going from 100% to 90%, as the release of the digital button maps the trigger to 0%. This leads to the output device's trigger jumping around sharply.

Potential solution:
Create a distinct event for trigger-button-with-axis-present versus trigger-button-with-no-axis. The former should be mapped to nothing for most games, as the trigger axis events do all the work.

I am low on name suggestions though. I feel the digital trigger should get to keep the "tr2" name. What about the other?

tr2_axis_btn
tr2_extra_btn
????
@TiZ-HugLife
Copy link
Contributor

How would this distinct event be used? Would the internal tr2 event be ignored in most configs, and this distinct event then used to trigger an external tr2 event at the desired threshold no matter what the controller?

To answer the question, I like tr2_axis_btn.

@jgeumlek
Copy link
Owner Author

It would be ignored in most configs, as most configs are likely matching a 360 controller (either as the virtual output device or as the physical input device). It has become the de facto standard controller interface, and it has no such button event. Most profiles would assume it doesn't exist.

I don't quite follow your last question. If we have analog values, and we want to report analog values, we should avoid mapping this digital button to analog values as it will clobber it. If we don't have analog values (like w/ Wii U Pro), then mapping the button is the best we can (and should) do.

Should MoltenGamepad ever support alternative controller styles for the virtual output controllers, it might be interesting to be able to configure at what threshold this extra virtual button fires. But that is an issue for another day.

It would purely be a case of our input device exposing an event, and it feeling wrong to not expose that event for the user. For example, some one might want to write a profile that maps a gamecube controller to keyboard keys. In this case, it would be best to use the controller's existing button event instead of the axis values as it corresponds to a satisfying tactile click of the trigger.

@TiZ-HugLife
Copy link
Contributor

TiZ-HugLife commented Oct 10, 2016

Your response kind of makes me confused as to what the issue is meant to address. The original issue created here mentions the different behaviors on how the device's internal tr2 event fires in relation to the analog axis's value, and how it's a problem because DS4s fire on the slightest push and GCs unfire at the slightest release. But your response here makes it kind of sound like you don't want to actually do anything about that kind of inconsistency, and instead just expose another special new event. Could I trouble you to take it from the top and tell me what problem you're trying to fix here?

@jgeumlek
Copy link
Owner Author

No problem.

You are correct, this would not fix that inconsistency. That is a problem for a different day since we currently do not make virtual DS4s or virtual GCs. I simply included two examples. This issue applies even when talking solely about DS4s, so let us ignore GCs for now.

From the top:
Currently with a DS4, both the digital event and the analog events get mapped to the virtual 360 pad trigger. This is because the tr2 (digital) event internally gets translated to the analog trigger values.

Recall that our virtual 360 pad has no digital tr2 event, only tr2_axis.

For the DS4, the correct action is to ignore the digital tr2 event. Our virtual output device has no such event, and the analog tr2_axis events do everything we want already. Trying to use tr2 can't help us, and using it clobbers the virtual tr2_axis events already coming the DS4's analog events.

But what about the Wii U Pro? In this case the correct action is to map the digital tr2 to the virtual tr2_axis. Otherwise the Wii U Pro is unable to make the virtual 360 pad triggers move at all.

So sometimes we want to use the tr2 events. Sometimes we don't. This new event tr2_axis_btn would allow us to differentiate these two cases and write profiles accordingly. So if the user writes a profile as such:

gamepad.tr2 = tr2
gamepad.tr2_axis = tr2_axis
gamepad.tr2_axis_btn = nothing

These assignments would propagate to all gamepads, and each will do the correct thing, whether they are physically a Wii U Pro, a 360 pad, or a DS4.

The current set up as the DS4 doing the wrong thing. We could just drop this extra digital event from the gamepad interface as being too exotic, but I think it deserves a standardized name.

@TiZ-HugLife
Copy link
Contributor

I think I understand now, thank you. I like the name tr2_axis_btn. :)

@TiZ-HugLife
Copy link
Contributor

Okay, so there seems to be some sort of problem regarding this event. I have no idea why, but on devices that have digital triggers but not analog triggers, tl2 works fine, but tr2 does not. That is to say, if I use this event alias, then tr2 does not work. It works fine on devices that have analog triggers, or if I just remove the event entirely. I can't figure out why tl2 works but tr2 doesn't.

@jgeumlek
Copy link
Owner Author

I can confirm tr2 working on my end for a controller I have with purely digital triggers. Which device/config in particular is giving you trouble?

Perhaps there is a miscommunication here. Are you saying you have tr2_axis_button defined for a device without analog triggers? If so, you should remove that, and use tr2 instead.

If a device has only digital triggers, use tl2 and tr2.

If a device has only analog triggers, use tl2_axis and tr2_axis

If a device has both, use tl2_axis and tr2_axis along with tl2_axis_btn and tr2_axis_btn.

A device should not have both tr2 and tr2_axis_btn defined. I see your configs linked in the other issue violate this. (Also, what xpad device exposes key code 312?)

(As for the asymmetry, I am assuming it is due to what order events are read from the hash map, which does not guarantee any particular order.)

@TiZ-HugLife
Copy link
Contributor

TiZ-HugLife commented Oct 18, 2016

Hmm... Okay, but I'm using subset to catch controllers that might fall into any of those three cases, especially those pesky generics. What do I do then? Should I just split the configs into analog/digital trigger configurations?

As for your xpad question, it's possible to configure the xpad module to map triggers to buttons instead of axes. I think some xpad devices do that by default. There are other module options that do similar things, like for the dpad. The xpad config should be able to catch any combination of those configurations but I haven't tested it extensively.

@jgeumlek
Copy link
Owner Author

If there are two devices that share an event code, and one needs tr2 and the other needs tr2_axis_btn for that same event code, then you'd need two separate configs to distinguish them.

You should be able to combine case 1 with case 2, or even case 2 with case 3. The trouble is in combining case 1 with 3.

Otherwise when we read that event code, we'd need to do some ugly logic to determine which event of those events it should fire, or add a messy way of doing conditional aliases.

I agree having essentially duplicated configs is not the best, but once it is set up it should just work. Each configuration was intended to match only one physical device. If there is something that strongly makes this unviable, I'll revisit what can be done here.

@TiZ-HugLife
Copy link
Contributor

TiZ-HugLife commented Oct 18, 2016

Duplicating configs isn't a huge deal because once it is set up it should indeed just work. The driver namespace will get kind of cluttered, but I don't especially mind that. I think I'll test for the presence of analog triggers first and then fall back to digital triggers. I should only need to split the ds4 config and xpad config this way because those are the two that are most geared toward generics.

EDIT: Actually, that seems to be combining case 1 and case 3, which is exactly the thing I shouldn't be doing. :/

@jgeumlek
Copy link
Owner Author

jgeumlek commented Oct 18, 2016

This should work, I think:

Let Config A be all events minus tr2_axis and tl2_axis. Use subset as the match, and create the tr2 and tl2 aliases. Anything with the analog axes won't match. Using subset lets us account for dpad shenanigans.

Let Config B be like A, but with the analog trigger axes as well. Here we don't create the tr2 and tl2 aliases, and instead offer the _axis_btn ones. Only devices with analog triggers should be matched here, as we gave config A priority.

So our cases:
Case 1: Matches A. It's fine.
Case 2: Rejected by A, Matches B. It is also fine. It doesn't even have the extra trigger buttons, but we don't care. Not much we can do about that, and most control mappings won't be using these buttons anyway.
Case 3: Rejected by A, Matches B. This is also fine.

In all three cases, a device ends up with the correct set of aliases (we never set both tr2 and tr2_axis_btn on the same device, as neither A nor B has both).

@TiZ-HugLife
Copy link
Contributor

TiZ-HugLife commented Oct 18, 2016

I see! With it set up like that, I was able to get it to work well. I also had to do a bit of a hack with the PS4's touchpad click button to allow it and the share button to share the select button. Four very different devices were all speaking the same language. Perfect!

@jgeumlek
Copy link
Owner Author

jgeumlek commented Oct 18, 2016

Glad to hear it. I'm going to do a bit more cleaning up/error checking before putting these new features on master.

I looked at your repo to see what you did with the select button for the PS4. Is that actually working? I'm a bit surprised. All the code assumes aliases are one-to-one and that event names are unique per device. I can't guarantee things will work that way in the future.

The intended way to achieve that would be to have the touch pad and share buttons be separate events, but use MoltenGamepad's profile system to map both the select button. For example, sending these commands to MoltenGamepad as it is running:

playstation4-dt.touchpad = select
playstation4-at.touchpad = select

You can use moltengamepad.cfg to load profiles as MoltenGamepad starts.

@TiZ-HugLife
Copy link
Contributor

It is actually working. But moltengamepad.cfg isn't. When I specify the config directory path, it ignores the moltengamepad.cfg contained within. Want me to open a separate issue for that?

@jgeumlek
Copy link
Owner Author

Yes, please do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants