[Merged by Bors] - Doc/module style doc blocks for examples #4438

themasch · 2022-04-07T08:29:05Z

Objective

Provide a starting point for #3951, or a partial solution.
Providing a few comment blocks to discuss, and hopefully find better one in the process.

Solution

Since I am pretty new to pretty much anything in this context, I figured I'd just start with a draft for some file level doc blocks. For some of them I found more relevant details (or at least things I considered interessting), for some others there is less.

Changelog

Moved some existing comments from main() functions in the 2d examples to the file header level
Wrote some more comment blocks for most other 2d examples

TODO:

2d/sprite_sheet, wasnt able to come up with something good yet
all other example groups...

Also: Please let me know if the commit style is okay, or to verbose. I could certainly squash these things, or add more details if needed.
I also hope its okay to raise this PR this early, with just a few files changed. Took me long enough and I dont wanted to let it go to waste because I lost motivation to do the whole thing. Additionally I am somewhat uncertain over the style and contents of the commets. So let me know what you thing please.

IceSentry · 2022-04-07T15:03:24Z

Your commit style is fine, we always squash the PR in a single commit using the PR title+description as the commit message so it doesn't matter how you commit while working on it.

IceSentry · 2022-04-07T15:05:58Z

examples/2d/contributors.rs

@@ -1,3 +1,32 @@
+//! This example collects a list of all contributors to the bevy source code and renders a bouncing


Personally, I would prefer if comments explaining a specific system were directly on that system and not all at the top. I think the top level doc should just be a short explanation of what it is. I would start with having the same short description found in the examples README

Completely agree here :)

alice-i-cecile

Hey, thanks! I agree with IceSentry: we can keep the "module-level" comments pretty terse to start.

More detailed explanation (such as "why do we add LogPlugin) belongs with that code, because it's a) more helpful to users there and b) less likely to go out of sync.

themasch · 2022-04-08T10:45:16Z

First, thanks for you fast and helpful feedback! I really dont know what I was thinking when doing the contributors comment/novel.

Your commit style is fine, we always squash the PR in a single commit using the PR title+description as the commit message so it doesn't matter how you commit while working on it.

Great, thanks!

I think the top level doc should just be a short explanation of what it is. I would start with having the same short description found in the examples README

I agree with IceSentry: we can keep the "module-level" comments pretty terse to start.

I agree that especially the contributors one is way to big, and really need to be ~~split up~~ shortened. But on the other hand, I think that the description in the examples README list is a bit to terse for a file doc. I think, especially in examples, a comment should add a little bit more than just "sprite.rs renders a sprite".

Maybe a way between those extremes would be useful? Do you thing the comments for mesh2d.rs and mesh2d_manual.rs are okay?

I will rework the changes by (re-)moving details and keeping the file level doc block terse.

alice-i-cecile · 2022-04-08T14:57:23Z

Maybe a way between those extremes would be useful? Do you thing the comments for mesh2d.rs and mesh2d_manual.rs are okay?

I really like these comments :) They strike a nice balance, and provide some essential context on the examples.

IceSentry · 2022-04-08T15:07:52Z

Oh, yeah, I was just suggesting the README comments as a starting point. I definitely agree that more detail is necessary. My issue was specifically about the comments explaining specific systems not being directly next to the system,

cart · 2022-04-08T19:57:11Z

I'm on board with this general approach. Whenever y'all think this is ready, feel free to merge!

themasch · 2022-04-17T15:50:41Z

So, I think this is ready for review/feedback. All examples should have comments, I used the ones from the readme if I was not able to come up with something short and better (most of them), used existing comments where they existed.

alice-i-cecile · 2022-04-17T18:07:11Z

@bevyengine/docs-team can I get another set of eyes on this? :)

IceSentry

I didn't look at everything in detail, but it looks good to me except a couple of nitpicks

IceSentry · 2022-04-18T02:17:02Z

examples/stress_tests/many_cubes.rs

@@ -1,8 +1,21 @@
+//! Simple benchmark to test per-entity draw overhead
+//!
+//! To a realistic performance result, run this in release mode.


It seems like there's a missing word here. "To see a" or "To get a" or something like that.

Suggested change

//! To a realistic performance result, run this in release mode.

//! To measure performance realistically, be sure to run this in release mode.

