johnnovak / gridmonger

Your trusty old-school cRPG mapping companion
https://gridmonger.johnnovak.net/
Do What The F*ck You Want To Public License
46 stars 2 forks source link

scDrawWallRepeat doesn't trigger with single shift #1

Closed Th30n closed 1 year ago

Th30n commented 1 year ago

Hey, I've been following the manual. In this section: https://gridmonger.johnnovak.net/manual/basic-editing.html#draw-wall-repeat the docs suggests that "Draw Wall Repeat" mode is triggered by holding any shift key with either W or S (it should be R). On my machine (5.19.7-arch1-1 #1 SMP PREEMPT_DYNAMIC Mon, 05 Sep 2022 18:09:09 +0000 x86_64 GNU/Linux, with libinput and ibus input method) to enter this "Draw Wall Repeat" mode I need to hold both shifts down at the same time. This is quite inconvenient, and I'm assuming is a bug.

I've inspected the code src/main.nim, and I can see there are 2 bindings for scDrawWallRepeat shortcut. One binding is for the left shift, and the other for the right, but both also accept that they are modified by shift itself. After inspecting the event received from koi.currEvent(), it seems that only an unmodified shift key is registered for me. This is the output of echo ke I inserted in the code:

(kind: ekKey, key: left shift, action: down, mods: {})

I've removed the modifier requirements for scDrawWallRepeat bindings, and this does resolve the issue on my machine. But I'm not sure if this is the right solution, as I have a feeling that it works for you as expected -- with 1 shift pressed. This may also be an underlying issue in koi or glfw. So, I haven't created a PR, but the diff of my local fix is below. Additionally, my fix introduces another quirk in the behaviour. If I press & hold the other shift as well, everything works fine, but as soon as I release one, the "Draw Wall Repeat" mode is exited. I fear you have the same behaviour with existing code, because the code reacts on "key up" event, but it doesn't check if any other shift remains pressed.

diff --git a/src/main.nim b/src/main.nim
index b0dac65..ca47abd 100644
--- a/src/main.nim
+++ b/src/main.nim
@@ -920,8 +920,8 @@ let g_appShortcuts = {
   scDrawWall:                  @[mkKeyShortcut(keyW,      {})],
   scDrawSpecialWall:           @[mkKeyShortcut(keyR,      {})],

-  scDrawWallRepeat:            @[mkKeyShortcut(keyLeftShift,  {mkShift}),
-                                 mkKeyShortcut(keyRightShift, {mkShift})],
+  scDrawWallRepeat:            @[mkKeyShortcut(keyLeftShift,  {}),
+                                 mkKeyShortcut(keyRightShift, {})],

   scPreviousSpecialWall:       @[mkKeyShortcut(keyLeftBracket,  {})],
   scNextSpecialWall:           @[mkKeyShortcut(keyRightBracket, {})],
johnnovak commented 1 year ago

Thanks for the report @Th30n, and for taking the time to investigate what's really going on under the hood, that certainly helped! 👍🏻

I've fixed it slightly differently here: https://github.com/johnnovak/gridmonger/pull/2 Can you please test that branch on your machine?

"Draw Wall Repeat" mode is triggered by holding any shift key with either W or S (it should be R)

No, it should be really W and S. Just follow the spiral tutorial from the manual and you'll see why :)

If I press & hold the other shift as well, everything works fine, but as soon as I release one, the "Draw Wall Repeat" mode is exited. I fear you have the same behaviour with existing code, because the code reacts on "key up" event, but it doesn't check if any other shift remains pressed.

Nah, I don't care about that. People shouldn't hold both Shift keys, just one.

johnnovak commented 1 year ago

Let me know if it's working correctly for you know @Th30n , then I'll merge it in and release a new version.

Th30n commented 1 year ago

Ah sorry, I've put a comment in the PR. Feel free to merge and close the issue.