Heroic-Games-Launcher / HeroicGamesLauncher

A games launcher for GOG, Amazon and Epic Games for Linux, Windows and macOS.
https://heroicgameslauncher.com
GNU General Public License v3.0
8.2k stars 430 forks source link

incorrect/outdated button mappings for dualsense controller #1584

Closed Etaash-mathamsetty closed 2 years ago

Etaash-mathamsetty commented 2 years ago

Describe the bug

I have a dualsense controller and the moment I press any trigger, heroic games launcher gets stuck scrolling up and there is no way to fix it other than restarting the app through the system tray

Add logs

(17:05:20) INFO:    [Legendary]:       Running command: /tmp/.mount_Heroic15ClAN/resources/app.asar.unpacked/build/bin/linux/legendary --version
(17:05:20) INFO:    [Legendary]:       Legendary location: /tmp/.mount_Heroic15ClAN/resources/app.asar.unpacked/build/bin/linux/legendary
(17:05:20) INFO:    [Gog]:             GOGDL location: /tmp/.mount_Heroic15ClAN/resources/app.asar.unpacked/build/bin/linux/gogdl
(17:05:20) INFO:    [Backend]:         

Heroic Version: 2.4.0-beta.2 Caesar Clown
Legendary Version:  0.20.27 Dark Energy (hotfix)
OS: Arch KERNEL: 5.18.12-arch1-1 ARCH: x64
CPU: AMD Ryzen 7 4700U with Radeon Graphics @2 GOVERNOR: schedutil
RAM: Total: 15 GiB Available: 7.42 GiB
GRAPHICS: GPU0: gfx90c:xnack- VRAM: 512MB DRIVER:  
PROTOCOL: wayland

(17:05:20) INFO:    [Gog]:             Getting data about the user
(17:05:21) WARNING: [Backend]:         Protocol already registered.
(17:05:22) INFO:    [Backend]:         AreWeAntiCheatYet data downloaded
(17:05:22) INFO:    [Gog]:             Saved user data to config
(17:05:22) INFO:    [Frontend]:        Refreshing Library
(17:05:22) INFO:    [Legendary]:       Refreshing library...
(17:05:22) INFO:    [Legendary]:       Refreshing Epic Games...
(17:05:22) INFO:    [Gog]:             Getting GOG library
(17:05:22) INFO:    [Legendary]:       Running command: /tmp/.mount_Heroic15ClAN/resources/app.asar.unpacked/build/bin/linux/legendary list
(17:05:22) INFO:    [Gog]:             Number of library pages: 1
(17:05:23) INFO:    [Gog]:             Saved games data
(17:05:23) INFO:    [Legendary]:       Updating game list
(17:05:23) INFO:    [Legendary]:       Game List Updated
(17:05:24) INFO:    [Legendary]:       Checking for game updates.
(17:05:24) INFO:    [Legendary]:       Running command: /tmp/.mount_Heroic15ClAN/resources/app.asar.unpacked/build/bin/linux/legendary list-installed --check-updates --tsv
(17:05:24) INFO:    [Backend]:         Downloaded Winetricks
(17:05:24) INFO:    [Legendary]:       Found 0 game(s) to update
(17:05:24) INFO:    [Gog]:             Found 0 game(s) to update

Steps to reproduce

  1. open heroic games launcher
  2. press any trigger

Expected behavior

right thumbstick should control scrolling not the triggers

Screenshots

No response

System Information

Additional information

I am pretty sure this is caused by outdated controller mappings or the usage of the wrong controller mapping

arielj commented 2 years ago

hmmm does your controller have drift? you can disable the navigation of heroic using the controller with this option in the settings image

can you share what you see when you open this website and use the controller? https://gamepad-tester.com

a screenshot and some comments about what you see there and what you are actually doing would help

Etaash-mathamsetty commented 2 years ago

hmmm does your controller have drift? you can disable the navigation of heroic using the controller with this option in the settings image

