Skip to content
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

Add arc method to Turtle #82

Open
26 tasks
sunjay opened this issue May 18, 2018 · 4 comments
Open
26 tasks

Add arc method to Turtle #82

sunjay opened this issue May 18, 2018 · 4 comments

Comments

@sunjay
Copy link
Owner

sunjay commented May 18, 2018

Note: This issue has been rewritten to reflect the current details as of Oct 5, 2020. See the edit history for the prior (now outdated) versions.

Goal: Provide some utility for drawing arcs. This is currently approximated in many turtle programs by many short line segments with small angles between them. For example:

turtle/examples/snowman.rs

Lines 191 to 201 in cc40975

fn arc(turtle: &mut Turtle, radius: f64, extent: f64, steps: u32) {
let circumference = 2.0 * PI * radius;
let distance = circumference * extent / 360.0;
let step = distance / steps as f64;
let rotation = extent / steps as f64;
for _ in 0..steps {
turtle.forward(step);
turtle.right(rotation);
}
}

The circle function in the Python implementation is an example of a potential API for this that works well in Python. Unfortunately, this is a bit tricky to copy directly into turtle because Rust doesn't have optional function arguments. Even if it did, I think we can do better in terms of API design in several ways:

  • The Python function is called circle which makes sense in Python because the call circle(radius) reads very naturally with all the optional arguments omitted
    • In Rust, we don't have optional arguments, so we'd probably want to call it arc so it reads naturally when both radius and extent are provided, e.g. arc(radius, extent)
  • The Python implementation takes a steps parameter because it approximates the curve with straight lines
  • The Python implementation deviates from the rest of the turtle API by forcing the user to use a positive/negative radius convention to choose the direction of the arc
    • We have the opportunity to add arc_left and arc_right methods instead that are consistent with the left and right methods
    • Note that both left and right support passing in negative values so arc_left and arc_right would as well
    • The Python implementation is the way it is because of the ARC command in the LOGO programming language (We are not bound by the design decisions of LOGO or Python, so it makes sense to try to do better here)

Aside: The Builder Pattern

Usually, when an API needs optional arguments in Rust, we turn to the builder pattern. (We could use Option to simulate optional arguments, but that is unidiomatic and difficult to read.) For example, we could have an Arc struct with an API that looks like:

turtle.arc(Arc::new(5.0) // arc with radius 5.0
    .extent(45.0));      // extent of 45 degrees (optional, default = 360.0)

This definitely makes sense when the struct being initialized has lots of options and reasonable defaults. In this case we only have two arguments (maybe 3 if you include Python's steps). A default of 360° for extent only makes sense if the method is called circle. We could rename the method and the builder, but turtle.circle(Circle::new(5.0)) doesn't really improve anything and is far too verbose.

Mentoring Instructions

To make this issue easier to implement and easier to review, I've split it into two parts: 1) adding arc_left and arc_right methods that we simulate using forward and right and 2) implementing the full circular arc animation system. That way we can get the method implemented and documented without needing you to master the math required for the animation right away.

Part 1: Adding the methods

Read through all the instructions first before getting started.

  • Goal: Add an arc_left(radius: Distance, extent: Angle) method and arc_right(radius: f64, extent: f64) method to Turtle
  • Both methods draw a circular arc with the given radius
  • The arc_left method turns the turtle left (counterclockwise) as it draws and the arc_right method turns the turtle right (clockwise) as it draws
  • The radius argument causes the center of the arc to extend out to the left of the turtle in arc_left and to the right of the turtle in arc_right
    • if the radius argument is negative, the center of the arc flips to the opposite side of the turtle
  • The extent argument dictates how much of a circle to draw
    • if the extent argument is negative, the turtle moves backwards instead of forwards
    • note that extent can be more than 360 degrees (in that case, the turtle would draw a complete circle and then keep drawing around the same circle until it reaches the desired angle)
  • Step 1: Add methods on Turtle called arc_left and arc_right that block on corresponding methods on AsyncTurtle (add them underneath the wait method)

    turtle/src/turtle.rs

    Lines 191 to 193 in cc40975

    pub fn left(&mut self, angle: Angle) {
    block_on(self.turtle.left(angle))
    }
  • Step 2: Add arc_left and arc_right async methods on AsyncTurtle that await on a new circular_arc method on ProtocolClient (add them underneath the wait method)
    pub async fn left(&mut self, angle: Angle) {
    let angle = self.angle_unit.to_radians(angle);
    self.client.rotate_in_place(self.id, angle, RotationDirection::Counterclockwise).await
    }
  • Step 3: Add a method on ProtocolClient with signature circular_arc(radius: Distance, extent: Radians, direction: RotationDirection) (add the method underneath rotate_in_place)
    • You're welcome to change this signature as needed, but make sure you're using the Radians type to disambiguate the unit of the angle and RotationDirection to disambiguate between calls to arc_left and arc_right
    • For now, implement this method by approximating it with straight lines (see the code snippet at the start of this issue for a good starting point)
    • Use the move_forward and rotate_in_place methods on ProtocolClient
    • Add methods to Radians as needed
  • Documentation (only on Turtle::arc_left and Turtle::arc_right)
    • The bullets after Goal above give a good outline of the major points that need to be covered
    • See the documentation for forward, backward, left, and right for some examples how this could be structured
    • See the documentation of the circle function in the Python implementation for how they described it
    • (Optional) An example that illustrates the different values you can provide to this method would be helpful (example)
  • (Optional) Add an example to the examples/ directory that draws something cool using this feature
  • Test the method with different pen sizes
  • Make sure you add a check in ProtocolClient to see if the extent or radius are zero/inf
    if !angle.is_normal() {
    return;
    }
  • Mark the methods added to Turtle as unstable since we'll want people to try out the API before we decide to keep it

    turtle/src/drawing.rs

    Lines 396 to 397 in de9fa48

    #[cfg(feature = "unstable")]
    #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
  • Run the tests and generate the documentation with the commands listed in CONTRIBUTING.md
    • Please include a screenshot of the documentation in the PR

Part 2: Arc Animation

TODO: Please ask for these mentoring instructions when Part 1 has been completed.

Notes:


As always, please don't hesitate to questions if anything is unclear or needs more explanation!

@DeltaManiac

This comment has been minimized.

@PaulDance
Copy link
Contributor

@sunjay I'd like to help with this, I think it should be a good way to get familiar with the implementation details.
Looking at your comments, it seems that some things need to be reconsidered, should we try to explore possibilities together on Zulip?

@sunjay
Copy link
Owner Author

sunjay commented Sep 25, 2020

@PaulDance Great idea! I am not very available this week, but if you could ping me on or after Thursday next week I'd be happy to discuss it. I think I mainly just need to rewrite the mentoring instructions. If you want to poke around at it in the meantime, you may be able to figure out what to do by looking at my comments in #189.

@sunjay
Copy link
Owner Author

sunjay commented Oct 5, 2020

@PaulDance The instructions are updated now. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants