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

Deprecate Window foreign kwarg and Window.focus input_only kwarg #3234

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

ankith26
Copy link
Member

I was looking at compiling Window under SDL3, and seems like SDL_WINDOW_FOREIGN is now gone, with no mention or a replacement in SDL3. The easiest thing to do now seems to be undocumenting, and the code now raises a deprecation warning if the foreign kwarg is received.

@ankith26 ankith26 requested a review from a team as a code owner November 22, 2024 16:54
@ankith26 ankith26 changed the title Deprecate Window foreign kwarg Deprecate Window foreign kwarg and Window.focus input_only kwarg Nov 22, 2024
@ankith26
Copy link
Member Author

Also added another deprecation for the input_only kwarg which does not have an SDL3 equivalent.

Now the only think left to address in porting Window in my view, is fullscreen_desktop. I think SDL3 didn't technically remove it, but they seem to have changed it a lot and I may be stuck on how to handle this one

Copy link
Member

@damusss damusss left a comment

Choose a reason for hiding this comment

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

Makes sense, LGTM 👍

Copy link
Contributor

@yunline yunline left a comment

Choose a reason for hiding this comment

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

Accroding to this commit the SDL_WINDOW_FOREIGN is not removed. It's renamed to SDL_WINDOW_EXTERNAL.

However I approve to deprecate this kwarg.

In Window.__init__ we create a SDL window, However SDL_WINDOW_EXTERNAL is a flag for "window not created by SDL". Setting this kwarg doesn't make any sence.

Copy link
Contributor

@yunline yunline left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM I guess, external would be a clearer keyword anyway if we need it for some reason (integration with Tkinter maybe?)

@ankith26 ankith26 added this to the 2.5.3 milestone Dec 2, 2024
@ankith26 ankith26 merged commit 64b5320 into main Dec 2, 2024
25 checks passed
@ankith26 ankith26 deleted the ankith26-foreign-deprecate branch December 2, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants