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 Doxygen-styled comments to parts of the libretro API #15641

Merged
merged 132 commits into from
Mar 10, 2024

Conversation

JesseTG
Copy link
Contributor

@JesseTG JesseTG commented Aug 23, 2023

Description

This pull request will add Doxygen-styled comments to various parts of the libretro API to simplify their use. Even if HTML pages are not generated or hosted, Doxygen is common enough that IDEs will recognize it. Here's what it looks like in CLion:

Screenshot 2023-08-23 075155

The comments are written to adhere to SemBr for easier diffs and clearer sentence structure. Essentially, line breaks are treated like punctuation.

This PR will not:

  • Violate C90. All additions come as /** standard C90 comments */.
  • Change how types, functions, macros, etc. are declared.
  • Generate documentation pages. This just adds comments.
  • Document RetroArch internals. This is for the benefit of libretro-common.

This PR, as of this writing, is only a sample. If @LibretroAdmin approves the general style, then I will proceed to document the rest of <libretro.h> (plus some other libretro-common APIs I've used) in the same format.

Reviewers

@LibretroAdmin @RobLoach

Status

The goal of this PR is to document all public-facing the parts of libretro-common that I've used. Here's where that stands:

@RobLoach
Copy link
Member

I've been wanting doxygen in here FOREVER. Happy to help out on its transition, if the rest of the libretro team are 👍

@JesseTG
Copy link
Contributor Author

JesseTG commented Aug 24, 2023

I've been wanting doxygen in here FOREVER. Happy to help out on its transition, if the rest of the libretro team are 👍

Great! How do you wanna divvy up the work? I'm happy to flesh out the docs for the environment calls plus any APIs that I've used so far. (That includes the VFS, task queue, and image scaler.) I'm also happy to proofread for spelling and clarity.

The APIs I've contributed already use Doxygen comments.

@JesseTG
Copy link
Contributor Author

JesseTG commented Dec 27, 2023

I'm still working on this. I've been prioritizing melonDS DS, but I've been squeezing in some work here whenever I'm blocked on something. I've finished with most parts of libretro.h. Once I complete it (and one other header whose changes I haven't pushed yet), I'll go around requesting reviews.

@JesseTG JesseTG marked this pull request as ready for review January 3, 2024 17:21
@JesseTG
Copy link
Contributor Author

JesseTG commented Jan 3, 2024

In the interest of not letting this languish while I move on to something else, I've decided to call this branch done. Any gaps in it will be filled in future PRs. In the meantime, could I get some eyes on the contents?

@JesseTG JesseTG requested a review from Jamiras January 3, 2024 17:23
Copy link
Member

@XerTheSquirrel XerTheSquirrel left a comment

Choose a reason for hiding this comment

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

I know I am not a originally requested reviewer, but I took a look at this and reviewed. I really do like the work that was done despite it being incomplete, I do agree that it should get merged in sooner so there is no gigantic feature creep kind of thing. This will also make development through IDEs must easier as well. I did not see any glancing issues at all as most of the documentation is just a transfer over and more clarification, which was fine for me.

The conflicts do need to be resolved before a final merge however.

Copy link
Member

@RobLoach RobLoach left a comment

Choose a reason for hiding this comment

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

@LibretroAdmin I believe this is good to go. Had another look through and there isn't any API or function definition that is changing, it's all documentation. Expanding to the rest of libretro-common could likely happen after this is merged in. Touching the majority of libretro.h is an incredible accomplishment here 👍

I don't see any conflicts on merging. Perhaps rather than a merge, a Squash and Merge would be better to ease the git history.
Screenshot from 2024-01-04 18-08-34

@JesseTG
Copy link
Contributor Author

JesseTG commented Jan 5, 2024

@LibretroAdmin I believe this is good to go. Had another look through and there isn't any API or function definition that is changing, it's all documentation.

One exception; I do add a new enum here for RETRO_ENVIRONMENT_GET_AUDIO_VIDEO_ENABLE. However, the size and range of the enum's values are consistent with the hardcoded numbers in the original documentation, so I don't foresee any ABI problems.

