Skip to content

fix docs for Entity::from_row#21652

Merged
mockersf merged 1 commit into
bevyengine:mainfrom
zacryol:from-row-docsfix
Oct 25, 2025
Merged

fix docs for Entity::from_row#21652
mockersf merged 1 commit into
bevyengine:mainfrom
zacryol:from-row-docsfix

Conversation

@zacryol

@zacryol zacryol commented Oct 25, 2025

Copy link
Copy Markdown
Contributor

Objective

The docs state that Entity::from_row creates an entity with a generation of 1, which was the case previously. With Entity internals being reworked in 0.17 however, this is no longer the case, as the generation used is now EntityGeneration::FIRST, currently equivalent to 0.

Solution

This PR changes the docs to state an "unspecified" generation to allow further changes to implementation without needing another rewrite, as the docs rather consistently refer to Entity, EntityRow, and EntityGeneration as being opaque identifiers.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events X-Contentious There are nontrivial implications that should be thought through S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 25, 2025
@mockersf

Copy link
Copy Markdown
Member

@ElliottjPierce ElliottjPierce left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ooh, nice catch. I forgot to update that.

IMO, we have two options:

  • Change docs to say "unspecified" as is done here.
  • Specify that it is EntityGeneration::FIRST and rename to first_id_at_row or something to reflect it in the name.

Personally, I would prefer the first option, the current implementation, because that gives us the most flexibility and because if anyone cares about what generation they get, they will use from_row_and_generation instead anyway.

@Victoronz Victoronz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think marking it unspecified is fine for now.

@Victoronz Victoronz added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 25, 2025
@mockersf mockersf added this pull request to the merge queue Oct 25, 2025
Merged via the queue into bevyengine:main with commit ca13690 Oct 25, 2025
47 checks passed
@zacryol zacryol deleted the from-row-docsfix branch October 25, 2025 20:45
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 S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants