Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add input event callback to DisplayServerHeadless #92806

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

mihe
Copy link
Contributor

@mihe mihe commented Jun 5, 2024

This adds input capabilities to DisplayServerHeadless. by simply adding an input event callback to it, in pretty much the exact same way as how DisplayServerMock does it, while also removing the now redundant one from DisplayServerMock.

This could perhaps be considered a mildly controversial change, seeing as how input handling is tied to windows in the eyes of the display server API, but in the interest of sparking a discussion about it, here it is.

The motivation for this isn't too complicated: When dealing with things like unit tests that run on a headless CI, it can be desirable to manually inject input events (with something like Input.parse_input_event) in order to test the input code paths, or perhaps simulate an actual user more accurately.

This is technically possible already by relying on Viewport.push_input instead, but there you run into the problem that's mentioned in its documentation, namely that input emulation (like input_devices/pointing/emulate_touch_from_mouse) doesn't exist in that context, leaving you to having to recreate/copy this emulation into your own code instead, which is somewhat unfortunate and potentially brittle. This emulation also can't be added to Viewport.push_input since that's in turn used by Input.parse_input_event, so you'd end up not only with double emulation, but emulated events themselves triggering further emulation.

The much simpler solution to this seems to be to just add an input event callback to DisplayServerHeadless, allowing you to use Input.parse_input_event and its emulation just fine.

@Calinou
Copy link
Member

Calinou commented Jun 5, 2024

Does this PR work with #92198?

@bruvzg
Copy link
Member

bruvzg commented Jun 5, 2024

Does this PR work with #92198?

#92198 simulate native OS events on the OS API level, therefore it needs real DisplayServer which can handle OS events to work. It won't work with DisplayServerHeadless, or interfere with this PR in any way.

@mihe

This comment has been minimized.

@mihe mihe requested a review from a team as a code owner June 5, 2024 18:20
@mihe

This comment has been minimized.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

This is pretty important for unit testing so I'll merge despite the feature freeze for 4.3, and consider it for a 4.2.3 cherry-pick.

@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jul 1, 2024
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jul 1, 2024
@akien-mga akien-mga merged commit d4fdf16 into godotengine:master Jul 1, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@mihe mihe deleted the headless-input branch July 1, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release enhancement topic:input
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants