Skip to content

Don't remove entry from ResourceEntities when removing a resource by ID#23716

Merged
alice-i-cecile merged 3 commits intobevyengine:mainfrom
chescock:remove-resource-by-id
Apr 8, 2026
Merged

Don't remove entry from ResourceEntities when removing a resource by ID#23716
alice-i-cecile merged 3 commits intobevyengine:mainfrom
chescock:remove-resource-by-id

Conversation

@chescock
Copy link
Copy Markdown
Contributor

@chescock chescock commented Apr 8, 2026

Objective

Make the behavior of World::remove_resource and World::remove_resource_by_id consistent. Currently, remove_resource will leave the entity in the ResourceEntities map so that re-initializing the resource keeps the same Entity, but remove_resource_by_id will remove it from the map.

Solution

Change remove to get in remove_resource_by_id.

Testing

Updated the existing unit test to ensure that re-initializing the resource keeps the same Entity.

@chescock chescock added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 8, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in ECS Apr 8, 2026
Copy link
Copy Markdown
Contributor

@kfc35 kfc35 left a comment

Choose a reason for hiding this comment

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

This issue was mentioned in another PR: #23174 (comment), so this is a great find!

@alice-i-cecile alice-i-cecile 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 Apr 8, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 8, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 8, 2026
@kfc35
Copy link
Copy Markdown
Contributor

kfc35 commented Apr 8, 2026

With #23616 merged, this just needs to readjust for the fact that Entity is returned, not &Entity

Remove unused `remove` method and remove dereferences from new `get` calls.
@chescock
Copy link
Copy Markdown
Contributor Author

chescock commented Apr 8, 2026

Yeah, sorry, I knew that conflict would happen. I just didn't expect both PRs to be merged so fast!! I also need to remove the now-unused remove method. ... Okay, pushed up some fixes!

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 8, 2026
Merged via the queue into bevyengine:main with commit b764189 Apr 8, 2026
38 checks passed
@github-project-automation github-project-automation bot moved this from Needs SME Triage to Done in ECS Apr 8, 2026
viridia pushed a commit to viridia/bevy that referenced this pull request Apr 10, 2026
…y ID (bevyengine#23716)

# Objective

Make the behavior of `World::remove_resource` and
`World::remove_resource_by_id` consistent. Currently, `remove_resource`
will leave the entity in the `ResourceEntities` map so that
re-initializing the resource keeps the same `Entity`, but
`remove_resource_by_id` will remove it from the map.

## Solution

Change `remove` to `get` in `remove_resource_by_id`.  

## Testing

Updated the existing unit test to ensure that re-initializing the
resource keeps the same `Entity`.
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-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants