iolivia / rust-sokoban

Rust Sokoban book and code samples
https://sokoban.iolivia.me
MIT License
156 stars 29 forks source link

Space removal, typos and style fixes #28

Closed mysterycommand closed 4 years ago

mysterycommand commented 4 years ago

Hi Olivia! Thanks for letting me play along with this tutorial. It's looking really great. There are so few practical and yet beginner friendly ECS/project tutorials out there. I think this is really going to help a lot of people get started. Really great job.

This PR includes mostly trailing white space removals (my IDE removes trailing white space on save) and a couple of fixes to typos I noticed.

I also recommend using tree for printing directory listings. I think there was actually a typo in one of the listings that made it seem like main.rs belonged in/was being moved to resources. The --dirsfirst option is nice for sorting the output like an IDE would.

I added one note where code would run but with warnings that'd get fixed up later, and I expanded a couple code blocks to reference new use statements that were needed to make the code run, or to include nearby/related changes to an impl.


Two other general thoughts:

Maybe consider using Prettier on the markdown files? It applies some conventions in order to hopefully avoid quibbling about syntax/formatting (I don't think I'm alone in having my IDE format on save).

Maybe name the code folders after the chapter to which they're related (e.g. chapter 2, section 4's code is in code/rust-sokoban-07 … why not code/rust-sokoban-02-04)? This'd make for easy mapping from chapter to example in the case that I've cloned the repo and am running the book locally and mapping to the examples (getting the example's "index" from the #include blocks is fine, but I think it'd be cool to "just see" the relationship).


These two are more preference, formatting, repo organization, and not really related to the content itself … which I'll reiterate, is great!

I think I saw someone else ask for an expanded explainer on lifetimes, and I'd second that … it seems like maybe digging into why impl<'a> System<'a> for RenderingSystem<'a> { needs that third lifetime ref, but impl<'a> System<'a> for InputSystem { doesn't would be an interesting way of approaching that super confusing subject?

Other things that would be cool to learn about:

  1. Performance? Input seems laggy to me, but maybe it's just because I'm running the debug build?
  2. More complex graphics stuff?
  3. Procgen'd maps?
  4. Resolution independence, scaling, centering, etc?
  5. Sound!

Okay, I think that's it from me … thanks again, and great work, can't wait to see what's next!

iolivia commented 4 years ago

@mysterycommand thank you so much for your thorough review and for your awesome feedback and this PR! PR looks amazing 🚀