rsubtil / controller_icons

Automatic keyboard/mouse/controller icons and remapper for Godot.
https://godotengine.org/asset-library/asset/1429
MIT License
243 stars 18 forks source link

Godot 4.3-dev4: GCController change means different controller names on Mac #83

Closed plink-plonk-will closed 2 weeks ago

plink-plonk-will commented 6 months ago

Hi!

This is more a heads-up on an issue I found in Godot 4.3-dev4, and offer to fix where I can once this release is in stable.

I noticed that in Godot 4.3-dev4 the macOS backend for input changed to use GCController instead of the IOHIDDeviceRef. What this does is change the name of the controller being returned: eg. "PS5 Controller" now becomes "DualSense Wireless Controller" (on Mac only!). Once 4.3 is released as stable I'm more than happy to add the alternatives that I can if you like for this.

rsubtil commented 6 months ago

Thanks for the heads up, and yeah, feel free to change the detection system once those changes are on a stable release!

The addon tries to target the lowest v4 version it can (currently v4.1.2-stable), so when that change is widely deployed, it would be better to make it backwards-compatible with the current detection strings.

plink-plonk-will commented 6 months ago

Excellent - I will keep an eye on this then 😄

francoisdlt commented 3 weeks ago

@plink-plonk-will I would be delighted if you could start working on this 😇

plink-plonk-will commented 3 weeks ago

I'm not sure the best way to approach this. Simply adding in additional string checks for the controller id doesn't seem very robust: and I only have a few controllers I can check- so the implementation wouldn't be complete.

It is caused by the mentioned here: https://github.com/godotengine/godot/issues/95588#issuecomment-2293825513

On other platforms, I think the GUID for manufacturer and device is used to lookup the controller name in the SDL Controller Database. This is no longer done on Mac with the change above, and instead the value the system determines for the name is what is returned instead. I briefly looked to see if there was a way of getting the old GUID to do the SDL DB look up, but that doesn't seem available in the new framework being used. As it appeared to be non-trivial, I moved it to a "future problem" list :)

I'm not sure if this should be logged as a bug in Godot instead, or whether it would be dismissed as a platform incompatibility. I'll post about it in the Discord channel to see if I can get any feedback on that.

plink-plonk-will commented 3 weeks ago

While posting in the Godot Discord server, I found this which will be useful when implementing a fix in the engine (if that's deemed the correct approach)

https://stackoverflow.com/questions/33509296/supporting-both-gccontroller-and-iohiddeviceref

plink-plonk-will commented 3 weeks ago

I tried the above, and it doesn't seem to work. There's no reliable way of mapping GCController to IOHIDDevice. What you can do is monitor for IOHIDDevice connections, and then you can tell if GCController will handle it. From the IOHIDDevice you can pull out the vendor id and product id that's needed for the proper database lookup. This could potentially all be fine in setups with a single controller... but if you have multiple controllers connected I can't find a way of determining reliably which would relate to which.

I took a different approach, and it looks as SDL solved it in a slightly different way: they simple have a bunch of string compares on the vendor name that GCController returns, and map that to the original USB ids. This leads me to believe the accepted approach would be to just extend the checks that are already done, eg: if controller_name == "PS5 Controller" or controller_name.contains("DualSense"). It is horrible though.

francoisdlt commented 3 weeks ago

i'll take "horrible and working" anyday over "not working at all" 🤣

plink-plonk-will commented 3 weeks ago

I opened an issue here: https://github.com/godotengine/godot/issues/96756

Once I have clarity on where the problem should be solved, I'll look into fixing it and open a PR in the appropriate place :)

rsubtil commented 3 weeks ago

I agree with @francoisdlt that the current solution of using the reported controller name is, well, all that we have to go with for now. Any other detection method would very likely need some engine support to be feasible.

I already noticed a change that seems to expose vendor/product IDs on Linux, and put a pin on it at https://github.com/rsubtil/controller_icons/issues/66. Maybe this discussion would be better to have in there, to consolidate it on the same ticket.

For the current case, you can for now add another string to detect the PS5 controller on macOS. Checking for Dualsense seems a safe decision to do.

plink-plonk-will commented 2 weeks ago

I opened #113 to fix this for DUALSHOCK 4 and DualSense by checking for the different names until a decision on if this belongs in Godot or not is made.

rsubtil commented 2 weeks ago

Closed by #113.