Skip to content

created readme and forgot to make pull request ><#27

Open
jpung wants to merge 23 commits into
wdi-sg:masterfrom
jpung:master
Open

created readme and forgot to make pull request ><#27
jpung wants to merge 23 commits into
wdi-sg:masterfrom
jpung:master

Conversation

@jpung
Copy link
Copy Markdown

@jpung jpung commented Sep 29, 2017

No description provided.

Comment thread assets/js/testScript.js
}, 30)

$body.on('keydown', function (e) { //player keyboard press detection
var $player = $('.player')
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

already defined at 122

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yeah was used for the checking portion when i was running the code, again i forgot to remove :(

Comment thread assets/js/testScript.js
$bulletLoc = $bullet.position()
$bulletExact = $bulletLoc.left
$bulletTop = $playPos.top
$bulletLeft = $playPos.left
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

$bulletLeft is not used in any part of the script. Any reason for this var?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

nope, forgot to take out after refactoring code

Comment thread assets/js/testScript.js
if (mana >= 10) {
mana -= 10
$mpBar.text(`${mana}/200`)
$mpBar.css('width', `${$mpBar.width() - 10}px`)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why not make the position = $player.position().left ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

its to reduce the width of the blue bar as the mana is used up, doesn't have any link to player position

Comment thread index.html

</div>
<script src="https://code.jquery.com/jquery-3.2.1.min.js"></script>
<script src="./assets/js/testScript.js" charset="utf-8"></script>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why not rename the testScript to script and remove the script.js since you dont need it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

lazy HAHAHA

@shumin13
Copy link
Copy Markdown

Glow

  • Interesting game
  • Nice design
  • Clear commit messages (but… there are some that doesn’t make sense)

Grow & Things to look out for

  • Need to work on the transitions and responsive design
  • Remember to remove those files that you do not need (2 script.js, 2 readme.md)
  • Game does not check for game over at boss level. Need to fix it before the M&G
  • The game is still quite buggy. Need to edit it before the M&G

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