Controller vs. Action

Hi there, I’m back again



I tried to write a test prog, that shows a disk spinning around its center y axis and which can be moved around by cursor keys.



I got it working, but I do have one issue here.



I let the disk spin by writing a Controller. Basically in this controller I take the time passed to the update method and then do the calculation of the rotation angle, like here:


/**
 * @see com.jme.scene.Controller#update(float)
 */
public void update(float time) {
    Quaternion rotation = new Quaternion();
    lastAngle += time * 2;
    rotation.fromAngleAxis(lastAngle, new Vector3f(0, 1, 0));
    node.setLocalRotation(rotation);
}


where lastAngle is an instance variable.
This works, as long as no KeyActions are in the code. Because when I attach, say, KeyNodeForwardAction and then I press the key, this action calls node.updateWorldData, which in turn calls my node controller too, so the rotation speed gets doubled (or even more, depending how many keys I press). I can easily come around that problem by rewriting the above method like


public void update(float time) {
    Quaternion rotation = new Quaternion();
    float absTime = (float)timer.getTime() / (float)timer.getResolution();
    lastAngle = (absTime * 2);
    rotation.fromAngleAxis(lastAngle, new Vector3f(0, 1, 0));
    node.setLocalRotation(rotation);
}


thus passing the Timer to the controller and using the absolute time, but what all of this really means to me is that the parameter "time" of the "update" method in controller loses its meaning of "time since last frame", because you cannot predict how often this method gets called for the current frame.

Am I totally wrong and missing the point here or is this worth consideration?

You’re absolutely right, the actions shouldn’t be calling updateWorldData. I think I originally placed the call there because it wasn’t certain that updateWorldData would be called each frame, but it should be, and so it shouldn’t be in the action.



I’ll clean up all the actions and commit. In the meantime, if you are working from CVS, remove that method call from your local version.

Thanks again for the quick response.



I am indeed working from CVS, because looking into the sources helps me learn how to use jME best, so I don’t flood this forum with too many too stupid questions…

is that why VZ1 was faster when you pressed a button??

Yeah, that would make sense.

Ok, I’ve removed all updates from actions. One thing to note, if you are using a camera node and a node controller: Be sure to add the camera node to the scene, otherwise, updates won’t be processed.

I bumped into a slightly different issue now.

I’d like to have an input action which gets called for different keys. That means, I’d like an action, say AvatarMoveAction, which I can register to ALL movement keys at the same time. It shall be called if at least one of the movement keys is pressed and needs a possibility to request which keys of the assigned ones are pressed currently.



The reason for this is that I 'd like to check for collision detection inside of the input action and if someone presses forward and strafe left simultaneously I don’t want to perform the check two times, once in each performAction. If I could register the input action to all four direction keys at the same time, I could calculate the complete possible movement in one go and then perform only one collision detection.



But since KeyBindingManager uses the command string as the key in an internal map

(I suggest the attribute definition should probably be private Map keyMap; btw. I always prefer the Interface type in variables definitions) I can assign only one key to a command key. Even if I instantiate my input action four times and assign different commands strings, it would end up calling the method four times.



I will now try to overwrite InputHandler in order to perform 4 hardcoded checks of isValidCommand for my 4 direction keys and then call something other than an input action with the collected information, but I don’t really like to violate your design so rudely.

I tried to allow for this already, but perhaps it’s not as good as it should be? Or probably just not documented well enough…



I can bind multiple keys to KeyBindingManager:


KeyBindingManager.getKeybindingManager().set("action", KeyInput.0);
KeyBindingManager.getKeybindingManager().add("action", KeyInput.1);


You now have a binding for 0 and 1 that is set to the same action.

Then you'd assign this binding to the action:

MyAction a = new MyAction();
a.setKey("action");


Now the "action" binding is assigned to MyAction. So pressing 1 or 0 will trigger this action.

Yes, but inside my action how do I know, which key press or combination of key presses actually triggered my action? I cannot ask the KeyBindingManager, since keyInput is a private attribute.

Ok, I see. The point was to abstract actions away from key presses, etc. Actions can be called if the time reaches 0 for instance, any input value for example.



So, I don’t want to have actions knowing anything about what caused them to be called.



Would something like KeybindingManager.getKeybinding().getLastPressed() help any? If so, I’ll see if that’s possible.

