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

Do not attempt to set cursor shape in headless mode. #89099

Merged

Conversation

chrisl8
Copy link
Contributor

@chrisl8 chrisl8 commented Mar 2, 2024

Every time I run anything in --headless mode I receive this warning:

WARNING: Custom cursor shape not supported by this display server.

This does not seem helpful or useful to me. Instead it distracts from the process and encourages us to get used to ignoring errors and warnings without reading them each time.

My, possibly naive, interpretation of the code is that it can skip this step always when running --headless with no negative effect, but if I'm wrong, feel free to reject.

It seems to work for me.

Copy link
Contributor

@Riteo Riteo left a comment

Choose a reason for hiding this comment

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

I do agree that we shouldn't do things that emit warnings in core code.

That said, the current approach is a bit too DisplayServerHeadless-specific, see the review comment.

editor/editor_node.cpp Outdated Show resolved Hide resolved
@Riteo Riteo added this to the 4.x milestone Mar 2, 2024
@akien-mga
Copy link
Member

Looks good! Could you squash the commits? See PR workflow for instructions.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Mar 2, 2024
@chrisl8 chrisl8 force-pushed the no_custom_cursor_error_in_headless branch from f5d5255 to 20e6321 Compare March 2, 2024 23:30
@chrisl8 chrisl8 force-pushed the no_custom_cursor_error_in_headless branch from 20e6321 to e6d0bf3 Compare March 2, 2024 23:45
@chrisl8 chrisl8 requested a review from Riteo March 2, 2024 23:48
Copy link
Contributor

@Riteo Riteo left a comment

Choose a reason for hiding this comment

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

Looks fine and dandy! :D

You started something really nice BTW, I think we should do this to other "special" calls, one of which comes to mind: the pesky icon setting code that prints a warning on startup on Wayland, as there's no support for "runtime" icons in the first place.

@akien-mga akien-mga merged commit 2d4c923 into godotengine:master Mar 4, 2024
16 checks passed
@akien-mga
Copy link
Member

akien-mga commented Mar 4, 2024

Thanks! And congrats for your first merged Godot contribution 🎉

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.

3 participants