This is a rather "bloggy" entry, but I want to share it with you. Of course this is simple stuff and totally uninteresting for the advanced jME guru. But maybe newbies can learn something from it, like I did.
Even if your class "works", you shouldn't stop improving it. With a little bit effort you can make your code so much cleaner and better. This is an easy example of a class I probably browsed through a hundred times and never took the time to take a closer look:
package jmetest.monkeymahjongg.game;
import com.jme.input.KeyInput;
import com.jme.input.controls.GameControl;
import com.jme.input.controls.GameControlManager;
import com.jme.input.controls.binding.KeyboardBinding;
import com.jme.math.Quaternion;
import com.jme.scene.Controller;
/**
*
* @author Pirx
*/
class CameraController extends Controller {
private static final long serialVersionUID = 1L;
private final static float MIN_ANGLE = 1.2f;
private final static float MIN_DISTANCE = 35f;
private final static float MAX_DISTANCE = 150f;
private final static float SPEED = 2f;
private float vAngle = 0;
private float hAngle = 0;
private float distance = 100;
private GameControl left;
private GameControl right;
private GameControl up;
private GameControl down;
private GameControl forward;
private GameControl backward;
private CameraGameState gameState;
public CameraController(CameraGameState gameState) {
this.gameState = gameState;
GameControlManager manager = new GameControlManager();
left = manager.addControl("left");
left.addBinding(new KeyboardBinding(KeyInput.KEY_LEFT));
right = manager.addControl("right");
right.addBinding(new KeyboardBinding(KeyInput.KEY_RIGHT));
up = manager.addControl("up");
up.addBinding(new KeyboardBinding(KeyInput.KEY_UP));
down = manager.addControl("down");
down.addBinding(new KeyboardBinding(KeyInput.KEY_DOWN));
forward = manager.addControl("forward");
forward.addBinding(new KeyboardBinding(KeyInput.KEY_PGUP));
backward = manager.addControl("backward");
backward.addBinding(new KeyboardBinding(KeyInput.KEY_PGDN));
}
public void update(float time) {
//System.out.println("rot:" + gameState.getCameraRotationNode().getLocalRotation());
//System.out.println("trans:" + gameState.getCameraDistanceNode().getLocalTranslation());
float newHAngle = hAngle + SPEED * time
* (right.getValue() - left.getValue());
if (-MIN_ANGLE < newHAngle && newHAngle < MIN_ANGLE) {
hAngle = newHAngle;
}
float newVAngle = vAngle + SPEED * time
* (down.getValue() - up.getValue());
if (-MIN_ANGLE < newVAngle && newVAngle < MIN_ANGLE) {
vAngle = newVAngle;
}
gameState.getCameraRotationNode().setLocalRotation(new Quaternion(new float[] { vAngle,
hAngle, 0f }));
float newDist = distance + 20 * SPEED * time
* (backward.getValue() - forward.getValue());
if (MIN_DISTANCE < newDist && newDist < MAX_DISTANCE) {
distance = newDist;
}
gameState.getCameraDistanceNode().setLocalTranslation(0, 0, distance);
}
}
Today I finally realized how poorly this class was written. This is especially bad as the MonkeyMahjongg project is intended to help newbies, not to confuse them.
Hmmm, what is wrong here? First of all, we do the same crap for GameControls again and again. Oh, and by the way, we can get the GameControls from the GameControlManager (that's the point of having names when adding it). Let's write a function for this:
private void bind(String name, int... keys) {
final GameControl control = manager.addControl(name);
for (int key : keys) {
control.addBinding(new KeyboardBinding(key));
}
}
Of course we need to have the GameControlManager as class member now, but we can throw the GameControl members away.
But somehow the Strings as controller names stink, so I replaced them by an enum:
enum Direction {LEFT, RIGHT, UP, DOWN, FORWARD, BACK};
...
private void bind(Direction direction, int... keys) {
final GameControl control = manager.addControl(direction.name());
for (int key : keys) {
control.addBinding(new KeyboardBinding(key));
}
}
Hmmm, how we get the values of a certain game control? OK, new function:
private float value(Direction direction) {
return manager.getControl(direction.name()).getValue();
}
Now to the update function. It works, but it's a mess. We calculate temporary values (like newHAngle), look if they are in range, and then copy them to the original value (like hAngle). That's stupid. I remembered there is a clamp function in FastMath, tried it out and the temporary variables were gone:
//too lazy to type
import static com.jme.math.FastMath.clamp;
import static jmetest.monkeymahjongg.game.CameraController.Direction.*;
...
float way = SPEED * time; //we need this several times
hAngle = clamp(hAngle + way * (value(RIGHT) - value(LEFT)), -MIN_ANGLE, MIN_ANGLE);
That's much better! Now the quaternion: It has an ugly constructor using a float array. Why? A short look in the jME source told me that it just sets the Euler angles using Quaternion.fromAngles, nothing else. So we can do this instead:
gameState.getCameraRotationNode().getLocalRotation().fromAngles(vAngle, hAngle, 0f);
Nice, no more Quaterion object creation needed.
All together, the class looks now like this:
package jmetest.monkeymahjongg.game;
import com.jme.input.controls.GameControl;
import com.jme.input.controls.GameControlManager;
import com.jme.input.controls.binding.KeyboardBinding;
import com.jme.scene.Controller;
import static com.jme.input.KeyInput.*;
import static com.jme.math.FastMath.clamp;
import static jmetest.monkeymahjongg.game.CameraController.Direction.*;
/**
*
* @author Pirx
*/
class CameraController extends Controller {
private static final long serialVersionUID = 1L;
enum Direction {LEFT, RIGHT, UP, DOWN, FORWARD, BACK};
private final static float MIN_ANGLE = 1.2f;
private final static float MIN_DISTANCE = 35f;
private final static float MAX_DISTANCE = 150f;
private final static float SPEED = 2f;
private final GameControlManager manager = new GameControlManager();
private final CameraGameState gameState;
private float vAngle = 0;
private float hAngle = 0;
private float distance = 100;
public CameraController(CameraGameState gameState) {
this.gameState = gameState;
bind(LEFT, KEY_LEFT);
bind(RIGHT, KEY_RIGHT);
bind(UP, KEY_UP);
bind(DOWN, KEY_DOWN);
bind(FORWARD, KEY_PGUP, KEY_ADD);
bind(BACK, KEY_PGDN, KEY_SUBTRACT);
}
private void bind(Direction direction, int... keys) {
final GameControl control = manager.addControl(direction.name());
for (int key : keys) {
control.addBinding(new KeyboardBinding(key));
}
}
private float value(Direction direction) {
return manager.getControl(direction.name()).getValue();
}
public void update(float time) {
final float way = SPEED * time;
hAngle = clamp(hAngle + way * (value(RIGHT) - value(LEFT)), -MIN_ANGLE, MIN_ANGLE);
vAngle = clamp(vAngle + way * (value(DOWN) - value(UP)), -MIN_ANGLE, MIN_ANGLE);
gameState.getCameraRotationNode().getLocalRotation().fromAngles(vAngle, hAngle, 0f);
distance = clamp(distance + 20 * way * (value(BACK) - value(FORWARD)),
MIN_DISTANCE, MAX_DISTANCE);
gameState.getCameraDistanceNode().setLocalTranslation(0, 0, distance);
}
}
That's short! And quite readable. The static imports are certainly a matter of taste (note that I had to made the Direction enum "package private" in order to get the static import working), but the enum itself is not: I got bitten often enough when using Strings as name tags. Just don't do it.
Another thing I should mention is that I made the member variables final when their references don't change. This is always good idea: For example you can see with one look that manager will be always the same old instance for the whole lifetime of the CameraController object. You can't make any mistakes when you try to add a final modifier, as the compiler (or the IDE) will point out when it is not possible to use it.
So what is the message here? Is there any? In fact there are several ones:
- DRY = Don't repeat yourself [A.k.a: Three times is a pattern] (GameControls)
- Know your libraries (clamp, Quaternion)
- static import can save you a lot of typing
- don't use Strings as name tags (i.e. use enums instead)
- make member variables final if possible
Refactoring your classes makes them more readable, more robust and maybe faster. You can also learn how to write new classes better from the very beginning.
I hope you enjoyed (and not only my funny English)!