Conversation
To reflect a better naming convention.
adding faster frame-rate (fixes #12)
load data from levels folder
maze generation
This reverts commit f094e31.
Fix wrong return type
ks129
left a comment
There was a problem hiding this comment.
Errors during running
Running the game itself didn't produce any errors (and the game was really interesting and fun), but if I tried to quit the game, I got the following error: openal.alc.ALCError: ALC_INVALID_DEVICE. I used MacOS to run the game.
Teamwork
All team members contributed to the project, made issues, and created PRs. Just fantastic.
README
Overall introductions were clear, but I would like to see more pictures inside introductions, not only in the images section, and even when you mentioned sounds source in code, it's a good idea to reference them in README. Maybe stack you used too.
PEP 8 Compliance
Perfect. Just perfect. Flake8 was speechless when it saw your code.
Code structure
The overall code structure is fine, but some big files should be split into smaller files in the module folder. But there is one file for which I can't find any usages.
Commit quality
This was the biggest fail. There are way too many meaningless commit titles and no commit body. You should definitely try to improve the quality of your commits.
Code quality
First, I want to say that the overall code quality is excellent and most of my code comments are opinionated, but there are few points that I would like to mention here:
- Using only one root logger is most of the time harder to track. I suggest using scoped loggers in the future.
- Log entries were really unclear, just dumping numbers sometimes.
- There are cases where code is pretty hard to follow, especially these algorithm codes.
| @property | ||
| def location(self) -> Vec: | ||
| """Get location of this character""" | ||
| """Get location of this character""" |
There was a problem hiding this comment.
I don't think double docstring is necessary.
| self.cells = [] | ||
| for y in range(self.height): | ||
| for x in range(self.width): | ||
| self.cells.append(Cell(x, y, [N, S, E, W])) |
There was a problem hiding this comment.
Nothing major, but I would like to see empty lines before and after this block.
| @@ -0,0 +1,27 @@ | ||
| """Convert copied maze from asciiflow to matrix |
There was a problem hiding this comment.
Where is this file used? I can't find any usages.
There was a problem hiding this comment.
This was used to create the levels. We created the diagrams in https://asciiflow.com/#/ and then copied the whole thing to get the matrix using this script.
| term = Terminal() | ||
|
|
||
|
|
||
| class AnimatedCharacter: |
There was a problem hiding this comment.
Nice usage of getters and setters in this class.
| class AnimatedCharacter: | ||
| """Animated character class | ||
|
|
||
| https://www.figma.com/file/KOvD3LAhx8ljCyvNJB4X2e/Untitled?node-id=0%3A1 |
There was a problem hiding this comment.
Such specifications should be created as a file inside the repository, not using a link to an external service.
| Anand>325.00 | ||
| Olivia>308.50 | ||
| Jason>383.97 |
There was a problem hiding this comment.
This shouldn't have been committed, I think.
| play_level_up_sound() | ||
| return self.action_on_choice(choice) | ||
| return "" | ||
| # return "" |
| self.player.start() | ||
| self.first_act = True | ||
|
|
||
| global prev_text, prev_text_loc |
There was a problem hiding this comment.
Globals should be definitely avoided because they add confusion.
| with term.cbreak(): | ||
| val = Keystroke() | ||
| while True: | ||
|
|
There was a problem hiding this comment.
This empty newline is unnecessary.
| TITLE = 0 | ||
| NEXT_SCENE = 1 | ||
| RESET = 2 | ||
| QUIT = 3 | ||
| PAUSE = 4 | ||
| PLAY = 5 | ||
| LOSE = 6 | ||
| CREDITS = 7 | ||
| TUTORIAL = 8 | ||
| END = 9 | ||
| LEADERBOARD = 10 | ||
| INFINITE = 11 |
There was a problem hiding this comment.
I think Enum would have been better here.
No description provided.