I’ve pondered over whether or not to suggest this class for the engine or not, but the more I use rotations the more it makes sense to avoid repeating the same code.
The class optionally forces the input angles to stay between negative and positive TWO_PI so that the angles don’t become ridiculously huge, and provides a convenience method to turn those angles into a quaternion to rotate scene objects. The get and set methods for each axis are also useful for binding to a GUI.
At first it appears to be a bit of “hand-holding” but I find myself repeating this code again and again and it seems a lot tidier and consistent to wrap the functionality into a class of its own. A lot of users get confused on how to “get and set” rotations, too - and I think this makes it a lot easier to understand. It enforces a standard of best practice that makes it difficult to deviate from and avoids them coming into contact with quaternions and getting/setting the values on them.
Please feel free to voice your opinion on the subject.
AxisRotation axisRotation = new AxisRotation(); // initialized to ( 0, 0, 0 )
AxisRotation axisRotation = AxisRotation.fromRadians(0, FastMath.HALF_PI, 0);
AxisRotation axisRotation = AxisRotation.fromDegrees(0, 90, 0);
float xInRads = axisRotation.getX();
float xInRads = axisRotation.get(Axis.X);
float xInDegs = axisRotation.getInDegreesX();
float xInDegs = axisRotation.getInDegrees(Axis.X);
// optional: don't force the angles to stay between -FastMath.TWO_PI and FastMath.TWO_PI
axisRotation.rotate(0, FastMath.HALF_PI, 0);
axisRotation.rotateInDegrees(0, 90, 0);
// apply the rotation to a scene object.
I see this being useful when you want to increment the current angle (eg a camera that rotates around a point) without going back and forth between toAngles/fromAngles or storing it in a global variable.
But i’m also not sure if this improves the current “manual” approach, that would be
You basically replace float with AxisRotation and * FastMath.DEG_TO_RAD with a dedicated setDegrees method.
Regarding the enforceRotationLimit, i don’t know if this is the desired behavior, but your code won’t work for values greather than FastMath.TWO_PI.
It will work, but it truncates the un-necessary size. If you rotated 365 degrees, it would return 5 degrees because the extra 360 degrees is un-necessary. One complete rotation + 5 degrees becomes just 5 degrees.
Where it won’t work is if you want to know how many times you’ve rotated - which is easily solved by adding a flag as to whether or not you wanted that behavior and bypassing the enforcement rule.
For 99% of applications involving 3-D rotations, I prefer quaternions to angles. Quaternions are much simpler for anything that doesn’t involve human input/output.
If you use angles, you should clearly document the order in which the rotations are applied and whether they are extrinsic or intrinsic. (There are 12 possibilities.) Similarly, all rotation methods should clearly document whether they rotate global axes or local ones.
I suggest AxisRotation be an add-on, not part of jme3-core.
UE4 returns a Vector3f which is interesting I guess.
Although we have .toAngles() and .fromAngles() it is the basic functionality of it.
I understand that JME is not a “feed you in small doses” engine, but I feel some areas such as this could do with a little help. I don’t feel so strong as to continue fighting the case, just explain that this is not an abnormal class to have in an engine.
Well, I hadn’t read the code so I was just going off the name which is very close to “AngleAxis” which is a form of rotation keeping using an angle and an axis… which is also what some of the sample code looked like it was doing. And in JME in 8 years, I’ve never kept a rotation this way.
In the end, knowing more about what it is, I think it’s likely a source for confusion more than anything else… current name or not. And maybe a long way to go to avoid writing rads % FastMath.PI. Also, even that’s going to require the user to sanitize pitch which more times than not is not rolled over but simply limited to +/- 90 degrees.
All to avoid some really simple code, to me.
But assuming others disagree and want to include this in the engine somewhere… some definite problems with the class:
the use of degrees at all. Encouraging users to keep things in degrees is going to be a huge source of bugs for them as they forget to convert back and forth in some other case. Angles should be in radians. Users should get used to that or quit math.
no way to have “non-quake camera” rotation ordering… though users that care about that probably don’t like your class anyway
no way to have truncated angles for things like pitch.
the automatic roll-over happens at two PI instead +/- 180 (PI)… which makes it unusable for anyone wanting to rotate back to center
As a snippet on the store or whatever, I think this is fine. Some users will certainly find this useful if for nothing else than seeing the math in one place.
Included in the engine, I’m sort of against it… but it would require a lot of changes to meet more use-cases. (Unusable in my own euler angle use-cases, for example.) Also, I think in real code, each call to this is only really replacing one properly written line of user code. So it might save them from typos in their math but not really reduce the size of their code any.
I concur with this. And after re-reading sgold’s comment, I will add to it that users already cling too hard to euler angles and need to ween themselves off of this. There is really only one use-case where keeping euler angles is appropriate but new users want to treat all rotations this way… at their own peril. Having a class that lets them will only encourage that.