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);
}
}
1 Answer
Thoughts from playing it:
- 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.
- There doesn't seem to be a way to exit the game without killing the process.
- Maybe a scoreboard with the number of apples eaten / high score would be a nice touch?
Onto the code...
I almost always implement
Debug
on every type.Don't forget you can implement methods on "plain data" types like
Position
. I moved such code fromtick
toPosition::move_by
.I like that you've created
Position
to bundle together values, but it's strange to use something calledPosition
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.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 aVecDeque
instead.I like promoting comments like "we died" to functions using the comment as the name.
Do you wish to allow a freshly-dead snake to immediately eat an apple?
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.The random position having a
0
lower bound in X and a1
lower bound in Y is interesting; extracting that to a method provides a nice place to hang an explanatory comment.You can glob-import an enum in a small scope (like a function) to avoid writing out the prefix over and over.
You can return a value from a
match
; this allows you to avoid writingself.velocity =
in each match arm.It appears that
1
and2
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...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.
I'd probably implement
render
on theState
and break out smaller functions for each specific thing to render.Extract a
State::new
function instead of inlining it inmain
.Extract the sleep duration out of the loop -- it doesn't need to be re-computed on each iteration.
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);
}
}
-
\$\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\$ Jul 7, 2017 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\$ Jul 7, 2017 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\$ Jul 7, 2017 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\$ Jul 7, 2017 at 20:49