Key::printable ignores modifiers

[abesto]

When pressing (on a US layout) Shift + . I expect the printable field of tcod::input::Key to be >. Instead, it is .. (The code: Text event seems to have the correct data in the text field). Based on https://github.com/tomassedovic/tcod-rs/pull/187 and https://tomassedovic.github.io/roguelike-tutorial/part-11-dungeon-progression.html I believe printable: '>' is the expected behavior in this scenario.

• tcod-rs version: 0.14.0
• rustc --version: rustc 1.34.2 (6c2484dc3 2019-05-13)
• OS: Windows 10 Pro (build 17134.766)

Events from pressing .

Key(Key { code: Char, printable: '.', pressed: true, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: false, alt: false, ctrl: false, text: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] })
Key(Key { code: Text, printable: '.', pressed: true, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: false, alt: false, ctrl: false, text: [46, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] })
Key(Key { code: Char, printable: '.', pressed: false, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: false, alt: false, ctrl: false, text: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] })

Events from pressing >

That is:

• shift down
• . down
• . up
• shift up

Key(Key { code: Shift, printable: '\u{0}', pressed: true, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: true, alt: false, ctrl: false, text: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] })
Key(Key { code: Char, printable: '.', pressed: true, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: true, alt: false, ctrl: false, text: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] })
Key(Key { code: Text, printable: '.', pressed: true, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: true, alt: false, ctrl: false, text: [62, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] })
Key(Key { code: Char, printable: '.', pressed: false, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: true, alt: false, ctrl: false, text: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] })
Key(Key { code: Shift, printable: '\u{0}', pressed: false, left_alt: false, left_ctrl: false, right_alt: false, right_ctrl: false, shift: false, alt: false, ctrl: false, text: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] })

I'd expect both code: Char events to show printable: '>' (note also, they both show shift: true).

Repro: https://github.com/abesto/tcod-modifier-repro/blob/master/src/main.rs

[abesto]

After some digging, it would appear tcod-rs only directly takes whatever it receives from tcod. http://rogueliketutorials.com/tutorials/tcod/part-11/ says:

*Note: I've used the Enter key here rather than the traditional '>' key. This is because the current Roguebasin tutorial's code for the '>' key does not work.

So this might be an issue with tcod itself? I guess the Rougebasin tutorial in question is http://www.roguebasin.com/index.php?title=Complete_roguelike_tutorial_using_C%2B%2B_and_libtcod_-_part_11:_dungeon_levels_and_character_progression - I've looked at the code, and it is indeed just using the characters directly as presented by libtcod.

[abesto]

https://github.com/libtcod/libtcod/issues/12 seems to be asking about the same symptoms, and the answer is that for anything other than special keys (escape, backspace, etc) we should rely on the TextInput event (whatever that is). This seems to match what the repro above shows, in that the text field contains the right data (even though the text field is private, so not directly usable in client code ATM)

[abesto]

Oops, I've just noticed the already existing public text method. I'm thinking it'd probably be nice to expose it as a field for pattern matching, but that's a minor annoyance; storing its return value and matching on that seems to work well (see https://github.com/abesto/rl/commit/bb8230340894d55810ec74babf7f438d42116f62#diff-83128b042459c6593f6f3810f519731cR96)

I'll leave this open as I still think printable is misleading. Also, the tutorial would I guess benefit from using text() instead of printable (which just plain doesn't work) :P

[ndouglas]

Thanks for figuring this out for me 😄

[tomassedovic]

The tutorial has been updated to use Key::text().

I agree that this is misleading. libtcod does expose the "printable" field (under the wonderfully descriptive name c). I wrote those Rust bindings and yet I made the same mistake in the tutorial.

I'm not really sure what to do here. As far as I can tell, using text() is always the right thing, but there may be situations I'm not aware of.

I'd be open to renaming printable back to c (this would need a version bump) pointing people at text in the field doc. Or dropping it entirely but like I said, there may be legitimate uses of it we don't know about.

We already have the text field in the Key struct. Except it's private right now and contains the internal FFI representation we got from libtctod:

https://github.com/tomassedovic/tcod-rs/blob/deda7eccb54fcd3ff71a6a687e4f3cb7f01c0d9d/src/input.rs#L92-L131

I'm not off the top of my head sure how to handle the c_char array to &str conversion without allocating memory (I'd rather not have a String field in the struct), but if someone wants to take a stab at that (or at least updating the docs), be my guest!

Unfortunately, given life circumstances, available time and priorities I am unlikely to contribute much to tcod-rs in the future.

[L3nn0x]

to convert from a c_char array to a &str, we should be able to use this: https://doc.rust-lang.org/std/ffi/struct.CStr.html

I'm not sure I'll have time to implement it, but I'll see what I can do.