Skip to content

Conversation

@31
Copy link
Contributor

@31 31 commented Feb 8, 2024

When following this link on the C# exports page:

Custom node classes can also be used, see C# global classes.

This sentence in the intro paragraph seems relatively easy to misinterpret:

... exported properties are restricted to instances of the global class or derived classes.

Someone took this to mean that exported custom node properties must be global classes (or derived from a global class). I hadn't read the intro to this doc yet when they brought it up, and reading it for the first time with this interpretation in mind (even though I was fairly sure it wasn't right), I also interpreted it that way.

After digging into it, I understand what it's trying to say, and I think it's worth clarifying. I added a bit more about how the restriction and filtering is helpful.

I retook the img/globalclasses_addnode.webp screenshot to get rid of some dead space that was making it harder to scroll through the page. (Seemed more noticeable once I added more content.)

@AThousandShips AThousandShips requested a review from a team February 8, 2024 10:24
@AThousandShips AThousandShips added enhancement topic:dotnet area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Feb 8, 2024
Copy link
Member

@paulloz paulloz left a comment

Choose a reason for hiding this comment

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

Thanks! It is way more understandable. It took me a couple of times re-reading the current page to understand what it meant.

Also, the new screenshot is a bit tiny, which is aesthetically unpleasing for me, but probably not a deal-breaker.

@31
Copy link
Contributor Author

31 commented Feb 8, 2024

the new screenshot is a bit tiny

On my screen (100% scaling, non-4k, FWIW) the font size seems the same as the existing images, which I was thinking is what matters. Just to make sure there's no rendering difference, here's what old (left) and new (right) look like to me:

image

And the unchanged screenshots:

image

I would agree the editor font used on this page overall is too small though.

Is there a contrib guide page that has something like "recommended screenshot theme settings"? I think having that on https://docs.godotengine.org/en/stable/contributing/documentation/docs_image_guidelines.html would be nice. (Maybe even with a note that it's ok to have mismatched sizes if a page's existing shots don't fit the guidelines.)

@31 31 force-pushed the dev/31/cs-global branch from 782e701 to 868d48d Compare February 8, 2024 19:33
@31
Copy link
Contributor Author

31 commented Feb 8, 2024

Updated with fixes from review. I was able to simplify the talk about the inheritance hierarchy because I seem to have been wrong about how that works anyway. Also, updated all the screenshots to use the default font size rather than a smaller one.

Wasn't able to find a StatsIcon.svg so made a new super simple one: StatsIcon

Copy link
Member

@paulloz paulloz left a comment

Choose a reason for hiding this comment

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

It's OK for me. Thanks ❤️

Copy link
Member

@raulsntos raulsntos 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 too. And thanks for the new icon 🙏.

@skyace65 skyace65 merged commit ec3e094 into godotengine:master Feb 15, 2024
@skyace65
Copy link
Contributor

Thanks!

@31 31 deleted the dev/31/cs-global branch February 15, 2024 01:17
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 enhancement topic:dotnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants