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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Add documentation to VisibleEntities and related #5100

Closed
wants to merge 5 commits into from
Closed

[Merged by Bors] - Add documentation to VisibleEntities and related #5100

wants to merge 5 commits into from

Conversation

djeedai
Copy link
Contributor

@djeedai djeedai commented Jun 26, 2022

Objective

Add missing docs

Solution

Add documentation to the VisibleEntities component, its related
check_visibility() system, and that system's label.

See Discord discussion here : https://discord.com/channels/691052431525675048/866787577687310356/990432663921901678

@djeedai
Add documentation to VisibleEntities and related
1af096f 
Add documentation to the `VisibleEntities` component, its related
`check_visibility()` system, and that system's label.
@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Rendering Drawing game state to the screen labels Jun 26, 2022
alice-i-cecile
alice-i-cecile requested changes Jun 26, 2022
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Very nice. We should add the current limitation that this has no effect on sprites to these docs, to avoid misleading users until that is fixed.

james7132
james7132 requested changes Jun 26, 2022
View reviewed changes
crates/bevy_render/src/view/visibility/mod.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/view/visibility/mod.rs Outdated Show resolved Hide resolved
djeedai and others added 3 commits June 27, 2022 09:57
@djeedai @james7132
Apply suggestion
e6eda20 
Co-authored-by: James Liu <contact@jamessliu.com>
@djeedai @alice-i-cecile
Commit suggestion about app world entities
4b2d05b 
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@djeedai
Add comment about sprite renderer
bb42e4f
james7132
james7132 approved these changes Jun 30, 2022
View reviewed changes
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

Please do fix the CI checks. Looks like a doc reference is failing.

@james7132 james7132 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 Jun 30, 2022
@djeedai
Copy link
Contributor Author

djeedai commented Jul 2, 2022

I've deleted the link. The clippy issue was the missing : between the link and its target (so the link was effectively malformed and ignored). But after that the link becomes broken. I think I had already issues like this in the past, when trying to link to another crate. I can't remember the details, but I think I had pinged the rustdoc folks and there was no good solution. Part of the issue is if I remember correctly that the sprite crate is bevy_sprite or bevy::sprite depending on context, and that confuses rustdoc. I can't remember all the details. Anyway, that works now.

Ping @alice-i-cecile for merge, pretty please 😉

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jul 2, 2022
@djeedai
Add documentation to VisibleEntities and related (#5100)
382cd49 
# Objective

Add missing docs

## Solution

Add documentation to the `VisibleEntities` component, its related
`check_visibility()` system, and that system's label.

See Discord discussion here : https://discord.com/channels/691052431525675048/866787577687310356/990432663921901678
@bors
Copy link
Contributor

bors bot commented Jul 2, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Add documentation to VisibleEntities and related [Merged by Bors] - Add documentation to VisibleEntities and related Jul 2, 2022
@bors bors bot closed this Jul 2, 2022
superdump
superdump reviewed Jul 2, 2022
View reviewed changes
crates/bevy_render/src/view/visibility/mod.rs
/// System updating the visibility of entities each frame.
///
/// The system is labelled with [`VisibilitySystems::CheckVisibility`]. Each frame, it updates the
/// [`ComputedVisibility`] of all entities, and for each view also compute the [`VisibleEntities`]
Copy link
Contributor

Choose a reason for hiding this comment

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

Too late, but ‘computes’

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, sorry I missed that. I'll try to make a new PR when I have some time.

@djeedai djeedai deleted the vis_docs branch July 2, 2022 11:18
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
@djeedai @inodentry
Add documentation to VisibleEntities and related (bevyengine#5100)
826a26f 
# Objective

Add missing docs

## Solution

Add documentation to the `VisibleEntities` component, its related
`check_visibility()` system, and that system's label.

See Discord discussion here : https://discord.com/channels/691052431525675048/866787577687310356/990432663921901678
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
@djeedai @james7132
Add documentation to VisibleEntities and related (bevyengine#5100)
69cfc0d 
# Objective

Add missing docs

## Solution

Add documentation to the `VisibleEntities` component, its related
`check_visibility()` system, and that system's label.

See Discord discussion here : https://discord.com/channels/691052431525675048/866787577687310356/990432663921901678
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@djeedai @ItsDoot
Add documentation to VisibleEntities and related (bevyengine#5100)
3bd0e17 
# Objective

Add missing docs

## Solution

Add documentation to the `VisibleEntities` component, its related
`check_visibility()` system, and that system's label.

See Discord discussion here : https://discord.com/channels/691052431525675048/866787577687310356/990432663921901678
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@superdump superdump superdump left review comments

@james7132 james7132 james7132 approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@djeedai @alice-i-cecile @superdump @james7132