Changes to Quaternion, with diff

wezrule · May 18, 2012, 6:01pm

I noticed there was a depreciated method normalize(), which stated that it didn’t follow practices and did the exact same as normalizeLocal(). First of all I fixed it (maybe) and there was a depreciated reference to normalize() inside, so if you were going to get rid of the function it would break, so i changed it. I also made them both (normalize() and normalizeLocal()) return a Quaternion instead of void, which goes in line with the other methods.

Also whats the proper way to do these patches? At the moment i’m just copying and pasting them into my folder, making the changes and applying the diff tool, i can’t test it or anything. Thanks

[patch]

— com/jme3/math/Quaternion.java in C:UsersWesleyAppDataRoaming.jmonkeyplatform3.0betalibsjMonkeyEngine3-sources.zip

+++ C:UsersWesleyDocumentsjMonkeyProjectsBasicGamesrcQuaternion.java

@@ -271,7 +271,7 @@

y = (sinRollXcosPitch * cosYaw + cosRollXsinPitch * sinYaw);

z = (cosRollXsinPitch * cosYaw - sinRollXcosPitch * sinYaw);

```
 normalize(); 
```

```
 normalizeLocal(); 
```

return this;

}

@@ -1082,27 +1082,25 @@

/**

<code>normalize</code> normalizes the current <code>Quaternion</code>

* @deprecated The naming of this method doesn't follow convention.<br />

* Please use {@link Quaternion#normalizeLocal() } instead.<br />

* The result is returned as a new quaternion.<br />

*/

@Deprecated
public void normalize() {

public Quaternion normalize() {

float n = FastMath.invSqrt(norm());

```
 x *= n; 
```
```
 y *= n; 
```
```
 z *= n; 
```
```
 w *= n; 
```

   return new Quaternion(x * n, y * n, z * n, w * n);<br />

}

/**

<code>normalize</code> normalizes the current <code>Quaternion</code>

```
* The result is stored locally. 
```

*/

public void normalizeLocal() {

public Quaternion normalizeLocal() {

float n = FastMath.invSqrt(norm());

x *= n;

y *= n;

z *= n;

w *= n;

+
```
 return this; 
```

}

[/patch]

normen · May 18, 2012, 7:57pm

Just open a checkout of jme3 as a project then you can run it to test etc.

Momoko_Fan · May 19, 2012, 9:55pm

I am getting “Failed to apply patch in the given context” error. Are you sure you’re using the SVN version of jME3 for this?

wezrule · May 20, 2012, 8:28am

No i wasn’t :P, i have followed this and it seems to be up to date now:

https://wiki.jmonkeyengine.org/legacy/doku.php/jme3:build_jme3_sources_with_netbeans

Quaternion.java

[patch]

— Base (BASE)

+++ Locally Modified (Based On LOCAL)

@@ -1,5 +1,5 @@

/*

- Copyright (c) 2009-2010 jMonkeyEngine

- Copyright (c) 2009-2012 jMonkeyEngine

All rights reserved.

*
Redistribution and use in source and binary forms, with or without

@@ -1082,27 +1082,25 @@

/**
<code>normalize</code> normalizes the current <code>Quaternion</code>

* @deprecated The naming of this method doesn't follow convention.<br />

* Please use {@link Quaternion#normalizeLocal() } instead.<br />

* The result is returned as a new quaternion.<br />

*/

@Deprecated
public void normalize() {

public Quaternion normalize() {

float n = FastMath.invSqrt(norm());

```
 x *= n; 
```
```
 y *= n; 
```
```
 z *= n; 
```
```
 w *= n; 
```

   return new Quaternion(x * n, y * n, z * n, w * n);<br />

}

/**

<code>normalize</code> normalizes the current <code>Quaternion</code>

* The result is stored interally.<br />

*/

public void normalizeLocal() {

public Quaternion normalizeLocal() {

float n = FastMath.invSqrt(norm());

x *= n;

y *= n;

z *= n;

w *= n;

+
```
 return this; 
```

}[/patch]

Had to modify FlyByCamera

FlyByCamera.java

[patch]

— Base (BASE)

+++ Locally Modified (Based On LOCAL)

@@ -279,7 +279,7 @@

Quaternion q = new Quaternion();

q.fromAxes(left, up, dir);

```
 q.normalize(); 
```

```
 q.normalizeLocal(); 
```

cam.setAxes(q);
}
[/patch]

pspeed · May 20, 2012, 8:51am

The problem I have with this patch is that it silently breaks all code that uses normalize() in the old way.

You would first have to go through some versions where normalize() is completely removed to make everyone convert to normalizeLocal(). Otherwise, the bugs this causes may be extremely hard to track down.

wezrule · May 20, 2012, 12:01pm

Yeh I removed normalized() from Quaternion to find which other classes used it, but the only one i found was FlyByCamera.java

pspeed · May 20, 2012, 3:51pm

@wezrule said:
Yeh I removed normalized() from Quaternion to find which other classes used it, but the only one i found was FlyByCamera.java

It's all of the user code that uses it that I'm worried about. The strange bugs that will take 10 posts back and forth to trace it back to a use of normalize() somewhere.

wezrule · May 20, 2012, 4:26pm

yeh I can fully understand that. It was marked depreciated though as it didn’t follow the specification, so hopefully people stopped using it. I’m just trying to make things more consistent

pspeed · May 20, 2012, 4:29pm

@wezrule said:
yeh I can fully understand that. It was marked depreciated though as it didn't follow the specification, so hopefully people stopped using it. Im just trying to make things more consistent :)

But the next step in deprecation is usually removal... not completely changing the implementation and breaking all usage.

wezrule · May 20, 2012, 4:48pm

ok, you can definitely apply the patch to FlyByCamera and to the date of Quaternion

and maybe the one to normalizeLocal() to return the Quaternion object

Momoko_Fan · May 20, 2012, 7:42pm

Okay I applied the patch except the normalize() part which was removed entirely.