ok, I added getKeyInput to KeyBindingManager()



This returns a KeyInput where you can say something like:



keyInput.isKeyDown(KeyInput.KEY_LEFT);



inside the action to determine which key caused the action to be called.



Now, this means you can’t dynamically remap keys.



I see where you are coming from, but it wasn’t designed to work how you want it. It’s just much more common to have one event trigger one action.

"mojomonk" wrote:
Ok, I see. The point was to abstract actions away from key presses, etc. Actions can be called if the time reaches 0 for instance, any input value for example.

So, I don't want to have actions knowing anything about what caused them to be called.
And I think that's a good design decision.

As I understand, it is possible to register actions for key combinations in the KeyBindingManager. So I could register "forward" for KEY_UP, "backward" for KEY_DOWN, "strafeleft" for KEY_LEFT and then "leftforward" for KEY_UP and KEY_LEFT together.

Now, if I could create a single instance of a multipurpose input action, which I then register for all these keys in the input handler and then they would be called by the input handler with the key that triggered them as a parameter of the performUpdate method...

...then this would currently break the interface of AbstractInputAction, because performUpdate would need another parameter. But these thoughts point into the direction I would like to go.
"mojomonk" wrote:
I see where you are coming from, but it wasn't designed to work how you want it. It's just much more common to have one event trigger one action.

Well, do you have any other idea to solve the problem of minimizing the number of collision detections by pressing several keys at once? Maybe there is a totally different approach that I didn't see...

Ah, I see. Light is starting to brighten up…



You’d like:


setKey(String key);
addKey(String key);


during input update:

instead of:

for(all actions) {
     if(isvalid(action key));
}



we'd have:

for(all actions) {
     for(all keys in action) {
        if(isvalid(action key));
     }
}



in ugly pseudo code.

I'm not against that (if that's what you are meaning).

However, you'd still need to know what key set off the action...

Ok, I’m adding the



addKey(String) method and a

setTrigger(String) method that will set an attribute of the input name that triggered the action.



I’ll let you know when I have it working.

Before you put too much effort into this, wait a minute. I’ll show you s’thing.


public class ThirdPersonHandler extends InputHandler {
    /**
     * Keeps the reference to the node moving action, which is always called.
     */
    private MoveNodeAction internalNodeAction;

    /**
     * Keeps the reference to the avatar root node.
     */
    private Spatial avatarNode;

    public ThirdPersonHandler(AbstractGame app, Camera cam, Spatial avatar,
            String api) {
        avatarNode = avatar;
        setKeyBindings(api);
        setMouse(cam);
        setActions(cam, app);
    }

    private void setKeyBindings(String api) {
        KeyBindingManager keyboard = KeyBindingManager.getKeyBindingManager();
        InputSystem.createInputSystem(api);

        keyboard.setKeyInput(InputSystem.getKeyInput());
        keyboard.set("screenshot", KeyInput.KEY_F12);
        keyboard.set("exit", KeyInput.KEY_ESCAPE);

        KeyBindingManager.getKeyBindingManager().set("node_forw",
                KeyInput.KEY_UP);
        KeyBindingManager.getKeyBindingManager().set("node_back",
                KeyInput.KEY_DOWN);
        KeyBindingManager.getKeyBindingManager().set("node_left",
                KeyInput.KEY_LEFT);
        KeyBindingManager.getKeyBindingManager().set("node_right",
                KeyInput.KEY_RIGHT);
        setKeyBindingManager(keyboard);
       
        /* create instance of my own move action */
        internalNodeAction = new MoveNodeAction(avatarNode, 6f);
    }

    private void setMouse(Camera cam) {
        RelativeMouse mouse = new RelativeMouse("Mouse Input");
        mouse.setMouseInput(InputSystem.getMouseInput());
        setMouse(mouse);
    }

    private void setActions(Camera cam, AbstractGame app) {
        KeyExitAction exit = new KeyExitAction(app);
        exit.setKey("exit");
        addAction(exit);
        KeyScreenShotAction screen = new KeyScreenShotAction();
        screen.setKey("screenshot");
        addAction(screen);
    }

