sdl.cr
sdl.cr copied to clipboard
Add 3 functions for getting the display mode
get_desktop_display_mode()get_current_display_mode()Window.display_index
(Maybe the first two functions should go in a new video file? SDL's wiki lists them under the Video category.)
Reworked the commit and MR to include Window.display_index, which is necessary to use the other two functions.
I made the changes that you requested. I was hoping that GitHub would update the diffs, but that doesn't to be the case.
What about dropping the get_ prefixes, so that we'd have SDL.desktop_display_mode and SDL.current_display_mode?
Personally I generally prefer that functions don't have names that would be typical of attributes. But it depends on the convention you're using for the project, so it's your call.
I'm wondering whether there could be a single function and a DisplayMode enum (Desktop, Current), so we'd call SDL.display_mode(:current, 0) and SDL.display_mode(:desktop, 0) for example, but that's probably overkill.
Personally I find that SDL2 is well designed overall for a C library and I prefer when bindings just adapt to the target language's idions. For example, in Rust I prefer to use the sys library that is just an unsafe wrapper around the C API, rather the full-fledged API which changes everything for no apparent reason.
I'm following Ruby/Crystal practice of avoiding the get/set/is/has prefixes when possible:
get_name->nameset_name->name=has_name->name?
For example window.cr is full of these.
If the goal is to make the method look like an attribute, then that seems in conflict with the fact that it takes a parameter (the display index).
I have added a commit with an alternative API. Typical usage would become SDL.desktop_display_modes[window.display_index]. One downside is that it makes the API slightly magical, which will confuse users if the abstraction leaks (e.g. in case of SDL error, users will have to realize it's not an actual array).
Update: added a size() method.
Now that sounds more complicated than necessary. I preferred the simple calls, just skipping the get_ prefix was enough. I like the addition of SDL_GetNumVideoDisplays. Just the following methods would be great; they're close enough to SDL:
SDL.desktop_display_mode(display_index: Int32) : SDL::DisplayMode
SDL.current_display_mode(display_index: Int32) : SDL::DisplayMode
SDL.num_video_displays() : Int32
SDL::Window#display_index() : Int32
Alternatively, for something that feels closer to Crystal, I'd introduce a struct SDL::Display with an @index attribute, and methods to access the display functions (e.g. #name, #bounds, #desktop_mode, #current_mode, ...). Add the following methods, and it would be very nice:
SDL::Window#display : SDL::Display
SDL.displays : Array(SDL::Display)
Sounds good, I removed the proposal commit and added a new one where I started working on the new API.
Can you give me feedback on the last commit?
Thanks for your patience!
Let's go with these :)