JesseTG and others added 3 commits January 24, 2024 18:00
# Conflicts:
#	libretro-common/include/libretro.h
libretro: Add three more environment callback doxygen docs
Copy link
Member

@RobLoach RobLoach left a comment

Choose a reason for hiding this comment

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

This is looking really solid. While there could be nitpick improvements to both the docs, and the formats, we could take those on as follow ups. Being able to read the docs directly in your IDE is nice, and it does pave the way to generating API docs automagically for libretro too...

Screenshot at 2024-01-29 11-21-52

I'd recommend merging this up, and then taking on individual parts of libretro-common as follow ups.

@LibretroAdmin
Copy link
Contributor

@Jamiras can still review it.

@RobLoach
Copy link
Member

Sounds good with me. The more eyes we have on this the better. When we decide to merge this forwards, it may be best to "Squash and merge" so that the git history doesn't get too crazy.
Screenshot from 2024-02-23 13-56-40

@LibretroAdmin LibretroAdmin merged commit b7ddac2 into libretro:master Mar 10, 2024
26 checks passed
Sunderland93 pushed a commit to Sunderland93/RetroArch that referenced this pull request Dec 26, 2024
)

* Touch up the documentation for a few environment calls

* Touch up more comments

* Update docs for more environment calls

* Add doc comments for more environment calls

* Change various @returns to indicate that the environment call is available

- Some environment calls might be recognized but ignored (e.g. when fast-forwarding during netplay)

* Note some deprecated symbols

* Touch up the docs for RETRO_ENVIRONMENT_SET_MESSAGE

* Touch up the docs for RETRO_ENVIRONMENT_SET_PIXEL_FORMAT

* Add more doc comments

* (libretro) Add more doxygen documentation for the libretro API

* (libretro) Add doxygen comments for the callbacks

* Document retro_init and retro_deinit

* Add comments for retro_log-related symbols

* Add a comment

* Clean up some camera-related comments

* Clean up frame time-related callbacks

* Correct some information about major callbacks

* Clarify some parameter info

* Fix incorrect info about retro_set_environment

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* (libretro) Add doxygen docs on RETRO_THROTTLE

* Touch up the docs for RETRO_ENVIRONMENT_SET_AUDIO_CALLBACK

* Touch up the docs for some macros

* Touch up the docs for some more environment calls

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* Tidy up the doc comments for clamping.h

- It was a low-hanging fruit

* Define some sections for constants

- Doxygen will group all contained symbols on one page

* Fix a duplicate @see

* Polish up the docs for the rumble interface

* Polish up the docs for RETRO_ENVIRONMENT_GET_INPUT_DEVICE_CAPABILITIES

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* Document INLINE

* Clean up some tags

* Touch up the docs for the sensor interface

* Add docs for RETRO_ENVIRONMENT_SET_PROC_ADDRESS_CALLBACK

* Update docs for RETRO_ENVIRONMENT_GET_INPUT_BITMASKS and accompanying names

* Update some group definitions

* Spiff up the docs for retro_dirent.h

* Document dylib.h

* Document base64.h

* Document crc32.h

* Touch up the docs for audio conversion functions

* Clean up some Doxygen tags

* Refine the docs for RETRO_ENVIRONMENT_GET_PERF_INTERFACE

* Fix incorrect infor in dylib.h

* Touch up the docs for RETRO_ENVIRONMENT_GET_CAMERA_INTERFACE

* Revise the docs for RETRO_ENVIRONMENT_SET_GEOMETRY

* Revise the docs for RETRO_ENVIRONMENT_GET_LOCATION_INTERFACE

* Revise a function's doc

* Touch up most of the rthreads docs

* Touch up the retro_timers.h docs

* Revise the subsystem docs

* Fix some incorrect @see's

* Touch up the docs for RETRO_ENVIRONMENT_GET_LED_INTERFACE

* Give the RETRO_ENVIRONMENT_GET_SAVESTATE_CONTEXT docs a makeover

* Slight cleanup to the microphone docs

* Slight cleanup to the device power docs

* Touch up serialization quirk docs

* Give the MIDI docs a haircut

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* Freshen up rtime's docs

