turtle Add arc method to Turtle

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:

https://github.com/sunjay/turtle/blob/cc40975c8b8d82dee2fdde0900e4516ff637a02b/examples/snowman.rs#L191-L201

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
- We have the opportunity to do better and draw a precise curve using the arc method on pathfinder_canvas
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) https://github.com/sunjay/turtle/blob/cc40975c8b8d82dee2fdde0900e4516ff637a02b/src/turtle.rs#L191-L193
[ ] 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) https://github.com/sunjay/turtle/blob/cc40975c8b8d82dee2fdde0900e4516ff637a02b/src/async_turtle.rs#L88-L91
[ ] 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 https://github.com/sunjay/turtle/blob/cc40975c8b8d82dee2fdde0900e4516ff637a02b/src/ipc_protocol/protocol.rs#L355-L357
[ ] Mark the methods added to Turtle as unstable since we'll want people to try out the API before we decide to keep it https://github.com/sunjay/turtle/blob/de9fa48c5a4af7af4d63f25161018424480c4ab8/src/drawing.rs#L396-L397
[ ] 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:

Probably a good idea to implement this without animation first and then add the animation later (gets all the protocol stuff out of the way)
- Fine to break the method temporarily since it is unstable anyway and the animation isn't that important
See https://github.com/sunjay/turtle/blob/cc40975c8b8d82dee2fdde0900e4516ff637a02b/src/renderer_server/animation.rs
We are essentially animating the parameters of arc in pathfinder_canvas
We may or may not need to linearly interpolate a circular arc

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

May 18 '18 05:05 sunjay

Hey @sunjay

Would the following implementation work for drawing an arc?

 pub fn arc(&mut self, radius:Distance, extent:Option<Angle>) {
        let angle = extent.unwrap_or(360.0);
        let arc_length = 2.0 * PI * radius * angle.abs() / 360.0;
        let step_count = (arc_length/4.0).round() + 1.0;
        let step_length = arc_length / step_count;
        let step_angle = angle / step_count ;
        for _ in 0..step_count as i32 {
            self.forward(step_length);
            self.right(step_angle);
        }
    }

I did a quick test locally and it does work except for when the radius < 0, i would prefer to enforce the radius > 0 condition.

Another thing we could do is consider the absolute value of the radius for calculation.

Sep 11 '18 15:09 DeltaManiac

@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?

Sep 25 '20 15:09 PaulDance

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

Sep 25 '20 15:09 sunjay

@PaulDance The instructions are updated now. 🎉

Oct 05 '20 18:10 sunjay