How should the speed levels in the turtle crate work?

[sunjay]

The Python turtle module has 10 speed levels. The turtle crate started with 10 speed levels, but after getting impatient with how slow many drawings were, I changed that to 25 speed levels.

Recently, while working on the new architecture in #173, I realized that because of physics (time = distance / speed) and because of IPC latency, there is a limit to how much increasing the speed can decrease the time it takes to draw an image:

Image generated with data collected from examples/speedtest.rs before the changes in #173

As you can see, after level 10, changing the speed impacts the time less and less. This is frustrating as a user because it means that 15 of the speed levels are pretty much useless. In #173, I rewrote the code so that changing the speed has a linear effect on the time:

Image generated with data collected from examples/speedtest.rs after the changes in #173

I thought this would be better, but it turns out to be even worse because now there is barely any visible difference between speed levels. The difference is always the same, but that turns out to not be all that intuitive because I guess our brains expect the time = distance / speed equation from physics.

Proposals

Proposal 1: We should go back to having 10 speed levels, just like the Python turtle module. We should set the speed to increase linearly and go back to time = distance / speed. We can play with the graphs shown above until we get a curve that provides a reasonable distribution of different times. Having two speed levels that result in almost the same time isn't very useful.

Proposal 2: Instead of a hardcoded set of 10 levels, the speed should become an actual quantity in pixels / second (or some other unit). That way the user is free to set their speed to whatever they want and there will be no limits. I like this idea because of the flexibility it provides, but it will be breaking compatibility with the Python turtle module and other turtle implementations. (Compatibility isn't an explicit goal, but it is nice to have.) It's also important to note that the speed value is used for rotation too, so we may have to think about how to map a speed to both px / sec and rad / sec.

So far, I am leaning towards Proposal 2. I think it's better to make that kind of breaking change now and give users the most flexibility. We will still have the speed strings (e.g. "slow", "normal", etc.). That means that there is an easier to teach option in addition to all the flexibility. We may even be able to come up with something to make that proposal backwards compatible so we would have both the speed levels and the ability to specify an absolute speed in px / sec or rad / sec.

Thoughts on as well as alternative proposals for what we should do are welcome.

Questions

These are questions that need to be resolved by the end of this:

• [ ] What are the speed levels (if any)?
• [ ] What speeds in px / sec do the speed levels map to?
• [ ] What speeds in rad / sec do the speed levels map to?
• [ ] What speeds do "slow", "normal", "fast", "faster", etc. map to?

The code, examples, and documentation then need to be updated with what we decide.

[PaulDance]

Hey @sunjay, I think it is indeed a good idea to overhaul this mechanism now, in my opinion for multiple reasons:

• The concept of "speed" should mean what it means, not an ETA.
• Levels introduce a counter-intuitive and non-necessary gradated wrapper around the actual speed.
• Sometimes, the highest level is not fast enough.

As you can see, I'm obviously leaning towards the second proposal as well. Then supposing we go for that and in order to reach maximum flexibility, here is what we could do:

1. Refactor the set_speed method so it accepts a value of either:
   • A linear speed in px/s mapped automatically to a rotational speed using an arbitrary function.
   • A couple of values where the first one is the linear speed and the second one the rotational speed.
2. Introduce new methods, say set_lin_speed and set_rot_speed, that enable the user to configure both linear (px/s) and rotational (rad/s) speeds independently. Corresponding getters should also exist.
3. Map the "normal", "fast", "faster", etc. levels to arbitrary values.

If keeping the current speed levels in order to reduce user changes required is desirable, then we could add a set_speed_level method, with a corresponding getter, that uses the current mapping to actual speeds. However, that may add more implementation complexity than anything else.

Also, only the linear-to-rotational automatic mapping is proposed for the set_speed method, as the current method accepts anything that may be turned Into<Speed> and a trait variant may not be implemented twice for the same type: vice-versa is just a floating-point number as well. If having the reverse automatic mapping is considered useful, then we could introduce an enumeration that enables differentiating between the two dimensions of speed, such as:

enum SpeedKind {
    Linear(f64),
    Rotational(f64),
}

This way, we could keep From<f64> for Speed as the linear-to-rotational default mapping, but also provide From<SpeedKind> for Speed so the user may specify explicitly which dimension is to be decided automatically. The first trait implementation would of course use the second one.

We could go even further by letting the user provide the arbitrary function used to convert from linear to rotational speeds and vice-versa, while still keeping a default one. That may be a bit too much however, considering the scope of this issue.

All these proposed changes would require breaking a lot of things and overhauling most of the Speed type, so it is of course subject to discussion. As a result, default conversion functions are not specified here, they should be discussed as well, but only when we decide to go forward with this proposal, if we ever do.

[sunjay]

Thanks for providing your thoughts @PaulDance. I definitely see the logic behind your proposals and I share your concerns about overcomplicating this API.

Overall, I am okay with making breaking changes here. We don't need to have integer speed levels. LOGO has no concept of speed (lines are drawn instantaneously). This was a concept added by the Python implementation. It's better to make these kinds of changes now. I'm glad we're discussing it.

Here's a proposal that merges both of our ideas, but makes it far less flexible in an attempt to keep it simple:

• We change set_speed to take one of three types:
  • f64 - a positive (> 0) value with an unspecified unit (must be normal)
  • "slow", "normal", "fast", "faster", etc. - map to reasonable arbitrary values that we choose (and are unspecified in the documentation)
  • "instant" or Speed::instant() - for the "instant" speed
• Passing an integer will now produce a compilation error (helpful for people updating from the previous speed levels)
• The Speed type will still be opaque and it will be impossible to get a value out of it (other than checking is_instant)
  • I am open to changing my mind about this, but it is probably better to be overly conservative in the beginning and add more flexibility later if we need it

Leaving things unspecified gives us some opportunity to change the inner implementation details as long as we don't change any of the behaviour of the resulting program. Having only one value but not specifying its unit allows us to choose a reasonable mapping to both translation speed and rotation speed.

I don't think most users will want to care about specifying precise/different values for translation and rotation speed. If they do, this API leaves the option open for us to provide another type later that allows them to do that. e.g. a SpeedLevels {translation: f64, rotation: f64} type could implement Into<Speed>.

As for the question of how we decide the mapping between speed value and translation/rotation speed: the goal should be for the translation and rotation speed to "feel" roughly the same. That is, if I set the speed to 100.0, the turtle should not move significantly faster than it rotates. To that end, I propose:

• The speed value provided to set_speed should be scaled so the user doesn't have to provide an extremely large value just to get the turtle moving at a reasonable speed
• Translation Speed: The scaled value maps to 1 px/sec where px is a logical pixel (as opposed to a physical pixel)
• Rotation Speed: The scaled value maps to x rad/sec where x is the number of radians it takes to rotate the tip of the turtle's shell by 1 px (circumference)

This creates a notion of "moving by 1 px" and "rotating by 1 px" which I am hoping will turn out to be quite intuitive and fulfill the goal stated above. We can always figure out something else if this doesn't work.

---

Unfortunately, the speed API is so important that we don't have the luxury of marking set_speed as unstable and experimenting with it. Whatever we choose up until the v1.0 release will get stabilized and end up being the API for the considerable future. Thanks for taking the time to think about this with me!

[PaulDance]

I think most of what you proposed is a good idea, but also that some details will reveal themselves upon implementing the changes.