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

Documentation for ECS internals #3366

Closed

Conversation

alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Dec 17, 2021

Objective

Solution

  • Document all the things.
  • Also improve existing documentation within bevy_ecs where I see the opportunity.

Status

This is a draft PR, so y'all can complain about my explanations as I work.

Unless there are strong objections, I'd like to tackle most of the crate in one pass, so then I can ensure that things aren't missed and the concepts are explained coherently.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 17, 2021
crates/bevy_ecs/src/archetype.rs Show resolved Hide resolved
crates/bevy_ecs/src/archetype.rs Show resolved Hide resolved
@@ -116,6 +123,9 @@ pub(crate) struct ArchetypeComponentInfo {
pub(crate) archetype_component_id: ArchetypeComponentId,
}

/// A unique set of components stored within the [`World`](crate::world::World)
Copy link
Member

Choose a reason for hiding this comment

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

With sparse set components, these can contain many sets of component, although each set is uniquely assigned to a single archetype.

@Nilirad
Copy link
Contributor

Nilirad commented Dec 17, 2021

Document all the things.

Start by putting #![warn(missing_docs)]

@alice-i-cecile
Copy link
Member Author

Start by putting #![warn(missing_docs)]

I think you mean #![forbid(missing_docs)]. One day!

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation and removed S-Needs-Triage This issue needs to be labelled labels Dec 18, 2021
Copy link
Contributor

@Carter0 Carter0 left a comment

Choose a reason for hiding this comment

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

Thank you for doing this! I was planning to look over this code at some point and this will be helpful

Probably out of scope for this PR
@cart
Copy link
Member

cart commented Dec 18, 2021

Document all the things

I propose documenting sections of bevy_ecs in multiple prs instead of all at once to make reviewing + merging easier. I won't block on that, but it has my preference.

@alice-i-cecile
Copy link
Member Author

I propose documenting sections of bevy_ecs in multiple prs instead of all at once to make reviewing + merging easier. I won't block on that, but it has my preference.

Sounds good @cart. I'm probably going to work in this PR, then close it out and cherrypick the commits into a dozen mini-PRs.

@alice-i-cecile
Copy link
Member Author

Alright, I think that's about all of it for a first pass. I'm going to set this down for a couple of days, and then come back to split the PRs and close this out when it's done.

Most of these docs could do with being expanded with context and examples, but that will be better handled in small PRs that experts can quickly review.

@alice-i-cecile
Copy link
Member Author

Split into #3384, #3385, #3386, #3387, #3388, #3389, #3390 and #3391 😂 Closing this out now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reflection allows users to clone arbitrary reflected components, but Reflect does not have a Clone bound
5 participants