InputHandler

archive development-discussion-jme2
1

I’ve been going over the InputHandler code and related stuff while putting together my UI test application.



I’d like to propose the following:



Right now to put together a binding to a particular key you have to work through InputHandler, the KeyBindingManager, and an AbstractInputAction of some kind in this manner:



InputHandler input = new InputHandler();

KeyBindingManager keyboard = KeyBindingManager.getKeyBindingManager();

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

input.setKeyBindingManager(keyboard);

KeyExitAction exit = new KeyExitAction( myApplication);
exit.setKey("exit");
input.addAction(exit);



Where KeyExitAction's performAction is implemented like this; which
is called by the input handler when it runs into the (int) KEY to (String) registered key -> Action.performAction:


    public void performAction(float time) {
        app.finish();
    }



Instead, I think it might be a good idea to register callback methods directly with the InputHander. This way allowing also for more than one callback action per bound key. This would also allow the user not to have to mess with the underlying KeyBindingManager. Something like this:


public callBackEscMethod1() {
   // some other code
}

public callBackEscMethod2() {
   this.finish();
}

InputHandler input = InputHandler();
input.registerCallback( KeyInput.KEY_ESCAPE, callBackEscMethod1);
input.registerCallback( KeyInput.KEY_ESCAPE, callBackEscMethod2);



What do y'all think? Oh, this would be good for handling button interaction from my UI objects as well, so I can't say I'm objective in my thoughts on this.
2

At minimum I’ll add this to InputHandler as a convienence method.



    public void addKeyboardAction( String key, int keyInputValue, AbstractInputAction action ) {
        if( keyboard == null) {
          KeyBindingManager keyboard = KeyBindingManager.getKeyBindingManager();
          keyboard.setKeyInput(InputSystem.getKeyInput());
            setKeyBindingManager(keyboard);
        }
        keyboard.set( key, keyInputValue);
        action.setKey( key);
        addAction( action);
    }

3

Sorry to keep adding…



If it’s not a good idea to do the Method callback, I understand. At least we could get rid of the String → int lookups and replace the HashMap in KeyBindingManager with a int/Vector HashMap instead of the String/int HashMap. And then have InputHandler work like this:



InputHandler input = new InputHandler();

input.addAction( KeyInput.KEY_ESCAPE, new KeyOtherAction( this) );
input.addAction( KeyInput.KEY_ESCAPE, new KeyExitAction( this) );



The two actions would be added to the Vector instead of overridding the first.
4

The reason keys are "abstracted" away as string descriptions is to allow easy switching of keys. That is, the current process is:


  1. The escape key is exit (key binding)
  2. if exit is called, quit (action)


keyboard.set("exit", KeyInput.KEY_ESCAPE);
KeyExitAction exit = new KeyExitAction(app);



Now, if I decide I want R to exit (in game menu selected by player).

I just do:

1. The R key is exit

keyboard.set("exit", KeyInput.KEY_R);



Not needed to figure out what action has the reference, etc. R will automatically trigger the "exit" action (which ever that may be).

The other part, regarding callbacks, you can register more than one action per key, at least you should be able to (if you can't it's a bug).

keyboard.set("jump", KeyInput.KEY_Y);
keyboard.set("shoot", KeyInput.KEY_Y);



should work. If not, it's a bug.

The system, as it is now, is not meant to make the programmers life easier while writing the code now, but make it easier to allow the player to change keys during the running of the game, hence the usage of the KeyBindingManager.

If you still feel that the callbacks are a better method, please continue to discuss.
5

If you’re currently looking into the InputHandler stuff, here’s something that might be worth thinking of, too.



It may be necessary to have actions which can react on several simultaneous key presses. Think of movement keys. You move left or forward by an amount, but if you press both keys at the same time you don’t want to simply add the amounts, since it would be a quicker movement. Instead you need to multiply the resulting vector by 1/sqrt(2).

I’ve written a completely nasty solution for that myself, setting bits in an int depending on the directional keys pressed and then evaluating this as a side effect in my action, which I call explicitly if any movement keys has been pressed, but I obviously would prefer a clean method.

Maybe we need some more complex action interface for this, where the action can test itself against all currently active commands. That is, you have action codes “forward”, “backward”, “strafeleft”, “straferight”, but for any of them you can register the same action and the action can query all the currently active codes which lead to its invocation. Obviously, the action should be called only once, even if more thant one action code is active.

6

mojo, dont you mean:



keyboard.set("jump", KeyInput.KEY_Y);
keyboard.add("shoot", KeyInput.KEY_Y);



Notice the second method is add rather than set?

DP
7

I follow…



Registering more than one KeyInput to a single command is what is not allowed right now in code… I got it backwards. You can’t do this:



keyboard.set("jump", KeyInput.KEY_Y);
keyboard.add("jump", KeyInput.KEY_SPACE);



Only space would work for the above. Which makes sense.

At first I was worried a little about a string compare issue, but now that I realize that it's not based on string compare but rather on the hash value of the string, it's faster.

Then you get a list of KeyInputs and do a compare against the currently down keys. Hmm... ;) still feels backwards. I've always started with which keys are down and then looked up actions. Not the other way around. But that's just me.

