tomassedovic
commented
7 years ago Hi!
First, a general debugging tip: if you run it with cargo run (without the --release option), it will be much slower, but the stack backtrace on panic will show the actual functions that were called instead of Rust's internals. You can do the same thing with --release if you put this in your Cargo.toml:
[profile.release]
debug = falseI think this line is the problem:
https://gist.github.com/omrisim210/3113be80c543bdd4725be588bc529756#file-main-rs-L112
Your MAP_WIDTH is set to 50 (the tutorial has it at 80), but accessing a Vec in Rust is 0-indexed, so by writing map[50], you're asking for the 51th element. The map is only 50 tiles wide, hence the panic. It's not a buffer overflow, Rust just does a bounds check to make sure you're not reading garbage memory.
Switching to a contiguous Vec would not solve this (it would just put the wall in a wrong position), but it a common (and generally more performant) way of handling maps like this, so I do encourage you to try it :-).
I don't see your comment as over-engineering: wrapping an internal representation (the contiguous Vec) in a struct with controlled access is perfectly normal. I do literally the same thing in my roguelike:
Here's my map struct:
https://github.com/tomassedovic/dose-response/blob/master/src/level.rs#L74-L78
And here's how to read and write to it:
https://github.com/tomassedovic/dose-response/blob/master/src/level.rs#L114-L122
Mine's more complicated because I have to translate between world and level coordinates (my world is infinite and each map is a small chunk of it), but the basic idea is the same: contiguous Vec + accessors.
If you want to get fancy, you can even implement the (std::ops::Index)[https://doc.rust-lang.org/std/ops/trait.Index.html] trait to get the same map[x][y] syntax :-).
Regarding getters and setters in Rust, you may have misunderstood a little. They are perfectly acceptable in cases where you want to hide the implementation or to check the inputs, etc. What is frowned upon is doing this where a public struct would be sufficient.
I.e. instead of:
pub struct Point {
x: i32,
y: i32,
}
impl Point {
pub fn new(x: i32, y: i32) -> Self {
Point{ x, y}
}
pub fn x(&self) -> i32 {
self.x
}
pub fn y(&self) -> i32 {
self.y
}
}(I believe this style of programming is common in some OOP languages/circles)
We generally prefer this:
pub struct Point {
pub x: i32,
pub y: i32,
}
ohmree
I'm getting an error when trying to compile. My code is basically part 2b, here's a gist of my code. I even ran
ediffon it and made sure that it's roughly the same as your version but in a slightly different order.Here's the error with a full backtrace:
It seems to be a buffer overflow of some sorts, but I don't really know how it's possible.
Did I do something wrong?
Unrelated:
I also tried to implement the continuous vector version of the map (thought that it may solve the issue, and also that it could be a good exercise), but that required me to either write C-style code full of "global" functions (which seemed kind of ugly) or turn
Mapinto a struct, which is overengineering things and also doesn't allow me to elegantly work with the map without getters and setters (they seem to be frowned upon in Rust). All in all this solution wasn't very elegant so I gave up on it.