IceSentry · 2022-04-18T02:18:28Z

examples/tools/scene_viewer.rs

@@ -1,3 +1,8 @@
+//! A simple way to view glTF models with Bevy.
+//! Just run `cargo run --release --example scene_viewer -- /path/to/model.gltf#Scene0`,


I don't think the -- is necessary

You are correct. Just tested this, and - at least with 1.60 on windows - cargo run seems to pass additional paramters to the target binary.
I'll remove all -- from cargo runs in the examples.

alice-i-cecile · 2022-04-18T02:25:21Z

examples/app/empty.rs

@@ -1,3 +1,5 @@
+//! An empty application (does nothing)


I don't love that this example exists, but that's not this PR's fault.

You want me to delete it?
Not sure if thats something that should be done in this context?

I was trying to say that this should be done in a separate PR, yeah.

examples/app/headless_defaults.rs

alice-i-cecile · 2022-04-18T02:39:27Z

examples/reflection/trait_reflection.rs

@@ -1,3 +1,5 @@
+//! Allows reflection with trait objects


This isn't terribly descriptive; but IMO we should tackle this as part of a push to properly document and explain bevy_reflect instead.

alice-i-cecile · 2022-04-18T02:39:46Z

examples/scene/scene.rs

@@ -1,6 +1,7 @@
+//! This example illustrates loading and saving scenes from files


This could really use a brief explanation of what a "scene" is in Bevy.

For me to add that, I'd need to understand that myself 😅. From what I understand, a Scene is a newtype wrapper around a World so we can put complete worlds - for example loaded from gltf assets - into our main world. That works by copying all components from the scene to the world its spawned in when the command is executed. Therefor, it does not really 'mount' other worlds into our app.world, it just uses the container to spawn a coupled set of components/entities into it?

a Scene is a set of entites that can be loaded from an asset and added to our world.?

That's about the right idea. They're effectively "ECS data, but on disk". We can clean this up later :)

alice-i-cecile · 2022-04-18T02:40:01Z

examples/shader/compute_shader_game_of_life.rs

@@ -1,3 +1,5 @@
+//! A compute shader simulating Conway's Game of Life


What's a compute shader? ;)

I added a sentence to add some context.

alice-i-cecile

Overall LGTM. Great work; and we can build on these further.

In general, I think these could benefit from having a bit more "background explanation" about the topics they're covering, but I'm fine to address those in follow-up PRs.

IMO we should clean up the few typos, and then merge this.

themasch · 2022-04-18T10:44:37Z

Thanks for the quick feedback, I tried to address all points.

alice-i-cecile · 2022-04-28T18:06:36Z

@bevyengine/docs-team + @Nilirad can I have another review or two on this? I'd like to get this in and start enforcing it as a standard.

Nilirad

I'm doing this review by pieces since the PR is quite big. I reached the load_gltf example. I mostly did consistency checks and, in a couple of cases, I suggested a longer, more specific suggestion.

Nilirad · 2022-05-01T09:00:25Z

examples/2d/mesh2d_manual.rs

@@ -1,3 +1,8 @@
+//! This example shows how to manually render 2d items using "mid level render apis" with  a custom


Suggested change

//! This example shows how to manually render 2d items using "mid level render apis" with a custom

//! This example shows how to manually render 2d items using "mid level render apis" with a custom

Double space.

Nilirad · 2022-05-01T09:03:04Z

examples/2d/mesh2d_manual.rs

+//! It doesn't use the [`Material2d`] abstraction, but changes the vertex buffer to include vertex color
+//! Check out the "mesh2d" example for simpler / higher level 2d meshes


Suggested change

//! It doesn't use the [`Material2d`] abstraction, but changes the vertex buffer to include vertex color

//! Check out the "mesh2d" example for simpler / higher level 2d meshes

//! It doesn't use the [`Material2d`] abstraction, but changes the vertex buffer to include vertex color.

//! Check out the "mesh2d" example for simpler / higher level 2d meshes.

Period at the end of sentence.

Nilirad · 2022-05-01T09:06:25Z

examples/2d/rect.rs

