roguelike-tutorial
roguelike-tutorial copied to clipboard
Remove excess as_ref, as_mut and unwraps
There's a few places where we could just match on Some(ref mut something) instead of calling as_mut. And ideally, we should get rid of all unwraps.
Just a note about this: in part 8, in the heal function, I believe this is mistaken:
let if Some(mut fighter) = self.fighter is incorrect (or it doesn't work for me; just learning rust!).
I believe it should be, as above:
let if Some(ref mut fighter) = self.fighter
Ah, good catch, thanks!
So I planned to remove the Copy trait and do only transformations that compile properly -- that should let us catch any copies that may not be obvious (including this one).
All right, this commit should fix the healing issue:
https://github.com/tomassedovic/roguelike-tutorial/commit/8b4b1446fe4a954f04bb9a8a869f2dfe875b8cff
Thanks!
I can give this a look, while the tutorial is still mostly fresh in my mind. If I removed the Copy trait wherever it appears, it should become clear where this isn't working correctly? I guess I should read up on the copy trait (lots of learning to do!)
Yeah, so when you assign a value to another variable or say a function directly (i.e. not by a reference), it gets moved and you can't access it from the original place. You can see it nicely in this example:
https://play.rust-lang.org/?gist=6124763952dcfdf8e932d94b18fc4894&version=stable&backtrace=0
So for example the healing potion bug you've discovered earlier was because the Fighter struct (which stores the hp) is copy and by not using ref in the pattern, I was silently copying the value, modifying the copy and leaving the original unchanged. This is one of risks of Copy so one needs to think carefuly what they implement it for. I'm still personally trying to feel out when it makes sense and when it doesn't.
So yeah if you do dig into this, try to remove Copy from Fighter (that's where most/all of these as_ref and as_mut calls are anyway) and see what breaks.
One more thing that's relevant: if you're pattern-matching an Option<SomeStruct> you can replace Some(some_struct) = option.as_ref() with Some(ref some_struct) = option and Some(some_struct) = option.as_mut() with Some(ref mut some_struct) = option.
So for example this:
https://github.com/tomassedovic/roguelike-tutorial/blob/11e50b1939b101a0adffa4e4c6fef957a20f593f/src/bin/part-13-adventure-gear.rs#L240
could have been written as: if let Some(equipment) = self.equipment.as_mut() {
which I like less, hence this issue.