sezero / quakespasm

QuakeSpasm -- A modern, cross-platform Quake game engine based on FitzQuake.
https://sourceforge.net/projects/quakespasm/
GNU General Public License v2.0
244 stars 97 forks source link

Fix on-screen keyboard showing up unnecessarily on Steam Deck #54

Closed Macil closed 1 year ago

Macil commented 1 year ago

On the Steam Deck, SDL_StartTextInput causes an on-screen keyboard to show up after about a second that you can control with the controller inputs. However, Quakespasm often calls SDL_StartTextInput while starting a map or loading a save which is an extremely janky experience. I often die in a map, press trigger to respawn from my last save, start running and shooting for a good second, and then the on-screen keyboard comes up, steals all my controller inputs until it goes away a half-second later, and my character stands helpless while enemies attack.

This PR fixes it so SDL_StartTextInput isn't called in two distinct cases: when key_dest == key_game && con_forcedup is true which is while starting a map or loading a save, and when key_inputgrab.active is true which is during the "Are you sure you want to start a new game? (y/n)" prompt (which already supports controller A/B buttons, so it's disruptive to have an onscreen keyboard after you've already instantly plowed through the menu). Other text input cases like the console, chat, and the name input in the multiplayer setup menu all still work as before calling SDL_StartTextInput and activating the on-screen keyboard.

(Okay, I haven't actually gotten Quakespasm specifically running on the Steam Deck, just vkQuake, but while exploring the code and debugging both on desktop I've found its behavior is consistent with vkQuake, and I figure getting input fixes like this upstream is good.)

If you set "in_debugkeys 1", you can see in the console whenever SDL_StartTextInput and SDL_StopTextInput are called, so someone without a Steam Deck can still test to see the cases it would show up. (The on-screen keyboard will show up for a good half second even if SDL_StartTextInput is called and immediately canceled with SDL_StopTextInput.)


Preventing SDL_StartTextInput from getting called prevents SDL_TEXTINPUT events from happening. If SDL_StartTextInput was prevented where it shouldn't be, then this could prevent the player from being able to type while they're supposed to.

The first commit handles the first case where key_dest == key_game && con_forcedup. I believe this case is when the console may be briefly visible during loading, but I don't think you're actually supposed to be able to type into the console during this so I don't think there's an issue there. If there's another case where this condition is true, then there could be an issue, but I haven't found one. (con_forcedup is set to !cl.worldmodel || cls.signon != SIGNONS, so I think it must only be relevant when a map isn't loaded or you're not connected to a server. That made me think it might also describe the case where you have startup demos turned off and you close the menu so the fullscreen console is present, but in that case key_dest == key_console instead of key_game so this PR doesn't affect that.)

The second commit handles the second case of when key_inputgrab.active is true. This is used just for y/n prompts (and some dead Con_NotifyBox function which waits for any key to be pressed) apparently so that keypresses would come through as SDL_TEXTINPUT events respecting the keyboard layout instead of it being disregarded for physical-location-based scancodes as usual. Instead of relying on SDL_TEXTINPUT events here, I'm handling this directly by reading the keycode from the SDL_KEYDOWN event and using it for key_inputgrab. This PR successfully preserves the behavior that at a y/n prompt, you need to press whatever your keyboard layout considers as y/n instead of the keys located in the US layout's y/n positions.


Since there's an overlap in people, I figured I'd proactively mention this PR was simultaneously developed for and tested with vkQuake: https://github.com/Novum/vkQuake/compare/master...Macil:vkQuake:fix-textinput. The first two commits are identical, but there was a third commit necessary in vkQuake to handle one more case because of a difference in how it handles quitting the game. Otherwise, vkQuake spawns an on-screen keyboard as it closes.

sezero commented 1 year ago

@ericwa: This looks OK to me. What do you say?

ericwa commented 1 year ago

Looks fine, thanks

sezero commented 1 year ago

This is in. Thanks.