Add Missing Joystick Button Definitions#2474
Conversation
|
Oops, right after merging this I saw something that I had somehow overlooked both other time I looked at this PR. It looks like you removed the static keyword from the JoyStick button String declarations and I did not realize that. Was this a mistake, or is there an intentional reason for doing so? I don't use Joysticks or controllers in my app, so maybe I'm mistaken. But I'm fairly sure that these fields are intended to be static so they can be referenced and used similar to static KeyTrigger values for normal key and mouse input. In which case setting them non static will break every app where a user is referencing them this way. Edit: this engine example cofirms that the JoyStickButton static vars are indeed intended to be left static, and examples like this would break if not static: https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-examples/src/main/java/jme3test/input/TestJoystick.java#L258 So I would suggest submitting another PR to add the static keywords back, or if you are busy and don't get aroudn to it soon then I could do so for you. Regardless of this mistake, your contribution is still appreciated and the added support for buttons 12-15 is good to have now. And I apologize for merging it before noticing this issue. |
I accidentally approved and merged this PR (#2474) without realizing that the static keyword had been removed from a set of String vars, and this mistake breaks any apps that are using JoyStickButton for controller input. Despite reviewing this fairly small PR twice, I unfortunately still didn't see this. So this PR re-adds the static keyword back. I will merge this in a few hours to ensure that the master branch isn't broken for too long. My apologies for the overlooking this mistake in my review, I should also be more hesitant to merge PRs in cases like that where I was the only reviewer, but unfortunately there is a lack of other active reviewers currently, so I just have to do my best I suppose. Thankfully there shouldn't have been any any harm done since this was only broken master for a day and I managed to catch it almost immediately after merging.
2 participants
Extends the
JoystickButtoninterface with definitions for buttons12through15, ensuring full coverage for common joystick layouts. These buttons are needed with a Logitec F310 andlwjgl3.