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

Port joystick and scrap to SDL3 #3208

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Port joystick and scrap to SDL3 #3208

wants to merge 2 commits into from

Conversation

ankith26
Copy link
Member

@ankith26 ankith26 commented Nov 2, 2024

This PR ports joystick and scrap to SDL3.

Porting scrap turned out to be surprisingly trivial, but it makes sense given that we are using like 3 functions of the SDL API, and the rest are custom implementations.
EDIT: There was some windows specific code that I initially missed when I made this comment, but that should hopefully be ported now as well.

@ankith26 ankith26 requested a review from a team as a code owner November 2, 2024 11:15
@ankith26 ankith26 force-pushed the ankith26-more-sdl3-3 branch from 2e19b25 to e6149e8 Compare November 2, 2024 12:27
@Starbuck5 Starbuck5 added the sdl3 label Nov 2, 2024
Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Starbuck5 Starbuck5 added joystick pygame.joystick scrap pygame.scrap labels Nov 3, 2024
@@ -200,18 +210,53 @@ joy_get_guid(PyObject *self, PyObject *_null)
guid = SDL_JoystickGetGUID(joy);
}
else {
#if SDL_VERSION_ATLEAST(3, 0, 0)
guid = SDL_GetJoystickGUIDForID(pgJoystick_AsID(self));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I didn't think about while making the PR: SDL_GetJoystickGUIDForID uses the instance ID and not the device ID returned by pgJoystick_AsID. This block needs more attention and thought

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general the joystick port will need some more thought. They removed device indices completely, meaning we need to rework the initialization of joystick objects in general. There is no easy way to emulate this either (and we shouldn't anyway!).

With that said, you can use SDL_GetJoystickID to get the instance_id of an already open joystick.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we may need to store something like initial_instance_id or last_known_instance_id on the pygame Joystick object.

at the minute it has these attributes:

jstick->id = id; // ID passed in by the user in range 0 to number of connected joysticks at time of creation - called 'device id'
jstick->joy = joy; // SDL Joystick Object
jstick->prev = NULL;
jstick->next = joylist_head;

It is a bit of an odd code path here, as you only go down it when the SDLJoystick object is invalid - which shouldn't happen, and I'm not sure why you'd want the GUID of an invalid SDL Joystick or if it would even produce any usable output. Are we testing this code path at all?

The instance id of an SDLJoystick changes to a unique number whenever a joystick is connected or disconnected. We are checking the instance ID in this case when we explicitly cannot get the SDL Joystick object - perhaps because it has been disconnected.

At the minute the joystick module has no support for 'hot plugging' or plugging & unplugging controllers and we probably shouldn't try and add it in as simple conversion PR - we also have the newer controller module for that sort of thing.

I would probably just store the initial instance id when we first create the joystick, like we do the device id, and then add another pg function to get it like we do the device ID. We could also update the last_known_instance_id in get_guid()

I notice there was also a lot of renaming in the joystick module:
libsdl-org/SDL#6881

I'm sure this has been covered somewhere, but are we intending to eventually rename our SDL function calls as well? Or just relying on the SDL 'old names' wrapper?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
joystick pygame.joystick scrap pygame.scrap sdl3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants