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

UI: Add analog speed limit mapping #15645

Merged
merged 7 commits into from
Jul 6, 2022

Conversation

unknownbrackets
Copy link
Collaborator

@unknownbrackets unknownbrackets commented Jul 4, 2022

This makes it possible to map an analog (most especially, a trigger) to an alternate speed. The FPS is then limited based on how much the analog is pressed, linearly. It can slow down or speed up, depending on what the limit is configured to, but it can't do both.

One trick is that on my DirectInput controllers, the trigger ends up mapped from negative to positive whereas XInput just has a linear positive mapping. I'm a bit concerned there might be other weird results for other controllers, so I added better auto-detection of pad identifiers. For now, kept it to axis ID which might just work.

The auto detection could probably allow better auto-configuration logic for buttons too.

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Jul 4, 2022

Quite a bit of logic, I'll review it tomorrow :) This is a really slick feature though.

@hrydgard hrydgard added this to the v1.13.0 milestone Jul 4, 2022
@@ -862,6 +862,8 @@ static ConfigSetting graphicsSettings[] = {
ReportedConfigSetting("AutoFrameSkip", &g_Config.bAutoFrameSkip, false, true, true),
ConfigSetting("FrameRate", &g_Config.iFpsLimit1, 0, true, true),
ConfigSetting("FrameRate2", &g_Config.iFpsLimit2, -1, true, true),
ConfigSetting("AnalogFrameRate", &g_Config.iAnalogFpsLimit, 240, true, true),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this belong to controls section? Related to #15406.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This isn't the button or anything, it's the limit. It's kinda more related to graphics.

-[Unknown]

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just wondering if we want to allow different control profile to have different limit or not. Not really interested in the semantic meaning here :)

Not a big deal anyway

@@ -337,6 +337,9 @@ bool KeyMappingNewKeyDialog::key(const KeyInput &key) {
if (key.keyCode == NKCODE_EXT_MOUSEBUTTON_1) {
return true;
}
// Only map analog values to this mapping.
if (pspBtn_ == VIRTKEY_SPEED_ANALOG)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we provide a message when user try to bind a key?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, it just doesn't react.

-[Unknown]

Copy link
Contributor

Choose a reason for hiding this comment

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

Mhhhh, it should react to at least "Back" button. If a controller have no analog axis this will brick the emulation basically.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I thought it did allow a way to cancel without touch already. Well, I let cancel go through. Thanks for pointing out.

-[Unknown]

UI/EmuScreen.cpp Outdated
PSP_CoreParameter().fpsLimit = FPSLimit::CUSTOM2;
osm.Show(sc->T("SpeedCustom2", "Speed: alternate 2"), 1.0);
} else if (PSP_CoreParameter().fpsLimit != FPSLimit::NORMAL) {
} else if (PSP_CoreParameter().fpsLimit == FPSLimit::CUSTOM1 && PSP_CoreParameter().fpsLimit == FPSLimit::CUSTOM2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a || here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, must've copy pasted something wrong there. Thanks.

-[Unknown]

Rather than just that it's a pad.  This tries to get the identifier if
possible.
Only shows up as a setting if mapped, to avoid cluttering settings.
@@ -335,6 +336,11 @@ void GameSettingsScreen::CreateViews() {
altSpeed2->SetZeroLabel(gr->T("Unlimited"));
altSpeed2->SetNegativeDisable(gr->T("Disabled"));

if (KeyMap::AxisFromPspButton(VIRTKEY_SPEED_ANALOG, nullptr, nullptr, nullptr)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't update when you get back from control mapping. If you exit to main menu and go back to setting screen it work tho'. Probably missing some RecreateView().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, you're right. Made it recreate. I had tested with it already mapped by the time I changed this...

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Jul 5, 2022

Code looks good, the MAPPED_TO_OPPOSITE stuff is a bit unintuitively named I feel, but okay I guess.

Will test it shortly before merging.

@hrydgard
Copy link
Owner

hrydgard commented Jul 5, 2022

Hm, on my 360 controller, it's almost as if it's missing some events - by releasing the trigger quickly I could often get it in a state where it's stuck in a speed considerably faster than 100%. Weird...

@hrydgard hrydgard added the User Interface PPSSPP's own user interface / UX label Jul 5, 2022
if (mode == AnalogFpsMode::MAPPED_DIRECTION) {
value = fabsf(value);
if (opposite)
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

About the XBOX controller getting stuck, could you try set the speed back to normal rather than just returning here?

I got a really vague memory about some problem with controller going [-1, 0] because 0 is somewhat positive.

Copy link
Owner

Choose a reason for hiding this comment

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

This does indeed help:

		value = fabsf(value);
		if (opposite) {
			value = 0.0f;
		}

instead of returning.

Though, I would argue that the code that tries to figure out an "opposite" key for a throttle already is wrong, but that's a separate bug. The value that comes in in that case is indeed 0.0, and returning there means that we miss the return to 0.0.

Copy link
Owner

@hrydgard hrydgard Jul 5, 2022

Choose a reason for hiding this comment

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

Yeah basically if it's a zero value, the mapping finds itself as an opposite with value 0.0, and skips it with the existing code :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm, I guess that's fine, I suppose any case where you don't want the negative (like mapping right analog X to speed) it'd make sense for it to clamp.

So direction is -1 for 0? I was expecting both to get called, because of this:

	if (axis.value > 0) {
		processAxis(axis, 1);
		return true;
	} else if (axis.value < 0) {
		processAxis(axis, -1);
		return true;
	} else if (axis.value == 0) {
		// Both directions! Prevents sticking for digital input devices that are axises (like HAT)
		processAxis(axis, 1);
		processAxis(axis, -1);
		return true;
	}

So sure, one would get skipped, but the other wouldn't. I guess it's probably not exactly equaling 0 or something?

-[Unknown]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Code looks good, the MAPPED_TO_OPPOSITE stuff is a bit unintuitively named I feel, but okay I guess.

Well, changed to MAPPED_DIR_TO_OPPOSITE_DIR. I guess it could be THROUGH or UNTIL or something, but already kinda long. I guess it could be MAPPED_FULL_RANGE or MAPPED_EDGE_TO_EDGE or something, none of them are great.

-[Unknown]

Copy link
Owner

Choose a reason for hiding this comment

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

Both do get called, so hm, I retract my statement about a bug, and now I'm slightly confused how this happens. The change does help though! I'm gonna debug some more because this doesn't quite make sense...

@hrydgard
Copy link
Owner

hrydgard commented Jul 5, 2022

Argh, I retract my whole report about clamping. It seems my controller just doesn't always send the 0.0 message, and I was imagining the cause, maybe I moved it differently with the fix, or something. Effectively, we need a deadzone here. I propose from the measurements that we rescale values with a ramp so that 0.15 -> 0.0, and 1.0 -> 1.0.

I can add that post-merge if you don't feel like it after me complaining about something that wasn't your fault at all :) Sorry!

@hrydgard
Copy link
Owner

hrydgard commented Jul 6, 2022

Thanks, works great now. It's a really cool feature!

@hrydgard hrydgard merged commit e1ce0e2 into hrydgard:master Jul 6, 2022
@unknownbrackets unknownbrackets deleted the analog-speed branch July 6, 2022 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
User Interface PPSSPP's own user interface / UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants