Frogger #479
Frogger #479
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 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. 😒 Please, make sure that your code follows the requirements Universal recommendations:
By the way, you may proceed to the next task before this one is reviewed and merged. Sincerely yours, |
fixed: assignment of constants, separation of functions
|
I have completed self-checks and fixed code accordingly. Please, check it. |
OleksiyRudenko
added
the
self-check-done
There was a problem hiding this comment.
@IhorOzerov good start.
However, not all requirements are met.
If anything is not clear feel free asking questions in Students' chat.
| player.x = config.PLAYER_CONST.x; | ||
| player.y = config.PLAYER_CONST.y; |
There was a problem hiding this comment.
Class definition should not refer a specific instance of the class.
There are means to refer to properties you actually use. Why this? (not a rhetoric question)
There was a problem hiding this comment.
I moved the character with coordinates and chose these two points for myself.
There was a problem hiding this comment.
Class definition should not refer a specific instance of the class.
Why do you do this? You will be closer to the solution once you figure out what forces you to take this approach.
There was a problem hiding this comment.
i am renamed specific instance, please check.
i am not pretty sure am i understand
There was a problem hiding this comment.
am i need to make some constants like a math decision, but not like "i am decided that"?
or there is problem exactly in specific instance?
There was a problem hiding this comment.
Is "i decided that" equivalent to "i just guessed and it works"?
Calculations provide explanations. As a developer tasked to contribute to a project developed before by someone else (often previous contributors are unreachable as people tend to quit companies) you will want to understand what is going on in the code, why and what is behind numbers in particular.
There was a problem hiding this comment.
Second option will be closer to truth.
After this fixes i am better see how prototype works and where it similar to class sintax. Thank you!
| const playerWidth = player.x + config.PLAYER_CONST.width; | ||
| const playerHeight = player.y + config.PLAYER_CONST.height; | ||
|
|
||
| if (player.x < enemyWidth && | ||
| playerWidth > this.x && | ||
| player.y < enemyHeight && | ||
| playerHeight > this.y) { | ||
| player.x = config.PLAYER_CONST.x; | ||
| player.y = config.PLAYER_CONST.y; |
There was a problem hiding this comment.
Requirements posted/linked by bot addresses specifically the issue we have in this fragment.
Please identify the requirement, post it here as a comment and fix code accordingly.
There was a problem hiding this comment.
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 instance of a game object (or objects) as an argument to other game objects they need to interact with.
There was a problem hiding this comment.
Yes, that a relevant requirement. Fix the code accordingly.
You may have the very same issue elsewhere in the code. This is just an example that happened to match this fragment, but the issue is described before the example.
There was a problem hiding this comment.
already fixed it, check please
| const playerWidth = player.x + config.PLAYER_CONST.width; | ||
| const playerHeight = player.y + config.PLAYER_CONST.height; | ||
|
|
||
| if (player.x < enemyWidth && | ||
| playerWidth > this.x && | ||
| player.y < enemyHeight && | ||
| playerHeight > this.y) { | ||
| player.x = config.PLAYER_CONST.x; | ||
| player.y = config.PLAYER_CONST.y; |
There was a problem hiding this comment.
Yes, that a relevant requirement. Fix the code accordingly.
You may have the very same issue elsewhere in the code. This is just an example that happened to match this fragment, but the issue is described before the example.
| player.x = config.PLAYER_CONST.x; | ||
| player.y = config.PLAYER_CONST.y; |
There was a problem hiding this comment.
Class definition should not refer a specific instance of the class.
Why do you do this? You will be closer to the solution once you figure out what forces you to take this approach.
improved refer to global variable 'player' in Enemy's and Player's methods
Frogger game
Demo |
Code base
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.