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

Adding Generic Trigger Effect to Joystick #4393

Closed
wants to merge 1 commit into from

Conversation

Electronicks
Copy link

Adds a generic trigger effect feature for joysticks and gamepads

Description

This PR builds upon the existing trigger rumble to generalize the effect that can be done on triggers.
The previous function function TriggerRumble still exists to rpeserve legacy, but the backend is completely replaced by the SetTriggerEffect nomenclature.
The effect is described as a struct to fill up by the caller and passing its pointer. The struct can then be expanded as necessary without changing the signature of the interface.
The different effects are identified in an exposed enum (such as rumble, constant or segment) in which the numeric value currently matches the Dualsense mode identifier (since it is the only controller handling more than a single effect).

Existing Issue(s)

Need to test on Xbox One controller, since I don't have one.
Need to find Dualsense rumbling values to match Xbox One Impulse Trigger Rumbling

I currently use these changes in JoyShockMapper

@sezero
Copy link
Contributor

sezero commented May 21, 2021

  • SDL_dynapi_overrides.h, SDL_dynapi_procs.h: Do not insert new procs,
    but append to the end. (Using the gendynapi.pl will do that.)

  • stdio.h shouldn't be needed in src/joystick/SDL_joystick.c

  • Many sysjoystick procs are updated wrongly for RumbleJoystickTriggers
    -> TriggerEffect signature change, e.g. android, bsd, darwin, dummy,
    emscripten, haiku, iphoneos, linux, os2, psp, vita. Fix for the os2
    driver is below, others should be like it:

diff --git a/src/joystick/os2/SDL_os2joystick.c b/src/joystick/os2/SDL_os2joystick.c
index 323c1d7..5e154c5 100644
--- a/src/joystick/os2/SDL_os2joystick.c
+++ b/src/joystick/os2/SDL_os2joystick.c
@@ -469,7 +469,7 @@ static int OS2_JoystickRumble(SDL_Joystick *joystick, Uint16 low_frequency_rumbl
        return SDL_Unsupported();
 }
 
-static int OS2_JoystickRumbleTriggers(SDL_Joystick *joystick, SDL_JoystickTriggerEffect left_effect, SDL_JoystickTriggerEffect right_effect)
+static int OS2_JoystickRumbleTriggers(SDL_Joystick *joystick, const SDL_JoystickTriggerEffect *left_effect, const SDL_JoystickTriggerEffect *right_effect)
 {
        return SDL_Unsupported();
 }
  • SDL_joystick.c emits the following warning:
    src/joystick/SDL_joystick.c:933: warning: suggest parentheses around '&&' within '||'
    Following patch should fix it:
index 1ce1618..809ada8 100644
--- a/src/joystick/SDL_joystick.c
+++ b/src/joystick/SDL_joystick.c
@@ -930,7 +930,7 @@ SDL_JoystickSetTriggerEffect(SDL_Joystick *joystick, const SDL_JoystickTriggerEf
     }
     /* else, just update the expiration because both effects are either nullptr or same as currrent */
 
-    if ((left_effect && left_effect->mode || right_effect && right_effect->mode) && duration_ms) {
+    if (((left_effect && left_effect->mode) || (right_effect && right_effect->mode)) && duration_ms) {
         // Some effect is active for some time
         joystick->trigger_effect_expiration = SDL_max(1, SDL_GetTicks() + SDL_min(duration_ms, SDL_MAX_RUMBLE_DURATION_MS));
     } else {
  • SDL_hidapi_xboxone.c needs c90 compatibility. Following patch should fix it:
diff --git a/src/joystick/hidapi/SDL_hidapi_xboxone.c b/src/joystick/hidapi/SDL_hidapi_xboxone.c
index 0faaae0..b616ad7 100644
--- a/src/joystick/hidapi/SDL_hidapi_xboxone.c
+++ b/src/joystick/hidapi/SDL_hidapi_xboxone.c
@@ -414,11 +414,11 @@ static int
 HIDAPI_DriverXboxOne_SetTriggerEffect(SDL_HIDAPI_Device *device, SDL_Joystick *joystick, const SDL_JoystickTriggerEffect *left_effect, const SDL_JoystickTriggerEffect *right_effect)
 {
     SDL_DriverXboxOne_Context *ctx = (SDL_DriverXboxOne_Context *)device->context;
+    int error_code = 0, result;
 
     if (!ctx->has_trigger_rumble) {
         return SDL_Unsupported();
     }
-    int error_code = 0;
     /* Magnitude is 1..100 so scale the 16-bit input here */
     if (left_effect) {
         if (left_effect->mode == SDL_JOYSTICK_TRIGGER_RUMBLE) {
@@ -446,7 +446,7 @@ HIDAPI_DriverXboxOne_SetTriggerEffect(SDL_HIDAPI_Device *device, SDL_Joystick *j
     }
     // else don't change right effect
     
-    int result = HIDAPI_DriverXboxOne_UpdateRumble(device);
+    result = HIDAPI_DriverXboxOne_UpdateRumble(device);
     return result != 0 ? result : error_code;
 }
 

There may be other issues, these are the ones I caught after a quick look.

Reuse the xbox one trigger rumble function to control the dualsense
adaptive triggers.
@slouken
Copy link
Collaborator

slouken commented Jul 8, 2021

As far as I know there is no way to emulate Xbox trigger rumble with the DualSense controller.

Given that this uses extremely specific and undocumented values for trigger effects and only works on the DualSense controller, I'd rather make an API that allows the application full control over the effects and is flexible enough to work with future controllers with different capabilities.

@slouken slouken closed this Jul 8, 2021
@slouken
Copy link
Collaborator

slouken commented Jul 8, 2021

FYI, the new API is in: d135c07

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

Successfully merging this pull request may close these issues.

None yet

3 participants