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

SDL API renaming: *Is* functions #6932

Closed
slouken opened this issue Dec 29, 2022 · 8 comments · Fixed by #6933
Closed

SDL API renaming: *Is* functions #6932

slouken opened this issue Dec 29, 2022 · 8 comments · Fixed by #6933
Milestone

Comments

@slouken
Copy link
Collaborator

slouken commented Dec 29, 2022

I am in the process of adding SDL_SetEventEnabled() and SDL_EventEnabled() instead of SDL_IsEventEnabled(), and I'm reviewing the other *Is* functions:

SDL_IsAndroidTV
SDL_IsChromebook
SDL_IsDeXMode
SDL_IsGamepad
SDL_IsGamepadConnected => SDL_GamepadConnected
SDL_IsGamepadSensorEnabled => SDL_GamepadSensorEnabled 
SDL_IsJoystickConnected => SDL_JoystickConnected
SDL_IsJoystickVirtual
SDL_IsPointInRect => SDL_PointInRect 
SDL_IsPointInRectF => SDL_PointInRectF 
SDL_IsRectEmpty => SDL_RectEmpty 
SDL_IsRectEmptyF => SDL_RectEmptyF
SDL_IsRenderClipEnabled => SDL_RenderClipEnabled
SDL_IsRenderTargetSupported => SDL_RenderTargetSupported
SDL_IsScreenKeyboardShown => SDL_ScreenKeyboardShown
SDL_IsScreenSaverEnabled => SDL_ScreenSaverEnabled 
SDL_IsShapedWindow
SDL_IsTablet
SDL_IsTextInputActive => SDL_TextInputActive
SDL_IsTextInputShown => SDL_TextInputShown
SDL_JoystickIsHaptic
SDL_MouseIsHaptic

I'm thinking about removing "Is" from any function that reads smoothly without it. e.g. leaving SDL_IsGamepad(), but changing SDL_GamepadConnected()

Thoughts?

@libsdl-org/a-team for discussion

@slouken slouken added this to the 3.2.0 milestone Dec 29, 2022
@slouken
Copy link
Collaborator Author

slouken commented Dec 29, 2022

Or maybe I should have made the functions SDL_SetEventEnabled() and SDL_GetEventEnabled() and leave these alone?

@slouken
Copy link
Collaborator Author

slouken commented Dec 29, 2022

Although if (SDL_EventEnabled()) flows so much better...

@icculus
Copy link
Collaborator

icculus commented Dec 29, 2022

I assume the haptic ones weren't changed because all of haptic is getting serious scrutiny outside of renaming.

"IsTablet" uses "is" as a form of "to be" ...like, the actual question is of its nature. They can stay.

The rest is just a superfluous word in the question and it flows as better English with if (RectEmpty) than if (IsRectEmpty)

@slouken
Copy link
Collaborator Author

slouken commented Dec 29, 2022

Perfect, thanks!

@slouken
Copy link
Collaborator Author

slouken commented Dec 29, 2022

I updated the original post with the proposed changes.

@slouken
Copy link
Collaborator Author

slouken commented Dec 29, 2022

@icculus, these functions went back to their SDL2 names:

SDL_PointInRect
SDL_RectEmpty
SDL_RenderTargetSupported

slouken added a commit that referenced this issue Dec 29, 2022
Feedback from @icculus:
"IsTablet" uses "is" as a form of "to be" ...like, the actual question is of its nature.

The rest is just a superfluous word in the question and it flows as better English with if (RectEmpty) than if (IsRectEmpty)

Fixes #6932
@1vanK
Copy link
Contributor

1vanK commented Jan 12, 2023

This renaming conflicts with other renamings. You delete Is but same time add Get (SDL_GetJoystickName for example).

C have no properties like C# and using Is, Get, Set is common practice

@1vanK
Copy link
Contributor

1vanK commented Jan 12, 2023

Same time even C# have IsX() for boolean functions. Char.IsDigit() for example https://learn.microsoft.com/en-us/dotnet/api/system.char.isdigit?view=net-7.0

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

Successfully merging a pull request may close this issue.

3 participants