Keyboard structs reform

[tomassedovic]

I think I've screwed up the design of the keyboard structs. Observe what happens when you press the number 4 on the US keyboard layout alone and with shift:

$ cargo run --example minimal
Pressed key: KeyState { key: Special(Number4), pressed: true, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: false }
Pressed key: KeyState { key: Special(Number4), pressed: true, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: true }

The latter case should give us some way of knowing that the $ character was entered. There are cases (especially on the number row) where libtcod returns both a printable character and keycode.

So here's how we could fix it:

struct Key {
    code: KeyCode,
    printable: Option<char>,  // None if TCOD_key_t.c == 0
    left_alt, right_alt, left_ctrl, right_ctrl, shift, // unchanged
    alt: bool,  // proposed for easier matching: true if left_alt || right_alt
    ctrl: bool,  // proposed for easier matching: true if left_ctrl || right_ctrl
}

Questions:

1. Am I just missing something obvious that would not require the rewrite of every codebase using tcod?
2. Do we want Option<char> or just char with a potential 0 value? (the latter is what libtcod does)
3. alt/ctrl convenience fields: yay or nay? We could have them as methods instead, but that would make for a more verbose matching.

Here's how the new struct could be used:

use tcod::input::KeyCode::*;
match root.wait_for_keypress(false) {
    Key { code: Enter, alt: true, .. } => { // maximise the window }
    Key { printable: '$', .. } => { // pay for items in a shop }
    Key { printable: 'w', .. } | Key { code: NumPad5, .. } => { // wait a turn }
}

What do you folks think?

[zsparal]

I've been thinking on this, but I don't think there's a better solution than this:

struct Key {
    code: KeyCode,
    printable: char,
    left_alt, right_alt, left_ctrl, right_ctrl, shift, // unchanged
    alt: bool,  // proposed for easier matching: true if left_alt || right_alt
    ctrl: bool,  // proposed for easier matching: true if left_ctrl || right_ctrl
}

Yes, I'm proposing dropping the Option too. It makes matching annoying, and doesn't really provide any benefit (even if you match agains 0, nothing bad will happen). I kind of dislike storing the same information in multiple places (alt and ctrl keys), but it really is convenient: even with match guards the individual match arms are really long without them.

[tomassedovic]

Yeah, agreed on both counts (zero char and not being happy about duplicating alt/ctrl but they're convenient).

[zsparal]

The only other solution for the alt thing that comes to mind is a thin wrapper around bitflags:

use tcod::input::KeyCode::*;
match root.wait_for_keypress(false) {
    Key { code: Enter, modifiers: mods, .. } if mods.pressed(LeftAlt|RightAlt) => { // maximise the window }
    Key { printable: '$', .. } => { // pay for items in a shop }
    Key { printable: 'w', .. } | Key { code: NumPad5, .. } => { // wait a turn }
}

Or we could return a tuple of (Key, Modifier):

use tcod::input::KeyCode::*;
match root.wait_for_keypress(false) {
    (Key { code: Enter, .. }, mods) if mods.pressed(LeftAlt|RightAlt) => { },
    (Key { printable: '$', .. }, _) => { // pay for items in a shop }
    (Key { printable: 'w', .. } | Key { code: NumPad5, .. }, _) => { // wait a turn }
}

Unfortunately this is not much more idiomatic: bitflags are not really used in idiomatic Rust code (as far as I know). But I can't think of an alternative (except for defining all possible combinations in an enum and matching on that):

enum Modifiers {
    LeftAlt,
    RightAlt,
    Alt,
    LeftCtrl,
    RightCtrl,
    Ctrl,
    Shift,
    LeftAltLeftCtrl,
    LeftAltRightCtrl,
    LeftAltShift,
    // ...
}

Not very elegant (and hand-coding 2^n possibilities for all possible subsets is not very efficient either).

[tomassedovic]

Yeah. Piston does something similar: it has all the distinct modifier keys in the KeyCode enum among all other keys and then a separate Modifiers bitflags struct that has the combinations.

But I'd like to keep this reasonably close to libtcod (although implementing Into<piston::Key> sounds interesting -- especially if when combined with a Piston window backend for tcod).

Let's just go with this for now, though. If you don't mind:

struct Key {
    code: KeyCode,
    printable: char,
    left_alt, right_alt, left_ctrl, right_ctrl, shift, // unchanged
    alt: bool,  // proposed for easier matching: true if left_alt || right_alt
    ctrl: bool,  // proposed for easier matching: true if left_ctrl || right_ctrl
}

[zsparal]

I don't mind at all, even with my other suggestions I think that's the best idea.