search

chip8-rust / chip8-ui

CHIP-8 graphical emulator built with the Rust programing language
MIT License
8 stars 0 forks source link

Validate the bits of `Addr` and `Nibble` #19

Closed robo9k closed 9 years ago

robo9k commented 9 years ago

Intention of this pull request is to prevent invalid values for Addr and Nibble. 0x0FFF is the largest valid address, yet the closest matching type u16 can also hold e.g. 0xFFFF.

Validating the bits (or rather ignoring the high bits) up-front allows to ignore error-checks later on, i.e. in the Vm as well as preventing Instructions with invalid operands directly (such as AddToI(0xFF) which would now be represented as AddToI(Addr::new(0xFF)) == AddToI(Addr::new(0x0F))).

I'm not entirely happy with naming the wrapped value bits, open for suggestions! I've chosen to make the wrapped value pub as it prevents having to implement e.g. ::bits().

robo9k commented 9 years ago

Maybe the bits can remain private when implementing std::num::ToPrimitive / std::num::NumCast or the like. Then again it might not be worth the effort :watch:

Also, pub here might not do what I think it does, need to re-validate with docs..

jakerr commented 9 years ago

I don't really have a better name than bits and I think public is fine for now. I thought about approaching this with some kind of trait like Deref or something but I think it's a bit of an abuse of the intended meaning so gave up on it. The traits you mentioned look better but are both marked unstable with reason = "trait is likely to be removed" I say go with this for now and see if we find a better way.

robo9k commented 9 years ago

The AddToI() example I gave does not match this pull request, but reminds me that I need to do a follow up pull request to fix Vx and Vy as well. Thanks for merging, I didn't find the time so far. I'll delete the branch as well.