I guess it depends on which list is larger.. the actions list which is looped through in the InputHandler or the Keys-that-are-down list which is built in the KeyBindingManager from the list of all available key codes associated with a particular command and the list of commands.

Bottom line for me is that right now you have to have at least 4 lines of code for each bound key (look at FirstPersonHandler).

1 for keyboard.set
1 for creating the action
1 for setting the command value
1 for adding it to the InputHandler

Plus it means at least 2 string copies... not a big deal really since it's during game init.

One of the above can be removed by creating the Action implementation to allow for passing in the command value during construction. Thus also combining two others which would end up being only two lines.

Don't know why I'm such a stickler for lines of code, my German heritage I guess :D.

Since we are limited on how the keyboard interaction works from LWJGL, where we only have one call... isKeyDown, and thus having to check for each key individually then I figure your way probably makes the most sense.

The direct callback to a Method is probably NOT a good idea, sticking with a documented interface via AbstractInputAction is just better design actually.

batman, probably the best solution to your issue is probably extending the KeyBindingManager so that the value part of the HashMap is not just one int value but rather a list. This could be done without changing how any other application works.

From the way I see it you'd add a new method to KeyBindingManager called setMulti that would work like the current set. This would accept an array or some such.

The HashMap's value associated with the command key would be an int array instead of just an int, and the current stuff would be converted to work with an array. set would be converted to add the value into the array, but just one value would be in it.

Then the isValidCommand would check if the array is larger than 1, if it was then instead of just checking if the single key is down, it would check if all the keys are down.

I can do this change for you if you'd like. I don't think anyone would have a problem with that since it wouldn't break anything in existance that I can see. Please correct me if I'm wrong.


now, finally...

suggestions on how UI Button to some Action correlation should work would be appreciated. Right now I am thinking that having a specialized UIInputHandler would be nessicary because the MouseInput handling just doesn't work for what I need to do. I need to allow for the same commands that are associated with an action to be run via the keyboard to be run via mouse clicking on buttons and such.
8

Oooh, just had an epiphany from the last part of that post…



I’m just going to allow the addition of AbstractInputActions to the actual UIObjects that can interact.



Still like to hear what others have to say.

9
mojo, dont you mean:

Code:

keyboard.set("jump", KeyInput.KEY_Y);
keyboard.add("shoot", KeyInput.KEY_Y);


Notice the second method is add rather than set?

DP


Nope, you use a set for each command, adds to add multiple keys to the same command. <
Guurk there is your answer. If it's not working (space only working, then you found a bug). The intended behavior is Y and Space would jump.
10

Ah, cool.



There ya go batman…



See, I’m just not reading all the code comments ://



But…



Then you really can’t have two different key combinations do the same action… or at least have the same action command name.

11

aaah, gotcha, its all so confusing when I have a brain the size of a peanut! XD

12
"DarkProphet" wrote:
have a brain the size of a peanut!

Ditto.
13

oh soz, i was refering to myself in the 3d perspective. I didn’t mean you ://



DP

14

So was I XD !

15

can’t help being a squirrel!



DP

16

I dunno… Seems like we should be building on the remaining missing features before we go back and pick apart stuff that works already. I see a lot of this on the forums lately to the point where I don’t read certain threads. :confused: Sure, they are all good ideas, but we haven’t reached the refactoring stage yet. (At least I haven’t noticed anyone saying we were feature complete…?)

17

Ok, agreed.

18

Nothing more than suggestions !


Think of movement keys. You move left or forward by an amount, but if you press both keys at the same time you don't want to simply add the amounts, since it would be a quicker movement. Instead you need to multiply the resulting vector by 1/sqrt(2).
I've written a completely nasty solution for that myself, setting bits in an int depending on the directional keys pressed and then evaluating this as a side effect in my action, which I call explicitly if any movement keys has been pressed, but I obviously would prefer a clean method.


Since this is a physical/cinematic issue, and not a rendering issue, i think modifying the KeyInput Stuff isn't a good idea.
I would suggest you to decouplate the movement in direction/speed.

that is all movement (forward, strafe, jump) keys pressed set the direction, the acceleration key (such as run) set the speed.

same things could be done for combo., keys pressed fill the combo array that is then checked with combo patterns.


dir=0 //vector!
speedMult = 1 //scalar
speed = 0 //scalar
//update keyboard input
...
//
speed *= speedMult
compute(pos,dir,speed);

where action are some thing like :
StrafeRightAction{ 
  dir += -camera.getLeft();
}
ForwardAction {     
  dir += camera.getDir();
}
JumpAction {         
  dir += camera.getUp();
  speedMult += 1.5; // let's say you jump faster than you walk !
}
RunAction {
  speed += 90;
}
WalkAction {
  speed += 10;
}



movement is then something like :
newPosition = position+speed*dir./|dir|
where speed could be |speed|

that is you don't have to check to 1/sqrt(2) in 2D or 1/sqrt(3) in 3D
19

Very good point arthemnos. In fact, fantastic point.



While I did put in the facilities to allow key combinations to call a single actions, for instance SHIFT+P might call a take screen shot action, it’s too difficult to foresee all needs of all games. So, it might be a very viable alternative to dumb down the actions and simply alter external values that will affect a game object in the update phase.