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

Fix gamepad L2/R2 in games #5623

Merged
merged 8 commits into from
Dec 9, 2024
Merged

Fix gamepad L2/R2 in games #5623

merged 8 commits into from
Dec 9, 2024

Conversation

rom1v
Copy link
Collaborator

@rom1v rom1v commented Dec 6, 2024

Fix #5362 (to be tested)

@Withoutruless
Copy link
Contributor

Withoutruless commented Dec 6, 2024

it works perfectly (on uhid), thank you.
I changed the vid:pid and the controller name to match an xbox 360 and thats it, no modifications required on android itself:

public final class UhidManager {
         ByteBuffer buf = ByteBuffer.allocate(280 + reportDesc.length).order(ByteOrder.nativeOrder());
         buf.putInt(UHID_CREATE2);
 
-        String actualName = name.isEmpty() ? "scrcpy" : "scrcpy: " + name;
+        int vid = 0;
+        int pid = 0;
+        String actualName;
+        if ("Xbox One Controller".equals(name)) {
+            actualName = "Microsoft X-Box 360 Pad";
+            vid = 0x045e;
+            pid = 0x028e;
+        } else {
+            actualName = name.isEmpty() ? "scrcpy" : "scrcpy: " + name;
+        }
         byte[] utf8Name = actualName.getBytes(StandardCharsets.UTF_8);
         int len = StringUtils.getUtf8TruncationIndex(utf8Name, 127);
         assert len <= 127;
@@ -183,8 +192,8 @@ public final class UhidManager {
 
         buf.putShort((short) reportDesc.length);
         buf.putShort(BUS_VIRTUAL);
-        buf.putInt(0); // vendor id
-        buf.putInt(0); // product id
+        buf.putInt(vid); // vendor id
+        buf.putInt(pid); // product id
         buf.putInt(0); // version
         buf.putInt(0); // country;
         buf.put(reportDesc);

aoa is gonna be broken as long as we cant change the vid:pid on it (or you could revert back to the old descriptor for aoa).

im doing the "if ("Xbox One Controller".equals(name)){" check so i dont affect other devices like a mouse or a keyboard, i dont know if it would work for people with other controllers (or other os).

but here's the modified server file with that check removed if anyone wants to try it (probably breaks the uhid keyboard):
scrcpy-server.zip

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 7, 2024

I changed the vid:pid and the controller name to match an xbox 360 and thats it

What if you keep 0:0, does it also work?

@Withoutruless
Copy link
Contributor

Withoutruless commented Dec 7, 2024

I changed the vid:pid and the controller name to match an xbox 360 and thats it

What if you keep 0:0, does it also work?

well, yes, but the rstick doesnt work and the triggers act as the rstick instead.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 7, 2024

well, yes, but the rstick doesnt work and the triggers act as the rstick instead.

Is that also true with "Game Controller Tester" (uk.co.powgames.gamecondiag)? On my devices, it works correctly in Gamepad testers with 0:0.

Can it be related to your changes in .kl files? (#5362 (comment))

@Withoutruless
Copy link
Contributor

well, yes, but the rstick doesnt work and the triggers act as the rstick instead.

Is that also true with "Game Controller Tester" (uk.co.powgames.gamecondiag)? On my devices, it works correctly in Gamepad testers with 0:0.

yup, its the same, which gamepad(s) did you try?

Can it be related to your changes in .kl files? (#5362 (comment))

nope, i reverted all of those changes, you dont need keylayout modifications to use xbox360 vid:pids with this update.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 7, 2024

yup, its the same, which gamepad(s) did you try?

With a PS4 controller, but that should not make any difference, since scrcpy uses the events received from SDL to generate UHID events.

The difference is probably the drivers on the device. On my devices, it works correctly with both 0:0 or 045e:028e (but not with the ids of the PS4 controller, because it is expected to behave differently I guess).

@Withoutruless
Copy link
Contributor

The difference is probably the drivers on the device. On my devices, it works correctly with both 0:0 or 045e:028e (but not with the ids of the PS4 controller, because it is expected to behave differently I guess).

how do games react to input? do they see your gamepad as an xbox or a ps one?
some ps4 keylayouts use generic identifiers so thats probably the issue you're having i reckon.
you can try the dualsense one: Vendor=0x054c, Product=0x0ce6
its axis identifiers are identical to xbox's, buttons might not work though.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 7, 2024

With 054c:0ce6, in Gamepad Tester, it is detected as a "PS5 Controller".
The right stick and L2/R2 axis are inverted (right stick changes L2/R2, and vice-versa).

With 0:0 or 045e:028e, it works correctly.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 7, 2024

I will add an option --gamepad-uhid-ids=xxxx:xxxx, and use 045e:028e by default.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 7, 2024

Or just force 045e:028e without option (setting another value would not change the HID reports, so it makes no sense to expose it).

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 7, 2024

I updated the PR.

Please test the latest version.

Here are binaries for Linux and Windows:

rom1v added 6 commits December 8, 2024 11:00
Use two separate callbacks for gamepad device added and gamepad device
removed.

It looks cleaner.

PR #5623 <#5623>
Use Z and Rz for L2/R2, which are more widely supported than
Brake/Accelerator.

The right stick must then be bound to Rx and Ry.

Fixes #5362 <#5362>
PR #5623 <#5623>
By default, initialize axis to 0, which is represented by 0x8000 as a
16-bit unsigned value.

PR #5623 <#5623>
Let the client choose the USB ids, that it transmits in UHID_CREATE
requests.

PR #5623 <#5623>
Use the vendorId and productId of an Xbox 360 controller for better
support (the HID gamepad protocol used in scrcpy is similar to that of
the Xbox 360 controller).

Fixes #5362 <#5362>
PR #5623 <#5623>
@rom1v
Copy link
Collaborator Author

rom1v commented Dec 8, 2024

@LHLaurini @yume-chan I'm also interested in your feedback if you have some time, since you worked on gamepads in the past 😉 Thank you.

@Withoutruless
Copy link
Contributor

I updated the PR.

Please test the latest version.

Here are binaries for Linux and Windows:

it works perfectly, but i think we should change the controller name as well, since some games seem to pay attention to that:

diff --git a/app/src/uhid/gamepad_uhid.c b/app/src/uhid/gamepad_uhid.c
index 2a063af5..b9016b39 100644
--- a/app/src/uhid/gamepad_uhid.c
+++ b/app/src/uhid/gamepad_uhid.c
@@ -36,7 +36,7 @@ sc_gamepad_uhid_send_open(struct sc_gamepad_uhid *gamepad,
     msg.uhid_create.id = hid_open->hid_id;
     msg.uhid_create.vendor_id = SC_GAMEPAD_UHID_VENDOR_ID;
     msg.uhid_create.product_id = SC_GAMEPAD_UHID_PRODUCT_ID;
-    msg.uhid_create.name = hid_open->name;
+    msg.uhid_create.name = "Microsoft X-Box 360 Pad";
     msg.uhid_create.report_desc = hid_open->report_desc;
     msg.uhid_create.report_desc_size = hid_open->report_desc_size;
 
diff --git a/server/src/main/java/com/genymobile/scrcpy/control/UhidManager.java b/server/src/main/java/com/genymobile/scrcpy/control/UhidManager.java
index 1d7678ec..c4867a3f 100644
--- a/server/src/main/java/com/genymobile/scrcpy/control/UhidManager.java
+++ b/server/src/main/java/com/genymobile/scrcpy/control/UhidManager.java
@@ -174,7 +174,7 @@ public final class UhidManager {
         ByteBuffer buf = ByteBuffer.allocate(280 + reportDesc.length).order(ByteOrder.nativeOrder());
         buf.putInt(UHID_CREATE2);
 
-        String actualName = name.isEmpty() ? "scrcpy" : "scrcpy: " + name;
+        String actualName = name.isEmpty() ? "scrcpy" : name;
         byte[] utf8Name = actualName.getBytes(StandardCharsets.UTF_8);
         int len = StringUtils.getUtf8TruncationIndex(utf8Name, 127);
         assert len <= 127;

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 8, 2024

i think we should change the controller name as well, since some games seem to pay attention to that

Did you observe a difference in behavior with just a different name?

If possible, I'd like to keep exposing the real controller name, but if it causes issues, ok.

@LHLaurini
Copy link
Contributor

LHLaurini commented Dec 8, 2024

Or just force 045e:028e without option (setting another value would not change the HID reports, so it makes no sense to expose it).

I believe having that option would only be useful for games and controllers that support stuff like gyro, adaptive triggers and haptic feedback. While I did find some references online saying some games support these features, I didn't find which games support them. In any case, I don't think scrcpy currently supports these features anyway, so it doesn't matter for now.

@Withoutruless
Copy link
Contributor

i think we should change the controller name as well, since some games seem to pay attention to that

Did you observe a difference in behavior with just a different name?

yes, in human fall flat (#5362), the game didnt recognize the controller cause of its name, changing the name fixed the issue.

Some games do not work without a known gamepad name.

Fixes #5362 <#5362>
Refs #5623 comment <#5623 (comment)>
PR #5623 <#5623>

Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator Author

rom1v commented Dec 8, 2024

@Withoutruless Thank you 👍 I added a commit under your authorship: 7418fd0.

(and I added a log when a gamepad is added or removed: 548d2c7)

Please confirm that everything works.

Add a log when a gamepad is added or removed.

PR #5623 <#5623>
@rom1v
Copy link
Collaborator Author

rom1v commented Dec 8, 2024

(scrcpy 3.1 including this PR is ready. I will publish it soon 🚀)

@rom1v rom1v merged commit 328bb74 into dev Dec 9, 2024
HighlanderDE pushed a commit to HighlanderDE/Highlanderfirstrep that referenced this pull request Dec 9, 2024
Use two separate callbacks for gamepad device added and gamepad device
removed.

It looks cleaner.

PR #5623 <Genymobile/scrcpy#5623>
HighlanderDE pushed a commit to HighlanderDE/Highlanderfirstrep that referenced this pull request Dec 9, 2024
HighlanderDE pushed a commit to HighlanderDE/Highlanderfirstrep that referenced this pull request Dec 9, 2024
Use Z and Rz for L2/R2, which are more widely supported than
Brake/Accelerator.

The right stick must then be bound to Rx and Ry.

Fixes #5362 <Genymobile/scrcpy#5362>
PR #5623 <Genymobile/scrcpy#5623>
HighlanderDE pushed a commit to HighlanderDE/Highlanderfirstrep that referenced this pull request Dec 9, 2024
By default, initialize axis to 0, which is represented by 0x8000 as a
16-bit unsigned value.

PR #5623 <Genymobile/scrcpy#5623>
HighlanderDE pushed a commit to HighlanderDE/Highlanderfirstrep that referenced this pull request Dec 9, 2024
Let the client choose the USB ids, that it transmits in UHID_CREATE
requests.

PR #5623 <Genymobile/scrcpy#5623>
HighlanderDE pushed a commit to HighlanderDE/Highlanderfirstrep that referenced this pull request Dec 9, 2024
Use the vendorId and productId of an Xbox 360 controller for better
support (the HID gamepad protocol used in scrcpy is similar to that of
the Xbox 360 controller).

Fixes #5362 <Genymobile/scrcpy#5362>
PR #5623 <Genymobile/scrcpy#5623>
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.

3 participants