Index bounds check

[smcmurray]

get_index should still vet its arguments.

✋ A similar PR may already be submitted!
Please search 🔎 among the open pull requests before creating one.

✋ If there isn't a similar PR, is there an open issue associated
with what this PR is trying to solve?
If not, let's create one before opening a PR. ✨

Now that you've checked, it's time to create your PR. 📝
Thanks for submitting! 🙏

For more information, see the contributing guide. 👫

Summary

Explain the motivation for making this change. What existing problem does the pull request solve? 🤔

• 👯 This PR is not a duplicate
• 📬 This PR addresses an open issue
• ✅ This PR has passed CI

[smcmurray]

Added issue #99 for this, as directed.

[fitzgen]

Thanks for bringing this up @smcmurray

There is no unsafety here (not that you implied it, just noting for posterity), as indexing arrays is bounds checked in Rust.

Given that this is only for catching logic errors, I think it makes most sense to have debug assertions like this:

debug_assert!(row < self.height);
debug_assert!(column < self.width);

Can you send a PR to https://github.com/rustwasm/wasm_game_of_life too?

Thanks again @smcmurray

[SomeoneToIgnore]

I like the way it's implemented in the pull request (using % instead of debug_assert), allowing the javascript code to receive the index for any row and column pair specified without any panics.

Consider an excersize from https://rustwasm.github.io/book/game-of-life/interactivity.html#exercises where a pulsar should be added: if a user clicks close enough to the edge of the universe, the part of the figure to be added will appear on the other side of the universe (since it's periodic).
If we go with the % approach, then implementing this behavior is rather trivial – we simply add some offsets to the clicked point row and column and call get_index for those.
With debug_assert! approach we will be forced to write extra boilerplate to ensure that those offset values are in the universe range instead.

If you're fine with the approach from the PR, I can create a PR into the https://github.com/rustwasm/wasm_game_of_life repo.

[fitzgen]

If we go with the % approach, then implementing this behavior is rather trivial – we simply add some offsets to the clicked point row and column and call get_index for those.

This should be handled further up the stack, before we ever call Universe::get_index.

I can create a PR into the https://github.com/rustwasm/wasm_game_of_life repo.

That would be very appreciated!

[SomeoneToIgnore]

Makes sense, here is the PR: rustwasm/wasm_game_of_life#38