Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IndexOutOfBounds in com.badlogic.gdx.controllers.desktop.ois.OisJoystick.buttonPressed #5857

Closed
nonchip opened this issue Dec 8, 2019 · 12 comments
Labels
bug controllers Concerning controllers extension desktop

Comments

@nonchip
Copy link

nonchip commented Dec 8, 2019

see

(plus probably the function following that one).

crashes in games like PokeMMO and (according to google) Slay the Spire (found this report when looking for the issue: https://steamcommunity.com/app/646570/discussions/1/1751232561619087220/), when tools like ckb-next (required for corsair input devices on linux) or xboxdrv are being used.

Error message I'm getting in PokeMMO:

[ERROR 2019-12-08 18-31-36 CET][LWJGL Application] f.Vv:? - Fatal render error.
java.lang.ArrayIndexOutOfBoundsException: 271
        at com.badlogic.gdx.controllers.desktop.ois.OisJoystick.buttonPressed(Unknown Source)
        at com.badlogic.gdx.controllers.desktop.ois.OisJoystick.update(Native Method)
        at com.badlogic.gdx.controllers.desktop.ois.OisJoystick.update(Unknown Source)
        at com.badlogic.gdx.controllers.desktop.ois.Ois.update(Unknown Source)
        at com.badlogic.gdx.controllers.desktop.OisControllers$1.run(Unknown Source)
        at com.badlogic.gdx.backends.lwjgl.LwjglApplication.executeRunnables(Unknown Source)
        at com.badlogic.gdx.backends.lwjgl.LwjglApplication.mainLoop(Unknown Source)
        at com.badlogic.gdx.backends.lwjgl.LwjglApplication$1.run(Unknown Source)
@mquickmann
Copy link
Contributor

Duplicate / Same Source affected: #2964

@nonchip
Copy link
Author

nonchip commented Jan 29, 2020

@Monarchis then we should probably change that issue, because i don't have any A4Tech hardware.

@mquickmann
Copy link
Contributor

@nonchip I refered to this Issue, because it also affects the same native library, that it returns wrong KeyCounts, in your case the array is also initialized with a wrong size.

Currently idk if it's a problem with the Hardware or the native library itself.

@nonchip
Copy link
Author

nonchip commented Jan 30, 2020

i think it's probably an issue with the library ((OIS::JoyStick*)->getNumberOfComponents(OIS::OIS_Button) seems to return too small a value, which it gets from its internal ((JoyStickState)mState).mButtons, initialized via a bunch of further abstraction, originally from DeviceComponentInfo::getComponentInfo in https://github.com/wgois/OIS/blob/master/src/linux/EventHelpers.cpp#L150).

maybe it's also somewhat related to the driver/hardware possibly skipping button IDs or something like that (e.g. i could imagine a device reporting button 0, axis 1, axis 2, button 3, resulting in a buttons[2] array trying to get indexed with 3), but this is literally the first time i have an issue like that, so if it's actually the driver doing that it would mean everyone else expects that and initializes their arrays differently, which kiiinda makes me think it's OIS who's offspec here, not the hardware/driver.

oh btw, looking further at that function, KEY_MAX is defined as 0x2ff in my linux/input-event-codes.h, which is definitely more than the 271 running out of bounds above, so it shouldn't be a kernel side issue with just having too large button indices, making just the ordering of stuff or skipped indices most likely imo. after all linux doesn't care if something's a button, axis, etc, they're all the same "component", so internally they all share the same index space, and are only split into multiple arrays by OIS, which seems to be messing up the resulting mapping around "missing" IDs.

EDIT:
or, rather, after all looks like libgdx might be doing it wrong in

, since the error is happening java side, not C side, and you are initializing that array for the button count, then indexing into it directly without any mapping from the buttonId, which seems to be a "component" id (from said shared index space, thus possibly larger than the buttonCount).

EDIT2:
I just built OIS myself to run their ConsoleApp demo, that's what it said (skipping unimportant stuff, text in [brackets] are my comments):

Total JoySticks: 5
        Device: OISJoyStick Vendor: ckb1: Corsair Gaming GLAIVE RGB Mouse vKB
        Device: OISJoyStick Vendor: ckb1: Corsair Gaming GLAIVE RGB Mouse vM
        Device: OISJoyStick Vendor: ckb2: Corsair Gaming K70 RGB RAPIDFIRE Keyboard vKB
        Device: OISJoyStick Vendor: ckb2: Corsair Gaming K70 RGB RAPIDFIRE Keyboard vM
        Device: OISJoyStick Vendor: Xbox Gamepad (userspace driver) [from xboxdrv]

Creating Joystick 1
        Axes: 0
        Sliders: 0
        POV/HATs: 0
        Buttons: 254
        Vector3: 0

[then creating joystick 2..4 with EXACTLY the same values, interestingly no joystick 5 for xboxdrv]
[then i pressed a keyboard button, not sure anymore which, i think it was shift:]
ckb2: Corsair Gaming K70 RGB RAPIDFIRE Keyboard vKB. Button Pressed # 28
ckb2: Corsair Gaming K70 RGB RAPIDFIRE Keyboard vKB. Button Released # 28
[then I clicked the mouse, first left then right:]
ckb1: Corsair Gaming GLAIVE RGB Mouse vM. Button Pressed # 271
ckb1: Corsair Gaming GLAIVE RGB Mouse vM. Button Released # 271
ckb1: Corsair Gaming GLAIVE RGB Mouse vM. Button Pressed # 272
ckb1: Corsair Gaming GLAIVE RGB Mouse vM. Button Released # 272

note those button IDs being larger than the reported number of buttons, which seems to confirm the "mixed/skipped IDs" theory. pressing another bunch of keyboard and mouse buttons, it seems to be like ckb-next (the corsair userspace driver) just appends the mouse's id space to that of the keyboard, since all key events, even esotheric macro keys and stuff like media buttons are <250, and the mouse events start at 270.

then again no events in ckb-next are actually bound to emulate joysticks, so those virtual devices shouldn't actually be used to begin with (and don't show up in any other games/etc that allow me to select joysticks/controllers, those do show ONLY the xboxdrv input for which OIS didn't even initialize a joystick), maybe there's some kind of issue with a flag being ignored or something like that resulting in accidentally registering those as joysticks with OIS just because they theoretically could be used as such?

seems like it's doing it literally the wrong way around, trying to access devices that aren't supposed to and not even touching the one that is enabled.

TL;DR:

seems like ckb-next is behaving in an "unintuitive" (but technically in-spec for linux events) way, triggering weirdness with OIS's buttonCount-vs-buttonId mapping you're relying on, and also a bunch of other issues (such as OIS not even initializing the right devices).

the easiest solution to force your code to behave in-spec would be to use the KEY_CNT constant from linux/input-event-codes.h instead of buttonCount, and just leaving any unused fields in your array empty, thus wasting at most (KEY_CNT)*sizeof(*buttons) bytes of ram, but ensuring the IDs always fit in your array.

@mquickmann
Copy link
Contributor

mquickmann commented Jan 31, 2020

@nonchip Yes that's exact the reason, why I marked it as duplicate, there are more Issues with the same Problem of the native implementation.

the native code behind this Line is just wrong. This also affects other Issues:

That leads to a wrong size initialization at:

That the Exeption is thrown on Line:

Maybe i'll try to fix it those days, else you or someone else could also create a pr ;)

@nonchip
Copy link
Author

nonchip commented Jan 31, 2020

well i don't know if the code to get the "button count" is wrong, or just you using the "count" as "max value of index".
see also my observation of ckb-next starting those indices at a relatively high number. just because you have 5 buttons doesn't mean they have to be button[0..4]. [234..238] would still just be a "count" of 5.
which is why i'd suggest using the KEY_CNT value from the kernel headers, which is defined as (KEY_MAX+1) (==768 on my machine), thus results in an array that always fits any KEY_ index returned by evdev.

@mquickmann
Copy link
Contributor

@nonchip I observed, that OIS, works with offsets off buttons (mappings), so may there could be also be the problem...
OIS is mapping buttons down to a smaller array with the button count.
May there is any Problem with the version of OIS, a newer one is already released.

Actually I also don't know how to fix this.

@tatokis
Copy link

tatokis commented Mar 15, 2020

While this still remains an issue, I have pushed a workaround in ckb-next.
ckb-next/ckb-next@afd0e19

@nonchip
Copy link
Author

nonchip commented Mar 16, 2020

@tatokis i'm not sure ckb-next should be stripped of functionality just because libgdx isn't as quick as we might like in replacing the call to getButtonCount by KEY_MAX/KEY_CNT though... ckb was doing nothing wrong, and your change seems to prevent custom buttons if i'm reading it correctly.

@tatokis
Copy link

tatokis commented Mar 16, 2020

@nonchip It's not stripping or removing any functionality.

For every physical device, it makes a virtual keyboard and a virtual mouse.
When you bind a keyboard key to a mouse, the key gets sent through the virtual keyboard, not the virtual mouse.

All that commit does is remove the keyboard keys from the virtual mouse, which were not being used in the first place.

Either way, this is off topic in this thread.

@nonchip
Copy link
Author

nonchip commented Mar 16, 2020

@tatokis ok yeah just realized the SCAN_MOUSE part. still though, from the point of this issue, we shouldn't rely on a ckb workaround, pretty sure as soon as something else comes along using uinput there's a nonzero chance it will break down again.

@MrStahlfelge MrStahlfelge added bug controllers Concerning controllers extension desktop labels Aug 28, 2020
@MrStahlfelge
Copy link
Member

Hi, please take a look at gdx-controllers v2 prerelease snapshot. It should solve your issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug controllers Concerning controllers extension desktop
Projects
None yet
Development

No branches or pull requests

4 participants