can you share what you see when you open this website and use the controller? https://gamepad-tester.com

a screenshot and some comments about what you see there and what you are actually doing would help

image

it's not drift, it's common across all apps that don't support this controller properly, the values for the right analog stick and triggers are swapped edit: i am correct, (i ran it through ds360) the right analog stick values and the trigger values are swapped aka incorrect/outdated button mapping

Etaash-mathamsetty commented 2 years ago

mappings appear to be present in chromium 102.0.5005.162 (not sure about earlier versions of chromium 102)

arielj commented 2 years ago

I'll check if I can change the mapping, the thing is we already have a mapping for the PS5 controller and this seems to have the same id but it's a different mapping so I don't know how I will differentiate them (I'll try to find the messages in discord when a user shared the ps5 controller mapping to compare)

flavioislima commented 2 years ago

I can confirm the issue. Maybe we can ignore these buttons or use them to navigate on the sidebar or filters.

Etaash-mathamsetty commented 2 years ago

I'll check if I can change the mapping, the thing is we already have a mapping for the PS5 controller and this seems to have the same id but it's a different mapping so I don't know how I will differentiate them (I'll try to find the messages in discord when a user shared the ps5 controller mapping to compare)

if you already have the mapping, that means that the mappings are set incorrectly, or it was the wrong mapping in the first place

arielj commented 2 years ago

this is the original layout that was shared on discord when in idle state image

I wonder if the problem is that chrome was detecting a buggy layout when we implemented this and some electron/chrome upgrade fixed it and now it's detected with the correct standard layout

the problem I see is that if both are correct I don't have a way to differentiate them since they both have the same name vendor and product values

Etaash-mathamsetty commented 2 years ago

this is the original layout that was shared on discord when in idle state image

I wonder if the problem is that chrome was detecting a buggy layout when we implemented this and some electron/chrome upgrade fixed it and now it's detected with the correct standard layout

the problem I see is that if both are correct I don't have a way to differentiate them since they both have the same name vendor and product values

both are not correct, there is only one button mapping for the dualsense and the one that is currently being used is wrong. Unless the person who you were referring to on discord says the controller works fine for them I don't think it is a chromium bug either, when I was using google chrome 102, the controller worked great with the correct mappings and whatnot

Etaash-mathamsetty commented 2 years ago

I upgraded to a more recent build of the beta branch and issue is still present (the one where they upgraded to electron 19.0.8), let me compile heroic with electron 20 and see what happens

arielj commented 2 years ago

this is the original layout that was shared on discord when in idle state image I wonder if the problem is that chrome was detecting a buggy layout when we implemented this and some electron/chrome upgrade fixed it and now it's detected with the correct standard layout the problem I see is that if both are correct I don't have a way to differentiate them since they both have the same name vendor and product values

both are not correct, there is only one button mapping for the dualsense and the one that is currently being used is wrong. Unless the person who you were referring to on discord says the controller works fine for them I don't think it is a chromium bug either, when I was using google chrome 102, the controller worked great with the correct mappings and whatnot

what I meant is that the handling of the PS5 controller was coded based on that shared layout, so the chromium browser used by electron at that time, at least for that user, was reporting that weird mapping and we used that without more information

maybe it was a chromium issue, or a drivers issue, or an electron issue, I asked them to check again if they still see the same weird layout of if they now see the correct standard layout

I want to have more information of that other case before changing that mapping, since I don't have a way to differentiate both of them to detect which one to apply if the identifier is the same

arielj commented 2 years ago

what I'm planning is to add a pick the layout of your controller selector in the settings, something like: autodetect / standard / nintendo switch / etc, that way we can allow users to try different layout instead of only using the autodetection

Etaash-mathamsetty commented 2 years ago

what I'm planning is to add a pick the layout of your controller selector in the settings, something like: autodetect / standard / nintendo switch / etc, that way we can allow users to try different layout instead of only using the autodetection

