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

Keys stick - glfwGetKey() continues to return GLFW_PRESS even after onblur, loses focus #40

Closed
satoshinm opened this issue Apr 4, 2017 · 6 comments

Comments

@satoshinm
Copy link
Owner

satoshinm commented Apr 4, 2017

Haven't track down the steps yet, but both the native and web-based clients are affected: sometimes, a key acts as if it is continuously held down, even when it isn't, usually occurs when switching in/out of focus to the app. For example, "A" may be held down and keep moving the player left. Pressing and releasing the key again unsticks it, but shouldn't it losing focus also release all keys?

@satoshinm satoshinm added the ui label Apr 4, 2017
@satoshinm
Copy link
Owner Author

satoshinm commented Apr 5, 2017

Here's an easy technique to trigger it, much of the time:

  1. Right-click to place a block, verify it works
  2. Cmd-Tab to another window and back
  3. Repeat step 1

Step 3 acts like Cmd-Right-Click, illuminating the block, because Cmd is stuck down from step 2.


Also, often seen with #46 Cmd-Backquote types the backquote keystroke to write on signs, Shift-Enter types a blank character (space?) in text input, instead of finishing the command. So if shift is stuck, text input mode can be unintuitive to get out of (return typing space, escape exiting pointer lock. The workaround is: tap shift to unstick it, then enter).

@satoshinm satoshinm added this to the important milestone Apr 9, 2017
@satoshinm
Copy link
Owner Author

satoshinm commented Apr 9, 2017

This is related to #44 Mouse look triggers even when pointer lock inactivated - which was fixed by "Sync glfw cursor to pointer lock disable state.": 9f8a56f (but maybe a better fix would be in emscripten itself?). Should released pointer lock also release key states?


It is possible to interact with the game using keyboard events, when pointer lock is not enabled, so clearing modifier states on pointer lock release is not necessarily a complete fix. Example steps:

  1. Hold down Space to continuously jump
  2. Cmd-Tab to another window and back

You continue jumping, even while the window is unfocused. There are visibility APIs: http://kripken.github.io/emscripten-site/docs/api_reference/html5.h.html#visibility though seem to be more centered around power management, maybe clear on EMSCRIPTEN_VISIBILITY_VISIBLE?


Clear modifier states on focus callback, onblur/focusout?


Craft uses glfwGetKey(), so shouldn't this be fixed there?

@satoshinm satoshinm changed the title Keys stick - missing keyup event when losing focus? Keys stick - glfwGetKey() continues to return GLFW_PRESS even after onblur, loses focus Apr 9, 2017
satoshinm added a commit to satoshinm/emglfwbugs that referenced this issue Apr 9, 2017
@satoshinm
Copy link
Owner Author

Issue in emscripten: emscripten-core/emscripten#5122

On the fence whether should attempt to workaround in the app.. is it even possible? Maybe add blur/focus callbacks, set a global blurred=1, then replace all glfwGetKey with !blurred && glfwGetKey but could get ugly fast. Maybe not too bad. There are 11 instances of glfwGetKey in Craft.

@satoshinm
Copy link
Owner Author