@@ -9,6 +11,8 @@ fn main() {

 fn setup(mut commands: Commands) {
    commands.spawn_bundle(OrthographicCameraBundle::new_2d());
+
+    // Since this Sprite is not generated from an image, we have to specify its size in `custom_size`.


Suggested change

// Since this Sprite is not generated from an image, we have to specify its size in `custom_size`.

// Since this `Sprite` is not generated from an image, we have to specify its size in `custom_size`.

Monospace consistency.

Nilirad · 2022-05-01T09:08:09Z

examples/2d/rotation.rs

@@ -1,3 +1,5 @@
+//! Demonstrates rotating entities in 2D with quaternions.


Suggested change

//! Demonstrates rotating entities in 2D with quaternions.

//! Demonstrates rotating entities in 2D using quaternions.

Just my opinion, feel free to skip if using “with” is sufficient.

Nilirad · 2022-05-01T09:16:46Z

examples/2d/texture_atlas.rs

@@ -1,7 +1,8 @@
+//! In this example we generate a new texture atlas (sprite sheet) from a folder containing
+//! individual sprites


Suggested change

//! individual sprites

//! individual sprites.

Sentence period.

Nilirad · 2022-05-01T09:20:56Z

examples/3d/3d_scene.rs

@@ -1,3 +1,5 @@
+//! Simple 3D scene with basic shapes and lighting


Overall, I would think about changing the description into something less generic, that makes the reader expect more what they are about to see, since the focus of the example is on the scene itself; maybe something like:

A simple 3D scene with light shining over a cube sitting on a plane.

Nilirad · 2022-05-01T09:22:09Z

examples/3d/lighting.rs

@@ -1,3 +1,5 @@
+//! Illustrates various lighting options in a simple scene


Suggested change

//! Illustrates various lighting options in a simple scene

//! Illustrates various lighting options in a simple scene.

Sentence period.

Nilirad · 2022-05-01T09:27:46Z

examples/3d/lighting.rs

@@ -1,3 +1,5 @@
+//! Illustrates various lighting options in a simple scene


I think the description should talk about the fact that there are used different light types (point, directional, ...) and different colors, instead of simply stating “various lighting options”.

Yeah, sounds good. I'll try to come up with something more helpful!

Nilirad · 2022-05-01T09:33:28Z

examples/3d/load_gltf.rs

@@ -1,3 +1,5 @@
+//! Loads and renders a gltf file as a scene


Suggested change

//! Loads and renders a gltf file as a scene

//! Loads and renders a glTF file as a scene.

This is the capitalization that Khronos uses. If one is referring to the file extension instead, they should write “.gltf file” instead. It is ok to say “glTF file” since it is another wording for “file with glTF format”.

I also added the period.

Nilirad · 2022-05-01T09:34:56Z

examples/2d/text2d.rs

@@ -1,3 +1,10 @@
+//! Example to show text rendering with moving, rotating and scaling text.


Suggested change

//! Example to show text rendering with moving, rotating and scaling text.

//! Shows text rendering with moving, rotating and scaling text.

Saying it is an example doesn't convey any new or useful information.

themasch · 2022-05-01T11:58:46Z

Thanks for the annotations, @Nilirad! I went through the list and added periods where they where missing in the header comment. I did not fix punctuation for all existing comments (yet), as I think that would increase the pull request size even more.
If you want me to tackle this as well, let me know.

The plan here is having a short, but helpful, introduction to each example. Stating what is meant to be shown, without going into details.

I used the text from the README as a default, or comments from the `main` if they existed in the example, sometime adding a bit more context. Some slight re-wording or layout changes where also applied.

re-sorted two groups alphabetically.

themasch · 2022-05-15T08:19:17Z

Thanks again @Nilirad, I copied all your suggestions. At least I hope I got them all correct, at this point I doubt my ability to do even that.
Sorry for this terrible attempt, I guess I should have left this to someone more capable.

Anyhow, I rebased on the current main branch, as there was a merge conflict (2d/rect.rs got deleted). A few new examples have been added in the meantime, some of them without commets, so I tried to add at least a basic one. Basically longer versions of the file name 😕.

Nilirad · 2022-05-16T08:09:52Z

You did a great job with this PR! The amount of files made it a gigantic task, and that's normal to make some mistakes. That's why reviews are fundamental.

I'll do now a quick final pass to ensure there aren't misplaced descriptions. I'll ignore typos, grammar, punctuation or form so we can merge it ASAP.

Nilirad

It looks like there are no grave mistakes. Thank you for accomplishing this task @themasch, and congratulations for your first merged contribution! 🎉

alice-i-cecile · 2022-05-16T13:53:04Z

bors r+

# Objective Provide a starting point for #3951, or a partial solution. Providing a few comment blocks to discuss, and hopefully find better one in the process. ## Solution Since I am pretty new to pretty much anything in this context, I figured I'd just start with a draft for some file level doc blocks. For some of them I found more relevant details (or at least things I considered interessting), for some others there is less. ## Changelog - Moved some existing comments from main() functions in the 2d examples to the file header level - Wrote some more comment blocks for most other 2d examples TODO: - [x] 2d/sprite_sheet, wasnt able to come up with something good yet - [x] all other example groups... Also: Please let me know if the commit style is okay, or to verbose. I could certainly squash these things, or add more details if needed. I also hope its okay to raise this PR this early, with just a few files changed. Took me long enough and I dont wanted to let it go to waste because I lost motivation to do the whole thing. Additionally I am somewhat uncertain over the style and contents of the commets. So let me know what you thing please.

bors · 2022-05-16T14:10:06Z

Pull request successfully merged into main.

Build succeeded:

# Objective Provide a starting point for bevyengine#3951, or a partial solution. Providing a few comment blocks to discuss, and hopefully find better one in the process. ## Solution Since I am pretty new to pretty much anything in this context, I figured I'd just start with a draft for some file level doc blocks. For some of them I found more relevant details (or at least things I considered interessting), for some others there is less. ## Changelog - Moved some existing comments from main() functions in the 2d examples to the file header level - Wrote some more comment blocks for most other 2d examples TODO: - [x] 2d/sprite_sheet, wasnt able to come up with something good yet - [x] all other example groups... Also: Please let me know if the commit style is okay, or to verbose. I could certainly squash these things, or add more details if needed. I also hope its okay to raise this PR this early, with just a few files changed. Took me long enough and I dont wanted to let it go to waste because I lost motivation to do the whole thing. Additionally I am somewhat uncertain over the style and contents of the commets. So let me know what you thing please.

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 7, 2022

IceSentry added C-Docs An addition or correction to our documentation C-Examples An addition or correction to our examples and removed S-Needs-Triage This issue needs to be labelled labels Apr 7, 2022

IceSentry reviewed Apr 7, 2022

View reviewed changes

alice-i-cecile self-requested a review April 7, 2022 15:41

alice-i-cecile requested changes Apr 7, 2022

View reviewed changes

themasch force-pushed the doc/module-style-doc-blocks-for-examples branch from 4ba0af0 to cf66375 Compare April 17, 2022 11:34

themasch marked this pull request as ready for review April 17, 2022 15:45

IceSentry approved these changes Apr 18, 2022

View reviewed changes

alice-i-cecile reviewed Apr 18, 2022

View reviewed changes

examples/app/headless_defaults.rs Outdated Show resolved Hide resolved

alice-i-cecile reviewed Apr 18, 2022

View reviewed changes

themasch force-pushed the doc/module-style-doc-blocks-for-examples branch from 9afa163 to 16ed993 Compare April 18, 2022 10:23

alice-i-cecile approved these changes Apr 18, 2022

View reviewed changes

alice-i-cecile mentioned this pull request Apr 24, 2022

Add capability to render to cpu buffer or double buffer #4572

Closed

Nilirad suggested changes May 1, 2022

View reviewed changes

themasch added 16 commits May 15, 2022 10:02

Add a simple module doc block to the 2d/sprite_flipping example.

b8c6817

moved the comment from main to the file head

1c5c560

Add a module doc block to the 2d/text2d example.

c2e78cf

Cut the long comments, reworded others.

9c0c7b1

The plan here is having a short, but helpful, introduction to each example. Stating what is meant to be shown, without going into details.

Add missing module doc blocks to examples.

301d75b

I used the text from the README as a default, or comments from the `main` if they existed in the example, sometime adding a bit more context. Some slight re-wording or layout changes where also applied.

Some small changes to the examples README.md

445327c

re-sorted two groups alphabetically.

Last missing blocks after rebase.

1b3e16b

ran cargo fmt --all to fix whatever the IDE auto format broke.

7b18e71

re-enable the markdownlint-ignore

c1b1da0

fixed some clippy md complaints in the new doc blocks

583acd4

remove unnecessary argument separator in cargo run calls

f9fd8b9

some rewording, the old one was missing words and therefore meaning.

8313bac

add some detail.

64d11cb

Add/fix some punctuation.

dad654a

applied @Nilirad's suggestions for example docs

b260137

added basic docs for the new example that had none.

cbc2470

themasch force-pushed the doc/module-style-doc-blocks-for-examples branch from c796c38 to cbc2470 Compare May 15, 2022 08:11

Nilirad approved these changes May 16, 2022

View reviewed changes

bors bot changed the title ~~Doc/module style doc blocks for examples~~ [Merged by Bors] - Doc/module style doc blocks for examples May 16, 2022

bors bot closed this May 16, 2022

This was referenced May 16, 2022

UI Texture Atlas support #3792

Closed

Module-style doc strings for examples #3951

Closed

		@@ -1,3 +1,32 @@
		//! This example collects a list of all contributors to the bevy source code and renders a bouncing

	//! To a realistic performance result, run this in release mode.
	//! To measure performance realistically, be sure to run this in release mode.

		@@ -1,3 +1,8 @@
		//! A simple way to view glTF models with Bevy.
		//! Just run `cargo run --release --example scene_viewer -- /path/to/model.gltf#Scene0`,

		@@ -1,6 +1,7 @@
		//! This example illustrates loading and saving scenes from files

		@@ -1,3 +1,5 @@
		//! A compute shader simulating Conway's Game of Life

		@@ -1,3 +1,8 @@
		//! This example shows how to manually render 2d items using "mid level render apis" with a custom

		//! It doesn't use the [`Material2d`] abstraction, but changes the vertex buffer to include vertex color
		//! Check out the "mesh2d" example for simpler / higher level 2d meshes

	// Since this Sprite is not generated from an image, we have to specify its size in `custom_size`.
	// Since this `Sprite` is not generated from an image, we have to specify its size in `custom_size`.

		@@ -1,3 +1,5 @@
		//! Demonstrates rotating entities in 2D with quaternions.

		@@ -1,7 +1,8 @@
		//! In this example we generate a new texture atlas (sprite sheet) from a folder containing
		//! individual sprites

		@@ -1,3 +1,5 @@
		//! Simple 3D scene with basic shapes and lighting

		@@ -1,3 +1,5 @@
		//! Illustrates various lighting options in a simple scene

	//! Loads and renders a gltf file as a scene
	//! Loads and renders a glTF file as a scene.

		@@ -1,3 +1,10 @@
		//! Example to show text rendering with moving, rotating and scaling text.

	//! Example to show text rendering with moving, rotating and scaling text.
	//! Shows text rendering with moving, rotating and scaling text.

[Merged by Bors] - Doc/module style doc blocks for examples #4438

[Merged by Bors] - Doc/module style doc blocks for examples #4438

Conversation

themasch commented Apr 7, 2022 • edited Loading

Objective

Solution

Changelog

IceSentry commented Apr 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

themasch commented Apr 8, 2022 • edited Loading

alice-i-cecile commented Apr 8, 2022

IceSentry commented Apr 8, 2022 • edited Loading

cart commented Apr 8, 2022

themasch commented Apr 17, 2022

alice-i-cecile commented Apr 17, 2022

IceSentry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IceSentry Apr 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

themasch Apr 18, 2022 • edited Loading

Choose a reason for hiding this comment

alice-i-cecile Apr 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

themasch commented Apr 18, 2022

alice-i-cecile commented Apr 28, 2022

Nilirad left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nilirad May 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

themasch commented May 1, 2022

themasch commented May 15, 2022

Nilirad commented May 16, 2022

Nilirad left a comment

Choose a reason for hiding this comment

alice-i-cecile commented May 16, 2022

bors bot commented May 16, 2022

themasch commented Apr 7, 2022 •

edited

Loading

themasch commented Apr 8, 2022 •

edited

Loading

IceSentry commented Apr 8, 2022 •

edited

Loading

IceSentry Apr 18, 2022 •

edited

Loading

themasch Apr 18, 2022 •

edited

Loading

alice-i-cecile Apr 18, 2022 •

edited

Loading

Nilirad left a comment •

edited

Loading

Nilirad May 1, 2022 •

edited

Loading