Skip to content

Commit

Permalink
This change did not make sense to be part of the web config modificat…
Browse files Browse the repository at this point in the history 
…ion.
  • Loading branch information
@arntsonl
arntsonl committed Jun 12, 2023
1 parent 9bb4bca commit ddf59c3
Showing 1 changed file with 1 addition and 4 deletions.
5 changes: 1 addition & 4 deletions src/gamepad.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,7 @@ void Gamepad::setup()
mapButtonR3 = new GamepadButtonMapping(convertPin(pinMappings.pinButtonR3), GAMEPAD_MASK_R3);
mapButtonA1 = new GamepadButtonMapping(convertPin(pinMappings.pinButtonA1), GAMEPAD_MASK_A1);
mapButtonA2 = new GamepadButtonMapping(convertPin(pinMappings.pinButtonA2), GAMEPAD_MASK_A2);

if(options.inputMode == INPUT_MODE_PS4 && options.switchTpShareForDs4)
f1Mask = GAMEPAD_MASK_A2 | GAMEPAD_MASK_S2;


uint16_t maskS1 = options.inputMode == INPUT_MODE_PS4
&& options.switchTpShareForDs4 ? GAMEPAD_MASK_A2 : GAMEPAD_MASK_S1;
mapButtonS1 = new GamepadButtonMapping(pinMappings.pinButtonS1, maskS1);
Expand Down

4 comments on commit ddf59c3

@teikai1216
Copy link
Contributor

@teikai1216 teikai1216 commented on ddf59c3 Jun 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If we don't make this change, the Hotkey F1 will display 'TouchPad+Options' when the Touchpad button and Share button are swapped on WebConfig. However, we still need to use the Share + Options combination to activate Hotkey F1. Making this change can ensure that the Hotkey F1 is consistent with the webconfig.

Another reason why I suggest this change is that regardless of whether the buttons are swapped or not, making this change will ensure that the Hotkey F1 uses PIN16 and PIN17 on the GP2040 to activate. This consistency applies to modes other than the PS4 mode.

Also, we can use this code after line 123 to instead. It is cleaner.

f1Mask = maskS1 | GAMEPAD_MASK_S2;

image

image

@bsstephan
Copy link
Contributor

Choose a reason for hiding this comment

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

I see now that what you are describing is true because of this code:

	uint16_t maskS1 = options.inputMode == INPUT_MODE_PS4
	               && options.switchTpShareForDs4 ? GAMEPAD_MASK_A2 : GAMEPAD_MASK_S1;
	mapButtonS1  = new GamepadButtonMapping(pinMappings.pinButtonS1,  maskS1);

	uint16_t maskA2 = options.inputMode == INPUT_MODE_PS4
	               && options.switchTpShareForDs4 ? GAMEPAD_MASK_S1 : GAMEPAD_MASK_A2;
	mapButtonA2  = new GamepadButtonMapping(pinMappings.pinButtonA2, maskA2);

I think this is also questionable, but now I understand why this is so confusing. IMO S1 should always be S1. I think we should move the switchTpShareForDs4 check into Gamepad::getPS4Report(), and then the feature works and S1 is always S1. (I feel this way because the hotkey rewrite is going to use the button masks directly and there's no great way to encapsulate this toggle there.)

@arntsonl
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I like this solution a lot, move our specialized switch over to the getPS4Report() so it doesn't impact anything else.

@bsstephan
Copy link
Contributor

Choose a reason for hiding this comment

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

Opened #329 for this.

Please sign in to comment.