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

Include Comments in Code examples #529

Open
CitrusWire opened this issue Oct 2, 2020 · 5 comments
Open

Include Comments in Code examples #529

CitrusWire opened this issue Oct 2, 2020 · 5 comments

Comments

@CitrusWire
Copy link

Godot Documentation Code examples should have comments

Currently the most of the Godot Engine Demos do not have code comments. Code as a rule should of course be commented, but demo code doubly so.

Reasons this should be done:

  • Code is write-once, read-many.
  • Documentation code is write-once read-thousands.
  • It gets Godot users used to comments in code and hopefully they'll follow the practice, which will benefit them.
  • Documentation has the purpose of teaching people about something. It's really hard to learn about something if that something is not explained.
  • Code comments lower the barrier to entry. Not everyone is an UberHack3r who can immediately parse and comprehend any code.
  • A documented function (i.e. with a docstring) explains succinctly to the user what the function is doing, what the input expected is, and what the output expected is. All things that are extremely useful to know at a glance for any function, but doubly so for documentation functions.
  • Allows users to easily skip over functions that don't do what they already know.

I've been playing with the translation one today for example. The code is 17 lines, no comments. Simple enough. Yet despite spending 15 minutes fiddling with it, I still have no idea how Godot knows to go from TranslationServer.setlocale(x) to "change the image/audio to the right file". I know I can find out from a dozen different places online, but that then obviates the point of this demo.

Or the code comments (which do exist) in "Background Thread Loading Demo" - they're written for someone who already knows the Godot systems. # Call deferred to configure max load steps. as the first comment isn't something that's going to be helpful to a new user. Or # Update progress bar, use call deferred, which routes to main thread. - which appears to be assuming I understand Godots threading system, but if someone did that, why would they need to be looking at a Godot demo of it?

But as with the docs, they're not all terrible. "Loading Autoload Demo" has a well code-commented global.gd. (The other two scripts in the project could benefit from having a line in though).

I've looked at 6 "Godot Engine" authored Demo's in total: 2 had basically no comments, 2 did and they weren't useful, 2 had decent comments. So 2/3rds of the sample could use some work.


(Filed as a "bug" because I figure a demo that's undocumented is failing its primary purpose, and therefore buggy.)

@Calinou
Copy link
Member

Calinou commented Oct 2, 2020

(Filed as a "bug" because I figure a demo that's undocumented is failing its primary purpose, and therefore buggy.)

That's not how it works…

I have to remind you that we have a code of conduct. Please drop this passive-aggressive attitude; it's not helping the project advance faster. Most Godot contributors are volunteering their free time after all.

I've been playing with the translation one today for example. The code is 17 lines, no comments. Simple enough. Yet despite spending 15 minutes fiddling with it, I still have no idea how Godot knows to go from TranslationServer.setlocale(x) to "change the image/audio to the right file". I know I can find out from a dozen different places online, but that then obviates the point of this demo.

This is handled by the TranslationServer's resource remapping system, which is handled internally by Godot. Nothing in the script controls it. I suppose we could have a comment in the script to make this explicit, still.

@CitrusWire
Copy link
Author

CitrusWire commented Oct 2, 2020

(Filed as a "bug" because I figure a demo that's undocumented is failing its primary purpose, and therefore buggy.)

That's not how it works…

Heh. Allow me to put it this way then: I'd consider these bugs in my own projects. :-)

I appreciate the comment came across as passive-agressive, and why, but it wasn't meant that way. I included it because it's a very important point about documentation. Documentation (and demo's) that doesn't explain something is faulty. Bugs are things that are faulty. Thus, faulty documentation = buggy documentation in my world view.

@CitrusWire
Copy link
Author

This is handled by the TranslationServer's resource remapping system, which is handled internally by Godot. Nothing in the script controls it. I suppose we could have a comment in the script to make this explicit, still.

Yup, I ended up concluding that but I have no idea where /that/ happens or what it entails (again, could google if I actually wanted to know, but for the purposes of a demo project, that defeats the point).
I agree it's definitely something that should be explicitly included in the code comment. Without the comment the code is little more than an example of one API call. It doesn't allow you to understand how the pieces are working together which is the point of a demo.

@aaronfranke
Copy link
Member

@CitrusWire If you can, I encourage you to work on this and submit pull requests. Adding comments to every demo is a lot of work and sometimes it's hard to know what information users need, so any assistance is greatly appreciated.

@CitrusWire
Copy link
Author

@aaronfranke - Thanks for the very kind offer/request. Unfortunately for various reasons I'm not going to be helping with PR's. But I am trying to provide my QA skills, and a fresh pair of eyes. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants