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

Control map multiple keys to one output #17215

Merged
merged 9 commits into from
Apr 1, 2023
Merged

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Apr 1, 2023

Reimplementation of #17168, using the new control mapping system, but supports up to 3-key combos.

One thing I've been thinking about is that with this, if say you map RTrigger + Y to take a screenshot, it'll still send the individual RTrigger and Y while you're pressing it. I'm thinking that when you do a combo like that, maybe the individual press of the second key could be suppressed (can hardly suppress the first one..)...

@hrydgard hrydgard added the Input/Controller Input and controller issues label Apr 1, 2023
@hrydgard hrydgard added this to the v1.15.0 milestone Apr 1, 2023
@unknownbrackets
Copy link
Collaborator

unknownbrackets commented Apr 1, 2023

Hm, thinking as a user I do think suppressing the second key is logical. This is fairly consistent with existing interfaces, like Ctrl-F etc. so I think it would make the most sense. Probably doesn't need to be within this pull, though, but a good idea.

-[Unknown]

Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad this handles previous config parsing well. Makes sense.

-[Unknown]

Core/ControlMapper.cpp Show resolved Hide resolved
Core/ControlMapper.cpp Outdated Show resolved Hide resolved
@unknownbrackets
Copy link
Collaborator

Accidental submodule updates slipped in.

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Apr 1, 2023

Oh, no.

@hrydgard hrydgard enabled auto-merge April 1, 2023 18:29
@hrydgard hrydgard merged commit 47785aa into master Apr 1, 2023
@hrydgard hrydgard deleted the multi-mapping-input branch April 1, 2023 18:47
Comment on lines +115 to +116
confirmKeys.push_back(InputMapping(DEVICE_ID_PAD_0, NKCODE_BUTTON_2));
cancelKeys.push_back(InputMapping(DEVICE_ID_PAD_0, NKCODE_BUTTON_3));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of controllers have different opinions on which buttons are which - I'm definitely not a fan of hardcoding these specific ones. Why do we need to do this and didn't before?

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan either, but these match with the "autodetect" mappings we use for DInput pads on Windows - which happen to match the PS4 controller I have.

I can't get it to navigate PPSSPP's menus with the PS4 controller without this. Does that work for you somehow?

But maybe we should just pick up from the bindings, at least on PC where you always have other options to navigate anyway...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can check later. It used to work just fine, of course. I'm sure there are some people who do couch gaming with dinput controllers on media pcs, still...

I thought this did use the bindings automatically, and I already have buttons mapped. I used to use (and still have) a controller that used different keys and it used to work fine in PPSSPP as well - after mapping it once. Forcing the wrong mappings seems bad. It's like the same stupid mistake the worst console->PC ports have made.

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've always had hardcoded bindings just for the PPSSPP menus so that you can always navigate even if you accidentally clear the configurable bindings - would be really screwed on the NVIDIA Shield for example without that.

It's possible I broke something though, let me check an older build.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we already have code to try to pick up extra confirm/cancel buttons from the bindings. So I think I did break something.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this was just me screwing up. Fixed in the new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Input/Controller Input and controller issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants