Documentation suggestions

@andybak I would like to use my knowledge of things like ModelWidget, LightWidget, BreakModelApartCommand, etc to write some docs in Developer Notes and add more comments to the codebase. (1) what do you think about this XML Documentation Comment? This SetLightType method is in LightWidget. I think some methods like this one could have these comments added, as when you hover over them you see the description. But something like LightWidget.Clone may not need comments (except maybe some remarks) as Clone is pretty standard and you can read the code to see what it does. I think this SetLightType method is a good example because I personally genuinely thought it was used to modify the lightwidget's Light type.
/// <summary>
/// Activates all lights of the specified type and deactivates all other lights that are children of this LightWidget's GameObject.
/// </summary>
/// <param name="lightType">The type of light to activate</param>
/// <returns>
/// The last <see cref="Light"/> object in the hierarchy that is activated, or <c>null</c> if no matching light type is found.
/// </returns>
private Light SetLightType(LightType lightType)
{
var lights = gameObject.GetComponentsInChildren<Light>();
Light activeLight = null;
foreach (var l in lights)
{
l.gameObject.SetActive(false);
if (l.type == lightType)
{
l.gameObject.SetActive(true);
activeLight = l;
}

}
return activeLight;
}
/// <summary>
/// Activates all lights of the specified type and deactivates all other lights that are children of this LightWidget's GameObject.
/// </summary>
/// <param name="lightType">The type of light to activate</param>
/// <returns>
/// The last <see cref="Light"/> object in the hierarchy that is activated, or <c>null</c> if no matching light type is found.
/// </returns>
private Light SetLightType(LightType lightType)
{
var lights = gameObject.GetComponentsInChildren<Light>();
Light activeLight = null;
foreach (var l in lights)
{
l.gameObject.SetActive(false);
if (l.type == lightType)
{
l.gameObject.SetActive(true);
activeLight = l;
}

}
return activeLight;
}
14 Replies
andybak
andybak2w ago
(2) in Developer Notes there could be a section that contains something like this: Widgets explained
GrabWidget ... LightWidget LightWidgets are currently created from (1) ModelWidgets that only have Lights and no mesh data (2) tilt files ModelWidget Represents a grabbable object from a 3d model ObjModelScript
a utility script that allows other components (e.g ModelWidget) to gain information about the children mesh under ObjModelScript's GameObject. Used by SketchControlsScript.cs to check how many vertices are in the current selection Maybe some helpful images like this one that shows how a freshly imported 3d model is setup in Unity's Hierarchy:
andybak
andybak2w ago
No description
andybak
andybak2w ago
(moved to a thread so it doesn't get lost) I've got a weird dislike of xml comments. Maybe it's just an IDE setting but for me they seriously clutter up the code and make it harder to skim a file and get a feel for it's "shape". Also XML was never meant to be seen by humans so they made a poor choice right out of the gate! but I realise I'm an outlier here so maybe just ignore my preferences on this. More documentation is nearly always a good thing. And the closer documentation is to the thing it's documenting - the better. SetLightType just seems really badly named. It's better to fix the naming in this case. but a method like this probably needs a better name AND a better comment. The name tells you "what" and the comment should tell you "why" If we have stuff in the developer notes then link to it from comments. But dev notes should be topic focused and aimede at giving an overview of a particular thing. Comments are better than separate dev docs in most cases.
eerop
eeropOP2w ago
ok can you give example from the codebase that has a good comment, e.g on a method?
andybak
andybak2w ago
not off the top of my head. we don't have many good comments! actually - that's not true.. Paul wrote some real corkers although some of them are waaaaaay over my head
eerop
eeropOP2w ago
should a comment explain where the method is currently used? to me it feels correct because: - it's a part of the question "why" the method exists. - e.g SetLightType exists because a LightWidget may have multiple types of Lights as its children It feels incorrect because: - if the method is sometimes in the future used for something else, this might lead to incorrect information I do realize there's no definitive answer to what is a good comment. Some developers find them useful and some don't. I also realize that I personally find it helpful to write about how something works. I do it a lot in my own notes, but sometimes the comments are a more convenient place to put them. Maybe I should look into a local comment add-on for Rider lol author "Paul Du Bois"?
andybak
andybak2w ago
yep
eerop
eeropOP2w ago
for me it only shows 4 commits:
$ git log --all --author="Paul Du Bois" --oneline
aa0d0739 (tag: 0.2.23) Fix #77 and #79 (#78)
a1868a7f yournamehere -> openbrush
f4008d09 More files that Unity really wants to use LF
1f6fa446 Disable irritating warning
$ git log --all --author="Paul Du Bois" --oneline
aa0d0739 (tag: 0.2.23) Fix #77 and #79 (#78)
a1868a7f yournamehere -> openbrush
f4008d09 More files that Unity really wants to use LF
1f6fa446 Disable irritating warning
there's a lot in developer notes written by him though
andybak
andybak2w ago
commit history was wiped when it was open-sourced so nothing is attributed that happened prior to that
eerop
eeropOP2w ago
ahh ok
eerop
eeropOP2w ago
I'm going to iterate different approaches. I made a version that adds overviews for ModelWidget and LightWidget. Can you let me know your thoughts? @andybak https://docs.openbrush.app/~/changes/KlRqpomXTmzvC0OXrGgt/developer-notes/ui-elements?r=O736sWmqJth3OpoxlP1q For this iteration, assumptions about the reader: - Knows C# and Unity - Is one of us here in the dev chat, or Foundation Team, or Contributor - Has Open Brush in Unity and an IDE like Rider opened at the same time Note: Maybe the assumptions should be changed, as the user could also maybe be developing a similar game and they wanna know how Open Brush implements Widgets. For this iteration, the goal of the docs: The reader isn't supposed to understand how the entire implementation of ModelWidget and LightWidget works after reading. They will understand the entire implementation if/when they go through the source code themselves, and e.g. make changes. The reader is supposed to get a quick overview of ModelWidget and LightWidget. They read the overview and they think: - Ok, so when I see ObjModelScript in ModelWidget, I'll know that it's a utility script used for getting info about the meshes - I'll also be aware of the BreakModelApartCommand.cs file, that implements breaking up ModelWidget - These images and videos are helpful for building a mental model of how ModelWidget and LightWidget work in Unity
andybak
andybak2w ago
Bear in mind that the developer docs were literally my rough notes in Google Drive that I decided to add to the main Open Brush docs. I gave it very little thought as I thought "something is better than nothing" - so the same applies here You can always improve it later. Don't overthink it - just get it up
eerop
eeropOP2w ago
Ok. I do wanna improve the docs. I think it can benefit us. Atleast me. It's helpful to understand how each of us developers use Developer Notes differently: How often do you use the Developer Notes? for example: (1) a tab open everytime working on OB in unity (2) occasionally (anywhere from once a day to once a week) for some technical details, but also use IDE or own personal docs (3) never or very rarely use it (like once a month or not even that often)
andybak
andybak2w ago
Rarely - sometimes I remember something is in there that I've forgotten how to do (used to be popups or creating a new panel but i remember that without needing the docs now) Or I think of something I should add (which i really need to do more often!)
Want results from more Discord servers?
Add your server