ForNeVeR / ForNeVeR/Gyges

[Gyges] Code review results

Open
#8 0 comments 0 reactions 0 assignees View on GitHub
code review
Dominant language
F#
Stars
4
Forks
1
PR merge metrics
No merged PRs in 30d

Description

1. I don't think that `Gyges.Math.offset` function is good enough. While it mutates the rect passed to it, its API doesn't suggest that (because by default I expect that such a function that **returns** a rect would be side effect-free). Either change the function to return `unit` or change it to never mutate the object passed.
2. I think that this code could be reduced: https://gitlab.com/gsomix/sitc-game-jam/blob/8b3740037a2f7e13b4f83eb31973f7eb6fb15175/Gyges/Program.fs#L178-185
3. `Program.every` looks suspicious: I'm not convinced that it will work well and won't generate the same action multiple times in the neighbour frames.

Otherwise, the code looks well and interesting, good work!

### Naming Nitpicks

Obviously this part is very subjective and thus discussional.

1. `Gyges.Utils.Map.addWithGuid`: maybe rename to `addNew`?
2. I don't like the name `GameLoop`. It looks more like `GameState` or even just `GygesGame` (because it inherits from `Game`).
3. `GameLoop.makeConfig` → `make`? You provide no ability to make it without config anyway, right?
4. `Program.updateEmenies` → `updateEnemies`, `clearEmenies` → `clearEnemies`, `collideEmenies` → `collideEnemies` :)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.