fix: modifier keys shift, ctrl, alt are not released when a different key is released, related to #1313

AlmasB · AlmasB · commit 386473af1ff4 · 2024-02-27T18:50:35.000Z
diff --git a/fxgl-core/src/main/kotlin/com/almasb/fxgl/input/Input.kt b/fxgl-core/src/main/kotlin/com/almasb/fxgl/input/Input.kt
@@ -400,7 +400,10 @@ class Input {
     }
 
     private fun handleReleased(event: InputEvent) {
-        val releasedTriggers = activeTriggers.filter { it.isReleased(event) || (it is KeyTrigger && isIllegal(it.key)) }
+        val releasedTriggers = activeTriggers.filter {
+            // input modifiers are never released under the standard check, so we need the second branch
+            it.isReleased(event) || (it is KeyTrigger && isIllegal(it.key) && event is KeyEvent && event.code == it.key)
+        }
         releasedTriggers.forEach {
             handleTriggerReleased(it)
         }
diff --git a/fxgl-core/src/test/kotlin/com/almasb/fxgl/input/InputModifierTest.kt b/fxgl-core/src/test/kotlin/com/almasb/fxgl/input/InputModifierTest.kt
@@ -11,6 +11,7 @@ import org.hamcrest.CoreMatchers.`is`
 import org.hamcrest.MatcherAssert.assertThat
 import org.junit.jupiter.api.Assertions.assertFalse
 import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 
 /**
@@ -20,6 +21,13 @@ import org.junit.jupiter.api.Test
  */
 class InputModifierTest {
 
+    private lateinit var input: Input
+
+    @BeforeEach
+    fun setUp() {
+        input = Input()
+    }
+
     @Test
     fun `Convert Shift from MouseEvent`() {
         val e = mouseEvent(true, false, false)
@@ -116,13 +124,44 @@ class InputModifierTest {
         assertTrue(InputModifier.NONE.toKeyCode() == KeyCode.ALPHANUMERIC)
     }
 
-//    private fun mouseEvent(shift: Boolean, ctrl: Boolean, alt: Boolean): MouseEvent {
-//        return MouseEvent(MouseEvent.ANY, 0.0, 0.0, 0.0, 0.0, MouseButton.PRIMARY, 1,
-//                shift, ctrl, alt,
-//                false, false, false, false, false, false, false, null)
-//    }
-//
-//    private fun keyEvent(shift: Boolean, ctrl: Boolean, alt: Boolean): KeyEvent {
-//        return KeyEvent(KeyEvent.ANY, "", "", KeyCode.A, shift, ctrl, alt, false)
-//    }
+    @Test
+    fun `Do not release illegal keys when main key is released`() {
+        var callsShift = 0
+
+        input.addTriggerListener(object : TriggerListener() {
+            override fun onKey(keyTrigger: KeyTrigger) {
+                if (keyTrigger.key == KeyCode.SHIFT) {
+                    callsShift++
+                }
+            }
+
+            override fun onKeyEnd(keyTrigger: KeyTrigger) {
+                if (keyTrigger.key == KeyCode.SHIFT) {
+                    callsShift = 999
+                }
+            }
+        })
+
+        input.update(0.0)
+        assertThat(callsShift, `is`(0))
+
+        // press shift
+        input.mockKeyPress(KeyCode.SHIFT)
+        input.update(0.0)
+        assertThat(callsShift, `is`(1))
+
+        // press arbitrary key
+        input.mockKeyPress(KeyCode.D)
+        input.update(0.0)
+        assertThat(callsShift, `is`(2))
+
+        // release that arbitrary key
+        input.mockKeyRelease(KeyCode.D)
+        input.update(0.0)
+        assertThat(callsShift, `is`(3))
+
+        // shift must still remain to be active
+        input.update(0.0)
+        assertThat(callsShift, `is`(4))
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -400,7 +400,10 @@ class Input {`
`400`	`400`	`}`
`401`	`401`
`402`	`402`	`private fun handleReleased(event: InputEvent) {`
`403`		`- val releasedTriggers = activeTriggers.filter { it.isReleased(event) \|\| (it is KeyTrigger && isIllegal(it.key)) }`
	`403`	`+ val releasedTriggers = activeTriggers.filter {`
	`404`	`+ // input modifiers are never released under the standard check, so we need the second branch`
	`405`	`+ it.isReleased(event) \|\| (it is KeyTrigger && isIllegal(it.key) && event is KeyEvent && event.code == it.key)`
	`406`	`+ }`
`404`	`407`	`releasedTriggers.forEach {`
`405`	`408`	`handleTriggerReleased(it)`
`406`	`409`	`}`