SDL: Some keybinds broken due to missing character lookup

[Desour]

Opening the chat with / is broken on my keyboard setup. I write it with shift+7. It's instead just interpreted as 7 (= select item in horbar).

[numberZero]

Shouldn’t key bindings depend on key positions and not on symbols? Otherwise switching a layout would break them (and e.g. I do that often: not all scripts are Latin)

[Desour]

Ideally binding both types of keys should be possible. In the case of /, the written character makes much more sense as the key position.

The thing is, minetest currently allows binding symbols, so to not change user experience, SDL should support this too, imo.

[numberZero]

The thing is, minetest currently allows binding symbols, so to not change user experience, SDL should support this too, imo.

Fixing bugs is changing user experience, too.

Ideally binding both types of keys should be possible.

In theory, maybe. But in practice, that would only confuse players. Key location is simple to understand, simple to code, and always works. While characters... aren’t. There are just too many things that may go wrong with them.

P.S. Default keybindings may need some rethinking too. E.g. I for inventory isn’t all that good, it’s too far from WASD.

[sfan5]

Key location is simple to understand, simple to code, and always works. While characters... aren’t. There are just too many things that may go wrong with them.

A bit off-topic but I don't think it's all that simple.
When you do this you get into the business of needing to display the correct key name to the user (do you save the raw keycode in minetest.conf?). Plus it gets messy when the layout changes.

[numberZero]

do you save the raw keycode in minetest.conf?

Scancode. Not keycode. That’s the whole point.

display the correct key name to the user

SDL_GetKeyName(SDL_GetKeyFromScancode(...)) maybe?

Plus it gets messy when the layout changes.

SDL_GetKeyName apparently uses the layout that was active when SDL_Init was called. Totally fine where the current layout is per-application (so at the init call time, user’s default layout is current). Not that good for global-current-layout systems but acceptable IMO. Much better than movement keys not working due to starting Mesebox with the wrong current layout.

[Montandalar]

I can see two distinct issues here:

• Certain keys don't register properly, even on the US layout such as `, and the / on the German layout as Desour noted. Those keys can't be bound, or existing custom binds used when bound in earlier, non-SDL versions.
• The default settingtypes values for increasing and decreasing render distance are invalid for SDL2 compared to pre-SDL irrlichtMt. Either non-SDL needs to be dropped and the defaults fixed, or some kind of compatibility layer needs to be there to ensure the binds work no matter SDL is used or not.

[Montandalar]

@nephele-gh reported needing to rebind sneak from the default "Shift" to "Left Shift" upon switching from 5.8 to 5.9.0-dev on macOS. Render distance is one thing, but having a core gameplay control like sneak broken by default is not good.

I'll reinforce the point in my last comment. I think a clear upgrade path for existing controls is needed: something like either a layer to translate old/existing bind names to SDL ones or at least working new defaults.

[grorp]

Note that SDL2 isn't (yet) enabled by default on macOS. The cause of nephele-gh's bug is probably this, which should be disabled when compiling with SDL:

https://github.com/minetest/minetest/blob/bceef8f5293fb935a78d143976fe1a0eecfdf5de/src/defaultsettings.cpp#L541-L544

PR'd a fix: #14754

[grorp]

It seems that SDL fails to provide the information we need here. Like Desour, I have a German keyboard where slash is Shift + 7. SDL has a keycode named SDLK_SLASH, so I thought we could just listen for that keycode and everything would work.

diff --git a/irr/src/CIrrDeviceSDL.cpp b/irr/src/CIrrDeviceSDL.cpp
index ecea5bac3..6f08d7fa4 100644
--- a/irr/src/CIrrDeviceSDL.cpp
+++ b/irr/src/CIrrDeviceSDL.cpp
@@ -2,6 +2,7 @@
 // This file is part of the "Irrlicht Engine".
 // For conditions of distribution and use, see copyright notice in irrlicht.h
 
+#include <iostream>
 #ifdef _IRR_COMPILE_WITH_SDL_DEVICE_
 
 #include "CIrrDeviceSDL.h"
@@ -817,6 +818,9 @@ bool CIrrDeviceSDL::run()
                        irrevent.KeyInput.Shift = (SDL_event.key.keysym.mod & KMOD_SHIFT) != 0;
                        irrevent.KeyInput.Control = (SDL_event.key.keysym.mod & KMOD_CTRL) != 0;
                        irrevent.KeyInput.Char = findCharToPassToIrrlicht(mp.SDLKey, key);
+                       std::cerr << "got " << (SDL_event.type == SDL_KEYDOWN ? "SDL_KEYDOWN" : "SDL_KEYUP") << ". " <<
+                                       "SDL_event.key.keysym.sym = " << (int)SDL_event.key.keysym.sym << "; " <<
+                                       "Irrlicht key = " << (int)key << "; assumed char = " << irrevent.KeyInput.Char << std::endl;
                        postEventFromUser(irrevent);
                } break;
 

When pressing Shift + 7, I get the following output:

got SDL_KEYDOWN. SDL_event.key.keysym.sym = 1073742049; Irrlicht key = 160; assumed char = 0
got SDL_KEYDOWN. SDL_event.key.keysym.sym = 55; Irrlicht key = 55; assumed char = 55
got SDL_KEYUP. SDL_event.key.keysym.sym = 55; Irrlicht key = 55; assumed char = 55
got SDL_KEYUP. SDL_event.key.keysym.sym = 1073742049; Irrlicht key = 160; assumed char = 0

1073742049 = SDLK_LSHIFT
55 = SDLK_7

SDLK_SLASH is not sent.

While searching the SDL issue tracker, I found this: libsdl-org/SDL#3559

[grorp]

With the changes to SDL3 coming from libsdl-org/SDL#3559, SDL3 makes it easy to reimplement the behavior of other Irrlicht devices in CIrrDeviceSDL. I have a version that fixes the slash issue here: grorp@48db7e5 (please note that I didn't do the SDL2 -> SDL3 migration properly) (the numpad +/- keys are still not working properly, haven't investigated)

[grorp]

Numpad +/- fixed by #14780.

[ExeVirus]

So to clarify: the issues remaining are:

• MacOS using non-SDL backend
• shift-F7 not mapping to /
• ` not mapping on US layouts

Anything else?

[grorp]

The SDL people have done more fancy things to key input for SDL 3 btw: libsdl-org/SDL#10156

[y5nw]

` not mapping on US layouts

Not just the backtick key. Umlaut keys (e.g. ü) are not mapped either.

Anything else?

Dead keys appear to be mapped to the "Left Button" (left mouse button).

[rubenwardy]

Related: #14940

[sfence]

Looks fine on Mac OS, both in the Minetest build for Intel and ARM.

I tested two keyboard layouts where "/" required normal key press and shift + key press. Works as expected in both states.

[grorp]

Looks fine on Mac OS, both in the Minetest build for Intel and ARM.

I tested two keyboard layouts where "/" required normal key press and shift + key press. Works as expected in both states.

Our macOS builds don't use SDL by default. To reproduce the issue, you have to build with -DUSE_SDL2=1 explicitly.

[sfence]

Looks fine on Mac OS, both in the Minetest build for Intel and ARM.
I tested two keyboard layouts where "/" required normal key press and shift + key press. Works as expected in both states.

Our macOS builds don't use SDL by default. To reproduce the issue, you have to build with -DUSE_SDL2=1 explicitly.

Oh yes. In the Minetest built on Mac with -DUSE_SDL2=1, shift + key does not open a chat.