Snake Game design pattern review

code review
1

Hey,

I have spent 99% of my dev life using dynamic typed OO languages, and this is my first real attempt at a typed language.

I decided to implement the snake game as a pretty easy first project.

I have spent hours deliberating back and forth, pretty frustrated at some design decisions that I'm not happy with. I would love some insight into how to make it better.

Specifically the areas I don't like...

  1. the fact that an Apple::new needs to know about a Snake as well as a Board so it can generate a random point which is within the bounds of the game, and not on a snake. This method feels very wrong, but I've tried maybe a dozen different ways to do this and they all feel bad. This is my main question on how to improve this...
  2. Same is true for Snake::new... it has the same problem needing to know about Board. I cant decide if a Snake should know that stuff, or it should just be told a specific point and the Game handles that. If Game handles that then making a Game::generate_apple() also feels wrong...
  3. I thought about making Snake be Snake(Vec<Point>) instead of a struct, however then down below I have to call self.0[0] which feels wrong. If I make a method called head() then i also need a mutable reference to head....... so does that mean i should make head() and mut_head() or do it a different way?
use rand_derive2::RandGen;
use rand::Rng;

#[derive(Debug, PartialEq)]
struct Point {
    x: u32,
    y: u32,
}

#[derive(Debug, RandGen)]
pub enum Direction {
    N,
    E,
    S,
    W,
}

#[derive(Debug)]
struct Apple(Point);

impl Apple {
    pub fn new(board: &Board, snake: &Snake) -> Self {
        loop {
            let point = board.random_point();
            if !snake.tail.contains(&point) && point != snake.head {
                return Self(point);
            }
        }
    }
}

enum SnakeMove {
    AteApple,
    AteSelf,
    Move,
}

#[derive(Debug)]
struct Snake {
    head: Point,
    tail: Vec<Point>,
}

impl Snake {
    pub fn new(board: &Board) -> Self {
        Self {
            head: board.random_point(),
            tail: Vec::new(),
        }
    }

    // (0, 1)
    // [(0, 2), (0, 3), (0, 4), (0, 5)]
    fn move_snake(&mut self, board: &Board, apple: &Apple, direction: &Direction) -> SnakeMove {
        // move existing head into tail and replace with new head
        let new_head = board.next_point(&self.head, direction);
        let old_head = std::mem::replace(&mut self.head, new_head);
        self.tail.insert(0, old_head);

        let ate_apple = self.head == apple.0;
        if !ate_apple {
            self.tail.pop();
        }

        if self.tail.contains(&self.head) {
            SnakeMove::AteSelf
        } else if ate_apple {
            SnakeMove::AteApple
        } else {
            SnakeMove::Move
        }
    }
}

#[derive(Debug)]
struct Board {
    width: u32,
    height: u32,
}

impl Board {
    pub fn new(width: u32, height: u32) -> Self {
        Self { width, height }
    }

    fn random_point(&self) -> Point {
        Point {
            x: rand::thread_rng().gen_range(1..=self.width),
            y: rand::thread_rng().gen_range(1..=self.height),
        }
    }

    // 0 0 0 4
    // 0 0 3 0
    // 0 2 0 0
    // 1 0 0 0
    fn next_point(&self, point: &Point, direction: &Direction) -> Point {
        match direction {
            Direction::N => match point.y {
                y if y == self.height => Point { x: point.x, y: 1 },
                _ => Point {
                    x: point.x,
                    y: point.y + 1,
                },
            },
            Direction::E => match point.x {
                x if x == self.width => Point { x: 1, y: point.y },
                _ => Point {
                    x: point.x + 1,
                    y: point.y,
                },
            },
            Direction::S => match point.y {
                1 => Point {
                    x: point.x,
                    y: self.height,
                },
                _ => Point {
                    x: point.x,
                    y: point.y - 1,
                },
            },
            Direction::W => match point.x {
                1 => Point {
                    x: self.width,
                    y: point.y,
                },
                _ => Point {
                    x: point.x - 1,
                    y: point.y,
                },
            },
        }
    }
}

