rsgbds icon indicating copy to clipboard operation
rsgbds copied to clipboard

Rewrite RGBFIX in Rust

Open Oneirical opened this issue 1 year ago • 4 comments

This is an introductory project for me as a way to familiarize myself with the port of a lower level C++ program into Rust. I have 4 months of Rust experience, please do not expect senior-level quality.

How it works

Flags and arguments are taken in using clap's derive feature, editing a CLIOptions struct with all relevant data. The filename is sent to process_filename, which reads the file and sends it to process_file.

Then, each if statement checks if a relevant flag/argument was provided by the user using "if let Some" syntax, and overwrites bytes in the .gb file if necessary.

Questions & Weird Things

  • [x] I'm still not 100% sure when clap automatically parses strings for me, or when I need to do something special, like how I needed to add the clap_num crate to parse hexadecimal. It would be worth testing every flag with a dummy value to see if everything works.
  • [x] I am not sure if RGBFIX is supposed to accept multiple files or just a single one. There is this weird check repeated a few times which checks if the input file is the same as the output file, which, logically, should always succeed since input/output are one and the same?
  • [x] There is this assert repeated twice: "static_assert(0x10000 * BANK_SIZE <= SSIZE_MAX, "Max input file size too large for OS");". I have no idea how it could ever fail, so is it worth keeping around? Static asserts are not obvious to do in Rust, as far as I know.
  • [x] C++ code abuses integer overflow a couple of times in the original RGBFIX. I tried to use wrapping_add/sub/mul to mirror this, but I'm particularly doubtful of my wrapping_mul implementation. The rest should be fine.
  • [ ] process_filename is weird. It checks if the filename is composed of nothing but a dash '-'? And if not, it passes the input file as both the input and output? Rust won't let me do that due to ownership rules, so I try to clone it with try_clone(), but this gives a new file descriptor to the clone, which makes them not truly equal. Super weird.
  • [x] I'm not fully certain if the code ACTUALLY edits the file or not. I imagined at first that a .gb file would crash my emulator before running it through RGBFIX, and become functional afterwards... But hello-world.gb from the tutorial works even without getting fixed. I wonder how I can actually debug and test my program if what it actually does is not obvious.

TODOs

  • [ ] Add more help text.
  • [x] Fix cargo.toml to properly have this coexist with RGBASM.
  • [x] If the cartridge type is a TPP1 subtype, the C++ code calls upon a convoluted "tryReadSlice" function with a switch statement that parses a string char-by-char. I have not fully understood yet how to to handle this in my Rust code.

Oneirical avatar Mar 04 '24 05:03 Oneirical

I'm not fully certain if the code ACTUALLY edits the file or not. I imagined at first that a .gb file would crash my emulator before running it through RGBFIX, and become functional afterwards... But hello-world.gb from the tutorial works even without getting fixed. I wonder how I can actually debug and test my program if what it actually does is not obvious.

This script should help:

head -c 336 < /dev/urandom > random.gb
cp random.gb fixed.gb
rgbfix $* fixed.gb
xxd random.gb > random.txt
xxd fixed.gb > fixed.txt
diff random.txt fixed.txt

rm -f random.gb fixed.gb random.txt fixed.txt

Invoke as:

sh script.sh <rgbfix flags>

evie-calico avatar Mar 09 '24 17:03 evie-calico

Thank you! This works perfectly and confirms to me that the file is indeed getting edited, and that the padding flag works. I will be using this for further testing.

Oneirical avatar Mar 09 '24 17:03 Oneirical

process_filename is weird.

I can explain how it works: RGBFIX has two ways of working,

  • The "traditional" one where it's passed filenames, and each of those files is modified in-place.
  • And the newer "pipe" one, where it's passed a single dash (standard Unix parlance for "what I really want is stdin/stdout [depending on context]") and it reads from stdin and writes to stdout.

This should be able to be emulated in some way. I'm considering something like

enum InOut {
    File(File),
    Stdio,
}

(...feel free to come up with a better name >_>) And then you can impl Read for InOut, impl Write for InOut, and use the appropriate APIs; or use a custom API if that's more ergonomic; you're in a better place to make that decision than me right now ^^

ISSOtm avatar Apr 18 '24 23:04 ISSOtm

Turns out the patharg crate does exactly that. I should replace dash_stdio.rs with it, too.

ISSOtm avatar Jul 05 '24 20:07 ISSOtm