5
\$\begingroup\$

I am working on this project to learn Rust and a bit of Web Assembly. The rendering is done in JavaScript which I do not really care about so I will not post. I would like to receive some harsh criticism about my rust code and my design patterns.

I want to expand this simulation to hold many types of elements (acid, salt, smoke...) of different phases (liquid, solid, gas, immovable...). However, before I move forward, I wanted to get some insight so that I can fix some of the patterns before the project becomes bigger. The topic of having more elements and avoiding repeating code is really bothering me. A lot of "traits" will be shared across elements of similar kind. Also I did not use dyn traits because I thought that this would be a lot more performant, but I am open for feedback.

use wasm_bindgen::prelude::*;
use web_sys::js_sys::Math::random;
mod utils;

const WIDTH: u32 = 64;
const HEIGHT: u32 = 64;
const SPREAD_FACTOR: u32 = 3;

/// Javascript can only store C style enums memory buffer
#[wasm_bindgen]
#[repr(u8)]
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum CellType {
    Dead = 0,
    Sand = 1,
    Water = 2,
}

#[derive(PartialEq, Eq)]
enum Phase {
    Dead,
    Solid,
    Liquid,
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct Cell {
    id: CellType,
    energy: u32,
    has_been_updated: bool,
}

impl Cell {
    fn set_cell(&mut self, ct: CellType) {
        self.id = ct;
    }

    fn new(ct: CellType) -> Self {
        return Cell {
            id: ct,
            energy: 0,
            has_been_updated: false,
        };
    }

    fn phase(&self) -> Phase {
        let id_as_num = self.id as u8;
        if id_as_num == 0 {
            return Phase::Dead;
        }
        if id_as_num < 2 {
            return Phase::Solid;
        }

        return Phase::Liquid;
    }
}

#[wasm_bindgen]
pub struct Universe {
    width: u32,
    height: u32,
    cells: Vec<Cell>,
}


impl Universe {
    fn get_index(&self, row: u32, column: u32) -> usize {
        (row * self.width + column) as usize
    }

    fn is_empty_and_inbound(&self, row: u32, col: u32) -> Option<(u32, u32)> {
        if !(row < self.height && col < self.width) {
            return None; // This also works for -1 which gets converted to u32MAX
        }
        let idx = self.get_index(row, col);
        if self.cells[idx].id == CellType::Dead {
            return Some((row, col));
        }
        None
    }

    fn is_phase(&self, row: u32, col: u32, ele: Phase) -> Option<(u32, u32)> {
        if !(row < self.height && col < self.width) {
            return None; // This also works for -1 which gets converted to u32MAX
        }
        let idx = self.get_index(row, col);
        if self.cells[idx].phase() == ele {
            return Some((row, col));
        }
        None
    }


    fn find_valid_positions(&self, positions: Vec<(u32, u32)>) -> Vec<(u32, u32)> {
        positions
            .iter()
            .map(|x| self.is_empty_and_inbound(x.0, x.1))
            .take_while(|x| x.is_some())
            .flatten()
            .collect::<Vec<_>>()
    }

    fn find_valid_positions_for_solid(&self, positions: Vec<(u32, u32)>) -> Vec<(u32, u32)> {
        positions
            .iter()
            .map(|x| {
                self.is_empty_and_inbound(x.0, x.1)
                    .or(self.is_phase(x.0, x.1, Phase::Liquid))
            })
            .take_while(|x| x.is_some())
            .flatten()
            .collect::<Vec<_>>()
    }


    fn switch_cells(&mut self, old_idx: usize, new_idx: usize) {
        self.cells.swap(old_idx, new_idx)
    }


    fn update_sand(&mut self, row: u32, col: u32) {
        let idx = self.get_index(row, col);
        self.cells[idx].has_been_updated = true;
        let cell_energy = self.cells[idx].energy / 4;

        let downwards_positions: Vec<_> = (1..=cell_energy + 1).map(|i| (row + i, col)).collect();
        let left_positions = vec![(row + 1, col - 1)];
        let right_positions = vec![(row + 1, col + 1)];
        let side_positions = if random() > 0.5f64 {
            // cant use system dependant rand in wasm
            vec![left_positions, right_positions].concat()
        } else {
            vec![right_positions, left_positions].concat()
        };

        let empty_downwards_positions = self.find_valid_positions_for_solid(downwards_positions);
        let empty_side_positions = self.find_valid_positions_for_solid(side_positions);

        if let Some(down_pos) = empty_downwards_positions.last() {
            self.cells[idx].energy += 1; // When objects are falling they gain energy
            let new_idx = self.get_index(down_pos.0, down_pos.1);
            self.switch_cells(idx, new_idx);
        } else if let Some(side_pos) = empty_side_positions.last() {
            let new_idx = self.get_index(side_pos.0, side_pos.1);
            self.switch_cells(idx, new_idx);
        } else {
            self.cells[idx].energy = 0;
        }
    }

