[Merged by Bors] - Improve bevy_ecs and bevy_app API docs where referenced by the new Bevy Book #2365

Nilirad · 2021-06-20T13:20:50Z

Objective

The upcoming Bevy Book makes many references to the API documentation of bevy.

Most references belong to the first two chapters of the Bevy Book:

This PR attempts to improve the documentation of bevy_ecs and bevy_app in order to help readers of the Book who want to delve deeper into technical details.

Solution

Add crate and level module documentation
Document the most important items (basically those included in the preludes), with the following style, where applicable:
- Summary. Short description of the item.
- Second paragraph. Detailed description of the item, without going too much in the implementation.
- Code example(s).
- Safety or panic notes.

Collaboration

Any kind of collaboration is welcome, especially corrections, wording, new ideas and guidelines on where the focus should be put in.

Related issues

Fixes add_startup_system_to_stage is broken #2246

crates/bevy_app/src/app.rs

alice-i-cecile · 2021-06-20T18:20:24Z

The appbuilder methods are where a lot of the interesting content is. I kind of want those documented, but then migrate them over to the new API.

Nilirad · 2021-06-20T18:36:09Z

Ok. I may be able to do that in a couple of days.

Nilirad · 2021-06-22T17:59:42Z

Status update

I added stub documentation for all the AppBuilder methods.

Design decisions to be taken

All public items of the app_builder module have now documentation. Should we add a #![warn(missing_docs)] at the top of the file to ensure it remains fully covered?
Most methods are just a wrapper around World and Schedule methods. How do we manage to avoid too much doc duplication between bevy_app and bevy_ecs? Since bevy_app depends on bevy_ecs, i think the meat should be on the latter, and bevy_app adds a link to bevy_ecs for more details. I did something like that for the AppBuilder::stage method.

Further work

Add an example for each method.
Add a second paragraph where applicable to describe the item more deeply.

crates/bevy_app/src/app_builder.rs

crates/bevy_ecs/src/system/system.rs

Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>

crates/bevy_ecs/src/system/mod.rs

Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>

crates/bevy_app/src/app_builder.rs

Nilirad · 2021-06-25T08:23:46Z

@alice-i-cecile I added in the last commit a link stub for the book. I think this may make a good addition for people who want to have a better insight into Bevy's design philosophy, if you find it appropriate.

This would likely go into chapter 2. I thought about expanding the scope of this PR to cover chapters 1 and 2, since they extensively touch topics that span over bevy_ecs.

Nilirad · 2021-08-18T09:56:25Z

@alice-i-cecile, @mockersf Merge conflicts resolved. I also deleted references in documentation to the removed WithBundle query filter.

@Ratysz I also documented part of the schedule API. Probably you can spot potential inaccuracies here better than many others.

Switching from draft to regular PR.

Nilirad · 2021-09-14T08:21:31Z

I fixed all rustdoc warnings when generating with command cargo doc -p bevy_ecs --no-deps and cargo doc -p bevy_app --no-deps

alice-i-cecile · 2021-09-15T17:28:15Z

@cart, when you get a chance could you do a quick skim and then merge this? I think this is critical to get in for 0.6 and the sooner we merge it the less of a merge-conflict risk it poses (and the better the developer experience on main).

cart · 2021-09-16T05:40:24Z

Sure thing! I'll throw it on the review stack :)

cart

Lots of solid improvements here. Great work!

crates/bevy_app/src/app.rs

crates/bevy_ecs/src/entity/mod.rs

crates/bevy_ecs/src/query/mod.rs

crates/bevy_ecs/src/system/query.rs

crates/bevy_ecs/src/world/mod.rs

cart · 2021-09-17T00:52:16Z

crates/bevy_ecs/src/world/mod.rs

+///
+/// # Resources
+///
+/// The world can store information not associated to entity data, called *resources*. They


I prefer a slight rewording of this: "Worlds can also store Resources, which are unique instances of a given type that don't belong to a specific Entity. There are also "non send resources", which can only be accessed on the main thread."

I wouldn't call Local a resource. It isn't stored the same way / World doesn't store them. Therefore we don't need to distinguish between "global" and "local" resources. Resources are "global" by definition.

