Quaternion.normalize() normalizes the "current" Quaternion.
I feel that .normalizeLocal() is more aligned with JME naming scheme.
This involves changing other 14 references and deprecating Quaternion.normalize(). If ever implemented, its signature would be different but the change would not cause existing code to fail, so this it should remain deprecated for a good long time (maybe forever in JME 2).
What do you guys think?
(I'd also check other Math methods and see if they are named correctly in terms of the Local suffix naming pattern).
Quaternion.negate() should also be Quaternion.negateLocal().
However, Quaternion.inverse() creates a new object and Quaternion.inverseLocal() is named as expected.
I am happy to review this if you are happy with deprecation and pretty massive code changes. I'm quite confortable at refactoring so this shouldn't be a big deal though.
Contrib Depot!
The problem is that other objects are using a New suffix as a solution to this problem.
For example, Matrix3f has transpose() and transposeNew() methods. But here, transpose() operates locally and transposeNew() transposes to a new Matrix3f.
Now I see no solution, other than documentation :(.
I wonder if these shouldn't be changed in the 2.0.x branch (although there has been very little activity on it)…
Here's my crazy proposal: get rid of the XXX() methods and rename all the XXXLocal() methods to XXX(). Make it explicit (or as explicit as reasonable) when objects are being created.
Now, OK, this would be a huge change, but here me out. I've been playing around with a private branch/fork of JME (mainly for educational purposes and as a kind of code playground) and this is one of the first changes that I made, and I've found that it uncovered a lot of sloppy code related to maths classes. Lots of unnecessary garbage creation.
If anyone wants to agree with me on this it may be something that we could discuss for V3.0 ?
i think someone should just change all of the inconsistencies (Quaternion, Matrix3f, …) without deprecating, just rename the methods, put it in the CD and check in to branch 2.0.x.
Why branching? Merging it later would be very painful. It's way better to make changes to the trunk, but that would have a huge impact on people's code.
Also, we would still need to agree on naming convention.
Personally I think we should go for "xxxNew" and "xxxLocal" for all of them…
I am not a fan of such naming normally but I think it's justified given the current situation and the impact of these methods. Also, this wouldn't break as much code as the other approach and at least wouldn't cause semantic changes. This would also fit correctly methods already called "transposeNew" and "normalizeLocal".
Again, in my opinion these are huge changes. If we agree in petit-comite, we still need to propose a poll and make sure enough people votes.
the way it usually goes:
once trunk is stable -> create branch 2.1 -> release 2.1 jars
- only merge bugfixes to branch no new features.
continue to develop in trunk
once trunk is stable -> create branch 2.2 -> release 2.2 jars
- only merge bugfixes to branch no new features.
So i guess it would be a good idea to create a branch now, then those changes can be made in trunk.
To see what possible code breaks between versions, one should be able to look in the not yet existing changelog
edit: link to changelog
An idea I got right now is that we should change the naming convention and only have these two instead as example:
// note, this was named addLocal before
public Vector3f add(Vector3f vec) {
x += vec.x;
y += vec.y;
z += vec.z;
return this;
}
// static when it's supposed to create a new vector
public static Vector3f add(Vector3f v1, Vector3f v2) {
return new Vector3f(v1.x+v2.x,v1.y+v2.y,v1.z+v2.z);
}
But I have not thought about it much so it might have downsides… But I have had a lot of bugs by forgetting to name my methods local. But thankfully they are easy to detect.