4
\$\begingroup\$
extern crate pancurses;
extern crate rand;

use rand::Rng;
use std::io::Write;

#[derive(Copy, Clone, Eq, PartialEq)]
struct Position(i32, i32);

struct State {
    player: Position,
    velocity: Position,
    size: Position,
    tail: Vec<Position>,
    length: usize,
    apple: Position,
}

impl State {
    fn tick(&mut self) {
        self.tail.push(self.player);
        if self.tail.len() > self.length {
            self.tail.remove(0);
        }

        self.player.0 = (self.player.0 + self.velocity.0 + self.size.0) % self.size.0;
        self.player.1 = (self.player.1 + self.velocity.1 + self.size.1) % self.size.1;

        if self.tail.iter().any(|&position| position == self.player) {
            // We died.
            self.length = 5;
            self.tail.clear();
        }

        if self.player == self.apple {
            self.length += 1;
            let mut rng = rand::thread_rng();
            self.apple = Position(rng.gen_range(0, self.size.0), rng.gen_range(1, self.size.1));
        }
    }

    fn input(&mut self, input: pancurses::Input) {
        match input {
            pancurses::Input::KeyUp => self.velocity = Position(0, -1),
            pancurses::Input::KeyDown => self.velocity = Position(0, 1),
            pancurses::Input::KeyLeft => self.velocity = Position(-1, 0),
            pancurses::Input::KeyRight => self.velocity = Position(1, 0),
            _ => {}
        }
    }
}

fn render(window: &pancurses::Window, state: &State) {
    window.attrset(pancurses::COLOR_PAIR(1));
    for position in &state.tail {
        window.mvaddch(position.1, position.0, '#');
    }
    window.mvaddch(state.player.1, state.player.0, '#');
    window.attrset(pancurses::COLOR_PAIR(2));
    window.mvaddch(state.apple.1, state.apple.0, '#');
}

fn main() {
    let window = pancurses::initscr();
    let result = std::panic::catch_unwind(|| {
        pancurses::start_color();
        pancurses::init_pair(1, pancurses::COLOR_RED, pancurses::COLOR_RED);
        pancurses::init_pair(2, pancurses::COLOR_GREEN, pancurses::COLOR_GREEN);
        pancurses::nl();
        pancurses::noecho();
        pancurses::curs_set(0);

        window.nodelay(true);
        window.keypad(true);

        let mut state = State {
            player: Position(10, 10),
            velocity: Position(0, 1),
            size: Position(window.get_max_x(), window.get_max_y()),
            tail: Vec::new(),
            length: 5,
            apple: Position(20, 20),
        };

        loop {
            window.clear();
            render(&window, &state);
            window.refresh();
            std::thread::sleep(std::time::Duration::from_millis(100));
            state.tick();
            if let Some(keypress) = window.getch() {
                state.input(keypress);
            }
        }
    });
    pancurses::endwin();

    if let Err(e) = result {
        if let Some(e) = e.downcast_ref::<&'static str>() {
            writeln!(&mut std::io::stderr(), "Error: {}", e).unwrap();
        } else {
            writeln!(&mut std::io::stderr(), "Unknown error: {:?}", e).unwrap();
        }
        std::process::exit(1);
    }
}
\$\endgroup\$
2
\$\begingroup\$

Thoughts from playing it:

  1. Because my terminal's characters are taller than wide, moving in an up-down direction feels faster than left-right. This makes it hard to anticipate when to turn.
  2. There doesn't seem to be a way to exit the game without killing the process.
  3. Maybe a scoreboard with the number of apples eaten / high score would be a nice touch?

Onto the code...

  1. I almost always implement Debug on every type.

  2. Don't forget you can implement methods on "plain data" types like Position. I moved such code from tick to Position::move_by.

  3. I like that you've created Position to bundle together values, but it's strange to use something called Position for values like velocity. This is how the Mars Climate Orbiter crashed. I'd encourage creating new types for each thing that shouldn't be treated the same, such as velocity and size.

  4. Removing from the head of the array feels inefficient, but I don't know what better data structure to use would be. You could use a VecDeque instead.

  5. I like promoting comments like "we died" to functions using the comment as the name.

  6. Do you wish to allow a freshly-dead snake to immediately eat an apple?

  7. Getting the thread RNG each time isn't the most efficient; I would normally provide it as a parameter to the State and reuse it.

  8. The random position having a 0 lower bound in X and a 1 lower bound in Y is interesting; extracting that to a method provides a nice place to hang an explanatory comment.

  9. You can glob-import an enum in a small scope (like a function) to avoid writing out the prefix over and over.

  10. You can return a value from a match; this allows you to avoid writing self.velocity = in each match arm.

  11. It appears that 1 and 2 are defining colors, and they are reused across functions. It'd be better to define a constant so that they stay in sync and are understandable. For example, your snake is red and the apple is green, which seems backwards to me...

  12. I see that ncurses requires different types for the two methods (sigh), which seems less than ideal... I'd probably use a type that is guaranteed to fit in both and perform casts at the call sites.

  13. I'd probably implement render on the State and break out smaller functions for each specific thing to render.

  14. Extract a State::new function instead of inlining it in main.

  15. Extract the sleep duration out of the loop -- it doesn't need to be re-computed on each iteration.

  16. The constructor assumes that the window is at least 20x20, but that may not be the case. If it's not, the apple is off the screen.

