tomassedovic / tcod-rs

Rust bindings for libtcod 1.6.3 (the Doryen library/roguelike toolkit)
Do What The F*ck You Want To Public License
229 stars 45 forks source link

make Color::new a const function #281

Closed ElectricCoffee closed 5 years ago

ElectricCoffee commented 5 years ago

Rust 2018 has a notion of "const functions", which are functions that can evaluate at compile-time if it makes sense to do so.

Proposal

This issue proposes changing

pub fn new(r: u8, g: u8, b: u8) -> Color {
    Color {
        r: r,
        g: g,
        b: b,
    }
}

to

pub const fn new(r: u8, g: u8, b: u8) -> Color {
    Color { r, g, b }
}

Motivation

The motivation is rather simple, really; it lets us write this:

const COLOR_DARK_WALL:    Color = Color { r: 0,   g: 0,   b: 100 };
const COLOR_LIGHT_WALL:   Color = Color { r: 130, g: 110, b: 50  };
const COLOR_DARK_GROUND:  Color = Color { r: 50,  g: 50,  b: 150 };
const COLOR_LIGHT_GROUND: Color = Color { r: 200, g: 180, b: 50  };

like this:

const COLOR_DARK_WALL:    Color = Color::new(0, 0, 100);
const COLOR_LIGHT_WALL:   Color = Color::new(130, 110, 50);
const COLOR_DARK_GROUND:  Color = Color::new(50, 50, 150);
const COLOR_LIGHT_GROUND: Color = Color::new(200, 180, 50);

Which (to me) is a bit ergonomic than having to use the full struct syntax.

Other similar functions could also be changed in similar ways to allow for compile-time resolution.

PS There's no reason to write r: r, g: g, b: b when the shorthand syntax just lets you insert variables with the same names as the fields directly. So I took the liberty to simplify that in the example

tomassedovic commented 5 years ago

Thanks, I agree!

I'm actually pretty sure this is available under Rust 2015 as well though I don't remember when they were introduced.

I'm absolutely in favour of doing this, but I don't expect to have a lot of time doing this myself. If you (or anyone else reading this) were interested in putting together a PR, I would be thrilled to review & merge it.

ElectricCoffee commented 5 years ago

I'll see what I can do :)

ElectricCoffee commented 5 years ago

PR made: https://github.com/tomassedovic/tcod-rs/pull/283

tomassedovic commented 5 years ago

The #283 PR is merged. Closing.