Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in part 2b: index out of bounds: the len is 50 but the index is 50 #23

Open
ohmree opened this issue Oct 1, 2017 · 3 comments
Open

Comments

@ohmree
Copy link

ohmree commented Oct 1, 2017

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 ediff on 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:

~/Documents/projects/roguelike:no-branch*? λ RUST_BACKTRACE=1 cargo run --release
    Finished release [optimized] target(s) in 0.0 secs
     Running `target/release/roguelike`
24 bits font.
key color : 0 0 0
24bits greyscale font. converting to 32bits
Using SDL renderer...
thread 'main' panicked at 'index out of bounds: the len is 50 but the index is 50', /checkout/src/liballoc/vec.rs:1564:14
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:396
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:497
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:92
   9: core::panicking::panic_bounds_check
             at /checkout/src/libcore/panicking.rs:68
  10: roguelike::main
  11: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  12: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:458
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:59
  13: __libc_start_main
  14: _start

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 Map into 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.

@tomassedovic
Copy link
Owner

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 = false

I 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
Copy link
Author

ohmree commented Oct 3, 2017 via email

@tomassedovic
Copy link
Owner

No worries! If you're unclear about something, feel free to ask me here or on IRC (I'm shadower, you can find me in #rust and #rust-gamedev in Mozilla's server).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants