Closed Tairesh closed 3 years ago
I'm definitely in favour of adding these! I just need to give some thought to the naming/API consistency - will try to give a proper review in the next day or two.
Okay, I finally got round to reviewing this, and made the following tweaks:
raise
to focus
, to make it clearer that calling it will also steal input focus. I also added a disclaimer about this to the docs, as it's quite a disruptive action to take without the user being aware it's going to happen.i32
instead of u32
- I'm not sure if this is correct, but it's consistent with the rest of the window
API. If this does turn out to be the wrong choice, we can always change everything to u32
in 0.7 or something like that.
int
s for these functions, it's the sdl2-rs
bindings that are adding validation on top, which I think is a bit overkill.sdl2::video::WindowPos
from the public API and replaced it with tetra::window::WindowPosition
, for two reasons:
sdl2
or glow
should never be exposed in Tetra's API - the idea behind the platform
module is to abstract over these crates so that they can be switched out later (e.g. if I wanted to port to consoles or the web).WindowPos
doesn't expose SDL2's functionality for centering a window on a second monitor - I was able to bring this back via some slightly hacky code.get_border_size
entirely, as I don't think that's something that other windowing libraries expose (it seems more common to have seperate functions for getting the client area and the outer size of the window), so it wouldn't be portable.I've merged this PR manually with my tweaks included (b2a1022), so closing this now :)
@Tairesh @17cupsofcoffee I needed a set of these functions as well, but I was hesitant to send a PR because I didn't think I should publish functions that could be highly dependent on SDL. Thanks for adding them.
@sumibi-yakitori: If you (or anyone else) are ever considering a PR but aren't sure if it'd get accepted, please open an issue or a discussion thread - I'm always happy to do a bit of research and let you know my thoughts 🙂
I'm usually happy to expose SDL2 functionality as long as there's a good use case for it and the API isn't miles away from what other windowing libraries have available - for example, I don't think all other windowing libraries have built-in 'center the window' functionality, but it'd not be too difficult to build it on top of their functions for getting the monitor size/setting the window position.
@17cupsofcoffee
OK, I'll do that next time! Do you have any plans to make tetra compatible with WASM? I don't really need it at the moment, but if you do, it will affect the design of APIs around Window.
@sumibi-yakitori: I would like to make Tetra WASM-compatible in the future (I actually have a prototype branch of it!), but desktop is probably going to remain the main focus, and I wouldn't be opposed to having stuff that's desktop-only as long as it's clearly documented.
I don't know if it is necessary for anyone else, but I needed some SDL functions like SDL_SetWindowPosition that Tetra currently doesn't provide, so I added some of them. I'm a very inexperienced Rust programmer so I may have just made some mess and nonsense, but if it would be useful I'd be glad to share this. Also sorry for my English.