Refactor Sprite rotation function names

[cugone]

The rotation functions in the Sprite class have poor and potentially confusing names and arguments.

Refactoring as

Sprite
{
//...
    void rotation(float angleDegrees) const;
    float rotationDegrees() const;
    float rotationRadians() const;
//...
};

is much more clear and less confusing.

[DanRStevens]

You should provide a bit more context on this. I assume you're talking about the Sprite class. This also relates to Renderer.

---

One concern I have here is the inability to overload based on angle type if they are both float. In your example, you have both rotationDegress and rotationRadians to account for that, while the other direction only accepts a single measure type.

Maybe what we really want is an Angle type, that can be initialized by factory functions for either measure, and convert to either measure. Then there would be no need for overloads to handle either measure type. You could pass Angle to any function that needs angles, and the measure of the angle would be specified unambiguously when the type is constructed.

[cugone]

Forcing the user to stick to one measurement unit, degrees or radians, from outside the class is less error prone. If the internals expect something else they can be converted.

For non-mathy types, like Sprite, specifying the angle in degrees is more clear. (Really, unless otherwise known and expected to be in radians like std::sin, etc., I would prefer every function take in an angle argument as Degrees; however, it's way more clear to name the variable angleDegrees anyway).

Mathy-types can have a degrees implementation for ease-of-use like so:

float CosDegrees(float degrees)
{
    const auto radians = MathUtils::ConvertDegreesToRadians(degrees);
    return std::cos(radians);
}

float SinDegrees(float degrees)
{
    const auto radians = MathUtils::ConvertDegreesToRadians(degrees);
    return std::sin(radians);
}

float Atan2Degrees(float y, float x)
{
    const auto radians = std::atan2(y, x);
    return MathUtils::ConvertRadiansToDegrees(radians);
}

[DanRStevens]

I think my point may have been missed. An Angle is a concept that can be represented as either degrees or radians, and those two representations are equivalent. This is a bit like how a point can be specified as either Cartesian coordinates or polar coordinates. They can be freely converted between each other. You can have an Angle class that can be initialized from either degree or radian data, and to return values in either degree or radian form. This is like how a point class can be initialized in either Cartesian form or polar form, and return equivalent data in either form. The internal format doesn't matter from the outside in terms of correctness. It might only matter in terms of efficiency, and how often conversions are performed. An Angle might be most effectively represented internally with radians if it's being used a lot in formulas, much like how a point might be most efficiently stored in Cartesian coordinates when used for 2D raster images. Either can be displayed in an alternative form for user display, depending on the appropriateness of the context.

Example:

renderer.drawImageRotated(image, Angle::FromDegrees(90));
renderer.drawImageRotated(image, Angle::FromRadians(Math::Pi / 2));

Example:

float SinDegrees(Angle angle)
{
    return std::sin(angle.radians());
}

[cugone]

That seems to be adding undue complexity of an entire class where there doesn't need to be; as well as punting the conversion to the user where it could easily be used incorrectly. Just by looking at the examples, I couldn't tell you what Angle represents.

[DanRStevens]

That's the point though. You don't need to know the internal representation of Angle. Users of Angle will know what measure they want, and request the correct type. Providers of angles will know what they have, and can create from the available type.

It also avoids having to create overloads for different angle types. Not that you could create overloads with either this nor the previous idea, since there is only one type, and hence no type distinction to provide an overload. I suppose I should say there's no need for near duplicate functions for every consumer of Angle values.

---

At any rate, the simpler solution is to just not worry about different angle measures and define only one correct type, and try to use that everywhere. Internally, I think radian measure makes the most sense. It's the natural measure of angles, and allows for the highest efficiency of use. For user input, degree makes sense, and can be converted on the fly when loading data, or when presenting data to the user.

What I don't want, is to provide multiple functions for every possible consumer of angles.

[cugone]

At any rate, the simpler solution is to just not worry about different angle measures and define only one correct type, and try to use that everywhere. Internally, I think radian measure makes the most sense. It's the natural measure of angles, and allows for the highest efficiency of use. For user input, degree makes sense, and can be converted on the fly when loading data, or when presenting data to the user.

What I don't want, is to provide multiple functions for every possible consumer of angles.

I totally agree. User-facing should be degrees (again, except for the trig functions, i.e. sin/cos/tan/atan because those are designed from C++ to be in radians). Internally we should decide on one and stick with it. Mathematically, radians would be better; but degrees are easier to understand.

[ldicker83]

I'm a little torn here as I'm not sure if I like the idea of an Angle class, though considering the direction we've taken with NAS2D so far, it would make sense. The user can specify their Angle in whatever unit they want and internally functions that use Angle would handle it in whatever way is most efficient.

At the moment all angle parameters are generally expected to be in degrees. That's was the easiest way for me to think about it so that's how the functions ultimately handle them.

Going with the example in the first post, I wonder if it would make sense to expect all functions/parameters in degrees but offer a stand-alone function that can convert to radians as needed? Now that I think about it those functions already exist in the Trig.h file. It's a bit crude but it suited my needs at the time.

[DanRStevens]

I think that solution is still suitable, and is the one I'm leaning towards. Have just a single correct way to specify angles, and separate helper methods to convert between angle measures. No need to start adding overloads everywhere to handle either angle measure when it can just be a separate wrapped call. That's basically what we already have, and I'm fine with that.