pygame-community / pygame-ce

šŸšŸŽ® pygame - Community Edition is a FOSS Python library for multimedia applications (like games). Built on top of the excellent SDL library.
https://pyga.me
777 stars 123 forks source link

Not all controller indices are actually controllers (3469) #1718

Open GalacticEmperor1 opened 1 year ago

GalacticEmperor1 commented 1 year ago

Issue №3469 opened by alters-mit at 2022-09-29 15:37:29

This bug was found by a user of my software. I don't have a Ducky One 3 keyboard so I can't reproduce the bug myself.

Environment:

You can get some of this info from the text that pops up in the console when you run a pygame program.

Current behavior:

Unhandled exception when you try to initialize a joystick with a Ducky One 3 keyboard plugged in.

The user who reported the bug says:

i can work around it by temporarily removing permissions from the device with sudo chmod 000 /dev/input/event<weird joystick-y device>

Steps to reproduce:

  1. Have a Ducky One 3 keyboard on Linux
  2. Run this:
import pygame
import pygame._sdl2.controller

pygame._sdl2.controller.init()
if pygame._sdl2.controller.get_count() == 0:
    return
j = pygame._sdl2.controller.Controller(0)

Stack trace

  File "src_c\cython\pygame\_sdl2\controller.pyx", line 112, in pygame._sdl2.controller.Controller.__init__
pygame._sdl2.sdl2.error: Index is invalid or not a supported joystick.
[91700] Failed to execute script 'game' due to unhandled exception!
```<hr>

#  Comments

# # #  **[ankith26](https://github.com/ankith26)* commented at 2022-10-03 13:02:26*

Thanks for the bug report!

Hmm... since this seems to be a very specific bug, we need the help of the original reporter to be fixing this.

pygame uses the SDL library behind the scenes for most things, if this is an SDL bug (and it most likely is), it should be reported upstream. The original issue reporter should test with an SDL equivalent of the pygame MRE, and incase the issue does not reproduce, we can do further follow-ups in here
<hr>

# # #  **[zoldalma999](https://github.com/zoldalma999)* commented at 2022-10-04 17:18:46*

I see you are using the experimental _sdl2 stuff. Note that anything in there may be changed or removed without any (or within a short) notice. But it is also nice to see some early adopters.

While it is weird that a keyboard is recognised as a joystick, there is also an issue in your code. Although controllers use the joystick indexes for initialization, not all joysticks are controllers (such as wheels, or joysticks). This can be checked using the `controller.is_controller` function. And confusingly, `controller.get_count` returns the number of *joysticks*, not controllers (so is the same as `joystick.get_count`). Which means you need to filter trough all the indices yourself. To find the first controller available, you would do something like this:
```py
def get_first_controller():
    for i in range(pygame._sdl2.controller.get_count()):
        if pygame._sdl2.controller.is_controller(i):
            return pygame._sdl2.controller.Controller(i)

\ Maybe we should remove controller.get_count and replace it with a function like controller.get_all, which would return/yield all the controller compatible indicies?


*alters-mit commented at 2022-10-06 19:23:30*

@zoldalma999

there is also an issue in your code.

I know that _sdl is experimental but this "bug" could probably be solved by better documentation i.e. some example code of how to initialize a controller. It's not clear right now that is_controller(index) is basically mandatory unless you are certain that the controller is supported.


*zoldalma999 commented at 2022-10-09 11:56:52*

Yes, I am aware. As you said, the module is experimental, and there is a lot of improvements to be made. I plan(ned) to do more to make the module easier to get started with and use, such as an example in the examples subdirectory, a migration guide from the joystick module and improved documentation. The docs I wrote up (and is on the site) is a start, but there are still a few things to clarify/explain better.

Also, at the end of my comment, I said a possible way to make things easier. Do you think it could work? Or do you have any other suggestions to make this less confusing? Any feedback is appreciated


*Starbuck5 commented at 2022-10-12 23:46:18*

Let's do it, all controller numbers should be controllers. @zoldalma999 do you think that's workable? I don't have any experience with the controller subsystem so I'm going out on a limb here. Changed the title to represent this as a possible agenda for the issue.


*zoldalma999 commented at 2022-10-13 17:06:31*

Well, my suggestion was adding a function that lists all controller compatible joystick device ids. So if you have a ps4 controller, a wheel and an xbox controller connected (in this order), it would return (0, 2). I would also remove the get_count function, as that is misleading still (since you could not just iterate trough all the ids, like you can with joysticks). And of course document everything better.

Not sure about adding another id system, since there is already the "new" instance_ids to get used to (and those will be used most of the time anyway), and I think it would cause more confusion.

zoldalma999 commented 1 year ago

Seems like what I suggested here is also where SDL is heading. They added a function for getting all the compatible JoystickIDs (see here). Will implement this once the port gets merged