Skip to content

Latest commit

 

History

History
138 lines (91 loc) · 5.91 KB

PeerReview-Exercise2-horbss.md

File metadata and controls

138 lines (91 loc) · 5.91 KB
Raw

Code Review for Programming Exercise 2

Due Date and Submission Information

See the official course schedule for due date.

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 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.

Solution Assessment

Peer-reviewer Information

Description

To assess 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.

Perfect

Cannot find any flaws concerning the prompt. Perfectly satisfied all stage objectives.

Great

Minor flaws in one or two objectives.

Good

A major flaw and some minor flaws.

Satisfactory

A couple of major flaws. Heading towards a solution, however, did not fully realize the solution.

Unsatisfactory

Partial work, but not really converging to a solution. Pervasive major flaws. Objective largely unmet.

Stage 1

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

Justification

The camera works perfectly and is locked to the vessel. The draw camera logic is good as well.

Stage 2

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

Justification

The autoscroll works as well, with the draw camera logic creating a box as the boundary for the vessel. My only addition would maybe to change the viewport to be the size of the viewport. I like that the user has to move with the autoscroll as well.

Stage 3

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

justification

The camera works here as well. The only thing I would maybe change is to have the camera move to the player itself faster so that it looks less like that player is snapping to the camera instead of the other way around.

Stage 4

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

justification

Works great! Perfect implementation of the camera.

Stage 5

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

justification

The camera works perfectly, with the different zones toggling different speeds. The drawn box is perfect as well.

Overall

I also noticed that you tried to add in scaling based on zoom amount which I thought was commendable and worth shouting out in this review. Fantastic implementation of this project.

Code Style

Description

Check the scripts to see if the student code follows the Godot style guide.

If sections don't adhere to the style guide, please permalink the line of code from GitHub and justify why the line of code has infractions of the style guide.

It should look something like this:

Here is an example of the permalink drop-down on GitHub.

Permalink option

Here is another example as well.

Code Style Review

The code is truly exemplary, with an abundance of comments that follow the style guide. The variables are all well defined and there is no code that is unclear or hard to read. The if statements are well structured and don't make use of multiline commands.

Style Guide Infractions

Maybe a little nitpicky, but I noticed some comments that are structured to break up the entire code like this.

exercise-2-camera-control-4Clover/Obscura/scripts/camera_controllers/camera_controller_four_way_speed.gd

Line 27 in 26cc168

################################## REFERENCE VARIABLES #########################################
Not sure if this is in line with the style guide, and I would stick to just keeping it as a regular comment.

Style Guide Exemplars

Great way to structure this variable, with the different parts of the boolean statement being split into multiple lines. https://github.com/ensemble-ai/exercise-2-camera-control-4Clover/blob/26cc1687f09320a833da5bdd3ffb4e02fbaf7c46/Obscura/scripts/camera_controllers/camera_controller_four_way_speed.gd#L74C1-L75C3

Best Practices

Best Practices Review

One thing I noticed is your use of frequent commits to the github, with about 15 commits made over the course of working on the project. That is a good practice. You use comments to detail what the code is doing and make use of multiple variables to make the code cleaner and easier to read, especially in your four way speed code. I also like that you used a separate script to create the crosshair for the cameras that required a crosshair.

Best Practices Infractions

Best Practices Exemplars

exercise-2-camera-control-4Clover/Obscura/scripts/camera_controllers/camera_controller_four_way_speed.gd

Lines 61 to 84 in 26cc168

var inside_inner_x = target_x > speed_left_bound and target_x < speed_right_bound
var inside_inner_z = target_z > speed_top_bound and target_z < speed_bottom_bound
var is_inside_inner_zone = inside_inner_x and inside_inner_z
var inside_outer_x = target_x > push_left_bound and target_x < push_right_bound
var inside_outer_z = target_z > push_top_bound and target_z < push_bottom_bound
var is_inside_outer_zone = inside_outer_x and inside_outer_z
var is_touching_outer_x = not inside_outer_x
var is_touching_outer_z = not inside_outer_z
# Flags for camera movement conditions
var is_between_zones = is_inside_outer_zone and not is_inside_inner_zone
var is_touching_one_side = ((is_touching_outer_x and inside_outer_z) or
(inside_outer_x and is_touching_outer_z))
var is_touching_corner = is_touching_outer_x and is_touching_outer_z
# Flags for moving towards the center from an outer zone
var moving_towards_center_x = ((target_x < speed_left_bound and moving_right) or
(target_x > speed_right_bound and moving_left))
var moving_towards_center_z = ((target_z < speed_top_bound and moving_up) or
(target_z > speed_bottom_bound and moving_down))
This is an example of you using multiple variables to make the code cleaner.