brson / rust-sdl

SDL bindings for Rust
MIT License
179 stars 52 forks source link

Some of the functions can return Null, possibly #16

Closed AngryLawyer closed 11 years ago

AngryLawyer commented 11 years ago

So, while writing a tutorial for playing with Rust, I realised that I was manually handling a Null that I probably shouldn't be:

let surface = sdl::video::set_video_mode(640, 480, 31, ~[sdl::video::HWSurface], ~[sdl::video::DoubleBuf]);

if surface == ptr::null() {
    return result::Err(#fmt("Unable to create surface: %s", sdl::get_error()));
}

I think there are other SDL functions that might be able to spit out nulls. Nulls are pretty against the whole ethos of Rust, so should we be intercepting these in the SDL library, and instead return an option type/result type? I'm happy to tackle this if so. The only downside I could feasibly see is that it stops being as much of a direct mapping to SDL.

brson commented 11 years ago

My general strategy for creating bindings these days is to create a set of low-level bindings (usually in a ll) module that maps directly to the native API, then high level bindings that encapsulate the low-level bindings into safe Rust, often following Rust naming conventions. The exact strategy for high-level bindings tends to depend on the native library, but ideally the library is object ordiented and has a reasonable memory-management strategy. In that case structs + dtors + impls tend to work very well.

Adding high-level SDL bindings would be a fine thing.

FWIW, these bindings were initially created a long time ago and I recall them being sort of awkward. There may be better ways to organize them, and I encourage you to make any modifications that you need.

AngryLawyer commented 11 years ago

Sounds like a good way of tackling it. I'll take a peek at some of your other projects to make sure I'm going the right way, and I'll tackle this in the coming weeks.

brson commented 11 years ago

The servo bindings tend to reflect my current thinking about bindings: https://github.com/mozilla-servo/

The one I'm working on now is https://github.com/mozilla-servo/rust-netsurfcss