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

[unittests] Send Mouse events via DisplayServer instead of push_input #71972

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Jan 24, 2023

Currently Unittests simplify mouse-events by just pushing them to Viewports. For dealing with mouse-screen-coordinates (caused by the introduction of multiple native Windows) it becomes necessary to extend the DisplayServer functionality for unittests.

This PR introduces DisplayServerMock based on DisplayServerHeadless, which additionally supports basic Mouse-Input handling.

This functionality is required for:

Updated 2023-01-29: Renamed DisplayServerTests to DisplayServerMock.

Copy link
Member

@Geometror Geometror left a comment

Choose a reason for hiding this comment

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

I haven't reviewed it properly yet, but at a first glimpse it looks great! I wouldn't, however, call it DisplayServerTests. Since it's basically a mock object I think that DisplayServerMock would be more descriptive.

Currently Unittests simplify mouse-events by just pushing them to Viewports.
For dealing with mouse-screen-coordinates (caused by the introduction of
multiple native Windows) it becomes necessary to extend the
DisplayServer functionality for unittests.

This PR introduces DisplayServerMock based on DisplayServerHeadless,
which additionally supports basic Mouse-Input handling.
@akien-mga
Copy link
Member

Haven't reviewed yet, but is there any reason why we wouldn't want to implement this mock support directly in DisplayServerHeadless?

@Sauermann
Copy link
Contributor Author

I also thought about this but took this safe approach with a new class, because I am not familiar with where DisplayServerHeadless is used and the relevant requirements there.
Let me know if I should move the changes to DisplayServerHeadless.

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.

Approved in PR review meeting.

@akien-mga akien-mga merged commit cf2bf08 into godotengine:master Jan 31, 2023
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants