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 explanation of lambdas to GDScript basics. #6276

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

HolonProduction
Copy link
Member

Fixes: #5832

Some examples and wordings were copied from the original proposal godotengine/godot-proposals#2431.

This also contains notion of one line functions.

The proposal also has an example of the factory pattern (one function that returns a lambda). I am not sure if this belongs into this section. It could be added though.

@TheYellowArchitect
Copy link
Contributor

TheYellowArchitect commented Oct 7, 2022

Looks good to me. Code works, I tested the lambda codeblocks you wrote 👍

I wish I could build the docs to ensure the code renders properly. Only nitpick is perhaps to change the 2 instances of "lambdas" to "lambda functions" but it doesn't really matter, as only someone who has never heard of lambdas will prefer "lambda functions" lol

Edit: This is not a nitpick:

.. note:: The frozen values behave like constants and are therefore "flat". If you declare an array or dictionary, it can still be modified afterwards.

The user cannot understand what you mean by "frozen" values, nor "flat". At least I don't, and I have used lambdas many times in the past. Documentation should be as comprehensive and crystal-clear as possible.

@HolonProduction
Copy link
Member Author

The user cannot understand what you mean by "frozen" values, nor "flat". At least I don't, and I have used lambdas many times in the past. Documentation should be as comprehensive and crystal-clear as possible.

I agree with you on "frozen". It is refering to the values that are copied from the outer scope. How about The values captured from the outer scope behave like...

The "flat" terminology was copied from the explaination of constants which was introduced before. When changing it here it should be changed there as well. (Line 845)

@HolonProduction
Copy link
Member Author

wish I could build the docs to ensure the code renders properly.

I rendered it with the make.bat file. Is there something wrong with this method?
gdscript_basics.zip

@TheYellowArchitect
Copy link
Contributor

wish I could build the docs to ensure the code renders properly.
I rendered it with the make.bat file. Is there something wrong with this method?
gdscript_basics.zip

I thought that building the docs requires a lot of programs, kinda like building godot itself. I remember I tried to install the programs required to build godot from source and 1 of them conflicted with a program I had, so I didn't continue my installation. I see in https://docs.godotengine.org/en/latest/community/contributing/building_the_manual.html that building the docs is simple.

tl;dr I haven't tried building. I did try that link you uploaded, opened it with my browser (brave) and it didn't render like the docs.

I agree with you on "frozen". It is refering to the values that are copied from the outer scope. How about The values captured from the outer scope behave like...

Sounds great, as I can understand it and I am certain more users as well. Though perhaps a change on the verb captured?
"The values of the outer scope" sounds simpler

The "flat" terminology was copied from the explaination of constants which was introduced before. When changing it here it should be changed there as well. (Line 845)

I went into that line, and am kinda stunned. Right above is a serious problem, which dwarfs the clarifications here:

You can also create constants inside a function, which is useful to name local magic values.

local magic values lol
Anyway, outside the scope of this PR, but had to mention.

Back on-topic: On the "flat", I honestly don't know with what phrase to replace. You did good to keep the consistency, but in this case, whoever wrote that "flat" example must have had issues expressing that concept. You can either replace both "flat" sections or just clear up yours at the lambda section, and some1 else cleans up that "flat" section when an appropriate phrasing is found.

Some examples and wordings were copied from the original proposal godotengine/godot-proposals#2431.

This also contains notion of one line functions.
@HolonProduction
Copy link
Member Author

tl;dr I haven't tried building. I did try that link you uploaded, opened it with my browser (brave) and it didn't render like the docs.

I did not include the style sheets. Feel free to build it yourself. It renders correct on my side.

@TheYellowArchitect
Copy link
Contributor

With the change of "Lambdas" to "Lambda functions", perhaps the last line also needs to change to "lambda functions", but anyway not important, especially since lambdas are clear now (e.g. no "flat" or "frozen" values) and I assume a new user who reads "lambdas" can do 1+1=2 and realize "lambdas = lambda functions"
tl;dr: Nitpick on naming consistency

Aside of that nitpick, cannot find any flaws at all 👍

@mhilbrunner mhilbrunner merged commit 0f77b03 into godotengine:master Oct 11, 2022
@mhilbrunner
Copy link
Member

Going without the above nitpick, good addition! Thanks for contributing (and congrats on having your first docs contribution merged!), and thanks TheYellowArchitect for not only contributing so much lately but also all the reviews, thats extremely helpful! 🎉

Thanks, both of you.

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.

Lambda functions needs to be documented
4 participants