Port the controller module to C.

[zoldalma999]

While the port is meant to be a full replacement, there are some changes made to the module. But apart from the removed functions, there are not a lot of differences, and they are meant to make the module more like the joystick module. However they are/can be breaking changes.

Changes:

• removed controller.set_eventstate
• removed controller.get_eventstate
• removed controller.update
• removed controller.name_forindex
• Controller.get_axis returns in the range of -1 to 1 now
• Controllers that have the same id are the same object
• Keyword argument names might not be the same everywhere

removed controller.set_eventstate
removed controller.get_eventstate
removed controller.update

I removed the controller.get/set_eventstate functions, since there is no other place we expose something like this. If someone wants to block controller events, they can use pygame.event.set_blocked anyway. And controller.update is only needed if one disables events with set_eventstate, so that got removed too.

removed controller.name_forindex

controller.name_forindex is kind of an awkward function, since it uses device indicies as its parameter, which is inconsistent, and should not be used a lot. Even SDL removed it in SDL3 to replace it with a similar function, just with instance_ids.

Controller.get_axis returns in the range of -1 to 1 now

Controller.get_axis returning -1 to 1 values is a change that should have been made a while ago honestly. Nowhere else do we use the -32768 to 32767 range. Changed the CONTROLLERAXISMOTION event as well, while I was at it.

Controllers that have the same id are the same object

This is done mostly so that it matches up with the joystick module's behaviour.

Questions:

• What to do with the docs of the removed functions? There is no "removed in" tag, I don't think?
• What to do with attributes (name, id)? For now I did not document them, but did add them, but there was some discussion about it (also id should be removed and replaced with instance_id anyway).
• What to do with the old module? Should it be removed now or should we wait a bit with it?

Test script and testing:

Adapted from the joystick example, this should test out most of the functionality of the module. Things to check:

• Runs at all?
• All information is correct on the screen (ie button presses or axis values are correct)
• When pressing the A button (or cross on ps controllers), the B (or circle) button should be the one returning True.
• Pressing the B (/circle) button should start a light rumble on the controller
• Hotplugging works as it should

If something is broken, set NEW_CONTROLLER_MODULE to False to see if it is a regression or not. You can also run the controller tests, though not sure how good those are.

Test script

import sys
import pygame

USE_NEW_CONTROLLER_MODULE = True
if USE_NEW_CONTROLLER_MODULE:
    import pygame._sdl2.controller

    pygame.controller = pygame._sdl2.controller
else:
    import pygame._sdl2.controller_old

    pygame.controller = pygame._sdl2.controller_old

pygame.init()
pygame.controller.init()


button_name_pairs = {
    pygame.CONTROLLER_BUTTON_A: "A",
    pygame.CONTROLLER_BUTTON_B: "B",
    pygame.CONTROLLER_BUTTON_X: "X",
    pygame.CONTROLLER_BUTTON_Y: "Y",
    pygame.CONTROLLER_BUTTON_DPAD_UP: "dpad up",
    pygame.CONTROLLER_BUTTON_DPAD_DOWN: "dpad down",
    pygame.CONTROLLER_BUTTON_DPAD_LEFT: "dpad left",
    pygame.CONTROLLER_BUTTON_DPAD_RIGHT: "dpad right",
    pygame.CONTROLLER_BUTTON_LEFTSHOULDER: "left shoulder",
    pygame.CONTROLLER_BUTTON_RIGHTSHOULDER: "right shoulder",
    pygame.CONTROLLER_BUTTON_LEFTSTICK: "left stick",
    pygame.CONTROLLER_BUTTON_RIGHTSTICK: "right stick",
    pygame.CONTROLLER_BUTTON_BACK: "back",
    pygame.CONTROLLER_BUTTON_GUIDE: "guide",
    pygame.CONTROLLER_BUTTON_START: "start",
}


