Hi, when browsing trough the transform code i found a few lines that are not really when looking at them. I am talking about:
[java]
parent
.rot
.multLocal(translation)
.addLocal(parent.translation);
[/java]
The syntax suggests that you are executing the .multLocal() and the .addLocal() on the parent quaternion. (If you follow the usual jme way of dooing stuff)
I tought i found a serious bug, but why should nobody else notice a bug of that severity?
As it turns out Quaterion.multLocal(Vector3f) does not multiply the quaternion, but the passed Vector3f
From my point of view, a clearer an more conform way of dooing it is:
Move Quaterion.multLocal(Vector3f) to Vector3f.multLocal(Quaterion)
The code used in Transform.combineWithParent(Transform parent) would then be:
[java]
translation
.multLocal(parent.rot)
.addLocal(parent.translation);
[/java]
NOTE: itās not really a bug, but from my point of view a very misleading design decision. The downside is, that a clean solution would probably break lots of usercode
Hi, when browsing trough the transform code i found a few lines that are not really clear when looking at them. I am talking about:
[java]
parent
.rot
.multLocal(translation)
.addLocal(parent.translation);
[/java]
The syntax suggests that you are executing the .multLocal() and the .addLocal() on the parent quaternion. (If you follow the usual jme way of dooing stuff)
I tought i found a serious bug, but why should nobody else notice a bug of that severity?
As it turns out Quaterion.multLocal(Vector3f) does not multiply the quaternion, but the passed Vector3f.
From my point of view, a clearer an more conform way of dooing it is:
*Move Quaterion.multLocal(Vector3f) to Vector3f.multLocal(Quaterion)
The code used in Transform.combineWithParent(Transform parent) would then be:
[java]
translation
.multLocal(parent.rot)
.addLocal(parent.translation);
[/java]
NOTE: itās not really a bug, but from my point of view a very misleading design decision. The downside is, that a clean solution would probably break lots of usercode
//Sorry for doublepost, it seems i cant edit the first postā¦
I agree that the name is misleading by multiplying a Quaternion by a Vector3f in a way that modifies the quaternion in some way doesnāt really make sense. So the alternative is that it doesnāt mean what it seems to mean.
Quaternion operations should not be a part of Vector3f, though.
Itās kind of a silly method anyway since Quaternion.mult(v, v) will give the same results and is more explicit about what is happening.
Agree that quaternion operation should not be in the vector class.
I would mark multLocal as deprecteted in the stable branch, and use the logic from .mult(v3,v3). In the next major update that deprected stuff could be removed than.
Here is a patch for that:
[java]
This patch file was generated by NetBeans IDE
It uses platform neutral UTF-8 encoding and \n newlines.
ā Base (BASE)
+++ Locally Modified (Based On LOCAL)
@@ -969,18 +969,9 @@
* the vector to multiply this quaternion by.
* @return v
*/
@Deprecated
public Vector3f multLocal(Vector3f v) {
float tempX, tempY;
tempX = w * w * v.x + 2 * y * w * v.z - 2 * z * w * v.y + x * x * v.x
+ 2 * y * x * v.y + 2 * z * x * v.z - z * z * v.x - y * y * v.x;
tempY = 2 * x * y * v.x + y * y * v.y + 2 * z * y * v.z + 2 * w * z
* v.x - z * z * v.y + w * w * v.y - 2 * x * w * v.z - x * x
* v.y;
v.z = 2 * x * z * v.x + 2 * y * z * v.y + z * z * v.z - 2 * w * y * v.x
- y * y * v.z + 2 * w * x * v.y - x * x * v.z + w * w * v.z;
Just a noteā¦ the āthisā is unnecessary and I canāt think of a good reason conventionally to put it there.
Heh, i am used to it, or i have trained me to use it.
Since the time i use static imports i donāt see a difference between calls on a remote static object, or inner calls of the object. āthisā makes it more visible.
On the other side, beside the 5 to 2 keystrokes (depending on the IDE you use) i also donāt see a downside
@zzuegg said:
Heh, i am used to it, or i have trained me to use it.
Since the time i use static imports i don't see a difference between calls on a remote static object, or inner calls of the object. "this" makes it more visible.
On the other side, beside the 5 to 2 keystrokes (depending on the IDE you use) i also donāt see a downside
Itās the most common way to call a methodā¦ and the only reason purported to use āthisā is to avoid confusion with static method imports which many conventions would just specifically disallow rather than pollute the rest of the code with redundant āthisā.
Not to mention that the static imports are easily located in the rarer case that they are used.
Yeah, i agree that itās the most common way to call a method. But itās most likely a leftover convention from then non OO time.
I mean, the systax for executing something in an OO environment would be something like:
[java]
(some object reference).(some method/object/variable reference)
[/java]
Allowing a method call without āthisā probably requires an additioanl syntax rule. I personall use not āthisā calls only when there is no other way. Like static methods and method local variables.
I also think that using this everywhere is the saver way of dooing stuff. Immagine:
[java]
class MyFancyObject{
int x;
//some hundred lines of code
public void myFancyMethod(){
//lotās of fancy calculations;
x=resultOfFancyCalculation;
}
}
[/java]
Now a year later you get fired and they hire a cheap junior instead. He browses trough the code, finds the fancy calculation routine and tries to make some optimisations. If he adds a tempory variable āint xā somewhere in his code he would break the result.
My point is, that from my side it really has no downsides using this. everywhere, but it has some real slight advantagesā¦ beside better readability.
For the longest time I didnāt use āthisā for method calls since it was unambiguous anyway, but Iāve changed my ways to using it all the time. It is more consistent I feel.
For variables Iāve always used āthisā since those could clash with method scoped variables and using āthisā was better than inventing some name-mangling to show if it was a member variable or method scope variable. But since static imports methods have the same sort of confusion. So why not be consistent all over.
Another, silly reason but still, is that I get command completion in the IDE when I type āthis.ā
Same thing with āfinalā, Iāve totally come around on that and write final all over the place. The code is more verbose but it also tells more about the assumptions Iāve made.
@zzuegg said:
Now a year later you get fired and they hire a cheap junior instead. He browses trough the code, finds the fancy calculation routine and tries to make some optimisations. If he adds a tempory variable "int x" somewhere in his code he would break the result.
Except that if he uses a decent IDE, he'll have a warning like "local variable hides a field".
Idk, I only use "this" when I can't do otherwise too...but I guess that's a matter of taste at some point.
@nehon said:
Except that if he uses a decent IDE, he'll have a warning like "local variable hides a field".
Idk, I only use "this" when I can't do otherwise too...but I guess that's a matter of taste at some point.
Likewise, when I see āthisā I then look for what Iād be hiding (parameter or whatever) if I didnāt. Otherwise, leave it off.
See I have my autofromatter set to always convert varaible to full qualified versions. So always this and always Class.staticvar
But yes matter of style.
What is kinda necessary for jme3 is a code convention for developers&contributors, + a full reformating of existing code to comply with it.
For some classes you have 3 different styles in then, depending on who did the last patch ^^