#[derive(PartialEq)]
pub enum GameStatus {
    Running,
    GameOver,
}

#[derive(Debug)]
pub struct Game {
    board: Board,
    snake: Snake,
    apple: Apple,
    direction: Direction,
}

impl Game {
    pub fn new(width: u32, height: u32) -> Self {
        let board = Board::new(width, height);
        let snake = Snake::new(&board);
        let apple = Apple::new(&board, &snake);
        let direction = Direction::generate_random();

        Self { board, snake, apple, direction }
    }

    pub fn change_direction(&mut self, direction: Direction) {
        self.direction = direction;
    }

    pub fn update(&mut self) -> GameStatus {
        let snake_move = self.snake.move_snake(&self.board, &self.apple, &self.direction);

        match snake_move {
            SnakeMove::AteSelf => GameStatus::GameOver,
            SnakeMove::AteApple => {
                self.apple = Apple::new(&self.board, &self.snake);
                GameStatus::Running
            }
            SnakeMove::Move => GameStatus::Running,
        }
    }
}

Here's how it's currently used. I haven't done any rendering logic yet:

#![allow(dead_code)]

use std::time::Duration;
use crate::engine::game::Game;

pub mod engine;

fn main() {
    let mut game = Game::new(100, 300);

    loop {
        println!("{:?}", game);

        let game_status = game.update();

        if game_status == engine::game::GameStatus::GameOver {
            break;
        }

        // randomly change direction
        if rand::random() {
            game.change_direction(engine::game::Direction::generate_random());
        }

        ::std::thread::sleep(Duration::new(0, 1_000_000_000u32 / 60));
    }
}
2

For #2, I recommend changing Snake::new to accept head as an argument. The game can get the random point from Board and pass it in.

Similar logic for #1, you can think of "what does an apple need to know?" without implicitly knowing the logic of the overall game. For example, what if you add multiple snakes in the future? Apple shouldn't have to know about multiple snakes, just all the places it can't spawn at.

Also, check if VecDeque gives you any advantage. I'm not sure it's well suited for this... since right now your setup with head/tail ensures the snake is not empty.

4 Likes
3

Neither of these seems all that bad to me, since the coupling to the other types is real. But if you don't like new having this coupling you can give it a different name or use a free function (a function at the module root), or an unbound method (a method that doesn't have a self param).

Syntax like self.0[0] is probably more common than you think, but there is an argument for using normal structs with field names when there is more than one field, simply so they have names and you don't have to remember which is 0 and which is 1.

With a tuple struct it is usually better to have methods for accessing the members rather than consumers of the struct using the .0 and .1 syntax. Making the consumer use .0 and .1 syntax is usually only done when the ordinal (0 and 1) has meaning in the domain.

1 Like
4

I would say it's the responsibility of the Game to pick a random free point, since it's the one that has all the information, and Apple is really just a thin wrapper (or "newtype") around a Point, or just the Point.

You have an edge case where the player "wins" as much as you can in Snake, where they eat the last apple in the last available square, and the new Apple loop will hang forever (it is also progressively worse performance as the board fills up, but not one likely to be an issue at typical Snake board sizes).

You can make Point derive Copy (which also requires Clone) so you don't need to use mem::replace and can just assign it like a primitive value. (Note that you should only derive Copy when you're sure that the value is "very dumb" in the sense that copying it will only ever be copying the direct bits of the members, so id types and points are good candidates, but anything you might want a String or Vec in at some point definitely isn't)

I would say Direction is a property of Snake, but that's super minor.

I expect from the MoveSnake result you initially started with passing in &mut Game and found you can't have two overlapping mutable references? That's a good solution and approach in general to avoid overlapping updates, but it's tricky to make generic for larger projects. Rust code has a habit of pushing up behavior to some "God object" that does everything with all mutable state available to it. There's no silver bullet here, unfortunately, but continuing on your current approach here leads to Effect systems, which are pretty neat: essentially everything returns a list of changes they'd like to see, separate both from figuring out what the changes are, and how to apply them. At the other end of the scale you have ECSs like bevy, where the entire game state is available to a sequence of "systems" (just functions, really) that apply updates essentially arbitrarily (there's a bunch of magic so you can keep these systems small and independent without huge performance impact).

