[Merged by Bors] - [Fixes #6059] `Entity`'s “ID” should be named “index” instead #6107

Edwox · 2022-09-26T16:06:02Z

Objective

Fixes #6059, changing all incorrect occurrences of id in the entity module to index:

struct level documentation,
id struct field,
id method and its documentation.

Solution

Renaming and verifying using CI.

Migration Guide

The Entity::id() method was renamed to Entity::index().

Edwox · 2022-09-26T16:08:27Z

Could someone help me apply the label specified in #6059? It was the C-Breaking-Change label.

tguichaoua

There is few places where "id" should be change into "index".

crates/bevy_ecs/src/entity/mod.rs

Nilirad · 2022-09-26T17:41:03Z

Could someone help me apply the label specified in #6059? It was the C-Breaking-Change label.

Done. (my bad, I forgot that only organization members can tag issues and PRs)

Edwox · 2022-09-28T14:05:00Z

There is few places where "id" should be change into "index".

My plan was to do the changes incrementally. So I started off by just changing the id in the entity struct and all resulting dependency issues. The plan was then to start working on the documentation and so on.

Could someone help me apply the label specified in #6059? It was the C-Breaking-Change label.

Done. (my bad, I forgot that only organization members can tag issues and PRs)

Thank you! :)

…nces.

Edwox · 2022-10-03T14:29:33Z

Could I get some help with CI / build (macos-latest)? It gets timed out.

Nilirad · 2022-10-03T18:33:06Z

bors try

bors · 2022-10-03T18:37:51Z

try

Build failed:

run-examples-on-wasm

Nilirad · 2022-10-03T19:01:01Z

Looks like there's some code that is only compiled in wasm builds. It wasn't changed probably because rust-analyzer checks for the default target.

If you do cargo check -p bevy_ecs --all-targets you may be able to reproduce the errors locally (-p bevy_ecs only restricts the check to that crate).

Edwox · 2022-10-03T22:54:48Z

Looks like there's some code that is only compiled in wasm builds. It wasn't changed probably because rust-analyzer checks for the default target.

If you do cargo check -p bevy_ecs --all-targets you may be able to reproduce the errors locally (-p bevy_ecs only restricts the check to that crate).

Thank you! I believe that the issue should be resolved now. 😃

xgbwei

A few variables named idx in sparse_set.rs need to be updated.

crates/bevy_ecs/src/storage/sparse_set.rs

Edwox

A few variables named idx in sparse_set.rs need to be updated.

okay, I agree with these changes. I will get right on it. 😄

…bevy_entity_id_to_index

alice-i-cecile

I'm in favor of this change, and this is well-done. Can you please resolve the comments as you address them to make it easier for reviewers to track what needs to be done?

Edwox · 2022-10-17T16:30:54Z

I'm in favor of this change, and this is well-done. Can you please resolve the comments as you address them to make it easier for reviewers to track what needs to be done?

Great, thank you! I think all comments should be resolved now.

tim-blackbird · 2022-10-17T17:38:33Z

crates/bevy_scene/src/dynamic_scene.rs

+            // Fetch the entity with the given entity index from the `entity_map`
+            // or spawn a new entity with a transiently unique index if there is


id is correct here.

tim-blackbird · 2022-10-17T17:40:26Z

crates/bevy_scene/src/dynamic_scene_builder.rs