extern crate pancurses;
extern crate rand;

use rand::Rng;
use std::io::Write;

const SNAKE_COLOR_ID: u16 = 1;
const APPLE_COLOR_ID: u16 = 2;
const SNAKE_COLOR: i16 = pancurses::COLOR_RED;
const APPLE_COLOR: i16 = pancurses::COLOR_GREEN;

#[derive(Debug, Copy, Clone, Eq, PartialEq)]
struct Position(i32, i32);

impl Position {
    fn move_by(&self, velocity: Position, size: Position) -> Position {
        let x = (self.0 + velocity.0 + size.0) % size.0;
        let y = (self.1 + velocity.1 + size.1) % size.1;
        Position(x, y)
    }
}

#[derive(Debug)]
struct State {
    player: Position,
    velocity: Position,
    size: Position,
    tail: Vec<Position>,
    length: usize,
    apple: Position,
}

impl State {
    fn new(width: i32, height: i32) -> Self {
        State {
            player: Position(10, 10),
            velocity: Position(0, 1),
            size: Position(width, height),
            tail: Vec::new(),
            length: 5,
            apple: Position(20, 20),
        }
    }

    fn tick(&mut self) {
        self.tail.push(self.player);
        if self.tail.len() > self.length {
            self.tail.remove(0);
        }

        self.player = self.player.move_by(self.velocity, self.size);

        if self.is_dead() {
            self.reset();
        }

        if self.player == self.apple {
            self.length += 1;
            self.apple = self.random_position_on_board();
        }
    }

    fn is_dead(&self) -> bool {
        self.tail.iter().any(|&position| position == self.player)
    }

    fn reset(&mut self) {
        self.length = 5;
        self.tail.clear();
    }

    fn random_position_on_board(&self) -> Position {
        let mut rng = rand::thread_rng();
        Position(rng.gen_range(0, self.size.0), rng.gen_range(1, self.size.1))
    }

    fn input(&mut self, input: pancurses::Input) {
        use pancurses::Input::*;

        self.velocity = match input {
            KeyUp    => Position(0, -1),
            KeyDown  => Position(0, 1),
            KeyLeft  => Position(-1, 0),
            KeyRight => Position(1, 0),
            _ => return,
        }
    }

    fn render(&self, window: &pancurses::Window) {
        self.render_snake(window);
        self.render_apple(window);
    }

    fn render_snake(&self, window: &pancurses::Window) {
        window.attrset(pancurses::COLOR_PAIR(SNAKE_COLOR_ID as u32));

        for position in &self.tail {
            window.mvaddch(position.1, position.0, '#');
        }
        window.mvaddch(self.player.1, self.player.0, '#');
    }

    fn render_apple(&self, window: &pancurses::Window) {
        window.attrset(pancurses::COLOR_PAIR(APPLE_COLOR_ID as u32));
        window.mvaddch(self.apple.1, self.apple.0, '#');
    }
}

fn main() {
    let window = pancurses::initscr();
    let result = std::panic::catch_unwind(|| {
        pancurses::start_color();
        pancurses::init_pair(SNAKE_COLOR_ID as i16, SNAKE_COLOR, SNAKE_COLOR);
        pancurses::init_pair(APPLE_COLOR_ID as i16, APPLE_COLOR, APPLE_COLOR);
        pancurses::nl();
        pancurses::noecho();
        pancurses::curs_set(0);

        window.nodelay(true);
        window.keypad(true);

        let mut state = State::new(window.get_max_x(), window.get_max_y());

        let sleep_time = std::time::Duration::from_millis(100);

        loop {
            window.clear();
            state.render(&window);
            window.refresh();

            std::thread::sleep(sleep_time);

            state.tick();

            if let Some(keypress) = window.getch() {
                state.input(keypress);
            }
        }
    });
    pancurses::endwin();

    if let Err(e) = result {
        if let Some(e) = e.downcast_ref::<&'static str>() {
            writeln!(&mut std::io::stderr(), "Error: {}", e).unwrap();
        } else {
            writeln!(&mut std::io::stderr(), "Unknown error: {:?}", e).unwrap();
        }
        std::process::exit(1);
    }
}
\$\endgroup\$
  • \$\begingroup\$ What's the reason behind #15? It seems to me that we don't get any mileage in reusing such a small struct. \$\endgroup\$ – Winston Ewert Jul 7 '17 at 20:39
  • \$\begingroup\$ @WinstonEwert Extracting the sleep duration out of the loop does two things: 1 it highlights that the sleep amount does not (and can not) change between iterations. This is useful for humans. 2 it avoids doing any repeated work of converting the milliseconds to the real representation. \$\endgroup\$ – Shepmaster Jul 7 '17 at 20:45
  • \$\begingroup\$ @WinstonEwert AIUI, many games actually calculate how long it's been since the last frame and modify the sleep duration. This keeps the framerate more consistent, regardless of what work occurred between frames. In this case, you would need to recompute it inside the loop. \$\endgroup\$ – Shepmaster Jul 7 '17 at 20:46
  • \$\begingroup\$ In my view 1) this is actually less clear now. I now have to read two lines of code to see that the sleep doesn't change rather than one. 2) given that the function is [inline] I'm thinking the work will be optimized out anyway. But yes, a more serious game would require a better sleep strategy anyways. Anyways: thanks for the feedback! \$\endgroup\$ – Winston Ewert Jul 7 '17 at 20:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.