Or you can just wrap RefCell around stuff so you can mutate things wherever you like, at the expense of it being real ugly and likely to yell at you :yum:

6 Likes
5

Wow this is really insightful... thank you. What's funny is your 2 refactors here:

  • responsibility of the Game to pick a random free point
  • I would say Direction is a property of Snake

I actually did those after I made this post, so I feel very good that matches what you said.

Your other points are also quite enlightening... especially about this "god object"

Very often when writing this, it became almost paralysis of decisions where I could visualize 30 different ways to do the thing... yet couldn't actually have the foresight to say which one was the right one, so it was just hours and hours... and hours of iterating over and over trying different ones.

Is there any general "rust design patterns" book or something? I dont want to overcomplicate it with a lot of complex design patterns, but major footguns patterns I should avoid would be nice.

Also... great point about the loop slowing down as the game fills up. I didn't think about that. What would be a good way to solve that if i have a 'sparse map' so to speak of coordinates with things IN them, when really I need to pick a point (in a non looping way) from the absense of those points, when the only information i have is a width/height, and those coordinates.

6

I especially like Jon Gjensets' YouTube channel and his book: "Rust for Rustaceans", but he is generally talking more about how to use the fine-grained functionality of Rust than larger scale design approaches (eg "how should you design a type" or "how should you use async", not "how should you decompose your application state") - I'd also like to find a great source for that, as Rust does push you away from broken designs somewhat, but not towards any nice designs (I'm currently trying to figure out how to clean up a project with a billion RefCells right now, in fact!)

For this specific case I'd say it's fine as the most ludicrous snake game with a 1000x1000 board would take forever to fill up and it would still only take on average a half million random picks (if I remember my stats correctly!), which is still perfectly reasonable for modern computers. But you can pick fairly in one step by picking an index in the number of available free spaces, then walk your board until you've seen that number of free spaces:

let mut index = random_in(0..free_spaces);
for x in 0..width {
  for y in 0.. height {
    if free(x, y) {
      if index == 0 {
        return index;
      }
      index -= 1;
    }
  }
}
2 Likes
7

My first instinct here would be to explicitly store the state of every cell on the board, and treat the game as a kind of cellular automaton:

#[derive(Copy,Clone)]
struct Point(usize, usize);

impl Point {
    fn step(self, dir:Direction)->Point { … }
}

#[derive(Copy,Clone)]
enum Direction { N, E, S, W }

#[derive(Copy,Clone,Default)]
enum Space {
    Empty,
    Apple,
    Head,
    Body ( Direction )
}

struct Board {
    spaces: Vec<Vec<Space>>,
    // Maybe other index data, as necessary
}

struct Snake {
    head: Point,
    tail: Point,
    current_len: u32,
    max_len: u32,
    dir: Direction
}

impl Snake {
    fn step(&mut self, board: &mut Board) {
        board[self.head] = Space::Body(self.dir);
        self.head = self.head.step(self.dir);
        match board[self.head] { … }
        self.current_len += 1;
        if self.current_len > self.max_len {
            let Space::Body(dir) = board[self.tail] else { unreachable!() }
            board[self.tail] = Space::Empty;
            self.tail = self.tail.step(dir);
        }
    }
}

struct Game {
    board: Board,
    snakes: Vec<Snake>,
    time:u32
}
3 Likes
8

Interesting! I also thought of that approach but i went with the "sparse representation" sort of method

These are definitely the things i went back and forth with forever in my mind, almost to a paralyzing degree

9

In case you're interested in turning your project into a terminal based application, I can recommend the ruscii crate. It is a terminal graphics engine and you can learn how to use it by going through the examples.

10

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.