brson / rust-sdl

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

Implement video::get_video_info #53

Closed robn closed 11 years ago

robn commented 11 years ago

Also with needed conversion for SDL_PixelFormat and SDL_Palette to their Rust counterparts.

This is the first Rust code I've written, so please point out anywhere that would be better served by some core function or idiom I haven't seen yet. In particular I think the palette member of PixelFormat should perhaps be an Option. You let me know and I'll update things :)

AngryLawyer commented 11 years ago

Nice.

Definitely agree that pallette should be an Option if it's ever going to possibly have some sort of Null. Makes it much easier to keep track of it.

robn commented 11 years ago

Ok, redone with an Option. Nicer.

As you see, I've implemented it like this:

palette: match fmt.palette {
    None => ptr::null(),
    Some(_) => ptr::addr_of(&unwrap_palette(fmt.palette.get_ref()))
},

I wanted to do:

    Some(p) => ptr::addr_of(&unwrap_palette(&p))

But that errors for reasons that I don't fully understand and can't find answers to:

src/video.rs:227:22: 227:25 error: use of partially moved value: `fmt`
src/video.rs:227         BitsPerPixel: fmt.bpp,
                                       ^~~
src/video.rs:223:23: 223:34 note: field of `fmt` moved here because the field has type core::option::Option<video::Palette>, which is moved by default (use `copy` to override)
src/video.rs:223         palette: match fmt.palette {
                                        ^~~~~~~~~~~

So this will have to do.

pcwalton commented 11 years ago

You need to use ref p in the pattern instead of using p. The error message should be clearer here.