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 visual shader node interface explanation #8459

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

ShlomiRex
Copy link
Contributor

Added visual shader port types, which greatly increases clarity for newbies.

image

@paddy-exe paddy-exe added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation content:images Issues and PRs involving outdated or incorrect images in articles labels Nov 12, 2023
@AThousandShips
Copy link
Member

Please convert the image to the .webp format.
you can read more about it in the Doc image guidelines.

@ShlomiRex ShlomiRex force-pushed the visual_shaders_interface branch from be1207e to a8cd252 Compare November 13, 2023 13:22
@ShlomiRex
Copy link
Contributor Author

@AThousandShips done

Copy link
Member

@mhilbrunner mhilbrunner 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, but I wonder how maintainable the example screenshots are, as we struggle to keep screenshots updated with Godot updates.

So we should consider how many of these we would expect to be adding for this, how often we expect them to change, and how much value they add. Happy to hear y'alls thoughts :)

@ShlomiRex
Copy link
Contributor Author

@mhilbrunner I agree that the screenshot represent UI that can change over time. However, its just an example and not a direct fact.
The screenshots of the UI help users visualize different types of ports by their color (which is the main reason for the screenshots, since I can't post the colors of the ports).
I dont expect we would add more to this list, because the point was to demonstrate the basic port types: scalar, vector, matrix, and so on. It's not subject to change.

@skyace65
Copy link
Contributor

If it's just these screenshots I would consider this maintainable

Copy link
Contributor

@skyace65 skyace65 left a comment

Choose a reason for hiding this comment

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

Missed this in the original review, apologies. But the outline for the vs_node image needs to be a different color. We use a specific shade of yellow (hex color is fffb44, fffb44ff if you're using something with transparency) so color blind people don't have issues reading the documentation.

Added port types
@ShlomiRex
Copy link
Contributor Author

Missed this in the original review, apologies. But the outline for the vs_node image needs to be a different color. We use a specific shade of yellow (hex color is fffb44, fffb44ff if you're using something with transparency) so color blind people don't have issues reading the documentation.

Thank you. I read the instructions in: https://docs.godotengine.org/en/stable/contributing/documentation/docs_image_guidelines.html

I should have read that before submitting. My mistake.
Fixed:
image

@ShlomiRex ShlomiRex force-pushed the visual_shaders_interface branch from a8cd252 to ee9e10e Compare November 18, 2023 23:12
@paddy-exe
Copy link
Contributor

I would perhaps hold off this PR from this for a while longer as there's a redesign coming in terms of both UX and Color Design: godotengine/godot#85017

@skyace65
Copy link
Contributor

I would perhaps hold off this PR from this for a while longer as there's a redesign coming in terms of both UX and Color Design: godotengine/godot#85017

I get it, but the work that's done for this PR is already done. I don't see holding off on this PR as giving us less work in the future.

@skyace65 skyace65 merged commit fabd885 into godotengine:master Nov 24, 2023
1 check passed
@skyace65
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation cherrypick:4.1 content:images Issues and PRs involving outdated or incorrect images in articles enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants