-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
71 collection #76
71 collection #76
Conversation
Codecov Report
@@ Coverage Diff @@
## master #76 +/- ##
=======================================
Coverage 33.33% 33.33%
=======================================
Files 1 1
Lines 18 18
Branches 6 5 -1
=======================================
Hits 6 6
Misses 12 12
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 Major issue: json levels don't work anymore. See review comments for details. After that's fixed, we can merge. :) Great job ! 🏆
src/scenes/Player.ts
Outdated
@@ -6,7 +6,8 @@ export class Player implements IPlayer { | |||
constructor( | |||
private graph: Graph, | |||
private location: ILocation, | |||
public stock = 3, | |||
// tslint:disable-next-line:variable-name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the reason for having to disabling the linter?
src/scenes/mainScene.ts
Outdated
@@ -86,6 +89,54 @@ export class MainScene extends Scene { | |||
this.playerTurnInfo.setText(this.player.turn.toString()); | |||
this.updateBuildFactoryButton(); | |||
this.updateCityInfos(); | |||
this.updateVisibility(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
who's visibility is updated? The + - buttons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name has been changed. New pull request will follow immediately. I was planning to extend the method to handle more GameObjects but it does not seem necessary at the moment.
src/scenes/mainScene.ts
Outdated
@@ -314,6 +375,9 @@ export class MainScene extends Scene { | |||
} | |||
|
|||
private drawEdges(cities: ICity[]) { | |||
this.travelPathLines = this.add.graphics({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line occurs 3 times, consider extract method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's done. New pull request will follow immediately.
No description provided.