Skip to content

On the other side#28

Open
itsgabrielu wants to merge 32 commits into
wdi-sg:masterfrom
itsgabrielu:master
Open

On the other side#28
itsgabrielu wants to merge 32 commits into
wdi-sg:masterfrom
itsgabrielu:master

Conversation

@itsgabrielu
Copy link
Copy Markdown

Readme and flowchart edit

Comment thread assets/js/script.js
var jumpLimit = true // switch for jump
var start1 = [] // indicates ball1 starting position, index position increases with level
var start2 = []
var currentLevel
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.

What does this variable for?

Comment thread assets/js/script.js
}
}
function wallCheck1Right () {
if (($ball1.position().left + $ball1.width() >= $('.wall1').eq(0).position().left) &&
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.

How would you rectified this repetitions?

Comment thread README.md
**1** | _Does not meet expectations._
**2** | _Meets expectations, good job!_
**3** | _Exceeds expectations, you wonderful creature, you!_
I also used wikipedia for the gravity formula, but it seems the page no longer looks the same, or I can't find it. But [here's](http://www.school-for-champions.com/science/gravity_equations_falling_displacement.htm#.WdXiRROCzVo) another resource you can use to see the gravity displacement.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The general equation is from kinematics, for displacement with constant acceleration.
See http://www.physicsclassroom.com/class/1DKin/Lesson-6/Kinematic-Equations.

Where d is the displacement, v(i) is initial velocity, a is acceleration (in this case, a = g) and t is time

Comment thread index.html
<body>
<script src="js/script.js" charset="utf-8"></script>
<h1> On The Other Side </h1>
<h3> <br> </h3>
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.

what's the <br> for?

Copy link
Copy Markdown
Author

@itsgabrielu itsgabrielu Oct 10, 2017

Choose a reason for hiding this comment

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

I had put it to push down the game halves lower from the h1 element

Comment thread index.html Outdated
<div id = "clear"></div>

</div>
<button type="button" id = "mute" class="mute"> I like sounds (M)</button>
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.

no space please => id="mute"

Comment thread index.html
<p> Help steer Blue and Red into the grey boxes to win. <br>
<strong>You can only move Blue directly</strong> with W,A,D or &uarr;,&larr;,&rarr;. <br>
Red will go opposite in the horizontal plane.<br></p>
<br><br>
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.

please group your html into a more logical grouping

Comment thread assets/js/script.js
@@ -0,0 +1,548 @@
var $ball1 = $('#ball1')
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.

Line 1 to line 18 should be after $(function () {

Comment thread assets/js/script.js
function ballMove (e) {
var unit = 20 // 20
if (e.key === 'd' || e.key === 'D' || e.key === 'ArrowRight') {
if (ball1Goal() === false && (borderCheck1Right()) && (wallCheck1Right()) && (!$('.fade').length)) ball1Hori(unit) // if ball1 doesn't exceed right border
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.

that's a really long condition, can we shorten this instead in a function?

Comment thread assets/js/script.js
}
if (e.key === 'w' || e.key === 'W' || e.key === 'ArrowUp') {
if (jumpLimit) {
if (!ball1Goal()) ball1Jump()
Copy link
Copy Markdown
Contributor

@primaulia primaulia Oct 10, 2017

Choose a reason for hiding this comment

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

Why can't just there be one function that checks goal for both balls?

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.

This is to let both balls be independent from each other, when a single ball goes into the goal, it will be frozen, while the other ball can still move/jump.

Comment thread assets/js/script.js
})

// this controls horizontal ball movement
function ball1Hori (a) {
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, and repeated.

What is a?

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.

a represents an argument that we eventually use, e.g.: in line, 31: var unit = 20 to indicate the amount of pixels moved per keyboard move:
ball1Hori(unit) to indicate movement to the right
ball1Hori(-unit) to indicate movement to the left

Comment thread assets/js/script.js
document.getElementById('ball1').style.left = (Number($ball1.css('left').replace('px', '')) + a).toString() + 'px'
}
function ball2Hori (a) {
document.getElementById('ball2').style.left = (Number($ball2.css('left').replace('px', '')) - a).toString() + 'px'
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.

mixing jquery with document.getElementById(), choose one and stick with 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.

Hmm i remember for this one, .css(top) and .position.top actually represent different values on the screen, so used .style to edit the ball placement,which seems to be only for non jquery manipulators ( i might be wrong!)

Comment thread assets/js/script.js
return true
}
}
function wallCheck2Right () {
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.

Refactor, refactor, refactor

Comment thread assets/js/script.js
break
}
}
}
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.

whew. what just happened

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.

this section of code represents when the user wins the current level. The logic which checks the winning condition is actually on a set interval, which also populates a nextLevel button. So in order to stop adding multiple buttons continously, we clear this interval, while adding a button which leads to the next level.

@primaulia
Copy link
Copy Markdown
Contributor

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

Glow

  • Interesting game concept, nice that you applied what you've learnt to what you really want to do
  • Delivery is concise and succinct
  • Applaud your attempt on doing this in a brute force attempt
  • Workflow was good, but quite messy

Grow & Things to look out for

  • Too many repetitions, I would ask you to repeat your codes line by line if this is not a 12 week course. I hope you've learnt that the amount of repetitions in your code here is alarming. Make sure that you look back on what you did every few minutes and not just piling up codes
  • A bad habit on not standardizing your way of coding, there are some parts that used space some that are not. Do look into this for your future development.

@shumin13
Copy link
Copy Markdown

Glow

  • Interesting game concept
  • Good implementation of the skip and reset button
  • Clear commit messages

Grow & Things to look out for

  • Good effort on the Readme but can be more concise. Can also look into https://help.github.com/articles/basic-writing-and-formatting-syntax/ for formatting syntax
  • Can look into https://coolors.co/ to enhance your design
  • Good to hide reset and skip button before the start button is clicked
  • Indicated the wrong year on the website
  • You may want to keep the sound credits to readme
  • The game is still quite buggy. May want 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.

4 participants