axis_name_pairs = {
    pygame.CONTROLLER_AXIS_LEFTX: "left x",
    pygame.CONTROLLER_AXIS_LEFTY: "left y",
    pygame.CONTROLLER_AXIS_RIGHTX: "right x",
    pygame.CONTROLLER_AXIS_RIGHTY: "right y",
    pygame.CONTROLLER_AXIS_TRIGGERLEFT: "trigger left",
    pygame.CONTROLLER_AXIS_TRIGGERRIGHT: "trigger right",
}


class TextPrint:
    def __init__(self):
        self.reset()
        self.counter = 0
        self.font = pygame.font.Font(None, 25)

    def tprint(self, screen, text):
        text_bitmap = self.font.render(text, True, (255, 255, 255))
        screen.blit(text_bitmap, (self.x, self.y))
        self.y += self.line_height
        self.counter += 1

    def reset(self):
        self.x = 10
        self.y = 10
        self.line_height = 22
        self.counter = 0

    def shift_right(self):
        self.x += 350
        self.y -= self.line_height * self.counter
        self.counter = 0

    def indent(self):
        self.x += 10

    def unindent(self):
        self.x -= 10


screen = pygame.display.set_mode((1280, 720))
clock = pygame.time.Clock()
controllers = {}
text_print = TextPrint()

done = False
while not done:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            done = True
        if event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE:
            done = True

        if event.type == pygame.CONTROLLERBUTTONDOWN:
            if event.button == pygame.CONTROLLER_BUTTON_A:
                controller = controllers[event.instance_id]
                if controller.rumble(0.2, 0.7, 500):
                    print(f"Rumble effect played on controller {event.instance_id}")

        if event.type == pygame.CONTROLLERDEVICEADDED:
            if not pygame.controller.is_controller(event.device_index):
                continue

            controller = pygame.controller.Controller(event.device_index)
            controllers[controller.as_joystick().get_instance_id()] = controller
            print(f"Controller {controller.as_joystick().get_instance_id()} connencted")
            mapping = controller.get_mapping()
            mapping["a"] = "b1"
            mapping["b"] = "b0"
            controller.set_mapping(mapping)

        if event.type == pygame.CONTROLLERDEVICEREMOVED:
            del controllers[event.instance_id]
            print(f"Controller {event.instance_id} disconnected")

    screen.fill((20, 20, 20))
    text_print.reset()

    joystick_count = pygame.controller.get_count()
    text_print.tprint(screen, f"Number of joysticks: {joystick_count}")
    text_print.indent()

    for controller in controllers.values():
        jid = controller.as_joystick().get_instance_id()

        text_print.tprint(screen, f"Controller {jid}")
        text_print.indent()
        text_print.tprint(screen, f"Controller name: {controller.name}")

        text_print.tprint(screen, "")
        text_print.tprint(screen, "Axes:")
        text_print.indent()
        for const, name in axis_name_pairs.items():
            text_print.tprint(screen, f"{name}: {controller.get_axis(const):.2f}")
        text_print.unindent()

        text_print.tprint(screen, "")
        text_print.tprint(screen, "Buttons:")
        text_print.indent()
        for const, name in button_name_pairs.items():
            text_print.tprint(screen, f"{name}: {controller.get_button(const)}")
        text_print.unindent()

        text_print.unindent()
        text_print.shift_right()
        text_print.tprint(screen, "Mapping:")
        text_print.indent()
        for name, value in controller.get_mapping().items():
            text_print.tprint(screen, f"{name}: {value}")
        text_print.unindent()

        text_print.shift_right()

    pygame.display.flip()
    clock.tick(30)

pygame.quit()

^{PS.: I wanted to (and should have) done this earlier. But late is better than never.}

[Starbuck5]

Unit tests failing because the ubuntu 20.04 build uses SDL 2.0.10, which doesn't have SDL_strtokr. Try using strtok_r instead, our compilers probably all support it?