that makes more sense, but isin't chromium supposed to handle this automatically?

Etaash-mathamsetty commented 2 years ago

just tried electron 20.0.0-beta.9 still broken (it works fine in google chrome beta 104 tho)

arielj commented 2 years ago

what I'm planning is to add a pick the layout of your controller selector in the settings, something like: autodetect / standard / nintendo switch / etc, that way we can allow users to try different layout instead of only using the autodetection

that makes more sense, but isin't chromium supposed to handle this automatically?

chromium detects something, but since it was not always consistent (multiple controllers would report standard layout incorrectly) so we started mapping what chromium reported with what we actually wanted

so, in theory, chromium should detect the correct layout and differentiate between standard and non-standards, but seems like the feature is not that reliable to just thrust what it says

I added a few of the layouts that were reported when implemeting this, most of them where correctly reported as standard and behave as standard, but some didn't (that reported PS5 was one of them) https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/tree/main/src/helpers/gamepad_layouts

looks like now chromium is reporting that correctly, so the mapping that was there to patch the wrong layout is now breaking the correct layout

Etaash-mathamsetty commented 2 years ago

what I'm planning is to add a pick the layout of your controller selector in the settings, something like: autodetect / standard / nintendo switch / etc, that way we can allow users to try different layout instead of only using the autodetection

that makes more sense, but isin't chromium supposed to handle this automatically?

chromium detects something, but since it was not always consistent (multiple controllers would report standard layout incorrectly) so we started mapping what chromium reported with what we actually wanted

so, in theory, chromium should detect the correct layout and differentiate between standard and non-standards, but seems like the feature is not that reliable to just thrust what it says

I added a few of the layouts that were reported when implemeting this, most of them where correctly reported as standard and behave as standard, but some didn't (that reported PS5 was one of them) https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/tree/main/src/helpers/gamepad_layouts

looks like now chromium is reporting that correctly, so the mapping that was there to patch the wrong layout is now breaking the correct layout

yes that is correct, I just checked in the dev console the mappings are correct! (edit: I am going to try removing the check for where you set the mappings, since chromium is literally giving the correct button inputs but heroic is using them incorrectly)

Etaash-mathamsetty commented 2 years ago

looking at the source code, I can tell these mappings are from when electron didn't support this controller properly https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/blob/bf0e2c4be42491d79c5c1cead559f1e25b24f73e/src/helpers/gamepad_layouts/ps.ts#L49 either we can completely ditch this input mapping thing and let chromium updates handle them (probably the best solution) or just update the mapping for the dualsense one

arielj commented 2 years ago

looking at the source code, I can tell these mappings are from when electron didn't support this controller properly

https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/blob/bf0e2c4be42491d79c5c1cead559f1e25b24f73e/src/helpers/gamepad_layouts/ps.ts#L49

either we can completely ditch this input mapping thing and let chromium updates handle them or just update the mapping for the dualsense one

yes, I asked the person that reported that layout for more input, I don't want to change this without asking them or I would break the controller for that person if I just remove this