    public void update(float time) {
        /* call base class method */
        super.update(time);

        /* collect pressed keys in a bit mask */
        int direction = 0;
        if (keyboard.isValidCommand("node_forw")) {
            direction |= MoveNodeAction.FORWARD;
        }
        if (keyboard.isValidCommand("node_back")) {
            direction |= MoveNodeAction.BACKWARD;
        }
        if (keyboard.isValidCommand("node_left")) {
            direction |= MoveNodeAction.STRAFELEFT;
        }
        if (keyboard.isValidCommand("node_right")) {
            direction |= MoveNodeAction.STRAFERIGHT;
        }
        if (direction != 0) {
            /* set the direction value */
            internalNodeAction.setDirection(direction);

            /* and perform the update */
            internalNodeAction.performAction(time);
        }
    }
}


That's my current input handler.


public class MoveNodeAction extends AbstractInputAction {
   
    private Spatial avatarNode;

    public static final int FORWARD = 1;
    public static final int BACKWARD = 2;
    public static final int STRAFELEFT = 4;
    public static final int STRAFERIGHT = 8;

    private int direction = FORWARD;
    private static final float ADJUST = FastMath.sqrt(2);

    /**
     * Constructor creates a new <code>KeyNodeForwardAction</code> object.
     * During construction, the node to direct and the speed at which to move
     * the node is set.
     *
     * @param node
     *            the node that will be affected by this action.
     * @param speed
     *            the speed at which the camera can move.
     */
    public MoveNodeAction(Spatial node, float speed) {
        this.avatarNode = node;
        this.speed = speed;
    }

    public void setDirection(int newDirection) {
        direction = newDirection;
    }

    /**
     * <code>performAction</code> moves the node along it's positive direction
     * vector at a speed of movement speed * time. Where time is the time
     * between frames and 1 corresponds to 1 second.
     *
     * @see com.jme.input.action.InputAction#performAction(float)
     */
    public void performAction(float time) {
        Vector3f loc = avatarNode.getLocalTranslation();
        Vector3f old = new Vector3f(loc.x, loc.y, loc.z);

        // we need to use the rotation of the torso!!
        Spatial rotationNode = ((BotTest)avatarNode).getBody();
        Quaternion quat = rotationNode.getLocalRotation();
        Vector3f currTranslation = new Vector3f(0f, 0f, 0f);
        Vector3f rotCol = null;
        float localSpeed = speed;
        /*
         * direction will be != 0; now if it is unequal to any single constant,
         * it must be a combination
         */
        if (direction != FORWARD && direction != BACKWARD
                && direction != STRAFELEFT && direction != STRAFERIGHT) {
            localSpeed /= ADJUST;
        }
        /* next calculate the current translation */
        if ((direction & FORWARD) != 0) {
            rotCol = quat.getRotationColumn(2);
            currTranslation = currTranslation.addLocal(rotCol.mult((localSpeed * time)));
        }
        if ((direction & BACKWARD) != 0) {
            rotCol = quat.getRotationColumn(2);
            currTranslation = currTranslation.addLocal(rotCol.mult((-localSpeed * time)));
        }
        if ((direction & STRAFELEFT) != 0) {
            rotCol = quat.getRotationColumn(0);
            currTranslation = currTranslation.addLocal(rotCol
                    .multLocal((localSpeed * time)));
        }
        if ((direction & STRAFERIGHT) != 0) {
            rotCol = quat.getRotationColumn(0);
            currTranslation = currTranslation.addLocal(rotCol
                    .multLocal((-localSpeed * time)));
        }
        loc.addLocal(currTranslation);

        /*
         * this is the place, where collision detection will have to adjust
         * loct
         */

        /* whatever remains, gets set as the local translation */
        avatarNode.setLocalTranslation(loc);
    }
}


And that's my current action. The thing about the rotation node is that I don't want to use the orientation of the avatars root node but of some child node so that's a detail.

This thing works. It's ugly, but it does, what I plan to do. And I don't need key definitions inside the action. On the other hand, it is no longer a simple action, since it relies on the setDirection call.

Ok, well, if you have a working solution, I’m not going to worry about it for the moment. As soon as there is a second case that it would make sense to support such Actions, we’ll discuss doing this again.

Anyway, thanks for your support. My solution is not very nice, but currently working and it emerged while we were posting replies here.



Currently I am still heavily learning to handle all this stuff, so I clearly need discussions more than solutions, since I am probably not always capable of explaining my thoughts in a correct way.



I hereby announce an upcoming question from my side about how to create a head up display - but that is postponed until I managed to sort out this terrain following stuff.