search

godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.63k stars 21.1k forks source link

Improvements on the Keyboard action handling #6826

Closed Faless closed 7 years ago

Faless commented 8 years ago

With the current input action system, if you press a modifier + key and there is no action registered for that combination no action gets pressed. Even if there was one which had that key without modifier.

E.g.:

ActionMap

forward = W
left = A
right = D
shoot = Shift

If at some point I'm holding Shift to shoot, I cannot move (because the input event won't be W but Shift+W). This forces the dev to declare ActionMap this way:

forward = W, Shift+W
left = A, Shift+A
right = D, Shift+D
shoot = Shift

Which is annoying and hard to maintain if you have a good number of controls and are using one or two modifiers.

My proposal is to handle modifier this way:

I can start working on a PR if there is interest in this. Any comment?

vnen commented 8 years ago

IIRC modifier+key pressed the action mapped to just key. In fact, I've seen some complaints about the behavior. When has this changed?

Anyway, this issue describes the intended behavior, so a PR would be great.

Faless commented 8 years ago

IIRC modifier+key pressed the action mapped to just key. In fact, I've seen some complaints about the behavior. When has this changed?

I think it all started with 5b96c3a5527c1b2989dbfbe625f1c763b8887334 which changed the behavior of is_action_pressed

vnen commented 8 years ago

I think it all started with 5b96c3a which changed the behavior of is_action_pressed

Can be. Likely related to #6388 then.

rgrams commented 8 years ago

I don't think modifier keys should be handled as modifier keys at all. It makes sense for a text editor, but no sense at all for games. It means several of the most common keys used in games are basically unusable. If someone wants to use modifier keys in their game, that would be really easy to do with GDScript, but as you said, working around the way it currently works is a major hassle. Unfortunately it seems to be built into the base structure of InputEventKey.

As far as I know it's always been like this. Certainly before is_action_just_pressed was added.

Faless commented 8 years ago

I've been studying the code and thinking about a solution for a while now, here are my thoughts.

Another, more complex but optimized solution, could be to allow selecting which key events would ignore modifiers (or be blocked by them). CryEngine seems to do it this way (just by reading the docs, I have absolutely no experience with it).

@akien-mga , @vnen opinions?

@rgrams

I don't think modifier keys should be handled as modifier keys at all. It makes sense for a text editor, but no sense at all for games.

Yes, it's not very often needed maybe, but when you need it it's really nice to have it integrated. Also, the editor uses them a lot. and other engines have it (e.g. Unreal), or somehow deal with it (e.g. CryEngine). I think if implemented properly it will be a good feature

akien-mga commented 8 years ago

I don't think modifier keys should be handled as modifier keys at all. It makes sense for a text editor, but no sense at all for games.

RTS games? Implementing unit grouping behaviours like in Age of Empires/Warcraft etc. is trivial when you support modifiers (1 selected, Ctrl+1 defines, Shift+1 removes selection, etc.). Basically any game with an advanced UI can benefit from modifiers. Sure it's not needed for the platformer demo, but keep in mind that Godot is meant to support all types of games.

rgrams commented 8 years ago

@akien-mga , @Faless OK, my comment was totally exaggerated. I'm sure it's debatable whether modifiers are used for the majority of games or not. But my point was: it's really easy to add them yourself and much harder to work around them if they're hard-coded in.

If you agree, then you could say on average it actually supports all games better to not have built-in modifier support :stuck_out_tongue:, Though I'm definitely not saying that's the right solution. I don't think a quick add-on fix for this is a good idea.

Here is the problem that I see: Say you have three actions bound to these keys/combos: A, Control, and Control+A, If you press Control+A (with a quick little gallop motion), you get the following sequence:

The problem here is that "A" never gets its "pressed" call, and "Control+A" never gets released. Two actions are stuck halfway. If you do the same keystroke with slightly different timing (release Control and A at exactly the same time), "Control+A" and then "Control" are released. That makes more sense, but still the "A" action is being skipped entirely. And that difference in timing is pretty small; not something you want to rely upon.

The same thing happens in any combination of course. If you press "A", press "Control", then release "A", then release "Control". You get: "A pressed", "Control pressed", "Control+A released", "Control released". "A" never gets released and "Control+A" never gets pressed. If you're doing, say a 3rd-person game with WASD movement and Shift to crouch, you're screwed.

I get why this is, a lot of the time you'll have "Ctrl+1" mapped to something and you don't want it to fire the command on "1" at the same time. In those cases you won't have anything mapped to Ctrl, so that first key press doesn't matter.

. . . hmm . . . kind of a black and white problem I guess. With modifiers you need to ignore a bunch of input, without them you need to get everything. Maybe that Cryengine "noModifiers" flag isn't a bad idea. (though "Ignore Modifiers" would be clearer.)

Faless commented 8 years ago

The problem here is that "A" never gets its "pressed" call, and "Control+A" never gets released. Two actions are stuck halfway

Yes, that is exactly the bug in #6388 but those cases are easily solvable by saying that the release events ignore modifiers. (eg here, I'll PR that soon).

If you're doing, say a 3rd-person game with WASD movement and Shift to crouch, you're screwed.

I agree, but it doesn't have to be a black or white situation. What I'd like to achieve is to support both behaviors without requiring game devs to code them.

We can solve it with a best match approach (eg. described at top) or a specify behavior one (eg. specifying in input maps if the key ignore or gets blocked by modifiers). There is even a third option I was thinking about which is setting engine behavior for every modifier via engine.cfg (eg, input/shift_as_modifier) and GDScript (and InputMap.set_shift_as_modifier(bool enable)).

I keep throwing in new ideas but this is not an easy problem and deserves a refined solution :tophat:

bojidar-bg commented 8 years ago

Hmm.. I think I have an idea, though I'm not sure how good it would be...

