Document when to use _unhandled_key_input over _unhandled_input

[groud]

As discussed in IRC with @reduz and @EricEzaM

[EricEzaM]

I don't think the right language is used here. The call doesn't actually "propagate" from what I can tell (see SceneTree::_call_input_pause).

The input event propagates up through the node tree

This language might mislead the user into thinking the call is repeated on parents until it is handled, or until the last parent. This is not correct. call_input_pause only calls the method on the node it is registered to call on. For example, MenuButtons are registered to receive unhandled_key_input. The _unhandled_key_input method is only called on the MenuButton - not parents, and not children.

I would probably change this to something more accurate:
"_unhandled_input is called on every node which is registered to receive it until one of the nodes handles the input."

However this relies on the information that "It is only called if unhandled input processing is enabled" so this might be better to place after the next line
The usage of the word "propagates" may also mislead the user into thinking at Object::propagate_call is used behind the scenes for this functionality.

It may also be worth noting that the order of the nodes it is called on is based on order in the scene tree - based on Node::is_greater_than.

[EricEzaM]

P.S. I know this isn't part of the PR but I thought since we are here, let's discuss

[groud]

You are right on all the points, but to be honest I don't think we should care that much.
The function is indeed not called on all the nodes, as they indeed have to implement the method. I am not sure it is worth mentioning to the user, as they probably know know an unexisting function cannot be called.

Regarding the "propagate" term, in the end it's the same. It's a similar behavior to calling Node::propagate_call, as the node order produced by Node::is_greater_than will lead to the same order as what a reversed propagate_call would do. That's why the documentation mentions "propagates up through the node tree".

We have to write the documentation from the user point of view, what happen behind the scene for performance reasons (like adding the nodes to a group, so that the function is only called on specific nodes, etc...) is not important. What matters is the end result.

[akien-mga]

Oldie, but still goodie? Would need a rebase and maybe re-discussion with current GUI experts.

[NathanLovato]

Yeah it's looking good.

[akien-mga]

Approved in PR review meeting to merge once rebased.

The discussion on existing language is not fully resolved but this can be reworked in a future PR / discussed in another issue.

[bruvzg]

Unhandled input processing was changed a bit, now there are 3 stages:

• shortcut_input - processed first, used for global shortcuts. Only Key and Shortcut events.
• unhandled_input - processed if event was not consumed by shortcut_input. All events.
• unhandled_key_input - processed if event was not consumed by unhandled_input, it's only used to handle text input with Alt+ and Ctrl+ modifiers. Only Key events.

[YuriSizov]

@groud Would you mind incorporating the changes that bruvzg mentions? (and fixing the typos mentioned in the review). Your additions touch directly on the need to use keyboard shortcuts, so I guess shortcut_input should be recommended as the first option.

[akien-mga]

Closing in on a 3 year anniversary here... @groud are you able to do the required adjustments?

[groud]

I missclicked the "ready for review" button, sorry. I'll have a look today

[groud]

Updated, let me know what you think. :)

[Sauermann]

The changes describe the implementation correctly as far as I can tell.

[YuriSizov]

Phew, finally done with this one! One day shy of a 3 year anniversary :D Thanks!