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

Document the differences between GDScript and GlobalScope #98438

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Document the differences between GDScript and GlobalScope #98438

merged 1 commit into from
Oct 24, 2024

Conversation

BrianBHuynh
Copy link
Contributor

Changed some wording in the documentation to make things seem more clear. The original wording used the words "in any script", and although it can be inferred with context clues that it is talking about GDScript specific code, it is not easy to parse from a glance as the sentence also used "related to" which may be vague.

Also moved GDScript specific to the end of the description for GDScript and added "which work in any language" when refering to GlobalScope in the same file to improve in readability as the difference was not clear prior.

@BrianBHuynh BrianBHuynh requested review from a team as code owners October 22, 2024 21:34
@BrianBHuynh
Copy link
Contributor Author

I had two wordings for the 2nd line of GDScript, the current wording I picked seems to have been slightly more clear and easier to parse for some people, however a concern raised is that it can be slightly ambiguous as any language could be misconstrued to also include other languages like rust (even though we don't officially support it as an engine).

As such if precise wording and low ambiguity is preferred over easy parsing this line could be used instead:

For the list of the global functions and constants not specific to GDScript see [@globalscope].

@fire
Copy link
Member

fire commented Oct 22, 2024

Thanks for clarifying the difference between the languages that extend Script and GDScript.

@fire fire changed the title Documentation clarification for the difference between GDScript and GlobalScope Documentat the differences between GDScript and GlobalScope Oct 22, 2024
@fire fire changed the title Documentat the differences between GDScript and GlobalScope Document the differences between GDScript and GlobalScope Oct 22, 2024
@BrianBHuynh
Copy link
Contributor Author

No problem :D we were talking about the docs in the discord when we noticed this slightly unclear wording

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Grammar and general tweaks

@Mickeon Mickeon removed the request for review from a team October 22, 2024 22:18
@BrianBHuynh
Copy link
Contributor Author

Thanks for the tweeks o7

@syntaxerror247
Copy link
Member

@BrianBHuynh You need to apply all of @Mickeon's suggestions and then squash your commits into a single one.

@BrianBHuynh
Copy link
Contributor Author

Oh shoot i wasn't aware, let me do that real quick! (Kinda new to this open source thing haha)

@BrianBHuynh
Copy link
Contributor Author

Squashed :D sorry for the trouble @syntaxerror247

Previous wording said that @GDscript referred to entries that could be accessed in any script. Although with common sense we could imagine that it is only refering to GDScript specific pieces of code, the wording is a little unclear.

In general there are small changes to the wording which makes it more clear and concise.
Wording change

Tried to match the wording up with my last change which should make it a bit easier to parse at a glance what the docs mean by "from any script"

Changed language from "not specific to" to "which work in any language"

After consulting multiple people the new wording seems easier to parse, even for non coders

Update doc/classes/@GlobalScope.xml

Update modules/gdscript/doc_classes/@GDScript.xml

Update modules/gdscript/doc_classes/@GDScript.xml

Co-Authored-By: Micky <66727710+Mickeon@users.noreply.github.com>
@syntaxerror247
Copy link
Member

Squashed :D sorry for the trouble @syntaxerror247

No Problem :)

I don't think that long commit description was needed though 😄

Copy link
Member

@syntaxerror247 syntaxerror247 left a comment

Choose a reason for hiding this comment

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

LGTM

@BrianBHuynh
Copy link
Contributor Author

Ahh i'll keep that in mind for the future o7 I've asked around and some people said I should add as much detail as relevant and others say to keep it curt so I never know what people prefer haha

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Triple approval energy blast

@BrianBHuynh
Copy link
Contributor Author

lmaooooo nice

@Repiteo Repiteo merged commit ab4de57 into godotengine:master Oct 24, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 24, 2024

Thanks, and congratulations on your first contribution! 🎉

@BrianBHuynh
Copy link
Contributor Author

Thankyou :D

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.

7 participants