Don't mark all input as handled when subwindow is focused

[0x0ACB]

Currently, Godot will mark all input as handled as soon as a subwindow is focused, even if local input handling of that window is off. This causes issues with input handling like #61758.

[akien-mga]

CC @Sauermann

[akien-mga]

Please use tabs for indentation instead of spaces. You can also run clang-format locally to ensure you follow the style guide: https://docs.godotengine.org/en/latest/contributing/development/code_style_guidelines.html#using-clang-format-locally

Once fixed, please also squash the commits. See PR workflow for instructions.

[0x0ACB]

Please use tabs for indentation instead of spaces. You can also run clang-format locally to ensure you follow the style guide: https://docs.godotengine.org/en/latest/contributing/development/code_style_guidelines.html#using-clang-format-locally

Once fixed, please also squash the commits. See PR workflow for instructions.

That was the change request of the other guy :D It was using tabs before

[Eoin-ONeill-Yokai]

That was the change request of the other guy :D It was using tabs before

I'm sorry, github doesn't like me using tabs in the browser as it tries to shift focus. I didn't mean for it to be applied as a commit directly.

[0x0ACB]

No problem. You would think that with them also offering Visual Studio Code in the browser the online editor would be better with that ^^

[Sauermann]

My understanding is, that this PR tries to solve #61758.

I am not entirely sure, if this can be possible, because #61758 describes a bug related to native windows, for which a workaround is to use embedded windows as described in https://github.com/godotengine/godot/issues/61758#issuecomment-1221406364. And this PR changes the behavior of embedded windows, but not of native windows.

Do you have a MRP available in order to test this PR? I am asking, because I am unable to replicate the behavior of the linked issue.

[0x0ACB]

This PR is not aimed at fully solving the issue in #61758 it does only help in certain circumstances. It mostly fixes issues with unhandled_input if you have open subwindows

[Sauermann]

@0x0ACB you seem to have some issues about unhandled_input in mind. Could you please provide a description of such an issue, that this PR fixes? Ideally a MRP would help for reviewing this PR.

[0x0ACB]

here you go: input.zip

Without fix:

• pressing keys ->color rect goes red
• open window -> press keys -> color rect stays green as long as the subwindow is focused even though nothing handles the input and handle input local is off

With fix:

• color rect goes red in both scenarios

Regarding #61758: So far I only ever had this issue with the editor, though I am not really using context menus in my game so that might not be a good indicator, but I can't really give you an MRP here. It just randomly happens sometimes. While this PR does not fully fix the issue it at least makes it much rarer in my experience. Case in point: doing this sample with the official 4.0.2 build I had the issue 2 times while I didn't have it at all the last couple of weeks using my patched version.

[Sauermann]

The use-case, that you are describing would lead to the following behavior, that I try to describe in a different setting:

1. Have a text-editor window open in your OS
2. Have a browser window open in your OS
3. Focus the text-editor window, then afterwards focus the browser window
4. Type in the key "a" (an example for a key, that isn't used by the browser)
5. The key "a" is not used by the browser
6. According to your use case, you would expect, that the key "a" should be sent to the text-editor, so that the letter "a" appears in the edited text, even though the text-editor window is not focused

I haven't seen this kind of behavior in any window manager (Linux & Windows), so this change would mean, that embedded Godot-windows diverge from the behavior of native OS-windows. I am not entirely convinced, that this is a good idea.

It would probably be best to open a proposal, where this feature-request can be discussed and perhaps there is enough support for it.

[0x0ACB]

So first of all this was the behavior in godot 3.x so having this changed actually breaks a lot of things and in a game I would expect to still be able to move around even if I have my inventory open for example. Also the behavior for non embedded windows should still be the same since the godot main window at least also doesn't mark unhandled input as handled. So if anything this change brings the behavior of embedded subwindows in line with how the main window behaves already.

Also your example is not entirely matching what this change achieves. This is only changing the behavior of unhandled_input. gui_input is not affected. So pressing "a" in your example would only be handled by the text editor if the text editor listens for unused input events not for the normal WM_CHAR. There is no passing down of input events to parent windows.

[Sauermann]

The Window class doesn't exist in 3.x but got introduced newly in 4.x. In 3.x you likely have used Control-derived nodes for inventories and it is still possible to use the same approach in 4.x without relying on Window nodes. So I don't see, how this could be considered a breaking change.

Thanks for the example with the inventory, this helps me to understand your use-case. Getting Key-input events to be treated in the way, you are describing for Windows, is something, that is not yet supported by the engine.

I believe, that you have found the correct location for this change. However I have concerns about using handle_input_locally. That flag is currently used for fixing a SubViewport-related behavior and it's description is rather non-helpful and it is possible, that it will get deprecated (#77926). Perhaps it would be better to introduce a new flag with a more descriptive name and description. I could see this being either a Viewport-flag (affects all its embedded windows) or a Window-flag (affects this window in its embedder).

So pressing "a" in your example would only be handled by the text editor if the text editor listens for unused input events not for the normal WM_CHAR

I am afraid, that this is not entirely true, because if the event is not handled by the first window, then it will be sent to the second (text-editor) window in which all input-processing steps happen (including gui_input and unhandled_input). Also please note, that a Window-node doesn't receive InputEvents via gui_input or unhandled_input, but only via Window::_window_input, so it behaved differently in comparison to a Control-node.

[0x0ACB]

The Window class doesn't exist in 3.x but got introduced newly in 4.x. In 3.x you likely have used Control-derived nodes for inventories and it is still possible to use the same approach in 4.x without relying on Window nodes. So I don't see, how this could be considered a breaking change.

In 3.x the Window class was called WindowDialog https://docs.godotengine.org/en/3.6/classes/class_windowdialog.html#class-windowdialog. The conversion script converts all WindowDialogs to Window. So that is why a lot of stuff did break in my project.

I am afraid, that this is not entirely true, because if the event is not handled by the first window, then it will be sent to the second (text-editor) window in which all input-processing steps happen (including gui_input and unhandled_input). Also please note, that a Window-node doesn't receive InputEvents via gui_input or unhandled_input, but only via Window::_window_input, so it behaved differently in comparison to a Control-node.

Adding _gui_input to the parent control in my example or even adding a LineEdit you will notice that unhandled keypresses or mouse movements are not forwarded to the parent window as long as you focus the subwindow. Eg adding

func _gui_input(event):
	print(event)

to the root Node2D will not print anything as long as the subwindow is focused.

[Sauermann]

We discussed this in today's PR-review-meeting but didn't come to a conclusion. The change needs to be investigated and tested more. Since this changes a behavior, it would be great if you could open a feature proposal at https://github.com/godotengine/godot-proposals/ so that the need for this change could be discussed.