KeyboardInputHandlerDevice not resettable

Hi everyone!:slight_smile:



As I already described here, I’m currently working on some kind of simulation, which is controlled by an external (Swing) GUI. One should be able to stop the current simulation, start a new one, stop this again and so forth, always having no more than one simulation created/running.





The problems arise when starting the 2nd instance of the simulation (successivly, after closing the 1st!). Due to the awesome support here 8) I already got rid of the graphical part, but now I’m facing some with my InputHandler! :-o



Problem:

Key bindings which are defined using InputHandler.addAction() are not working on the 2nd start of the simulation!

As I stepped through the jME-Code yesterday, this is - in my opinion - why:



Digging into the method, a saw that InputHandler got a private static final DevicesMap devices. On first sight this totally makes sense, as the devices are “static” naturally. But looking closer there is a problem with the KeyboardInputHandlerDevice (which is only created once due to the static context): It holds an instance of the TriggersKeyboardInputListener  (which has package-visbility, btw) and thus only registered once to the current KeyInput using KeyInput.get().addListener( this ).



When I’m destroying the KeyInput (on cleaning up the simulation), the TriggersKeyboardInputListener is still there and not recreated - though the KeyInput it listened to doesn’t exist anymore.



Here the relevant code-snippets:



InputHandler$DevicesMap:

    private static final DevicesMap devices = new DevicesMap();
  
    private static final class DevicesMap {
       private final ConcurrentMap<String, InputHandlerDevice> devicesMap = new ConcurrentHashMap<String, InputHandlerDevice>();
        private final Collection<InputHandlerDevice> devicesUnmod = Collections.unmodifiableCollection( devicesMap.values() );
       
       public DevicesMap() {
          addDevice( new MouseInputHandlerDevice() );
      addDevice( new KeyboardInputHandlerDevice() );                              // <--- HERE
      for ( int i = JoystickInput.get().getJoystickCount() - 1; i >= 0; i-- ) {
         Joystick joystick = JoystickInput.get().getJoystick( i );
         String name = joystick.getName();
         String uniqueName = name;
         for ( int j=2; devicesMap.get(uniqueName) != null; j++ ) {
            uniqueName = name + " ("+j+")";
         }
         addDevice( new JoystickInputHandlerDevice( joystick, uniqueName ) );
      }
       }

    [...]
}



KeyboardInputHandlerDevice:

    private TriggersKeyboardInputListener keyboardListener;

    public synchronized TriggersKeyboardInputListener getKeyboardListener() {
        if ( keyboardListener == null ) {
            keyboardListener = new TriggersKeyboardInputListener();         // <-- it NEVER gets here again
            keyboardListener.activate();
        }
        return keyboardListener;
    }



class TriggersKeyboardInputListener implements KeyInputListener {    // package... :-(

    public TriggersKeyboardInputListener() {
    }

    public void activate() {
        KeyInput.get().addListener( this );
    }

   [...]
}




Workarounds:
In fact there are several possibilities to avoid this behaviour, but none of them is very nice:

1. My first intention was to call TriggersKeyboardInputListener.activate() to simply register it to the new KeyInput, but the class has package-visibility.
2. Another  - yet not very clean - workaround would be to call InputHandler.addDevice( new KeyboardInputHandlerDevice() ) for every start of the simulation except the first.
3. Generally avoid the KeyInputHandler and instead use the KeyBindingManager directly.
4. ...

Suggestion:
If I'm right so far (and PLEASE correct me if I got anything wrong!) I would suggest to either
1. make TriggersKeyboardInputListener public (which isn't the best solution from the encapsulation point of view)
2. or to provide a mechanism that resets the input-middleware completly to enforce recreation (maybe with a cleanup()-method in InputSystem (which means a bit more work to do, and a bigger change, but more OO - at least in my opinion ;) ;))


I'm looking forward to your replies/opinions/comments! :)