* Improve the docs and accompanying definitions for RETRO_ENVIRONMENT_GET_AUDIO_VIDEO_ENABLE

- Revise the text of the documentation
- Introduce an enum that defines the flags (it's still an int, so ABI compatibility will be fine)
- Move the documentation for each bit to its corresponding enum

* Shine the shoes of RETRO_ENVIRONMENT_GET_INPUT_MAX_USERS's docs

* Freshen up the docs for fifo_queue.h

* Document most of task_queue.h

* Put retro_dirent's symbols in a group

* Finish documenting task_queue.h

* Document some compatibility headers

* Document read_stdin

* Document file_stream_transforms.h

* Document the VFS API

- Not the wrappers, just the plain API itself

* (Docs) Add doxygen notes about RETRO_DEVICE_*

* Fix some line breaks

* Revise RETRO_DEVICE docs

* Document strl.h

* Update the features_cpu.h docs

* Rewrite the docs for file_stream.h

* Update the docs for retro_endianness.h

* Update the docs for retro_miscellaneous.h

* Document the RETRO_VFS_SEEK_POSITION constants

* Finish documenting rthreads.h

* Document network_stream.h

* Put the RETRO_MEMORY defines in a defgroup

* Move a doc comment in retro_common.h to file scope

* Revise the docs for RETRO_ENVIRONMENT_SET_CONTROLLER_INFO, and accompanying symbols

* Fix the @param/in/out order in libretro.h's @param declarations

* Tidy up the docs for RETRO_ENVIRONMENT_GET_CORE_OPTIONS_VERSION

* Spiff up the docs for RETRO_ENVIRONMENT_GET_CURRENT_SOFTWARE_FRAMEBUFFER

* Fix some tags

* Polish up RETRO_ENVIRONMENT_GET_HW_RENDER_INTERFACE's docs

* libretro: Add header doxygen

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* Update libretro-common/include/libretro.h

Co-authored-by: Rob Loach <[email protected]>

* Clean up the docs for RETRO_ENVIRONMENT_SET_CORE_OPTIONS_DISPLAY

* Clean up the docs for RETRO_ENVIRONMENT_SET_HW_RENDER_CONTEXT_NEGOTIATION_INTERFACE

* Touch up some comment syntax for RETRO_ENVIRONMENT_GET_VARIABLE_UPDATE

* Fix some inaccuracies

* Re-add the license statement for libretro.h

* Touch up the docs for RETRO_ENVIRONMENT_SET_CORE_OPTIONS_V2

* Touch up docs for RETRO_ENVIRONMENT_SET_CORE_OPTIONS_DISPLAY

* Touch up docs for some options-related symbols

* Fix some syntax that was preventing most doc files from being generated

* Express retro_core_option_definition docs in terms of retro_core_option_v2_definition

* Finalize some core option-related docs

* Fix some incorrect info about achievements

* Polish up the docs for RETRO_ENVIRONMENT_SET_MEMORY_MAPS

* Polish up the docs for RETRO_ENVIRONMENT_GET_DISK_CONTROL_INTERFACE_VERSION

* Add a notice for `RETRO_ENVIRONMENT_GET_LOG_INTERFACE`

* Update the disk control interface docs

* Add a sentence to a doc comment

* Update a comment

* Remove an irrelevant @todo

* Touch up the docs for `retro_message_target`

* Touch up the docs for `retro_message_type`

* Touch up the docs for `RETRO_ENVIRONMENT_SET_MESSAGE_EXT`

* Touch up the docs for `RETRO_ENVIRONMENT_SET_AUDIO_BUFFER_STATUS_CALLBACK`

* Touch up the docs for `RETRO_ENVIRONMENT_SET_MINIMUM_AUDIO_LATENCY`

* Revise a comment

* Revise the docs for `RETRO_ENVIRONMENT_SET_VARIABLE`

* Add a `@see`

* Clean up the `RETRO_ENVIRONMENT_SET_FASTFORWARDING_OVERRIDE` docs

* Update the Doxyfile

* libretro: Add three more environment callback doxygen docs

* doxygen: Remove @example reference

---------

Co-authored-by: Rob Loach <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants