Skip to content

readme update#35

Open
thamjieying wants to merge 20 commits into
wdi-sg:masterfrom
thamjieying:master
Open

readme update#35
thamjieying wants to merge 20 commits into
wdi-sg:masterfrom
thamjieying:master

Conversation

@thamjieying
Copy link
Copy Markdown

No description provided.

Comment thread js/level1.js
[7,3,3,3,3,8]
]

//array indicating the exits
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's not necessary to split these two arrays (map and exit), but good that you justify why in the presentation.

Comment thread level1.html
@@ -0,0 +1,76 @@
<!DOCTYPE html>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good job on separating the concern of each level into different html 👍

I hope you realized that this is tedious, but this is inevitable in unit 1.

Comment thread js/script.js
// SET THE MAP
map.forEach(function(spotY,indexY) {
spotY.forEach(function(spot, indexX){
switch (spot) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One of the few good application of switch :)

Comment thread js/gamePlay.js
counter=0

//playerMovement
if(!isExit() && clicker){
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

indentation issue here

@primaulia
Copy link
Copy Markdown
Contributor

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

Glow

  • Very good well-thought game logic and game mechanism
  • Code is written in a very systematic manner and easy to follow
  • Project description is fine-grained and detailed, yet easy to understand

Grow

  • Your delivery can be more engaging and fun, the game is pretty interesting :)
  • Make your codes more DRY, look back on your project 1 after you're done with unit 2
  • Try to solve my challenges by randomizing the maze

Things to look for

  • MVC structure, we'll cover this on unit 2
  • Class & Constructor, perhaps this will help you to randomize the game better

Comment thread level1.html
@@ -0,0 +1,76 @@
<!DOCTYPE html>
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 can complete your game without split files of html. I believe you will figure that out. Focus on reducing repetition.

Comment thread js/gamePlay.js

//playerMovement
if(!isExit() && clicker){
switch(e.keyCode){
Copy link
Copy Markdown

@alexkimin alexkimin Oct 8, 2017

Choose a reason for hiding this comment

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

What will be cons and pros of switch statement with a few case?

if(!isExit() && clicker) {
     if(e.keyCode === 37 && !checkXBlockLeft(charPos)) charMoveLeft()
     if(e.keyCode === 38 && !checkYBlockTop(charPos)) charMoveUp()
     if(e.keyCode === 39 && !checkXBlockRight(charPos)) charMoveRight()
     if(e.keyCode === 40 && !checkYBlockBottom(charPos))charMoveDown()
     mummyFollowChar()
     meetMummy()
}

Comment thread js/level1.js
@@ -0,0 +1,30 @@
//layout for blockage
Copy link
Copy Markdown

@alexkimin alexkimin Oct 8, 2017

Choose a reason for hiding this comment

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

Without file splitting and linking to particular html file, I believe you can generate levels with javascript. :) It will be helpful for scalability of levels.

Comment thread js/mummyMovement.js
function mummyFollowChar(){
counter = 0
mumPos = map[mumY][mumX]
if(counter <2){
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 reduce some repetition here. How?

Comment thread js/playerMovement.js
@@ -0,0 +1,23 @@
function charMoveLeft(){
Copy link
Copy Markdown

@alexkimin alexkimin Oct 8, 2017

Choose a reason for hiding this comment

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

Your movement functions are called inside of callback function of the keyborad event listener. Then what if I put var position = $('#player').position() line to line 4 of gameplay.js?

Comment thread js/script.js
// SET THE MAP
map.forEach(function(spotY,indexY) {
spotY.forEach(function(spot, indexX){
switch (spot) {
Copy link
Copy Markdown

@alexkimin alexkimin Oct 8, 2017

Choose a reason for hiding this comment

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

Good 'switch approach with many cases. :)

And you may do this with 2-D Array and in statement for replacing each switch statement.

e.g.

var arr = [[1,5,6,9,11,12,14], [ ...... ], [......], .........]
if( spot in arr[0] ) $wrapper.eq(indexY).find('.box').eq(indexX).addClass('top')

Comment thread js/script.js

// SET THE MAP
map.forEach(function(spotY,indexY) {
spotY.forEach(function(spot, indexX){
Copy link
Copy Markdown

@alexkimin alexkimin Oct 8, 2017

Choose a reason for hiding this comment

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

What are the cons and pros of forEach()comparing with for-loop?

Comment thread js/script.js

//checing relative x-position of mummy to player
function checkXPos(mumX, charX){
if(mumX === charX){
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 refactor here

return mumX === charX

Comment thread js/script.js

//checking relative y-position from mummy to player
function checkYPos(mumY, charY){
if(mumY === charY){
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same as line 110

Comment thread js/script.js

//checking x-blockage
function checkXBlockLeft(x){
switch(x){
Copy link
Copy Markdown

@alexkimin alexkimin Oct 8, 2017

Choose a reason for hiding this comment

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

For your inspiration.

function checkXBlockLeft(x){
    var block = [4,5,7,10,11,13,14]
    return x in block
}

@alexkimin
Copy link
Copy Markdown

alexkimin commented Oct 8, 2017

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

Glow

Strong understanding of own logic and solid way of thinking.

Grow

You can try various method or approach for doing same things.
Check cons and pros of several Iteration and Condition flow.
If you follow DRY principle, the code will be more tidy and clean.

Things to look for

Class & Constructor.
How to avoid using a global scope.

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.

3 participants