Nathan Kotni
[email protected]
For this assignment, you will be giving feedback on the completeness of Exercise 2. In order to do so, we will be giving you a rubric to give feedback on. For the feedback, please give positive criticism and suggestions on how to fix segments of code.
You only need to review code modified or created by the student you are reviewing. You do not have to review the code and project files that were given out by the instructor.
Abusive or hateful language or comments will not be tolerated and will result in a grade penalty or be considered a breach of the UC Davis Code of Academic Conduct.
If there are any questions at any point, please email the TA.
A successful submission should consist of a copy of this markdown document template that is modified with your peer-review. The file name should be the same as it is in the template: PeerReview-Exercise1.md. You also need to include your name and email address in the Peer-reviewer Information section below. This review document should be placed into the base folder of the repo you are reviewing in the master branch. This branch should be on the origin of the repository you are reviewing.
If you are in the rare situation where there are two peer-reviewers on a single repository, append your UC Davis user name before the extension of your review file. An example: PeerReview-Exercise1-username.md. Both reviewers should submit their reviews in the master branch.
For assessing the solution, you will be choosing ONE choice from: unsatisfactory, satisfactory, good, great, or perfect. Place an x character inside of the square braces next to the appropriate label.
The following are the criteria by which you should assess your peer's solution of the exercise's stages.
Can't find any flaws in relation to the prompt. Perfectly satisfied all stage objectives.
Minor flaws in one or two objectives.
Major flaw and some minor flaws.
Couple of major flaws. Heading towards solution, however did not fully realize solution.
Partial work, not really converging to a solution. Pervasive Major flaws. Objective largely unmet.
- Perfect
- Great
- Good
- Satisfactory
- Unsatisfactory
The position locked camera works exactly as it should with the camera staying centered exactly on where the player's position is and is indicated by a cross drawn on the screen when the camera draw logic is on. The camera maintains functionality when hyperspeed is activated. A nice touch is that draw logic is on by default and can be turned off/toggled by pressing F, which is a small but convenient quality of life update from the starter code since it allows the user to identify which camera they are using more easily.
- Perfect
- Great
- Good
- Satisfactory
- Unsatisfactory
This camera does work in many ways that follow from the assignment description, like there being an auto-scrolling speed that the camera moves at, and the player being confined to a certain scrolling area/box at which point it is 'pushed' by the edges of that box, with this box being shown when draw logic is on, and the player staing inside the box even when hyperspeed is activated. However, when it comes to the player movement, there appears to be a major flaw that likely stems from the vagueness of the assignment directions for stage 2 on this particular subject.
The author's code actually ends up only moving the camera by the auto-scroll speed while the player gets left behind as the camera moves (when no user input is given). This is not the intended behavior of this camera, especially when considering responses clarifying this matter on the Class Discord. The intended behavior would be for the camera to be moving at a scrolling speed but also be moving the player at that speed as long as the camera is moving, leading to it appearing as if the player can freely move within the given screen space as it moves without being pushed a direction by default. This is definitely an understandable mistake to make though as the assignment directions and demo GIF don't clarify this matter much.
- Perfect
- Great
- Good
- Satisfactory
- Unsatisfactory
The Lerp smoothing with the 'lagging behind' effect of the camera works exactly as intended since it begins to lag the camera behind as the player moves up to a maximum distance of leash distance. This also maintains functionality when hyperspeed is activated and the player stays within the leash distance. The camera also appropriately catches up with the player once the player is no longer moving. The camera draw logic also accurately draws a cross when the draw logic is on.
- Perfect
- Great
- Good
- Satisfactory
- Unsatisfactory
Most things with the lerp smoothing target focus camera work correctly, like how the camera goes ahead of the player in the direction of movement, and maintains a leash distance from the player once it gets that far apart from it with regular player movement, it correctly has a delay for which the player must remain still for the camera to 'snap back' to where the player is, and the camera draw logic also correctly draws a cross.
The issue comes when the player activates hyperspeed, and the camera seems to go far beyond the leash distance instead of maintaining it. Since this only happens with the hyperspace feature, for which the behavior wasn't explicitly specified or focused on in the assignment directions, it can be considered as a non-major flaw.
- Perfect
- Great
- Good
- Satisfactory
- Unsatisfactory
This camera does not have all of the functionality it should when testing it. First of all, when the player enters any of the speed up zones and is moving in the direction of that edge, the camera does not move at all, not even by a ratio of player speed. This is beyond a major flaw because it now just seems to function as a pushbox with different draw logic of another box inside the big one. Also, movement in the perpendicular directions of movement to the closest edge in the speedup zone also doesn't lead to speeding up, which is another flaw in the design. Because this essentially functions as a pushbox, it can be considered that the right idea was in mind with the draw logic but it was not executed or achieved considering the pushbox code was given to us as starter code.
Check the scripts to see if the student code adheres to the dotnet style guide.
If there are sections that don't adhere to the style guide, please peramlink the line of code from Github and justify why the line of code has infractured the style guide.
It should look something like this:
- description of infraction - this is the justification.
Here is an example of the permalink drop down on Github.
Here is another example as well.
Following this style guide: https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/gdscript_styleguide.html
There were very few minor infractions that I could find within the author's code, firstly to do with line spacing in the code. While the line spacing is pretty good, in speed_up_push_zone.gd, target_focus.gd, lerp_smoothing.gd, and auto_scroll.gd, there aren't two lines surrounding the functions as the style guide suggests there should be and instead only 1 line of blank space. However, this is a rather nitpicky infraction, as it didn't affect code readability and was quite difficult to notice unless someone is specifically looking for it as I am.
One other minor infraction was the lack of usage of inferred types when appropriate like the Vector2 and Vector3 variables in speed_up_push_zone.gd and auto_scroll.gd definite the type then initialize the variables instead of using := for a non-ambiguous variable like the style guide suggests.
I looked at many other aspects of the style guide and couldn't really find any other infractions. I inspected things like the naming conventions, variable ordering, function ordering, and others.
The author followed the code style conventions very closely and followed naming conventions of exported variables/functions well by using snake_case for them with PascalCase used for class names in the declarations. The ordering of the code also follows the style conventions of Godot with class declaration, then @export variables, then public variables, then _ready, then _process, then public methods, and the best example of this is in the target_focus.gd file.
The author uses good spacing within functions between logical components, which helps with code readability and could be considered a coding style exemplar. This can be seen in really any of the files and one for example would be auto_scroll.gd.
If the student has followed best practices (Unity coding conventions from the StyleGuides document) then feel free to point at these segments of code as exemplars.
If the student has breached the best practices and has done something that should be noted, please add the infracture.
This should be similar to the Code Style justification.
- description of infraction - this is the justification.
The main thing that the code is lacking would be a few more comments explaining what some sections are doing especially in parts of the code like the code in target_focus.gd to help with understanding the process the author was intending.
Another thing is that there was one time that a variable was created and initialized but was never used, which was the cpos variable in [position_lock.gd], and this action generates a warning in the debugger on Godot, which adds a little bit to the seriousness of this infraction.
Those seem to be the only real Best Practices infractions that I noticed looking at the author's code.
Despite the slight lack of comments in the code, there are still some basic comments and the variable and function names were always very appropriate for what they were doing so this definitely helped anyone reading the code to understand what different parts of it are accomplishing, boosting code readability.
The author stores calculated values in variables in the functions for reusability and code readability, which is a good practice.
Another good practice that can be seen in some of the author's files is that they used helper functions when appropriate like the update_camera_position helper function called in _process in lerp_smoothing.gd.
The author also went ahead and made it so that for each one of the cameras, when the camera is switched it starts out centered on the player's position, which is a best practice for smooth gameplay experience.