+            for component_index in self.world.entity(entity).archetype().components() {
                let reflect_component = self
                    .world
                    .components()
-                    .get_info(component_id)
+                    .get_info(component_index)


component_id is correct here.

Edwox

Of course, my bad! Will fix this right away.

…ne_builder.rs``

…bevy_entity_id_to_index

alice-i-cecile

Rebase LGTM.

tim-blackbird · 2022-11-02T13:23:07Z

Looks like there are some new calls to .id() that need to be changed to .index() that came in from the merge.

Edwox · 2022-11-02T13:38:52Z

Looks like there are some new calls to .id() that need to be changed to .index() that came in from the merge.

Attempting to fix it right now.

alice-i-cecile · 2022-11-02T15:19:32Z

Can you please add the following to your PR description?

Migration Guide

The Entity::id() method was renamed to Entity::index().

bors r+

# Objective Fixes #6059, changing all incorrect occurrences of ``id`` in the ``entity`` module to ``index``: * struct level documentation, * ``id`` struct field, * ``id`` method and its documentation. ## Solution Renaming and verifying using CI. Co-authored-by: Edvin Kjell <43633999+Edwox@users.noreply.github.com>

Edwox · 2022-11-02T15:28:35Z

The description should be updated now.

bors · 2022-11-02T15:34:44Z

Pull request successfully merged into main.

Build succeeded:

build-and-install-on-iOS
build-android
build (macos-latest)
build (ubuntu-latest)
build-wasm
build (windows-latest)
build-without-default-features (bevy)
build-without-default-features (bevy_ecs)
build-without-default-features (bevy_reflect)
check-compiles
check-doc
check-missing-examples-in-docs
ci
markdownlint
run-examples
run-examples-on-wasm
run-examples-on-windows-dx12

Continuation of #6107 Co-authored-by: devil-ira <justthecooldude@gmail.com>

Continuation of bevyengine#6107 Co-authored-by: devil-ira <justthecooldude@gmail.com>

…tead (bevyengine#6107) # Objective Fixes bevyengine#6059, changing all incorrect occurrences of ``id`` in the ``entity`` module to ``index``: * struct level documentation, * ``id`` struct field, * ``id`` method and its documentation. ## Solution Renaming and verifying using CI. Co-authored-by: Edvin Kjell <43633999+Edwox@users.noreply.github.com>

Continuation of bevyengine#6107 Co-authored-by: devil-ira <justthecooldude@gmail.com>

Renamed id in the Entity struct to index

ba8279a

Edwox changed the title ~~[Fixes #6059] Renamed id in the Entity struct to index~~ [Fixes #6059] Entity's “ID” should be named “index” instead Sep 26, 2022

tguichaoua suggested changes Sep 26, 2022

View reviewed changes

Nilirad added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Sep 26, 2022

Renamed Entity's member id to index and for all its insta…

7f13187

…nces.

bors bot added a commit that referenced this pull request Oct 3, 2022

Try #6107:

ed0ffb8

Renamed Entity's member id to index in sparse_set.rs.

b3e5184

Merge branch 'main' into bevy_entity_id_to_index

6fc664e

Edwox marked this pull request as ready for review October 5, 2022 15:26

xgbwei suggested changes Oct 6, 2022

View reviewed changes

crates/bevy_ecs/src/storage/sparse_set.rs Outdated Show resolved Hide resolved

crates/bevy_ecs/src/storage/sparse_set.rs Outdated Show resolved Hide resolved

Edwox commented Oct 6, 2022

View reviewed changes

Edwox added 7 commits October 7, 2022 20:29

Updated variables named idx to index in sparse_set.rs.

208d8a5

Merge branch 'main' into bevy_entity_id_to_index

eefe35a

Merge branch 'main' into bevy_entity_id_to_index

ff31954

Renamed id to index

79a1e88

Merge main into bevy_entity_id_to_index

20dd979

Merge branch 'bevy_entity_id_to_index' of github.com:Edwox/bevy into …

da3b791

…bevy_entity_id_to_index

Renamed index to id for EntityMut

c51b587

alice-i-cecile approved these changes Oct 17, 2022

View reviewed changes

alice-i-cecile requested a review from Nilirad October 17, 2022 16:49

tim-blackbird suggested changes Oct 17, 2022

View reviewed changes

Edwox commented Oct 20, 2022

View reviewed changes

Edwox and others added 6 commits October 20, 2022 09:47

Renamed component_index back to component_id in ``dynamic_sce…

cf4ee06

…ne_builder.rs``

Renamed index to id in dynamic_scene.rs

89474b6

Merge branch 'bevyengine:main' into bevy_entity_id_to_index

e819f70

Merge branch 'main' into bevy_entity_id_to_index

66f3437

Merge branch 'bevy_entity_id_to_index' of github.com:Edwox/bevy into …

5ae4a08

…bevy_entity_id_to_index

Merge branch 'main' into bevy_entity_id_to_index

f0bd519

alice-i-cecile approved these changes Nov 2, 2022

View reviewed changes

Resolved a merge issue

f00552c

Edwox added 2 commits November 2, 2022 14:41

Resolved merge issues dynamic_scene_builder

e03a071

EntityMut should use .id() instead of .index()

8e95a72

tim-blackbird approved these changes Nov 2, 2022

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 Nov 2, 2022

bors bot changed the title ~~[Fixes #6059] Entity's “ID” should be named “index” instead~~ [Merged by Bors] - [Fixes #6059] Entity's “ID” should be named “index” instead Nov 2, 2022

bors bot closed this Nov 2, 2022

tim-blackbird mentioned this pull request Nov 22, 2022

[Merged by Bors] - Rename EntityId to EntityIndex #6732

Closed

bors bot pushed a commit that referenced this pull request Nov 22, 2022

Rename EntityId to EntityIndex (#6732)

a1607b8

Continuation of #6107 Co-authored-by: devil-ira <justthecooldude@gmail.com>

taiyoungjang pushed a commit to taiyoungjang/bevy that referenced this pull request Dec 15, 2022

Rename EntityId to EntityIndex (bevyengine#6732)

e105fb1

Continuation of bevyengine#6107 Co-authored-by: devil-ira <justthecooldude@gmail.com>

Edwox deleted the bevy_entity_id_to_index branch December 16, 2022 08:09

alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023

Rename EntityId to EntityIndex (bevyengine#6732)

9add320

Continuation of bevyengine#6107 Co-authored-by: devil-ira <justthecooldude@gmail.com>

ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023

Rename EntityId to EntityIndex (bevyengine#6732)

e1a667c

Continuation of bevyengine#6107 Co-authored-by: devil-ira <justthecooldude@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - [Fixes #6059] `Entity`'s “ID” should be named “index” instead #6107

[Merged by Bors] - [Fixes #6059] `Entity`'s “ID” should be named “index” instead #6107

Edwox commented Sep 26, 2022 •

edited

Loading

Edwox commented Sep 26, 2022 •

edited

Loading

tguichaoua left a comment

Nilirad commented Sep 26, 2022

Edwox commented Sep 28, 2022

Edwox commented Oct 3, 2022

Nilirad commented Oct 3, 2022

bors bot commented Oct 3, 2022

Nilirad commented Oct 3, 2022

Edwox commented Oct 3, 2022

xgbwei left a comment •

edited

Loading

Edwox left a comment •

edited

Loading

alice-i-cecile left a comment

Edwox commented Oct 17, 2022

tim-blackbird Oct 17, 2022

tim-blackbird Oct 17, 2022

Edwox left a comment

alice-i-cecile left a comment

tim-blackbird commented Nov 2, 2022 •

edited

Loading

Edwox commented Nov 2, 2022

alice-i-cecile commented Nov 2, 2022 •

edited

Loading

Edwox commented Nov 2, 2022 •

edited

Loading

bors bot commented Nov 2, 2022

		// Fetch the entity with the given entity index from the `entity_map`
		// or spawn a new entity with a transiently unique index if there is

[Merged by Bors] - [Fixes #6059] Entity's “ID” should be named “index” instead #6107

[Merged by Bors] - [Fixes #6059] Entity's “ID” should be named “index” instead #6107

Conversation

Edwox commented Sep 26, 2022 • edited Loading

Objective

Solution

Migration Guide

Edwox commented Sep 26, 2022 • edited Loading

tguichaoua left a comment

Choose a reason for hiding this comment

Nilirad commented Sep 26, 2022

Edwox commented Sep 28, 2022

Edwox commented Oct 3, 2022

Nilirad commented Oct 3, 2022

bors bot commented Oct 3, 2022

try

Nilirad commented Oct 3, 2022

Edwox commented Oct 3, 2022

xgbwei left a comment • edited Loading

Choose a reason for hiding this comment

Edwox left a comment • edited Loading

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

Edwox commented Oct 17, 2022

tim-blackbird Oct 17, 2022

Choose a reason for hiding this comment

tim-blackbird Oct 17, 2022

Choose a reason for hiding this comment

Edwox left a comment

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

tim-blackbird commented Nov 2, 2022 • edited Loading

Edwox commented Nov 2, 2022

alice-i-cecile commented Nov 2, 2022 • edited Loading

Migration Guide

Edwox commented Nov 2, 2022 • edited Loading

bors bot commented Nov 2, 2022

[Merged by Bors] - [Fixes #6059] `Entity`'s “ID” should be named “index” instead #6107

[Merged by Bors] - [Fixes #6059] `Entity`'s “ID” should be named “index” instead #6107

Edwox commented Sep 26, 2022 •

edited

Loading

Edwox commented Sep 26, 2022 •

edited

Loading

xgbwei left a comment •

edited

Loading

Edwox left a comment •

edited

Loading

tim-blackbird commented Nov 2, 2022 •

edited

Loading

alice-i-cecile commented Nov 2, 2022 •

edited

Loading

Edwox commented Nov 2, 2022 •

edited

Loading