Skip to content

Updated Readme#29

Open
soemn wants to merge 22 commits into
wdi-sg:masterfrom
soemn:master
Open

Updated Readme#29
soemn wants to merge 22 commits into
wdi-sg:masterfrom
soemn:master

Conversation

@soemn
Copy link
Copy Markdown

@soemn soemn commented Sep 29, 2017

No description provided.

Comment thread assets/js/script.js
let gameCounter = 0
let gameStatus = "playing"

class Character {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What could be the drawbacks of big-sized Class including many methods?

Comment thread assets/js/script.js

//appends to game board
addChar() {
let $char = $("<div>")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any problem if I change the line as const $char?

Comment thread assets/js/script.js
}

if (this.type === "player") {
$body.on("keydown", e => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I suggest moving those eventListners to the outside of Class method moveChar(), where you want to put that?

Comment thread assets/js/script.js
if (this.type === "player") {
//mouse targeting
shootSoundPlayer.play()
$gameBoard.on("mousemove", e => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this event listener is moved to the outside, what is the benefit?

Comment thread assets/js/script.js
if (pause === false) {
return
}
for (key in enemyList) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check for-in vs for-of

Comment thread assets/js/script.js
const shootSoundPlayer = new Audio("./assets/audio/shootSoundPlayer.wav")
const $startGame = $(".startGame")
let level = 1
let enemyList = []
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference of let and const?

Comment thread assets/js/script.js
@@ -0,0 +1,990 @@
const $gameBoard = $(".gameBoard")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will be the difference if you wrap the entire code with $(function() { ... }) ?

Comment thread assets/js/script.js
}

spawnEnemy()
for (key in enemyList) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may try another iteration like .forEach().

Comment thread assets/js/script.js
}

//60 fps
requestAnimationFrame(update)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any approach that makes your game loop looks slim?

@alexkimin
Copy link
Copy Markdown

alexkimin commented Oct 12, 2017

Project Workflow: 4 / 5
Technical Requirement: 5 / 5
Creativity: 5 / 5
Code Quality: 3 / 5
Problem Solving: 4 / 5

Glow

Strong mindset on difficult logic, ability to make the logic works.
Fast adoption of new techniques.
Adept understanding what game is.
Self-driven research and writing code from the scratch.
Beta-testing with friends for debugging and difficulty design.

Grow

You may use more classes with slim methods.
Split long functions into a shorter and smaller unit.
You may simplify your core logic(e.g. update loop) with calling functions, it will boost readability.
More comments may be better.

Things to look for

Try functional method like .forEach() also, check for-in, for-of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants