Add support for listening to joystick connection/disconnection.

[ghost]

Further to thread: https://hub.jmonkeyengine.org/t/gamepad-disconnect-re-connect/41822

This PR changes the following:

• You can add joysticks AFTER the game is loaded.
• You don’t need to plug the joystick in BEFORE the game starts.
• You are notified if a joystick is added or removed.
• No errors are thrown if a joystick is removed.

Connection listeners can be added, removed and cleared via the InputManager.

getInputManager().addJoystickConnectionListener(new JoystickConnectionListener(){

            @Override
            public void connectionChanged(int joystickId, JoystickState action) {
                System.out.println(action == JoystickState.CONNECTED
                        ? "Joystick Connected:" + joystickId
                        : "Joystick Disconnected:" + joystickId
                );
            }
        });

[Ali-RS]

Thanks @jayfella
Can you please fix this little thing:

jmonkeyengine/jme3-lwjgl3/src/main/java/com/jme3/input/lwjgl/GlfwJoystickInput.java

Line 52 in 318d6d0

private static final Logger LOGGER = Logger.getLogger(InputManager.class.getName());

it should be

private static final Logger logger = Logger.getLogger(GlfwJoystickInput.class.getName());

[pspeed42]

Also, on quick glance, it's a little weird to embed lwjgl3-specific magic numbers into a core class rather than translating them into something more general in the implementation layer. That means every other joystick layer that wanted to implement this would have to mimic lwjgl3 constants. Just define the Connected/Disconnected enum (enum values should not be all caps) and translate that in the glfw layer.

[tonihele]

Also the print line to system.out. should it go to logger debug instead?

…

On Sat, 11 May 2019, 21:22 Paul Speed, ***@***.***> wrote: Also, on quick glance, it's a little weird to embed lwjgl3-specific magic numbers into a core class rather than translating them into something more general in the implementation layer. That means every other joystick layer that wanted to implement this would have to mimic lwjgl3 constants. Just define the Connected/Disconnected enum (enum values should not be all caps) and translate that in the glfw layer. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1087 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB7VJPSLBOJTGDRPAKO2UITPU4FGFANCNFSM4HMIHXFQ> .

[ghost]

Ok.

• changed the logger to the correct class
• removed magic number from the enum constant.
• The LWJGL3 context now translates its event ID to a JoystickState enum constant.
• Removed InputManager field and use RawInputListener instead.

@tonihele the listener code in the comments is just an example written here of the JoystickConnectionListener entry point - unless I missed a line somwehere?

[pspeed42]

The first time I read JoystickState I wondered why you were making an app state. That's an unfortunate name collision. Not really sure I have a better idea (though a boolean "connected" might also work instead of an enum.).

Hmmm...

[Ali-RS]

Yes, I also think a boolean would be better here.

[ghost]

Yep. Makes sense. Done.

[ghost]

Are we all good with this now?

[Ali-RS]

Looks fine to me.

[pspeed42]

It's going to sound like I'm being nit-picky but this is going to become part of the public API and we will be stuck with it forever. So I gave it a little more thought and I'm sorry I didn't do that earlier when you were already making changes.

JME seems to follow a couple different patterns: 1) pass an event object that can be interrogated and have fewer listener methods (for example, we don't have separate pressed/released button listener methods), or 2) pass the raw event pieces and have more methods... (for example: ClientStateListener.onConnect()/onDisconnect(), ConnectionListener.connectionAdded()/connectionRemoved())... sometimes both. (CinematicListener.onPlay()/onPause()/onStop())

In that light, even if we only keep one method on the JoystickConnectionListener, I think it should pass the Joystick and not the joystick ID. It would better fit all of the other joystick listener methods to pass the actual object instead of making the listener look it up. It's also nice that one of the parameters would be strong-type tied to joysticks just from an idiomatic perspective for some wierdo who tries to implement a lot of interfaces on one object.

I don't have a strong opinion on JoystickConnectionListener.onConnect()/onDisconnect() versus a boolean... just something to think about. I think the Joystick versus ID thing is important, though.

[ghost]

No not at all. I'd rather it be as it should be. Referencing the joystick makes sense - from a user point of view the code seems a lot more descriptive.

getInputManager().addJoystickConnectionListener(new JoystickConnectionListener() {

            @Override
            public void onConnected(Joystick joystick) {
                System.out.println("Joystick connected: " + joystick.getName());
            }

            @Override
            public void onDisconnected(Joystick joystick) {
                System.out.println("Joystick disconnected: " + joystick.getName());
            }
        });

I actually went with separate methods because I think in the case of buttons it usually follows the same pattern (move left, move -left or whatever) but in a connection state it's a separate flow direction completely - it would need to follow a recovery or pause phase on disconnection. In my experience each method should only do one thing to keep code maintainable - and this way keeps in line with that.

[pspeed42]

I agree. I like separate methods better for all of the reasons you listed... I just didn't want to send you back to the drawing board if you weren't feeling it. It wasn't worth the argument when I really really wanted to have the Joystick parameter. :)

Changes look good to me... I'll let them percolate a little longer in case someone else wants to review the latest changes.

[Ali-RS]

@stephengold are you alright with this PR?

[stephengold]

I'm unfamiliar with JME's support for joysticks and therefore unqualified to review this PR.

[pspeed42]

I guess no one else but me and Jayfella are going to look at it so might as well just push it through. Open the gates, guys, I think I'm just going to start blind-merging everything. lol.

[empirephoenix]

Master is not stable, so go ahead, in the worst case it can always be reverted :)