Skip to content

Latest commit

 

History

History
154 lines (100 loc) · 5.85 KB

PeerReview-Exercise2.md

File metadata and controls

154 lines (100 loc) · 5.85 KB

Peer-Review for Programming Exercise 2

Description

For this assignment, you will be giving feedback on the completeness of assignment two: Obscura. To do so, we will give you a rubric to provide 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 check the code and project files that the instructor gave out.

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.

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. This review document should be placed into the base folder of the repo you are reviewing in the master branch. The file name should be the same as in the template: CodeReview-Exercise2.md. You must also include your name and email address in the Peer-reviewer Information section below.

If you are in a rare situation where two peer-reviewers are on a single repository, append your UC Davis user name before the extension of your review file. An example: CodeReview-Exercise2-username.md. Both reviewers should submit their reviews in the master branch.

Solution Assessment

Peer-reviewer Information

Description

For assessing the solution, you will be choosing ONE choice from: unsatisfactory, satisfactory, good, great, or perfect.

The break down of each of these labels for the solution assessment.

Perfect

Can't find any flaws with the prompt. Perfectly satisfied all stage objectives.

Great

Minor flaws in one or two objectives. 

Good

Major flaw and some minor flaws.

Satisfactory

Couple of major flaws. Heading towards solution, however did not fully realize solution.

Unsatisfactory

Partial work, not converging to a solution. Pervasive Major flaws. Objective largely unmet.

Solution Assessment

Stage 1

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

Justification

The controller/screen draws a 5 by 5 unit cross in the center of the screen when draw_camera_logic is true.


Stage 2

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

Justification

Every requirement is fulfilled. The controller draws a frame border box when draw_camera_logic is true. And if the player is touching the left edge box, the player would get pushed forward by the box edge. And exported the required variables.


Stage 3

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

Justification

The camera trails behind the players based on the follow_speed, and it also works during hyperspeed. Exported variables like catchup_speed and leash_distance work as described based on my testing. The only problem I notice is that the movement is jittery, but that isn't listed as a requirement for Stage 3. And more to do with the Vessell script from what i have heard.


Stage 4

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

Justification

The camera leads the player in the direction of the player input. But it doesn't work during hyperspeed. And the student exported all of the required variables and they're all working as intended from my testing. However, the problem with the player's jittery movement persists from stage 3.


Stage 5

  • Perfect
  • Great
  • Good
  • Satisfactory
  • Unsatisfactory

Justification

The player can push the box in the direction they're moving, at both normal and hyper-speed when it touches the edges and corners. But the push_ratio is not implemented, so it's not accounted for in the movement calculation.

func _process(delta: float) -> void:
if !current:
return
if draw_camera_logic:
draw_logic()
var tpos = target.global_position
var cpos = global_position
#boundary checks
#left
var diff_between_left_edges = (tpos.x - target.WIDTH / 2.0) - (cpos.x - box_width / 2.0)
if diff_between_left_edges < 0:
global_position.x += diff_between_left_edges
#right
var diff_between_right_edges = (tpos.x + target.WIDTH / 2.0) - (cpos.x + box_width / 2.0)
if diff_between_right_edges > 0:
global_position.x += diff_between_right_edges
#top
var diff_between_top_edges = (tpos.z - target.HEIGHT / 2.0) - (cpos.z - box_height / 2.0)
if diff_between_top_edges < 0:
global_position.z += diff_between_top_edges
#bottom
var diff_between_bottom_edges = (tpos.z + target.HEIGHT / 2.0) - (cpos.z + box_height / 2.0)
if diff_between_bottom_edges > 0:
global_position.z += diff_between_bottom_edges
super(delta)

Another problem is the Speed-up Zone and the Push Zone are not distinguished. In the Push Zone(not touching the corners), the camera doesn't follow the player and simply acts the same as the Speed-up Zone. The student also didn't export any of the required variables.

I mark this as "good" because there are 2 major flaws, a lack of zone distinction and push_ratio not being implemented. And some minor flaws like no exported variables.


Code Style

The code adheres to the style guide of this class. It's well-structured, easy to read, and no obsolete codes.

Style Guide Infractions

None.

Style Guide Exemplars

There are many descriptive and helpful comments for the code. For example, it's full of them for the Stage 3 script

# Called every frame. 'delta' is the elapsed time since the previous frame.
func _process(delta: float) -> void:
if !current:
return
# Calculate distance to target
var distance_to_target = Vector2(target.position.x - position.x, target.position.z - position.z).length()
# Calculate vessel's current speed
var vessel_velocity = Vector2(target.velocity.x, target.velocity.z).length()
var is_vessel_moving = vessel_velocity > 0.1 # Small threshold to detect movement
# Choose which speed to use based on vessel movement
var current_speed = follow_speed if is_vessel_moving else catchup_speed
# If beyond leash distance, move faster to catch up
if distance_to_target > leash_distance:
current_speed = max(current_speed, catchup_speed * 1.5) # Use faster speed when too far
# Update position with lerp using the determined speed
position.x = lerp(position.x, target.position.x, current_speed * delta)
position.z = lerp(position.z, target.position.z, current_speed * delta)
if draw_camera_logic or true:
draw_logic()
super(delta)

Best Practices

The code adheres to the best practices and the coding convention of GDScript. I don't see any Infractions other than a very minor indentation mistake.

Best Practices Infractions

One minor indentaion mistake for a comment.

# Reset catchup timer when there's input
time_since_last_movement = 0.0

Best Practices Exemplars

The code is very well-structured, it make it self-explanatory even without the comments.

func _process(delta: float) -> void:
if !current:
return
if draw_camera_logic or true:
draw_logic()
var input_dir = Vector2(
Input.get_action_strength("ui_right") - Input.get_action_strength("ui_left"),
Input.get_action_strength("ui_down") - Input.get_action_strength("ui_up")
)
var has_moved: bool = input_dir.length_squared() > 0.01
if has_moved:
# Reset catchup timer when there's input
time_since_last_movement = 0.0
# Calculate lead target position
var lead_offset = Vector3(input_dir.x, 0, input_dir.y) * leash_distance
target_position = target.position + lead_offset
# Move towards lead position
position = position.lerp(target_position, lead_speed * delta)
else:
# print("has not moved")
time_since_last_movement += delta
# If we've waited long enough, move back to player
if time_since_last_movement >= catchup_delay_duration:
target_position = target.position
position = position.lerp(target_position, catchup_speed * delta)
super(delta)