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

Update docs/Documenting.md #1265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 72 additions & 53 deletions docs/Documenting.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,57 @@
# Documentation Style Guide

This project uses [Doxygen](https://www.doxygen.nl/index.html) to generate documentation pages from comments found in the source files. This guide focuses on writing compatible comments and ensuring consistency across the codebase.
```diff
- Note -
As the codebase is constantly changing, only document what is complete, well-understood and not
already covered by good naming. This is especially true for function parameters and return values.
Also note that there is no obligation to completing the documentation steps for functions you
work on if you do not want to at the time.
```

However to keep the documentation readable in text and favor consistency, the Doxygen commands that should be used are restricted to what this document mentions.

To generate a doxygen manual for the project, ensure you have doxygen installed and then cd into the project root directory and run `doxygen Doxyfile`.

The documentation can then be browsed by opening `docs/doxygen/html/index.html` in a web browser.

## Documenting Functions
For functions, a description of the function's purpose should go above the function:

Any comments inside functions, except bugs ([see below](#documenting-bugs)), should use `//`-style comments, even if spanning over multiple lines.

A simple example of documenting a function withjust a description (note the leading `/**`):

Comment on lines +16 to +17
Copy link
Collaborator

Choose a reason for hiding this comment

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

need a space between "with" and "just"

```c
/**
* My description of this function
* Update the crawl sound timer, and play the crawling sound when it reaches 0.
*/
void foo(void);
void EnInsect_UpdateCrawlSfx(EnInsect* this) {
```
Further considerations:
- Any comments inside the function should follow the usual `//` or `/**/` comment styles.
- For documenting return values, place a `@return` at the bottom of the function comment followed by the description of the return value. This should only be done if the name of the function is not self-explanatory and is well-understood.
- For documenting parameters, place a `@param` between the comment and the @return (if applicable) followed by the name of the parameter and a brief description. This should only be done if the name of the parameter is not self-explanatory and is well-understood.
- All `@param`s should come before `@return` and be in the same order the parameters appear in the function declaration. Note that this does not mean you should add empty `@params` for parameters deemed self-explanatory.

A full example would be as follows: (however in practice functions such as this would be considered self-explanatory)
A more complete example:

```c
/**
* This is an example
* Request to either increase or consume magic.
*
* @param amount the positive-valued amount to either increase or decrease magic by
* @param type how the magic is increased or consumed.
*
* @param bar the input
* @return bar multiplied by 2
* @return false if the request failed
*/
s32 foo(s32 bar) {
return 2*bar;
}
s32 Magic_RequestChange(PlayState* play, s16 amount, s16 type) {
```

Note:

- Documentation for self-explanatory arguments (`@param`) and return values (`@return`) may be omitted.
- `@param` commands should not have empty lines in between.
- Different commands (main description, `@param`, `@return`, ...) should be separated by empty lines.

Other directives that may be used for documenting functions:

- `@see` to reference something else ([see below](#linking-related-information)).
- `@note` to bring attention to some of the function behavior.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is appropriate to have something stronger than @note for some things where stuff will break badly if instructions are not followed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah and i think the default should be @remark since @note is not for trivia stuff but it would end up being / already is used this way

The problem would be that graduation is completely arbitrary though, or can we come up with actual clear "rules"?

I made this to showcase @remark, @note, @attention, @warning

/**
 * Spawn an object file of a specified ID that will persist through room changes.
 *
 * This only starts loading the object, the data will not be available immediately when the function returns.
 *
 * Used for gameplay_keep, Link's object, and the subkeep if any.
 *
 * @return The new object slot corresponding to the requested object ID.
 *
 * @remark This function is not meant to be called externally to spawn object files on the fly.
 * @note When an object is spawned with this function, all objects that come before it in the entry list will be treated as
 * @attention persistent, which will likely cause the memory available for object space to run out.
 * @warning This function is only meant to be called internally on scene load, before the object list from any room is processed.
 */

image

Copy link
Contributor

@EllipticEllipsis EllipticEllipsis Jun 9, 2022

Choose a reason for hiding this comment

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

I usually use @remark like I'd use "Remark:" in a paper, as a tangential thing that's not usage(/argument)-specific, like what the original name of the function is if known.

Copy link
Contributor

Choose a reason for hiding this comment

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

I usually use @remark like I'd use "Remark:" in a paper, as a tangential thing that's not usage-specific, like what the original name of the function is if known.

Copy link
Contributor

Choose a reason for hiding this comment

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

@warning is tricky because it can be mixed up with compiler warnings, but I don't love @attention.


## Documenting Variables
Documentation of variables should include what this variable is used for if the name is not completely clear and if applicable whether a set of defines or enumerations should be used alongside it (which should be linked with `@see`, see below)

In case the name of a variable isn't completely clear, documentation can provide a description.

If applicable, it may refer to a set of defines or enumerations that should be used with it (those should be linked with `@see`, [see below](#linking-related-information)).

```c
/**
* My description of this variable
Expand All @@ -48,53 +61,59 @@ s32 foo;
```

## Documenting Files
File documentation should go near the top of the file, below includes. It should only feature information that is general to the file.

File documentation should be located at the top of the file, above `#include`s.

File documentation should only feature information that is general to the file or the system it implements.

```c
/**
* @file file_name.c
* @file z_fcurve_data_skelanime.c
* @brief Curve skeleton animation system
*
* My description of this file
* A curve skeleton has a fixed number of limbs, ...
...
*/
```

## Other

### Documenting Bugs:

Bugs should be documented on the line above where the bug begins.

```c
//! @bug description
//! @bug Missing early return
```

Bug described on multiple lines should still use the `//!` syntax, over multiple lines. For example:

```c
//! @bug this code was clearly meaning to print `abs(camera->camDataIdx)` as a
//! one-or-two-digit number, instead of `i`.
```

### Linking related information:

`@see` should be used to provide links to related information where appropriate, for example:

```c
/**
* Save File Data
* @see SaveContext
* Sets the next framebuffer to the framebuffer associated to `task`.
* If there is no current buffer or it is time to swap, this buffer will be swapped to
* immediately, otherwise it will be swapped to later in Sched_HandleRetrace.
*
* @see Sched_HandleRetrace
Copy link
Contributor

Choose a reason for hiding this comment

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

Does Doxygen generate the correct links if function names are not followed by ()?

*/
SaveContext gSaveContext;
void Sched_SetNextFramebufferFromTask(Scheduler* sc, OSScTask* task) {
```

In the case of functions, `@see` should come before the first `@param`.
### HTML
You can include html tags in your doc comments, however it is strongly advised against doing this if it greatly reduces readability of the code comments.
```c
/**
* My<br>
* Newline<br>
* Doc Comment
*/
```
### LaTeX
You can embed [LaTeX](https://wikipedia.org/wiki/LaTeX) in your doc comments if useful to do so.

For inline rendering:
```c
/**
* \f$ \textrm{Your LaTeX Here} \f$
*/
```
For centered rendering on a separate line:
```c
/**
* \f[ \textrm{Your LaTeX Here} \f]
*/
```
`@see` may also be used for documenting files or variables.

### HTML and LaTeX

It is possible to include HTML and LaTeX in documentation comments.

However it is prefered not to do so, so that the raw text stays readable when looked at as plain text, for example when browsing the source code.
Copy link
Contributor

@EllipticEllipsis EllipticEllipsis Jun 9, 2022

Choose a reason for hiding this comment

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

I do think there are systems where LaTeX is useful enough to make it worth it (the matrix stuff, probably sys_math3d), but I know I'm in the minority.

(I suppose fundamentally my point would be that "there is no way to write this clearly in the code, and the next best thing is for it to be clear in the Doxygen".)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we can find a compromise where LaTeX is its own part and doesn't add information besides math, and there's a main separate explanation part that is readable as plain text?

Do you have an example in MM maybe, there's not much LaTeX in OoT and idk how to find it

2 changes: 1 addition & 1 deletion src/code/z_actor.c
Original file line number Diff line number Diff line change
Expand Up @@ -3288,7 +3288,7 @@ Actor* Actor_GetProjectileActor(PlayState* play, Actor* refActor, f32 radius) {
actor = actor->next;
} else {
//! @bug The projectile actor gets unsafely casted to a hookshot to check its timer, even though
// it can also be an arrow.
//! it can also be an arrow.
// Luckily, the field at the same offset in the arrow actor is the x component of a vector
// which will rarely ever be 0. So it's very unlikely for this bug to cause an issue.
if ((Math_Vec3f_DistXYZ(&refActor->world.pos, &actor->world.pos) > radius) ||
Expand Down
8 changes: 4 additions & 4 deletions src/code/z_camera.c
Original file line number Diff line number Diff line change
Expand Up @@ -7125,7 +7125,7 @@ void Camera_PrintSettings(Camera* camera) {
sp50[i++] = '-';
}

//! @bug: this code was clearly meaning to print `abs(camera->camDataIdx)` as a
//! @bug this code was clearly meaning to print `abs(camera->camDataIdx)` as a
//! one-or-two-digit number, instead of `i`.
// "sp50[i++] = ..." matches here, but is undefined behavior due to conflicting
// reads/writes between sequence points, triggering warnings. Work around by
Expand Down Expand Up @@ -7264,7 +7264,7 @@ s32 Camera_UpdateWater(Camera* camera) {
}
Audio_SetExtraFilter(0);
}
//! @bug: doesn't always return a value, but sometimes does.
//! @bug doesn't always return a value, but sometimes does.
}

s32 Camera_UpdateHotRoom(Camera* camera) {
Expand Down Expand Up @@ -7917,8 +7917,8 @@ s32 Camera_ChangeDataIdx(Camera* camera, s32 camDataIdx) {
camera->unk_14A |= 4;
Camera_CopyDataToRegs(camera, camera->mode);
} else if (settingChangeSuccessful < -1) {
//! @bug: This is likely checking the wrong value. The actual return of Camera_ChangeSettingFlags or
// camDataIdx would make more sense.
//! @bug This is likely checking the wrong value. The actual return of Camera_ChangeSettingFlags or
//! camDataIdx would make more sense.
osSyncPrintf(VT_COL(RED, WHITE) "camera: error: illegal camera ID (%d) !! (%d|%d|%d)\n" VT_RST, camDataIdx,
camera->camId, 0x32, newCameraSetting);
}
Expand Down