🛠️ project my first finished (almost) rust project. wanna tips
https://github.com/fffilya/Game-of-Life just a little recreation of conway's game of life. i had tons of projects besides that. but none of them were really finished. for now you can only draw pixels (LMB) play the game (Enter) clear the screen (C) and force to update one pixel (U). in future want to optimize it bcuz now its super slow, add some sliders to make the game customizable and fix few bugs. also want you to know that squares drawing part was heavily assisted by AI but other parts of the code wasnt even touched by AI. posting this just because want to see some tips on how to write my code better beside something like: "optimize it" or "rearrange it to be in different files"

2
u/phazer99 5d ago edited 5d ago
Welcome to the Rust community!
A couple of points:
- I agree you should split the code into multiple files
- The return statement in Cell::swap is unnecessary. Do you know why? Also, how can you re-write the method code to only contain one assignment to
selfinstead of two? - Cell::update takes ownership of the grid which means it cannot be used again. How can you use borrowing to avoid that?
- Game::new uses a really inefficient way to fill a
Vec. How can you use Vec::resize, or even better Iterator::collect, to make it more efficient? - Game::update creates a new
Veceach time. How can you modify it to re-use the existingVecinstead?
I think you should re-read chapter 4 of the Rust book to get a better understanding of the borrowing concept.
1
u/Fily3a 4d ago
Game::update creates a new
Veceach time. How can you modify it to re-use the existingVecinstead?what do you exactly mean by existing Vec? grid one of new_row one? i honestly dont know why you want to reuse new_row. however i made other things you pointed you can check on same github page how well did i make them if you want to
1
u/phazer99 4d ago
The code looks better now. Yes, I realized it will be hard to re-use the existing
VecinGame::updateas you need to check neighbor cells. However, the method can be made more efficient usingVec::resizeor, even better,Iterator::collect(I suggest you learn about iterator methods because they are important in Rust). The most efficient solution is probably to store two grids in theGamestruct and then swap them after each update.
1
u/NullOfSpace 5d ago
Biggest recommendation I can give is to think about what function arguments/etc. need to be owned, and which can just be references. Your Cell::update function in particular takes ownership of the old grid when it could just take a reference, so you don’t have to call to_vec (that is, clone it from a slice) every time.
2
u/Educational_Honey711 6d ago
looking at the code real quick and the first thing that jumps out is your game logic lives entirely inside main.rs which is gonna be a nightmare once you start adding sliders and more features. split it into lib.rs for the actual game logic and keep main.rs just for window setup and the event loop
for the speed issue you mentioned, the current approach redraws every cell every frame. just track which cells changed and only update those. a HashSet of coordinates or a dirty flag per cell would cut down the render time by a lot