+static int blurred = 0;
+int craftGetKey(GLFWwindow *window, int key) {
+    // Workaround stuck keys in web, TODO: fix upstream in https://github.com/kripken/emscripten/issues/5122
+    if (blurred) return GLFW_RELEASE;
+
+    return glfwGetKey(window, key);
+}
+
+#ifdef __EMSCRIPTEN__
+EM_BOOL on_focus(int eventType, const EmscriptenFocusEvent *focusEvent, void *userData) {
+   switch(eventType) {
+       case EMSCRIPTEN_EVENT_BLUR:
+           blurred = 1;
+           printf("onblur (keyboard interaction disabled)\n");
+           break;
+       case EMSCRIPTEN_EVENT_FOCUS:
+           blurred = 0;
+           printf("onfocus\n");
+           break;
+   }
+   return EM_FALSE;
+}
+#endif
+
 void handle_movement(double dt) {
     static float dy = 0;
     State *s = &g->players->state;
@@ -2593,21 +2617,21 @@ void handle_movement(double dt) {
     int sx = 0;
     if (!g->typing) {
         float m = dt * 1.0;
-        g->ortho = glfwGetKey(g->window, CRAFT_KEY_ORTHO) ? 64 : 0;
-        g->fov = glfwGetKey(g->window, CRAFT_KEY_ZOOM) ? 15 : 65;
-        if (glfwGetKey(g->window, CRAFT_KEY_FORWARD)) sz--;
-        if (glfwGetKey(g->window, CRAFT_KEY_BACKWARD)) sz++;
-        if (glfwGetKey(g->window, CRAFT_KEY_LEFT)) sx--;
-        if (glfwGetKey(g->window, CRAFT_KEY_RIGHT)) sx++;
-        if (glfwGetKey(g->window, GLFW_KEY_LEFT)) s->rx -= m;
-        if (glfwGetKey(g->window, GLFW_KEY_RIGHT)) s->rx += m;
-        if (glfwGetKey(g->window, GLFW_KEY_UP)) s->ry += m;
-        if (glfwGetKey(g->window, GLFW_KEY_DOWN)) s->ry -= m;
+        g->ortho = craftGetKey(g->window, CRAFT_KEY_ORTHO) ? 64 : 0;
+        g->fov = craftGetKey(g->window, CRAFT_KEY_ZOOM) ? 15 : 65;
+        if (craftGetKey(g->window, CRAFT_KEY_FORWARD)) sz--;
+        if (craftGetKey(g->window, CRAFT_KEY_BACKWARD)) sz++;
+        if (craftGetKey(g->window, CRAFT_KEY_LEFT)) sx--;
+        if (craftGetKey(g->window, CRAFT_KEY_RIGHT)) sx++;
+        if (craftGetKey(g->window, GLFW_KEY_LEFT)) s->rx -= m;
+        if (craftGetKey(g->window, GLFW_KEY_RIGHT)) s->rx += m;
+        if (craftGetKey(g->window, GLFW_KEY_UP)) s->ry += m;
+        if (craftGetKey(g->window, GLFW_KEY_DOWN)) s->ry -= m;
     }
     float vx, vy, vz;
     get_motion_vector(g->flying, sz, sx, s->rx, s->ry, &vx, &vy, &vz);
     if (!g->typing) {
-        if (glfwGetKey(g->window, CRAFT_KEY_JUMP)) {
+        if (craftGetKey(g->window, CRAFT_KEY_JUMP)) {
             if (g->flying) {
                 vy = 1;
             }
@@ -2956,6 +2980,8 @@ int main(int argc, char **argv) {
     // OUTER LOOP //
     g_running = 1;
 #ifdef __EMSCRIPTEN__
+    emscripten_set_blur_callback(NULL, NULL, EM_TRUE, on_focus);
+    emscripten_set_focus_callback(NULL, NULL, EM_TRUE, on_focus);
     emscripten_push_main_loop_blocker(main_init, NULL); // run before main loop
     emscripten_set_main_loop(one_iter, 0, 1);
     //main_shutdown(); // called in one_iter() if g_inner_break

An insufficient workaround - partly works in that keyboard interaction is disabled when focus is lost, so say if you were pressing space when switching out, you stop jumping. But once you switch back, you begin jumping again.

@satoshinm
Copy link
Owner Author

diff --git a/src/main.c b/src/main.c
index 6952ab6..4690fae 100644
--- a/src/main.c
+++ b/src/main.c
@@ -2586,6 +2586,61 @@ void handle_mouse_input() {
     }
 }
 
+// Workaround stuck keys in web, TODO: fix upstream in https://github.com/kripken/emscripten/issues/5122 and delete all this
+// TODO: this fixes character keys, but not modifiers like Cmd :(
+static int blurred = 0;
+static int was_blurred = 0;
+static int last_pressed_key = 0;
+int craftGetKey(GLFWwindow *window, int key) {
+    if (blurred) {
+        //printf("key ignored since blurred: %d\n", key);
+        // When the window is not focused, take no key events.
+        return GLFW_RELEASE;
+    }
+
+    int status = glfwGetKey(window, key);
+    if (status == GLFW_PRESS) {
+        // Save the last pressed key for if the user switches to another window (onblur).
+        printf("set last_pressed_key=%d\n", last_pressed_key);
+        if (key == last_pressed_key && was_blurred) {
+            // After returning from onblur, back to onfocus, ignore this keystroke.
+            // An ugly hack - and creates a new bug: keys ignored right after returning,
+            // but hopefully better than sticking, and since the key is ignored, the user
+            // knows what they pressed that was ignored, so they can press it again.
+            printf("key %d ignored since last_pressed_key=%d and was_blurred=%d\n", key, last_pressed_key, was_blurred);
+            return GLFW_RELEASE;
+        }
+        last_pressed_key = key;
+    } else if (status == GLFW_RELEASE && key == last_pressed_key) {
+        printf("resetting last pressed key %d, since was released\n", key);
+        // ... until it is pressed and released again, then return to normal.
+        was_blurred = 0;
+        last_pressed_key = 0;
+    }
+    return status;
+}
+#ifdef __EMSCRIPTEN__
+EM_BOOL on_focus(int eventType, const EmscriptenFocusEvent *focusEvent, void *userData) {
+   switch(eventType) {
+       case EMSCRIPTEN_EVENT_BLUR:
+           blurred = 1;
+           printf("onblur (keyboard interaction disabled), last_pressed_key=%d\n", last_pressed_key);
+           if (glfwGetKey(g->window, last_pressed_key) != GLFW_PRESS) {
+               printf("resetting last_pressed_key since was since released\n");
+               last_pressed_key = 0;
+               was_blurred = 0;
+           }
+           break;
+       case EMSCRIPTEN_EVENT_FOCUS:
+           blurred = 0;
+           was_blurred = 1;
+           printf("onfocus\n");
+           break;
+   }
+   return EM_FALSE;
+}
+#endif
+
 void handle_movement(double dt) {
     static float dy = 0;
     State *s = &g->players->state;
@@ -2593,21 +2648,21 @@ void handle_movement(double dt) {
     int sx = 0;
     if (!g->typing) {
         float m = dt * 1.0;
-        g->ortho = glfwGetKey(g->window, CRAFT_KEY_ORTHO) ? 64 : 0;
-        g->fov = glfwGetKey(g->window, CRAFT_KEY_ZOOM) ? 15 : 65;
-        if (glfwGetKey(g->window, CRAFT_KEY_FORWARD)) sz--;
-        if (glfwGetKey(g->window, CRAFT_KEY_BACKWARD)) sz++;
-        if (glfwGetKey(g->window, CRAFT_KEY_LEFT)) sx--;
-        if (glfwGetKey(g->window, CRAFT_KEY_RIGHT)) sx++;
-        if (glfwGetKey(g->window, GLFW_KEY_LEFT)) s->rx -= m;
-        if (glfwGetKey(g->window, GLFW_KEY_RIGHT)) s->rx += m;
-        if (glfwGetKey(g->window, GLFW_KEY_UP)) s->ry += m;
-        if (glfwGetKey(g->window, GLFW_KEY_DOWN)) s->ry -= m;
+        g->ortho = craftGetKey(g->window, CRAFT_KEY_ORTHO) ? 64 : 0;
+        g->fov = craftGetKey(g->window, CRAFT_KEY_ZOOM) ? 15 : 65;
+        if (craftGetKey(g->window, CRAFT_KEY_FORWARD)) sz--;
+        if (craftGetKey(g->window, CRAFT_KEY_BACKWARD)) sz++;
+        if (craftGetKey(g->window, CRAFT_KEY_LEFT)) sx--;
+        if (craftGetKey(g->window, CRAFT_KEY_RIGHT)) sx++;
+        if (craftGetKey(g->window, GLFW_KEY_LEFT)) s->rx -= m;
+        if (craftGetKey(g->window, GLFW_KEY_RIGHT)) s->rx += m;
+        if (craftGetKey(g->window, GLFW_KEY_UP)) s->ry += m;
+        if (craftGetKey(g->window, GLFW_KEY_DOWN)) s->ry -= m;
     }
     float vx, vy, vz;
     get_motion_vector(g->flying, sz, sx, s->rx, s->ry, &vx, &vy, &vz);
     if (!g->typing) {
-        if (glfwGetKey(g->window, CRAFT_KEY_JUMP)) {
+        if (craftGetKey(g->window, CRAFT_KEY_JUMP)) {
             if (g->flying) {
                 vy = 1;
             }
@@ -2956,6 +3011,8 @@ int main(int argc, char **argv) {
     // OUTER LOOP //
     g_running = 1;
 #ifdef __EMSCRIPTEN__
+    emscripten_set_blur_callback(NULL, NULL, EM_TRUE, on_focus);
+    emscripten_set_focus_callback(NULL, NULL, EM_TRUE, on_focus);
     emscripten_push_main_loop_blocker(main_init, NULL); // run before main loop
     emscripten_set_main_loop(one_iter, 0, 1);
     //main_shutdown(); // called in one_iter() if g_inner_break

Continued iterating, but the state machine is becoming increasingly complex, and still doesn't quite fix all the edge cases.

@satoshinm
Copy link
Owner Author

Went with the partial fix, ignoring keystrokes onblur - still gets "stuck" when go back onfocus, but is a (slight) mild improvement in experience. May not be feasibly fixable without changing emscripten to change its keyboard state for glfw onblur, rather than layering another state machine over it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant