Key Remapping conflicts with menu option presses

[alfred]

Describe the bug When the Key Remapping plugin is toggled on, and an in-game menu is opened that accepts key presses for options (Spirit Tree, or Jewelry Box) is opened, you can't pre-select your option by holding the key down before the menu pops up.

To Reproduce Steps to reproduce the behavior:

1. Go to a spirit tree or jewelry box in-game.
2. Toggle the Key Remap Plugin on, and remap F1 to 1 (can be any remap, 1 is safe to test with as its on every menu type)
3. Hold down the number 1 on your keyboard
4. Click on the spirit tree menu
5. Your client won't select the 1 menu option immediately, without re-pressing the number 1.

Expected behavior In every in-game menu that can be passed by keyboard commands (holding space to continue through prompts, numbers for choosing menu options) if you're holding the key before the menu pops up, it will select the option you're holding down on the next tick after the menu is open, near instantly, and without you having to press the key again.

Screenshots I can do a screen recording, but you won't be able to see me pressing the key on my keyboard if I'm just recording the screen. I don't know how to record with voice over, but if this is needed I can figure it out.

Environment (please complete the following information):

• OS: macOS
• RuneLite version: 1.6.14
• Launcher version: 2.1.1 (I did "About RuneLite" for this value, no idea if that's the right place)

Additional context These are my logs I didn't see anything pop up while I was doing the steps to reproduce.

This isn't game-breaking, but it is a regression that only started happening on the most recent release, based on the currently open issues for the last few days, I'd guess it has something to do with keyboard bank pin?

[Adam-]

The old behavior was actually broken, since if you pressed 1 it would send: keypress f1, typed 1, keyrelease f1 - this was recently discovered and fixed due to the bank pin stuff, since pressing 1 would both hit f1 and also enter in bank pin 1. "Fixing" this again would sort of break the whole awt keylistener contract where you are getting keytyped events without keypresses, which is not great.

[alfred]

Does this mean skilling methods like this won't work anymore by just holding the key down the whole time?

That's really surprising though, I have gotten so used to that I thought it was kinda just a quirk of the game.

[alfred]

Is this something that RL would ever consider making an option to "re-break" or should I just close this issue?

I wouldn't want to try to make a PR for it if you don't think it fits.

[Adam-]

I don't know of a way to fix this that wouldn't reintroduce the original problem/be broken. Also I don't see a way to change this behavior back without sending keytyped events without any keypresses which seems somewhat sketchy. Do you?

[alfred]

I definitely don't understand the problem-space well enough to give a solution, but I guess I don't really see the problem with sending keytyped events and keypresses? since a key is being pressed afterall.

[Adam-]

Does adding a check for plugin.chatboxFocused() in the keyTyped event in the key remapping listener work? That might fix it actually. It might also break the pin stuff again though not sure, would have to test it.

[alfred]

I can try after work today. Thanks for humoring me through this issue so far!

[alfred]

I did what you said and added && plugin.chatboxFocused() to the end of this line https://github.com/runelite/runelite/blob/94482f5aed49604fa7c45631c98cddc2b0ebb208/runelite-client/src/main/java/net/runelite/client/plugins/keyremapping/KeyRemappingListener.java#L62

My original issue ✅ Keyboard Bank Pin ✅ F key remapping still working ✅

Working just as expected. Any other cases I should try testing? Are there unit tests that I should try to write to prep for a PR?

[Adam-]

Just see if the existing tests fail or not.

[alfred]

I'm actually having trouble with the tests, I think I'm missing some important dependency because even without my change the tests fail for me.

To test I changed my run config from maven install -DskipTests -f pom.xml to maven install -f pom.xml but I'm getting a bunch of NullPointerExceptions in the "Cache" section of the tests.

Here's a subset of the verbose test run output

Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.034 sec <<< FAILURE! - in net.runelite.cache.script.disassembler.DisassemblerTest
test(net.runelite.cache.script.disassembler.DisassemblerTest)  Time elapsed: 0.033 sec  <<< ERROR!
java.lang.NullPointerException: null
  at net.runelite.cache.script.disassembler.DisassemblerTest.test(DisassemblerTest.java:65)

Running net.runelite.cache.KitDumperTest
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.03 sec <<< FAILURE! - in net.runelite.cache.KitDumperTest
test(net.runelite.cache.KitDumperTest)  Time elapsed: 0.029 sec  <<< ERROR!
java.lang.NullPointerException: null
  at net.runelite.cache.KitDumperTest.test(KitDumperTest.java:68)

Running net.runelite.cache.ObjectManagerTest
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.005 sec <<< FAILURE! - in net.runelite.cache.ObjectManagerTest
test(net.runelite.cache.ObjectManagerTest)  Time elapsed: 0.005 sec  <<< ERROR!
java.lang.NullPointerException: null
  at net.runelite.cache.ObjectManager.load(ObjectManager.java:60)
  at net.runelite.cache.ObjectManagerTest.test(ObjectManagerTest.java:56)

Running net.runelite.cache.SpriteManagerTest
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.029 sec <<< FAILURE! - in net.runelite.cache.SpriteManagerTest
test(net.runelite.cache.SpriteManagerTest)  Time elapsed: 0.027 sec  <<< ERROR!
java.lang.NullPointerException: null
  at net.runelite.cache.SpriteManager.load(SpriteManager.java:58)
  at net.runelite.cache.SpriteManagerTest.test(SpriteManagerTest.java:55)

Running net.runelite.cache.TrackDumperTest
[main] INFO net.runelite.cache.util.Djb2Manager - Loaded 1402 djb2 hashes
Tests run: 2, Failures: 0, Errors: 1, Skipped: 1, Time elapsed: 0.032 sec <<< FAILURE! - in net.runelite.cache.TrackDumperTest
test(net.runelite.cache.TrackDumperTest)  Time elapsed: 0.029 sec  <<< ERROR!
java.lang.NullPointerException: null
  at net.runelite.cache.TrackDumperTest.test(TrackDumperTest.java:73)

Running net.runelite.cache.InterfaceManagerTest
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.008 sec <<< FAILURE! - in net.runelite.cache.InterfaceManagerTest
test(net.runelite.cache.InterfaceManagerTest)  Time elapsed: 0.007 sec  <<< ERROR!
java.lang.NullPointerException: null
  at net.runelite.cache.InterfaceManager.load(InterfaceManager.java:59)

[abextm]

Try deleting $TEMP/cache-165

[alfred]

@abextm Nice, that allowed me to get further in the tests, running the whole suite now.

[alfred]

Tests finished and they pass now