sofiak-hel / minesweeper Goto Github PK

Koodi haettu 27.4. klo 14.30.

En saa valmiiksi buildattuja paketteja toimimaan koneellani (Ubuntu Linux -fuksiläppäri). Virheviestinä tulee

./miinaharava: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found (required by ./miinaharava)
./miinaharava: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.33' not found (required by ./miinaharava)
./miinaharava: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.32' not found (required by ./miinaharava)
./miinaharava: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found (required by ./miinaharava)
./miinaharava: /lib/x86_64-linux-gnu/libm.so.6: version `GLIBC_2.29' not found (required by ./miinaharava)

En saa ongelmaa runsaassa tunnissa ratkaistua googlen avulla joten luovutin ja rakensin manuaalisesti. Rust ei ole minulle entuudestaan tuttu, mutta Readme:n ohjeilla ohjelma käynnistyy ja toimii kuten pitääkin. Ulkoasu on siisti ja peliominaisuudet ilahduttavan samanlaiset kuin alkuperäisessä klassikkopelissä. AI-pelin nopeudensäätö on mainio ominaisuus.

Koska kieli ei ole minulle tuttu, niin kovin syvällistä koodin kommentointia en osaa tehdä. Jako luokkiin ja metodeihin vaikuttaa kuitenkin johdonmukaiselta. Koodi näyttää siisltiltä ja hyvin kommentoidulta eivätkä luokat tai metodit ole liian pitkiä.

Dokumentaatio on huolella tehtyä ja kattavaa.

Project cloned and tested Apr 30 15.00

A little disclaimer about my review: I have no experience in game dev at all, so any conventions or best practices that may come from that world are unkown to me. That being said, here's some feedback:

• Code is very well commented throughout the project. This really helps with reviewing the code.
• As another reviewer already mentioned, I think it would make sense to move some stuff from minesweeper-ai/src/main.rs into separate files. Perhaps the stats (StateStats and GuessStats) could be moved into one file and VisualState into another.
• I was also wondering about the purpose of splitting the project into two different apps: the AI and the non-AI version. Personally, I would just make one app that woud be executed in either headless or gui mode (depending on a cl-flag), and then give the option to play it either in ai or PvP mode. But, if there's some specific reason for this type of implementation, then never mind.
• Test organization might be slighty unconventional (in Rust), but I think it's clean and seems to work fine, so I have no issues with that. The setup for generating coverage automatically is a nice one.
• Docs are well written. Especially the user guide is very detailed and helpful.

Overall, the project is quite impressive and you've clearly been putting a lot of effort into it. Good job!

Overall, the code seems to be well-organized and follows good coding practices, such as using appropriate struct names and visibility modifiers. The code is thoroughly documented with comments.

minesweeper-ai/src/main.rs has quite a lot of code. At least the VisualState and argument parsing could easily be refactored into their own modules.

There are some places in the code where lint rules are disabled. Particularly dead code is allowed at some points. The programs seems to be mostly complete so I'm not sure if they were left there for a reason or just forgotten leftovers from earlier development. I would remove the dead code unless there's an immediate use for it.

Some points about test:

• Tests seem to cover key functionality in the program and provide good test coverage.
• The naming of test is sometimes a bit cryptic. For example fn test_flag() does not tell me what about flag is being tested and what the expected behavior is. It would be nice to have the name give that information without reading through the test code.
• Some tests are also testing multiple things. Generally it would be nice to have one behavior per test, so that when a test fails you immediately know what went wrong.
• All tests are in a tests directory, which is to my understanding where integrations tests go according to rust conventions. Unit tests would go into the source file of the module being tested, so they have access to private functions. All tests being in the test directories tells me that private functions are not specifically unit tested, though the other tests probably cover them.