siggame / Viseur

Visualizer for the Cadre AI game framework
http://vis.siggame.io
MIT License
5 stars 10 forks source link

Checkers Code Review + Linter fixes for them #31

Closed JacobFischer closed 6 years ago

JacobFischer commented 7 years ago

I did a code review on Checkers for you @MatthewQualls

Overall it was good, but there were some potential problems:

So you had lines like these:

this.checker = (this.game.resources[`checker`] as RendererResource)
               .newSprite(this.container);

the correct way to code this is:

this.checker = this.game.resources.checker.newSprite(this.container);

Doing this has a few advantages:

  1. shorter code
  2. No unnecessary cast needed
  3. typescript can perform type checking on the checker property and knows what methods it can do
  4. Don't have to handle index not found errors because typescript knows what properties are present in the resources

In addition I've added a feature to ease() so that if you don't specify the easing type it defaults to cubic-in-out now, because we use that so much, so you can rewrite this:

ease(currentPosition.x, nextPosition.x, dt, "cubicInOut")

to this:

ease(currentPosition.x, nextPosition.x, dt)

Otherwise it all looks good. I just wanted to point out where you can improve your code and throw more of the workload on TypeScript instead of you the developer.

JacobFischer commented 6 years ago

I've merged this in respect to Matthew.