Hello!
Why godot source code havn't good comments(c++ comments) and documentation (doxygen). Eventually one or two years later, even main developers can't understand functions an classes in a less documented source code.
Blender has a policy in documentation and everyone who wants change some part of code must document and comment his changes carefully.
Do godot engine have a strategy for documenting.
Another question. Do godot engine have enough developers for reviewing?
Here is a sample of blender coding and reviewing workflow:
Engine's source code should at least have more comments and some documentation of starting point for anyone who would like to add/modify some parts of it.
At the current point, I don't go any further than making independent modules, because I'm pretty sure that with my current knowledge I would add more bugs than useful functionality if I would like to try modify the source.
Really... even 'tool' GDScript told me more about how the editor is made than current source code.
The source code needs more comments yes, but it's a long-running process as the main coder @reduz prefers writing self-explanatory code than overloading it with comments (with which I kind of agree, so for less experience coders some more comments would still be welcome).
Regarding documentation, there is http://docs.godotengine.org/en/latest/classes/_classes.html
I don't think that more documentation on the C++ classes themselves is needed apart from what is exposed to GDScript. Godot is a library used in 98% of cases via GDScript, so that's the only API we should care about documenting. The rest is anyway self-explanatory for C++ coders IMO, and doxygen-like documentation is quite often more annoying than useful when it's not properly maintained (and trust me, you won't want to maintain it).
Regarding code review, we already use PRs for most changes, and those are obviously up for review. Now, to get them actually _reviewed_ by other developers, it's another matter. I hardly manage to get PRs +1'ed by end users who might find the added functionality useful... but it's again a long process to build up a culture of peer review.
I think OP wanted to move from one xml
file with docs to doxygen
format, where docs for each method are attached to it, eg.:
//! A normal member taking two arguments and returning an integer value.
/*!
\param a an integer argument.
\param s a constant character pointer.
\return The test results
\sa QTstyle_Test(), ~QTstyle_Test(), testMeToo() and publicVar()
*/
int testMe(int a,const char *s);
or:
/**
* a normal member taking two arguments and returning an integer value.
* @param a an integer argument.
* @param s a constant character pointer.
* @see Javadoc_Test()
* @see ~Javadoc_Test()
* @see testMeToo()
* @see publicVar()
* @return The test results
*/
int testMe(int a,const char *s);
eg for: core/vector.h
/**
* @brief Removes all elements from the vector, leaving the container with a size of 0.
*/
_FORCE_INLINE_ void clear() { resize(0); }
/**
* @brief returns the number of elements in the Vector.
* @return The number of elements in the Vector.
*/
_FORCE_INLINE_ int size() const {
uint32_t *size = (uint32_t *)_get_size();
if (size) { return *size; }
else { return 0; }
}
/**
* @brief Returns Whether the vector is empty
* @note This function does not modify the container in any way. To clear the content of a
* vector, see Vector<T>::clear;
*/
_FORCE_INLINE_ bool empty() const { return _ptr == 0; }
1) All classes and most functions are properly documented in the class
reference
2) Example avobe for stuff like vector.h. If you don't really know what
those functions do by name, you should not even be looking at the source
code to begin with.
3) Many functions which are not as obvious have some documentation. If you
happen to find a function in the source code that you are not used what it
is for, and it lacks documentation, you can always ask for that.
On Tue, Mar 21, 2017 at 10:05 AM, Ramesh Ravone notifications@github.com
wrote:
eg for: core/vector.h
/**
- @brief Removes all elements from the vector, leaving the container with a size of 0.
*/
_FORCE_INLINE_ void clear() { resize(0); }/**
- @brief returns the number of elements in the Vector.
- @return The number of elements in the Vector.
*/
_FORCE_INLINE_ int size() const {
uint32_t *size = (uint32_t *)_get_size();
if (size) { return *size; }
else { return 0; }
}/**
- @brief Returns Whether the vector is empty
- @note This function does not modify the container in any way. To clear the content of a
- vector, see Vector
::clear;
*/_FORCE_INLINE_ bool empty() const { return _ptr == 0; }
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/5312#issuecomment-288071891,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z21udXkZ4qNvJQku-Ciyxi8JqlVfOks5rn8sVgaJpZM4I5oqk
.
Well I was just thinking if the core classes were documented, new developers can start developing faster rather than searching/waiting for community/core developers to replay.
And personally it looks @reduz is working a lot, alone
@reduz - If nothing else, your point 2 really annoys me. People have to start somewhere and you should not be discouraging people from contributing. Personally I feel that people should be looking at the source code no matter their skill level, and some degree of commenting should be there to explain it as necessary to help them to understand. Doesn't matter if the name is "obvious" or not.
@RameshRavone You can read about core classes in the online doc: http://docs.godotengine.org/en/stable/reference/_developing.html
@Sslaxx I understand, but Godot is huge and providing beginner level documentation on it is a task as big as writing the engine itself. We have much, much better things to focus our resources into.
I'm just saying if I where to PR some it's acceptable right
For core clases, sure, though i think documenting behavior is more
important than individual functions, unless those functions are not obvious.
On Tue, Mar 21, 2017 at 10:37 AM, Ramesh Ravone notifications@github.com
wrote:
I'm just saying if I where to PR some it's acceptable right
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/5312#issuecomment-288080122,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z29pvcCMifEVljspRz_IthlW3mit1ks5rn9KqgaJpZM4I5oqk
.
Maintaining comments which only have property lists are useless way of spending coding time and acts as obstacles while reading code.
Also over commenting code is the worst thing in the world and mostly the way of "working by doing nothing" in corporate environments.
If a code is not self explanatory comments will not help either to anybody so will be useless load on code base and will steal bandwidth and processing power in all operations.
So commenting all functions in code (especially doxygen style) will not speed up development process in any way I think.
Well its a point I can accept, But my students I don't think so, they are 13.
2) Example avobe for stuff like vector.h. If you don't really know what those functions do by name, you should not even be looking at the source code to begin with.
@reduz but they don't do what name says. Eg. size
for vector returns number of elements, and not it size in bytes, which is said by name.
I disagree with @reduz on this one. Godot's comments documentation is very lacking. A lot of times I've wasted several hours trying to figure out what something does, how something works or what an interface implementation is supposed to do.
The last time was the other day when I was working on C# script reloading. The interface for script reloading is quite confusing. We have methods like reload_all_scripts
and reload_tool_script
. What's the difference between both? When are they called? What are they supposed to do? Should they keep the state of instances? What is soft reloading? Etc.
Turns out not even @reduz remembered many of these things (he did remember it in the end, but after I already had spent some time with it), which is pretty normal considering he wrote that quite some time ago. I don't remember what some of the code I wrote two years ago does.
In the end, all those questions were answered, but not without wasting a few hours. This could have been avoided by simple documenting these interfaces. This is what I'm trying to explain: in many cases we don't need documentation to understand the code base, we just have to spend some time figuring things out or asking someone who may know it... or we can document the code base and spend that time on something more productive.
The Godot repository has lots of long files with lots of methods and code doing non-obvious stuff. By documenting them we make it easier for ourselves to maintain it in the future and for new contributors to join us.
I do not believe documenting code is such a time-consuming task. It doesn't have to be carefully written like the official documentation and class reference. We don't need to use something like doxygen and follow its style (although it would be better if comments had a consistent style).
Like I said previously, without documented code the same time (or even more) it would have taken you to document it ends up being spent trying to figure things out.
Also by documenting the code base we make things easier for those writing the official documentation and class reference.
We don't need to use something like doxygen and follow its style (although it would be better if comments had a consistent style).
I think it's a good idea to use standard docstrings, even if we don't use Doxygen ourselves. This way, we don't end up designing our own documentation comment style which new contributors would have to learn.
From a new contributor's point of view:
I am just exploring godot's source code and trying to get familiar with it.
I consider myself a somewhat experienced developer and I am quite familiar with reading sparsely documented code.
From what I saw of the sources so far, I must say that I could manage to get the ideas and meaning behind the code suprisingly (to me) quick. So at least for the parts of the code I was looking at, the self-documenting-thing did work quite well. *
But that does not mean that it couldn't be even better with more comments. Comments are not only a way of explaining what code does, they are also a way of helping developers detect their own misunderstanding of the code fast.
Take, for example, TextEdit::get_line_syntax_highlighting( .. )
:
Map<int, TextEdit::HighlighterInfo> TextEdit::_get_line_syntax_highlighting(int p_line)
It seams pretty clear to me what this method will probably do and what the returned map could represent.
To be sure about my assumption however, I still have to dig through the 180+ lines long method body, despite the clear naming.
If there was a comment above the method containing a short sentence about what it does, even without documenting details, I would instantly see if I am on the right track and save all that time that it takes just to be sure that everything is as it looks like.
Compared to that or even the time it takes to look up some documentation and keep that information in mind, the time of writing that short note is negligible, I think.
Btw: There was a good article on HN some days ago about the different types and the value of source code comments.
*) I do not doubt however, that there are many parts in greater depths of the code that are not as straight forward and therefore harder to understand than the lines that I had a look at.
I strongly agree with @neikeq. It is possible indeed to understand the behaviour of a piece of code through reading it, but why not making it easier and faster?
While it's not a replacement for inline docs, I'd like to note that it can be helpful to view a file's history on GitHub. That way you can see what things were changed and usually a note of what's happening.
The current policy is that comments are only added in functions that are not obvious when you look at them, or when you have to add a local hack that something far away in the code base needs to work.
Instead of going the suicide route of asking everyone to comment everything (which is an insanely huge waste of time, as we are not particularly lacking contributors), I suggest those trying to understand the code do their part and ask for help or comments on how the code works.
If there is something you don't understand, just ask. This way, those who understand it can document it.
We can't really tell in advance who will have a problem understanding what.
I already explained my problem is with this approach. And the objective is not only to attract new contributors but to make life easier current ones.
I put the scripting interface issue as an example. Obviously, we are not going to stop everything we're doing to document existing code. Instead, when we encounter a piece of code that needs documentation. we ask for help. Once we get an answer, we document that piece of code so the next guy doesn't have to go through the same process again.
We don't have to force contributors to document their new code neither. We just have to encourage it. I'm sure most would gladly do.
I don't think this is overkill. Actually, I explained in my comment how I think all the time (or even more) you save by not documenting code is later wasted trying to understand that undocumented code.
We can't really tell in advance who will have a problem understanding what.
I don't think this is an issue. Just document what you consider needed. We can document more in the future if needed. However "what you consider needed" is not the same as what we are documenting now, which is almost nothing.
There are some things I consider always or most of the times need documentation. For example, interfaces like the ones in core/script_language.h
. I and other contributors understand most of it at this point, because we spent the time. We should document it so other contributors don't have to go through that, and also for us because we can forget some of it in the future.
@neikeq yeah, for core interfaces, definitely agree that some doc on what functions are is valuable, since they are not exposed to the engine API.
So I just remembered this issue after encountering a file which I'm not sure what it does (area_sw). I agree that commenting everything (or actually anything) is counter-productive, as you can write code in a way that is rather obvious. However, it could be helpful if every source file had a very short tl;dr section explaining what this file is for in general and what is supposed to do. There are many files that you won't be familiar with after just using Godot, e.g. different editor docks or plugins.
It seams pretty clear to me what this method will probably do and what the returned map could represent. To be sure about my assumption however, I still have to dig through the 180+ lines long method body, despite the clear naming. If there was a comment above the method containing a short sentence about what it does, even without documenting details, I would instantly see if I am on the right track and save all that time that it takes just to be sure that everything is as it looks like.
If it seems clear to you, then just assume you are right and start figuring out how to use it for your contribution. Focus on making something work, instead of understanding every aspect and planning beforehand. If you do something wrong, it will just break, so you will see that you need to correct something.
I mean, in the past month I created quite a few PRs. I'm just looking at the list of issues and I can somewhat determine how this should be done and if I'd be able to fix it. Then I just locate the right source file and do the changes. I never had a feeling that some function should be commented so that I can understand it better, although it happened that some functionality was too cryptic for me to understand (and it was a case where a comment wouldn't really help).
To leave my 5 cents - I am currently reading/trying to read whole engine :) From the start, starting with 'core', and gradually one by one. Truth to be told - I have a mixed feelings - even large part of the code is mostly understandable for me, there are a lot of concepts (mostly undocumented macros and patterns - like family of classes: Reference, Ref, WeakRef, RID, RefPtr - which share some sort of commonalities) which still makes newly-aspiring contributor/developer to wrap his/hers head around :(
I am also finding a lot of inconsistencies while finding that sometimes, part of class implementation is found in *.h file, sometimes in *.cpp, even one-liners, like accessors. There are no strict rules behind declaration/implementation place, which makes jumping back and forth kinda tedious..
It would be mostly beneficial to have those core concepts explained somehow, so developers could easily connects to them, when reading through the whole code base.
There is a section of the docs for explaining core concepts. It should be very helpful to you. http://docs.godotengine.org/en/latest/development/cpp/index.html
There is a section of the docs for explaining core concepts. It should be very helpful to you. http://docs.godotengine.org/en/latest/development/cpp/index.html
yeah, I already know about that - I've read whole documentation, though, still - couple lines of explanation in source code wouldn't kill anybody :)
BUT - I am not complaining that much, because as you read more and more of source code you're getting more use cases for those constructs like reflection, reference management, etc. - they're just not that easy to spot on during first source-code read.
Edit: this post was in response to a now-deleted post.
I think you have misunderstood. Additionally, you have taken a comment completely out of context and assumed a bad intent by the author.
You can find the documentation here http://docs.godotengine.org/en/latest/
This issue thread is on documenting throughout the source code to help out engine contributors. Not on documenting the engine itself.
We do our best to document the engine as thoroughly as possible but it's incredibly hard as we have very few contributors. Users do not write C++ code based on engine internals. They access the engine the scripting API which is documented as best we can.
So please take that attitude elsewhere. We are working as hard as we can. Having access to the source code at all is a rare thing for a game engine.
I sincerely apologize for being rude in my reply to the issue, just because I'm a fan of well documented code, and comment my code as much as possible, that doesn't mean everybody else is, I understand the time is bigger factor that developers willingness, in fact majority of devs are not, everybody knows that.
I'll go ahead an delete my post, let not other people read and draw conclusions on code based on such comments.
I wish you guys all the best in your future development!
Most helpful comment
I disagree with @reduz on this one. Godot's comments documentation is very lacking. A lot of times I've wasted several hours trying to figure out what something does, how something works or what an interface implementation is supposed to do.
The last time was the other day when I was working on C# script reloading. The interface for script reloading is quite confusing. We have methods like
reload_all_scripts
andreload_tool_script
. What's the difference between both? When are they called? What are they supposed to do? Should they keep the state of instances? What is soft reloading? Etc.Turns out not even @reduz remembered many of these things (he did remember it in the end, but after I already had spent some time with it), which is pretty normal considering he wrote that quite some time ago. I don't remember what some of the code I wrote two years ago does.
In the end, all those questions were answered, but not without wasting a few hours. This could have been avoided by simple documenting these interfaces. This is what I'm trying to explain: in many cases we don't need documentation to understand the code base, we just have to spend some time figuring things out or asking someone who may know it... or we can document the code base and spend that time on something more productive.
The Godot repository has lots of long files with lots of methods and code doing non-obvious stuff. By documenting them we make it easier for ourselves to maintain it in the future and for new contributors to join us.
I do not believe documenting code is such a time-consuming task. It doesn't have to be carefully written like the official documentation and class reference. We don't need to use something like doxygen and follow its style (although it would be better if comments had a consistent style).
Like I said previously, without documented code the same time (or even more) it would have taken you to document it ends up being spent trying to figure things out.