Other fail is about the stubs of Controller __init__, not sure the best way to fix that one.

[Starbuck5]

Notes for self, review:

• Stuff non obviously changed should go in docs
• Keyword arguments? (docs vs source vs old source)

[MyreMylar]

LGTM. The testing code works well with my aged 360 controller - including hot plugging. Possibly we could refine the final design of the module/class a little more but the main thing is to get it converted to C so that process becomes easier to tackle.

I think the current proposed changes to the cython version all make sense - but I agree we should take a pass at the documentation before the module (pygame.controller I assume) reaches it's final form.

[ankith26]

While I don't have the hardware to test out these changes, I have gone through the code and found nothing questionable.

This change LGTM and should probably go in before the next dev release, so I'm approving. Thanks for the PR 🎉

[oddbookworm]

@zoldalma999 Once the merge conflicts are fixed, I'm willing to give this a final test with several controllers I own to make sure it works properly, and then merge it when all things look good. If you'd like, I can fix the merge conflicts and push to your branch

[Starbuck5]

Sorry to come up with this so late, but I’m not sure the “port to C PR” and the “change a couple APIs” PR should be the same PR. I’d be happier with this PR if it was a drop in replacement.

[zoldalma999]

@zoldalma999 Once the merge conflicts are fixed, I'm willing to give this a final test with several controllers I own to make sure it works properly, and then merge it when all things look good. If you'd like, I can fix the merge conflicts and push to your branch

I'd appreciate that. I'll message you whenever it is ready.

Sorry to come up with this so late, but I’m not sure the “port to C PR” and the “change a couple APIs” PR should be the same PR. I’d be happier with this PR if it was a drop in replacement.

The changes to the API are more like removes, apart from "Controller.get_axis returns in the range of -1 to 1 now". I can change it to be like the cython version, and add functions that will be removed in a later pr anyway, but it made sense for me to just not add those functions.

[Starbuck5]

The changes to the API are more like removes, apart from "Controller.get_axis returns in the range of -1 to 1 now". I can change it to be like the cython version, and add functions that will be removed in a later pr anyway, but it made sense for me to just not add those functions.

Thanks for pointing that out. I'd like it if you changed that to be like the Cython version, then that change can be evaluated independently.

[MyreMylar]

Looking at the fails I'm guessing this needs to make some modifications to the meson build now:

..\src_py\_sdl2\meson.build:2:17: ERROR: File controller.py does not exist.

[bilhox]

With my nintendo switch joycons, I get a segfault on Windows 11 when I reach this line in the test script :

# In pygame.CONTROLLERDEVICEADDED event check
mapping = controller.get_mapping()

EDIT : Traceback coming when I'll figure out which debugger can give the traceback I want on Windows.

Turned out this line (line 252) was the culprit :

        if (value[0] != '\0') {

Here you have to check if value[0] is not NULL.

[bilhox]

Overall this is a really good work ! 😄 🎉
An additional step for better controller support!

[bilhox]

Approving this since it looks pretty much stable enough. Good work ! 🎉

I also have a remark about something, since pygame.CONTROLLER_BUTTON|AXIS_<AXIS/BUTTON> is not something universal right now. This leads to unexpected behaviour and despite the fact we have set_mapping, I feel like we should have some kind of database that loads smart default mapping when the controller is initialized.
To support my idea, buttons in my nintendo switch joycons that are supposed to have a pygame constant are not detected by the test script. Furthermore with my SNES controller, if I press X, 2 buttons are shown activated.

Let me know if you like the idea (this would require a separate PR btw).

[MyreMylar]

OK, all looks good to me, on a retest. The example script works as I expect with rumble, hot-plugging and all the buttons on my controller working.

The tests also all pass. I think we just have to get this controller API out there and see it get wider user testing with different controller setups. I'm using an old X-box 360 controller on Windows, in cas that is useful ino.

[MyreMylar]

Merging this now.