I want to find a solution that works for both cases if it happend to report the wrong layout sometimes (that's why I want to add the selector to have standard and non-standard options for that incorrect layout)

Etaash-mathamsetty commented 2 years ago

patch for ditching the input mapping thing patch.txt (it works perfectly with this patch)

I want to find a solution that works for both cases if it happend to report the wrong layout sometimes (that's why I want to add the selector to have standard and non-standard options for that incorrect layout)

this would require the user to manually add mappings for every single button, because I have doubts that future playstation controllers are going to follow an already existing layout (If they do, it's only one company's controllers, there are 100s of other manufacturers who all use the xbox 360 mapping). Controllers from other manufacturers are probably going to use the xbox 360 button mapping, and that's the default fallback for nearly every program. So I don't exactly see a need for this, when we can just update electron instead of going through all the hard work to implement such a feature.

Etaash-mathamsetty commented 2 years ago

yes, I asked the person that reported that layout for more input, I don't want to change this without asking them or I would break the controller for that person if I just remove this

If chromium supports it, it probably should work fine for everyone. But it's still a good idea to ask just in case.

arielj commented 2 years ago

patch for ditching the input mapping thing patch.txt (it works perfectly with this patch)

I want to find a solution that works for both cases if it happend to report the wrong layout sometimes (that's why I want to add the selector to have standard and non-standard options for that incorrect layout)

this would require the user to manually add mappings for every single button, because I have doubts that future playstation controllers are going to follow an already existing layout (If they do, it's only one company's controllers, there are 100s of other manufacturers who all use the xbox 360 mapping). Controllers from other manufacturers are probably going to use the xbox 360 button mapping, and that's the default fallback for nearly every program. So I don't exactly see a need for this, when we can just update electron instead of going through all the hard work to implement such a feature.

like I said, if the issue is fixed for the person reporting this original layout I think it's safe for us to remove the patch and fallback to standard and move on becauase it means something in electron/chromium did the trick, but if the issue keeps happening for that person it means it can still happen to more people, that's why I want to get to a solution that keeps everyone's controllers working

I agree that, ideally, all controllers should map to the standard xbox 360 mapping if they are reported as standard, but it was not our experience when developing this feature, that's why we had to add the specific mappings (though most controllers did follow the standard, many didn't)

I'm not 100% confident that an update in electorn fixes the layout for all the controllers, so if we change something it will be only for the ps5 which is what we are talking about here, I wouldn't remove the other controllers without input for users of those controllers since removing the mapping for all the controllers like in your patch may break that for them

Etaash-mathamsetty commented 2 years ago

patch for ditching the input mapping thing patch.txt (it works perfectly with this patch)

I want to find a solution that works for both cases if it happend to report the wrong layout sometimes (that's why I want to add the selector to have standard and non-standard options for that incorrect layout)

this would require the user to manually add mappings for every single button, because I have doubts that future playstation controllers are going to follow an already existing layout (If they do, it's only one company's controllers, there are 100s of other manufacturers who all use the xbox 360 mapping). Controllers from other manufacturers are probably going to use the xbox 360 button mapping, and that's the default fallback for nearly every program. So I don't exactly see a need for this, when we can just update electron instead of going through all the hard work to implement such a feature.

like I said, if the issue is fixed for the person reporting this original layout I think it's safe for us to remove the patch and fallback to standard and move on becauase it means something in electron/chromium did the trick, but if the issue keeps happening for that person it means it can still happen to more people, that's why I want to get to a solution that keeps everyone's controllers working

I agree that, ideally, all controllers should map to the standard xbox 360 mapping if they are reported as standard, but it was not our experience when developing this feature, that's why we had to add the specific mappings (though most controllers did follow the standard, many didn't)

I'm not 100% confident that an update in electorn fixes the layout for all the controllers, so if we change something it will be only for the ps5 which is what we are talking about here, I wouldn't remove the other controllers without input for users of those controllers since removing the mapping for all the controllers like in your patch may break that for them

yeah you are correct for example here

 const // CUp = buttons[0],
 // CRight = buttons[1],
 CDown = buttons[2],
 // CUp = buttons[3],
 // L = buttons[4],
 // R = buttons[5],
 A = buttons[6],
 Z = buttons[7],
 B = buttons[8],

are different then what would be used in my patch

Etaash-mathamsetty commented 2 years ago

updated patch: patch.txt

arielj commented 2 years ago

in your case, probably just falling back to the xbox360 layout works so maybe just changing this line to say checkXbox instead of checkPS5 might work https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/blob/main/src/helpers/gamepad.ts#L246

Etaash-mathamsetty commented 2 years ago

in your case, probably just falling back to the xbox360 layout works so maybe just changing this line to say checkXbox instead of checkPS5 might work https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/blob/main/src/helpers/gamepad.ts#L246

if the ps5 button mappings are already present I might as well update them