    fn update_water(&mut self, row: u32, col: u32) {
        let idx = self.get_index(row, col);
        self.cells[idx].has_been_updated = true;
        let cell_energy = self.cells[idx].energy;

        let downwards_positions: Vec<_> = (1..=cell_energy + 1).map(|i| (row + i, col)).collect();
        let left_down_positions = vec![(row + 1, col - 1)];
        let right_down_positions = vec![(row + 1, col + 1)];
        let left_positions: Vec<_> = (1..=SPREAD_FACTOR).map(|i| (row, col - i)).collect();
        let right_positions: Vec<_> = (1..=SPREAD_FACTOR).map(|i| (row, col + i)).collect();

        let side_down_positions = if random() > 0.5f64 {
            vec![left_down_positions, right_down_positions].concat()
        } else {
            vec![right_down_positions, left_down_positions].concat()
        };
        let side_positions = if random() > 0.5f64 {
            vec![left_positions, right_positions].concat()
        } else {
            vec![right_positions, left_positions].concat()
        };

        let empty_downwards_positions = self.find_valid_positions(downwards_positions);
        let empty_side_positions = self.find_valid_positions(side_positions);
        let empty_side_down_positions = self.find_valid_positions(side_down_positions);

        if let Some(down_pos) = empty_downwards_positions.last() {
            self.cells[idx].energy += 1; // When objects are falling they gain energy
            let new_idx = self.get_index(down_pos.0, down_pos.1);
            self.switch_cells(idx, new_idx);
        } else if let Some(side_down_pos) = empty_side_down_positions.last() {
            let new_idx = self.get_index(side_down_pos.0, side_down_pos.1);
            self.switch_cells(idx, new_idx);
        } else if let Some(side_pos) = empty_side_positions.last() {
            let new_idx = self.get_index(side_pos.0, side_pos.1);
            self.cells[idx].energy = 0;
            self.switch_cells(idx, new_idx);
        } else {
            self.cells[idx].energy = 0;
        }
    }
}

/// Public methods, exported to JavaScript.
#[wasm_bindgen]
impl Universe {
    pub fn tick(&mut self) {
        for row in (0..self.height).rev() {
            for col in (0..self.width).rev() {
                let idx = self.get_index(row, col);
                let cell = self.cells[idx];
                if cell.has_been_updated {
                    continue;
                }
                match cell.id {
                    CellType::Dead => (),
                    CellType::Sand => self.update_sand(row, col),
                    CellType::Water => self.update_water(row, col),
                }
            }
        }

        for row in 0..self.height {
            for col in 0..self.width {
                let idx = self.get_index(row, col);
                self.cells[idx].has_been_updated = false;
            }
        }
    }

    pub fn new() -> Universe {
        utils::set_panic_hook(); // If our code panics, we want informative error messages to appear in the developer console

        let width = WIDTH;
        let height = HEIGHT;

        let cells = (0..width * height)
            .map(|_i| Cell::new(CellType::Dead))
            .collect();

        Universe {
            width,
            height,
            cells,
        }
    }

    pub fn render_to_console(&self) -> String {
        self.to_string()
    }

    pub fn width(&self) -> u32 {
        self.width
    }

    pub fn height(&self) -> u32 {
        self.height
    }

    /// This method will be called by javascript to get the memory buffer of our cells
    pub fn cells(&self) -> *const CellType {
        self.cells
            .iter()
            .map(|&c| c.id)
            .collect::<Vec<CellType>>()
            .as_ptr()
    }

    pub fn set_width(&mut self, width: u32) {
        self.width = width;
        self.cells = (0..width * self.height)
            .map(|_i| Cell::new(CellType::Dead))
            .collect();
    }

    pub fn set_height(&mut self, height: u32) {
        self.height = height;
        self.cells = (0..self.width * height)
            .map(|_i| Cell::new(CellType::Dead))
            .collect();
    }

    pub fn set_cell(&mut self, row: u32, column: u32, ct: CellType) {
        // The out of bounds check is done in javascript
        let idx = self.get_index(row, column);
        self.cells[idx].set_cell(ct);
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ It's a good idea to show your main. \$\endgroup\$
    – ggorlen
    Apr 12 at 15:18
  • \$\begingroup\$ @ggorlen I don’t believe there is a main? This appears to be a library compiled to wasm, called from JavaScript. \$\endgroup\$
    – Davislor
    Apr 12 at 16:34
  • \$\begingroup\$ @Davislor Makes sense, although it'd be good to see that build step or JS code so it can be run, which makes refactoring much easier, helping ensure there are no regressions. \$\endgroup\$
    – ggorlen
    Apr 12 at 16:47

1 Answer 1

2
+200
\$\begingroup\$

deltas vs absolute

pub struct Cell {
    ...
    has_been_updated: bool,

That attribute seems less convenient than it could be.

We iterate through an unbounded number of Generations, and track whether this Cell was updated on the last generation. So we'll have to clear all these flags to zero when beginning the next one. Which produces a large number of dirty cache lines to be flushed.

Consider making it last_updated: u32, which records the most recent generation in which it changed. Now instead of testing the flag we can test whether last_updated >= gen, which is also a cheap operation.

The phase() code seems to still be evolving, so I won't even comment on those magic numbers.

I don't understand wandering between u32 and the (possibly 64-bit?) usize, but that's fine, I imagine there's some reason behind that.

In is_phase(), consider turning ele: Phase into phase: Phase.

parameterize

        let empty_side_positions = self.find_valid_positions_for_solid(side_positions);
        ...
        let empty_side_positions = self.find_valid_positions(side_positions);

You want the helper which finds valid positions to accept a phase or similar argument, so it behaves differently for water and for solids.

The "divide energy by 4" variant wasn't completely clear. Consider breaking up the "update" into several phases, each of which can have a helper. For example, you might first compute a vector sum of forces acting on each particle, and then compute what effect that has on the particle. The indirect call to a get_force() function could be different based on phase or cell type.

gravity

The "downwards" identifiers suggest a hardcode of some massive body located in the "down" direction relative to particles. I live in a gravity field, as do most of the people I know. Consider introducing additional E and B fields, for charged and magnetized particles. This would force you to be explicit about where the field generator is located, in addition to its strength. Consider allowing for more than one such field generator.

unit tests

Set up a uniform universe of dead cells plus a handful of interesting ones, and place a Test Cell nearby. Now show us how changing its cell type changes the system behavior.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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