Key-Binding Related Crash
Darkosto opened this issue ยท 8 comments
Heya Algo,
I had a report of a user crashing with CnB after changing a keybinding:
http://paste.feed-the-beast.com/view/9be4d501
Original Report: https://forum.feed-the-beast.com/threads/client-crash-while-opening-radial-menu-from-chisel-and-bits.210338/#post-1811397
Mod Version: 1.2.9
Forge: 2185
Thank you!
Yeah, I was calling Keyboard#isKeyDown without checking if the the key binding's key code was negative (for mouse bindings) or greater than 256 (the standard "keyboard size", as explained in this vanilla MC bug report).
Simply calling GameSettings#isKeyDown would prevent crashing (as was done in micdoodle8/Galacticraft#469), but it would still mean that if C&B's radial menu binding is set to any non-standard key (keyCode >= 256) EBM's tools would not know if it is down or if was ever pressed.
I ended up fixing it another way, which was fine for 1.8.9; but since MC 1.9+ adds IKeyConflictContext checks (for both conflicts and activity) in KeyBinding#isKeyDown, I had to bypass C&B's activity check by accessing KeyBinding#pressed with reflection. It's not ideal, but it works: Phylogeny/ExtraBitManipulation@53fd157#diff-4d98f369749b00ef0fc3ca7ae8975287
This looks like Extra Bit Manipulation, @Phylogeny possibly something for you?
@Phylogeny if needed we could always extend the API to provide tools for whatever it is you need. like returning the keybind or something like that.
I'm certainly not against such things, if something makes things less hacky on your end feel free to put together a PR for it, I'd prefer if things were less hacky in general, less crashes for all of us that way.
Alright, here's what I have so far: Phylogeny@86eafeb
The only thing left is adding a means of accessing C&B's key bindings, which will save me from iterating through all key binds and checking their descriptions, as I've been doing. I think I'll do that tomorrow.
Of course you/I can change any of this now/later that you want changed; just let me know.
The code in the commit looks fine to me, I might be missing something obvious though.
What issue is the KeyConflict changes to resolve? Isn't the idea to use my keybind so you don't need one? I'm probably missing something extremely obvious to you, if you wouldn't mind explaining.
Any C&B key binding that uses a custom IKeyConflictContext (ModConflictContext) overrides isActive and only returns true if the held item is C&B item (as well as some other checks). That means pressing the key associated with the key binding will not allow KeyBinding#isKeyDown to return true unless one is holding a C&B item. I've added a means of bypassing that and just return true for items of classes registered via the API.