First of all, what about allowing all keys to act as modifiers? That way, we would have a (probably) better solution, since we would have to hardcode way less things.

Second, I think that we should say for each combination of keys/modifiers if it is "singular" "exclusive". Then, we might have the following:

rgrams commented 8 years ago

those cases are easily solvable by saying that the release events ignore modifiers.

Really? Won't that(#6862) just patch up some use cases but leave just as many issues? Then you can still get a release event without getting a pressed event. That's exactly the kind of "fix" that worries me.

Say you give the following input with your keyboard: Shift-pressed, A-pressed, A-released, Shift-released

If you make released events ignore modifiers: Result: Shift-pressed, Shift-A-pressed, Shift-A-released, A-released, Shift-released.

Current behavior: Result: Shift-pressed, Shift-A-pressed, Shift-A-released, Shift-released.

So in this situation it gets worse. The way I see it you're just moving the problem from one use case to another.

rgrams commented 8 years ago

@Faless The per-action specify method is starting to look like the best option to me. Your third option, specifying it globally could be cool too, but might exclude some possible uses. I'd say if you could specify it for each action, but set the global default, that would be best. That way you'd have all the options, but no one would ever be stuck toggling it for every action.

@bojidar-bg Allowing any key combinations sounds great to me. Perhaps a better word would be "exclusive"? I see a problem though. Say you have A and S as normal actions, and A+S as an "exclusive" action. Then wouldn't pressing A and S release both and press A+S? Or if only one key is released because of the exclusive action, how do you decide which one? Only the last?

bojidar-bg commented 8 years ago

@rgrams Ahhh, right, "exclusive" was the word!

Well, only one of them would get the release event, which would be the first one to press, since it would be the only one for which a press event was generated. When releasing a combo, there would be a quick press/release of the last released key, since we have no idea that you want to release the whole combo and not only a key from it. Maybe an additional option would be needed for this :/

vnen commented 8 years ago

This "exclusive" stuff seems very complicated. If you have an action mapped to A, it should be pressed/release when the A key is pressed/released, no matter what the other keys' states are.

If you have an action mapped to S+A, it should be pressed when both keys are pressed and released when either key is released.

Those aren't mutually exclusive, so you may have both pressed at the same time. Now if want "exclusive" behavior for some reason, it's easy to code:

if is_action_pressed('single') and not is_action_pressed('combo'):
    # do stuff

If you want combo to take precedence then it might be even easier:

if is_action_pressed('combo'):
    # do stuff
elif is_action_pressed('single'):
    # do other stuff

I'm not sure it's that useful to have such behavior embedded in the engine. Is there some use case as an example?

hubbyist commented 8 years ago

Keys must be treated as just keyboard signals IMO. None must be special for the engine.

So

"Control" is pressed. "A" is pressed. Thus --> "Control+A" is pressed. "Control" released. Thus --> "Control+A" is released. "A" released.

and

"B" is pressed. "A" is pressed. Thus --> "B+A" is pressed. "B" released. Thus --> "B+A" is released. "A" released.

Must always behave similar I think.

Faless commented 8 years ago

@rgrams thanks for reviewing my PR. It should be fixed now.

I also uploaded my input test project on github so you can do some tests ( https://github.com/Faless/godot-input-test ). It's still pretty simple.

It would be nice to create automated tests for input.

bojidar-bg commented 8 years ago

@vnen Maybe I indeed don't have an actual usecase, hehe. So, just think of all keys are modifiers, and leave exclusiveness to users..

bojidar-bg commented 7 years ago

What's the status on this issue?

Faless commented 7 years ago

What's the status on this issue?

Well, I guess I got sucked into the networking part and I forgot about this.

To sum it up, these are the alternative proposals:

  1. Drop modifiers (I don't like the idea of losing a feature)
  2. Use best match approach (i.e. use action F when Shift+F is pressed if no Shift+F action is defined) (could be counterintuitive)
  3. Add option for ignoring modifiers (i.e. checkbox in InputMap event), if true extra modifiers will be ignored eg. action ctrl+F with ignore_modifers = true will fire when ctrl+shif+F is pressed but not when shift+F is pressed. (gives good control over input without losing the modifier feature)

Additional optional feature of 2 or 3:

I would go for option 3 without the additional feature which could be coded via script anyway.

I think we also need an opinion from @reduz

punto- commented 7 years ago

I like option 3, it's pretty common to have for example ctrl+1 and 1 be different actions (like assign control group 1 and use control group one), you don't want the engine doing guess work behind your back

On 25 January 2017 at 13:33, Fabio Alessandrelli notifications@github.com wrote:

What's the status on this issue?

Well, I guess I got sucked into the networking part and I forgot about this.

To sum it up, these are the alternative proposals:

  1. Drop modifiers (I don't like the idea of losing a feature)
  2. Use best match approach (i.e. use action F when Shift+F is pressed if no Shift+F action is defined) (could be counterintuitive)
  3. Add option for ignoring modifiers (i.e. checkbox in InputMap event), if true extra modifiers will be ignored eg. action ctrl+F with ignore_modifers = true will fire when ctrl+shif+F is pressed but not when shift+F is pressed. (gives good control over input without losing the modifier feature)

Additional optional feature of 2 or 3:

  • Possibility to use any key as modifier (eg. A+E is a key combination) (could be heavy)
    • Ordered (seems complex)
    • Unordered (better, but it still seems too much for me)

I would go for option 3 without the additional feature which could be coded via script anyway.

I think we also need an opinion from @reduz https://github.com/reduz

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/6826#issuecomment-275157885, or mute the thread https://github.com/notifications/unsubscribe-auth/AGVmPby6gkr1acy3lF2_dueD5fe7m9arks5rV3lmgaJpZM4KW8fI .