-
-
Notifications
You must be signed in to change notification settings - Fork 185
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
Frogger js core code #92
Conversation
Thank you for contributing to this repo! ❤️️ Frogger Arcade Game -- JS OO exercise check listRelates to Object-Oriented JavaScript task. Let's do some self-checks to fix most common issues and to make some improvements to the code while reviewers find some time to dedicate it to your submission. Go through the checklist below, fix code as appropriate and mark fulfilled requirements when you genuinely believe you are done. Please, feel free to ask questions here or seek for help in the Students' chat. Check-list - definition of doneMinimal requirements to meet:
Sincerely yours, |
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.
Hey! There are some improvements that can be done, but in general, nice work!
You've chosen to make the character go not cell-by-cell but with smaller steps which are fine but it's a little strange that it can go so low, almost off the level, and stops going left so early. Please work on the limits of where the character can go.
Also, check comments below.
submissions/asaMitaka/frogger/app.js
Outdated
@@ -0,0 +1,169 @@ | |||
// Enemies our player must avoid |
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.
Seems like an irrelevant comment here.
submissions/asaMitaka/frogger/app.js
Outdated
@@ -0,0 +1,169 @@ | |||
// Enemies our player must avoid | |||
var newP = document.createElement('p'); |
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.
You should avoid using var
s. Most of the time to store a value const
is enough and only if you are 100% sure that you'll change this value in code later use let
.
submissions/asaMitaka/frogger/app.js
Outdated
newP.textContent = `Your score: 0 `; | ||
document.body.append(newP); | ||
|
||
var enemyStat = { |
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.
There is no reason for a separate object that stores information about enemies since you already has a whole class for them. Store these values there.
submissions/asaMitaka/frogger/app.js
Outdated
|
||
Player.prototype.win = function() { | ||
if (field.min >= this.posY) { | ||
console.log(field.min, this.posY); |
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.
console.log
is fine for debugging but should be removed from the final version of the code.
Did you forget to update your own repo? Github Pages demo still has problems listed in my previous review. |
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.
Also follow the requirements here #92 (comment)
Feel free asking questions here or in Students' chat if anything is not really clear.
When done, please confirm explicitly that all requirements are met.
Great job! |
Object oriented JS
Demo
Code
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.