-
-
Notifications
You must be signed in to change notification settings - Fork 184
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
Arcade game #289
Arcade game #289
Conversation
Hey! Congratulations on your PR! 😎😎😎 Let's do some self-checks to fix most common issues and to make some improvements to the code before reviewers put their hands on the code. Go through the requirements/most common mistakes listed/linked below and fix the code as appropriate. If you have any questions to requirements/common mistakes feel free asking them here or in Students' chat. When you genuinely believe you are done put a comment stating that you have completed self-checks and fixed code accordingly. Also, be aware, that if you would silently ignore this recommendation, a mentor can think that you are still working on fixes. And your PR will not be reviewed. 😒 This PR contains irrelevant files. You can see this under Files changed tab (you will find a couple of helpful tabs right under the PR title). Most likely, you added all files from your project, not only required and sufficient code review as instructed in section B123 of submission instructions. Also read an important note therein. Why we want only few files?
Please fix this issue by removing irrelevant files and add a comment stating that you did this. Universal recommendations:
Also take a note of the requirements above and follow them in all your future projects. By the way, you may proceed to the next task before this one is reviewed and merged. Sincerely yours, |
discarded (related to Friends App) |
Hey! Congratulations on your PR! 😎😎😎 Let's do some self-checks to fix most common issues and to make some improvements to the code before reviewers put their hands on the code. Go through the requirements/most common mistakes listed/linked below and fix the code as appropriate. If you have any questions to requirements/common mistakes feel free asking them here or in Students' chat. When you genuinely believe you are done put a comment stating that you have completed self-checks and fixed code accordingly. Also, be aware, that if you would silently ignore this recommendation, a mentor can think that you are still working on fixes. And your PR will not be reviewed. 😒 Frogger Arcade Game -- JS OO exercise check listRelates to Object-Oriented JavaScript task. Check-list - definition of done
Universal recommendations:
Also take a note of the requirements above and follow them in all your future projects. By the way, you may proceed to the next task before this one is reviewed and merged. Sincerely yours, |
@A-Ostrovnyy Hello! Can you help me delete irrelevant files? |
@A-Ostrovnyy hello, can you check my code? I'm add main constants and checked my code against the main check-list. |
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.
@Levi123 good job.
Let's take some care of peer developers that will need to collaborate on this project.
widthEnemy: 80, | ||
heightEnemy: 60, | ||
positionX: -100, | ||
positionY: [50, 140, 225], |
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.
As a peer developer I am tasked to add more rows and more enemies.
I'd like to this in terms or "rows" rather than pixels.
Can we make values calculable from some basic consepts like "row number", "block height" etc?
This also relates to other pixel-based numbers across the code base that are calculable from basic concepts.
As a bonus you will have more accurate positioning of characters on the field.
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.
As a peer developer I am tasked to add more rows and more enemies. I'd like to this in terms or "rows" rather than pixels. Can we make values calculable from some basic consepts like "row number", "block height" etc? This also relates to other pixel-based numbers across the code base that are calculable from basic concepts. As a bonus you will have more accurate positioning of characters on the field.
can you give me your telegram contact?
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.
As a peer developer I am tasked to add more rows and more enemies. I'd like to this in terms or "rows" rather than pixels. Can we make values calculable from some basic consepts like "row number", "block height" etc? This also relates to other pixel-based numbers across the code base that are calculable from basic concepts. As a bonus you will have more accurate positioning of characters on the field.
i refresh my PR and DEMO. I tried to implement line work for enemy creation instead of pixel work
setTimeout(() => { | ||
this.x = DEFAULT_PLAYER.positionX; | ||
this.y = DEFAULT_PLAYER.positionY; | ||
}, 300) |
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.
What is 300 semantically? I know it is delay. It delays what?
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.
What is 300 semantically? I know it is delay. It delays what?
when person move to waterline, we refresh him position to default. I added delay to smoother animation.
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.
Let the constant name say this somehow
@OleksiyRudenko Hello, can you review my changes? |
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.
@Levi123 good job!
A few improvements are still needed.
if (valueY === 1){ | ||
enemy = new Enemy(DEFAULT_ENEMY.positionX, firstRowEnemy); | ||
allEnemies.push(enemy); | ||
} | ||
|
||
if (valueY === 2){ | ||
enemy = new Enemy(DEFAULT_ENEMY.positionX, secondRowEnemy); | ||
allEnemies.push(enemy); | ||
} | ||
|
||
if (valueY === 3){ | ||
enemy = new Enemy(DEFAULT_ENEMY.positionX, thirdRowEnemy); | ||
allEnemies.push(enemy); | ||
} |
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.
As a peer developer I am tasked to add two more rows to the play field.
Can we map valueY
into enemy Y position?
Also is valueY
is a pixel or field row?
Let the code say that.
Let's also Array#map
[1,2,3]
directly into allEnemies
.
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.
As a peer developer I am tasked to add two more rows to the play field. Can we map
valueY
into enemy Y position? Also isvalueY
is a pixel or field row? Let the code say that.Let's also
Array#map
[1,2,3]
directly intoallEnemies
.
done
if(player.x < this.x + DEFAULT_ENEMY.widthEnemy && | ||
player.x + DEFAULT_ENEMY.widthEnemy > this.x && | ||
player.y < this.y + DEFAULT_ENEMY.heightEnemy && | ||
DEFAULT_ENEMY.heightEnemy + player.y > this.y){ | ||
player.x = DEFAULT_PLAYER.positionX; | ||
player.y = DEFAULT_PLAYER.positionY; |
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.
* [ ] classes do not refer to any global variables, like global variable `player`, which is an instance of `Player` class (referring to global constants and globals provided by the gaming platform like `Resources` is OK); Hint: pass `Player` instance as an argument to every enemy
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.
* [ ] classes do not refer to any global variables, like global variable `player`, which is an instance of `Player` class (referring to global constants and globals provided by the gaming platform like `Resources` is OK); Hint: pass `Player` instance as an argument to every enemy
done
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.
@Levi123 great job!
Object-Oriented JavaScript
Demo |
Code base
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.