Interesting. They tend to behave like resources from the end-user's perspective in other ways, but I see your point. The main problems from a pedagogical perspective are a) "local" is an adjective, not a noun so it begs the "local what" question b) where should they be discussed from a teaching perspective.

Would you just call them "local unstructured data storages" or something? Do we start describing the ECS as having 3 places it can store data?

@alice-i-cecile maybe we can call it "system local persistent data". It's persistent in the sense that it lasts across system calls but I hope it doesn't get confused with the more general "persistent storage" concept, opposed to "volatile storage".

Right 🤔 I think I like that.

Or "system local state". This time it can be confused with states though 😬

I think "system local persistent data" is fine if it's followed by the explanation that it's only persistent between frames

Yeah I think its enough to say that Local is "a persistent value stored locally in the system".

Nilirad · 2021-09-17T10:17:06Z

@cart Thanks for the review. I think I've applied all the suggestions.

cart · 2021-09-17T18:00:04Z

Looks good to me!

cart · 2021-09-17T18:00:10Z

bors r+

…vy Book (#2365) ## Objective The upcoming Bevy Book makes many references to the API documentation of bevy. Most references belong to the first two chapters of the Bevy Book: - bevyengine/bevy-website#176 - bevyengine/bevy-website#182 This PR attempts to improve the documentation of `bevy_ecs` and `bevy_app` in order to help readers of the Book who want to delve deeper into technical details. ## Solution - Add crate and level module documentation - Document the most important items (basically those included in the preludes), with the following style, where applicable: - **Summary.** Short description of the item. - **Second paragraph.** Detailed description of the item, without going too much in the implementation. - **Code example(s).** - **Safety or panic notes.** ## Collaboration Any kind of collaboration is welcome, especially corrections, wording, new ideas and guidelines on where the focus should be put in. --- ### Related issues - Fixes #2246

bors · 2021-09-17T18:27:02Z

Pull request successfully merged into main.

Build succeeded:

Nilirad added 4 commits June 20, 2021 13:49

Improve App docs

4d1040a

Move App doc example to AppBuilder

54954bc

Document App methods

6db1fce

Restore App example to original position

b7436de

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jun 20, 2021

TheRawMeatball added C-Docs An addition or correction to our documentation C-Enhancement A new feature and removed S-Needs-Triage This issue needs to be labelled labels Jun 20, 2021

TheRawMeatball reviewed Jun 20, 2021

View reviewed changes

crates/bevy_app/src/app.rs Outdated Show resolved Hide resolved

alice-i-cecile self-assigned this Jun 20, 2021

Clarify App::update method doc comment

71f4541

Document AppBuilder fns and some relative items

62258d4

MinerSebas reviewed Jun 22, 2021

View reviewed changes

crates/bevy_app/src/app_builder.rs Outdated Show resolved Hide resolved

MinerSebas reviewed Jun 22, 2021

View reviewed changes

crates/bevy_app/src/app_builder.rs Outdated Show resolved Hide resolved

crates/bevy_ecs/src/system/system.rs Outdated Show resolved Hide resolved

Nilirad and others added 5 commits June 23, 2021 12:48

Add StartupStage enum link to add_default_stages

3d34f02

Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>

Correct enum keyword monospace text

ba27b4f

Add docs for mod system

862ccb3

Point stage links to add_default_stages

cfe0755

Merge branch 'main' into api-docs-ch1

043b31a

MinerSebas reviewed Jun 23, 2021

View reviewed changes

crates/bevy_ecs/src/system/mod.rs Outdated Show resolved Hide resolved

Nilirad and others added 3 commits June 24, 2021 06:26

Update crates/bevy_ecs/src/system/mod.rs

c1fc50b

Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>

Correct add_startup_system_to_stage docs

f29e769

Document new AppBuilder methods

7c66d20

MinerSebas reviewed Jun 24, 2021

View reviewed changes

crates/bevy_app/src/app_builder.rs Outdated Show resolved Hide resolved

Nilirad added 2 commits June 24, 2021 19:09

Remove incorrect references to StartupStage

3f9d879

Add module-level documentation for entity

483f312

Merge branch 'bevyengine-main' into api-docs-ch1

762be6c

Nilirad marked this pull request as ready for review August 18, 2021 09:56

Nilirad added 2 commits August 27, 2021 18:37

Merge branch 'main' into api-docs-ch1

0136659

Merge branch 'bevyengine:main' into api-docs-ch1

b5bb71a

alice-i-cecile mentioned this pull request Sep 7, 2021

Incorrect doc syntax on World get and get_mut #2787

Closed

IceSentry approved these changes Sep 12, 2021

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Sep 12, 2021

Nilirad added 4 commits September 13, 2021 12:09

Resolve merge conflicts

1e6a53e

Merge branch 'bevyengine-main' into api-docs-ch1

0997fe4

fix

d740a96

Fix doc warnings

757aa22

Nilirad added 2 commits September 15, 2021 15:59

Resolve merge conflicts

37b5b40

Merge branch 'bevyengine-main' into api-docs-ch1

49f8e3d

alice-i-cecile added this to the Bevy 0.6 milestone Sep 16, 2021

cart reviewed Sep 17, 2021

View reviewed changes

Nilirad added 2 commits September 17, 2021 12:00

Apply review suggestions, part 1

f0ef44c

Move .spawn().insert_bundle alternative to spawn

4c88e01

bors bot changed the title ~~Improve bevy_ecs and bevy_app API docs where referenced by the new Bevy Book~~ [Merged by Bors] - Improve bevy_ecs and bevy_app API docs where referenced by the new Bevy Book Sep 17, 2021

bors bot closed this Sep 17, 2021

alice-i-cecile mentioned this pull request Oct 4, 2021

LD49 Post-Mortem -- First use of Bevy & learnings #2914

Closed

Nilirad deleted the api-docs-ch1 branch December 5, 2023 18:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Improve bevy_ecs and bevy_app API docs where referenced by the new Bevy Book #2365

[Merged by Bors] - Improve bevy_ecs and bevy_app API docs where referenced by the new Bevy Book #2365

Nilirad commented Jun 20, 2021 •

edited

Loading

alice-i-cecile commented Jun 20, 2021

Nilirad commented Jun 20, 2021

Nilirad commented Jun 22, 2021 •

edited

Loading

Nilirad commented Jun 25, 2021

Nilirad commented Aug 18, 2021

Nilirad commented Sep 14, 2021

alice-i-cecile commented Sep 15, 2021

cart commented Sep 16, 2021

cart left a comment

cart Sep 17, 2021

alice-i-cecile Sep 17, 2021

Nilirad Sep 17, 2021

alice-i-cecile Sep 17, 2021

Nilirad Sep 17, 2021

IceSentry Sep 17, 2021

cart Sep 17, 2021

Nilirad commented Sep 17, 2021

cart commented Sep 17, 2021

cart commented Sep 17, 2021

bors bot commented Sep 17, 2021

[Merged by Bors] - Improve bevy_ecs and bevy_app API docs where referenced by the new Bevy Book #2365

[Merged by Bors] - Improve bevy_ecs and bevy_app API docs where referenced by the new Bevy Book #2365

Conversation

Nilirad commented Jun 20, 2021 • edited Loading

Objective

Solution

Collaboration

Related issues

alice-i-cecile commented Jun 20, 2021

Nilirad commented Jun 20, 2021

Nilirad commented Jun 22, 2021 • edited Loading

Status update

Design decisions to be taken

Further work

Nilirad commented Jun 25, 2021

Nilirad commented Aug 18, 2021

Nilirad commented Sep 14, 2021

alice-i-cecile commented Sep 15, 2021

cart commented Sep 16, 2021

cart left a comment

Choose a reason for hiding this comment

cart Sep 17, 2021

Choose a reason for hiding this comment

alice-i-cecile Sep 17, 2021

Choose a reason for hiding this comment

Nilirad Sep 17, 2021

Choose a reason for hiding this comment

alice-i-cecile Sep 17, 2021

Choose a reason for hiding this comment

Nilirad Sep 17, 2021

Choose a reason for hiding this comment

IceSentry Sep 17, 2021

Choose a reason for hiding this comment

cart Sep 17, 2021

Choose a reason for hiding this comment

Nilirad commented Sep 17, 2021

cart commented Sep 17, 2021

cart commented Sep 17, 2021

bors bot commented Sep 17, 2021

Nilirad commented Jun 20, 2021 •

edited

Loading

Nilirad commented Jun 22, 2021 •

edited

Loading