NES Emulator in Rust

Hi all,

I'm fairly new to Rust and am writing an NES Emulator. Would appreciate any feedback. This project is still a WIP.

Thanks!

A few suggestions:

  • Use std::thread::sleep instead of busy loop in NESEmulator/src/main.rs at 951a3de211f0384eaad796293f5ad0153eb7c936 · MWJones96/NESEmulator · GitHub

    • Also use std::time::{Duration, Instant} instead of SystemTime. The former are for monotonic timing, the latter is for wall-clock.
  • Use std::path::PathBuf instead of String when dealing with file paths: NESEmulator/src/util.rs at 951a3de211f0384eaad796293f5ad0153eb7c936 · MWJones96/NESEmulator · GitHub

    • In this specific case, I would recommend something like:
    pub fn read_bytes_from_file<P: AsRef<Path>>(file_path: P) -> Result<Vec<u8>> {
        let file_path = file_path.as_ref();
    
        // ...
    
        Ok(bytes)
    }
    
    • The P: AsRef<Path> generic parameter will allow the caller to pass anything that can be referenced as a Path, including other paths, UTF-8 strings, and strings with platform-defined encoding (e.g. OsString).

    • Panicking when human-supplied inputs are invalid is not a nice user experience. You don't yet have human-supplied paths for the NES ROM, but you probably will someday. Return a Result and show a nice error message for the Err cases:

      • "NES ROM file not found, path: `/foo/bar/game.nes`"
      • "File is not a valid NES ROM, path: `/foo/bar/game/nes`"
      • Etc. anyhow and thiserror can help you with your error handling.
  • SDL2 should only be necessary on Redox because minifb uses platform-specific window manager code on other platforms. You could also try softbuffer or pixels as an alternative to minifb

    • In full disclosure, I am the author of pixels.
  • There's one dead_code warning.

  • It would be worth adding some documentation comments (and rustdoc more generally). Especially in places where you are currently using plain old boring comments like this one: NESEmulator/src/cpu/addr/imp.rs at 951a3de211f0384eaad796293f5ad0153eb7c936 · MWJones96/NESEmulator · GitHub

  • The code seems decently tested, and the project structure is nice. There aren't any thousand-line source code files. Rustfmt is used. No obvious uses of unsafe. Nothing really stood out as a red flag in my quick review.

5 Likes

Hey, thanks for the feedback. The README is actually out of date, so I will change that too.

I was recommended minifb by ChatGPT, just as a simple proof-of-concept. I'm leaning towards WebAssembly for the final UI, but I want to make my NES code decoupled from the UI layer as best as possible. I'm still learning clean code and clean architecture principles.

pixels works in WASM browser environments (currently with WebGL2, but in the future it will use WebGPU). Here's the bare minimum example: pixels/examples/minimal-web at main · parasyte/pixels · GitHub

Your code is already decoupled from the presentation layer, because all of it is in main.rs. :+1: As it grows, you could put it into a second crate that is specific to UI stuff.

Gotcha! Thank again :slight_smile:

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.