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

Add EntityMut::get_mut_by_id_unchecked #15603

Closed
alice-i-cecile opened this issue Oct 2, 2024 · 2 comments · Fixed by #16210
Closed

Add EntityMut::get_mut_by_id_unchecked #15603

alice-i-cecile opened this issue Oct 2, 2024 · 2 comments · Fixed by #16210
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@alice-i-cecile
Copy link
Member

I still think there should be a get_mut_by_id_unchecked. The apis added in #15593 could end up allocating in some tight loops which would be a perf footgun. The unchecked version would allow users to implement their own versions in userspace that could reuse the allocations.

Originally posted by @hymm in #15577
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Oct 2, 2024
@BenjaminBrienen BenjaminBrienen self-assigned this Oct 13, 2024
@BenjaminBrienen
Copy link
Contributor

BenjaminBrienen commented Oct 13, 2024 •
edited
Loading

I intend to work on this if no one else gets to it first

@BenjaminBrienen BenjaminBrienen removed their assignment Oct 13, 2024
@Soulghost Soulghost mentioned this issue Nov 2, 2024
Merged
@mockersf
Copy link
Member

it was also suggested that a variant checking unicity though a hashset could be better: #15577 (comment)

github-merge-queue bot pushed a commit that referenced this issue Nov 11, 2024
@Soulghost @hymm
Add EntityMut::get_mut_by_id_unchecked (#16210)
b83c0e1 
# Objective

- Fixes: #15603 

## Solution

- Add an unsafe `get_mut_by_id_unchecked` to `EntityMut` that borrows
&self instead of &mut self, thereby allowing access to multiple
components simultaneously.

## Testing

- a unit test function `get_mut_by_id_unchecked` was added.

---------

Co-authored-by: Mike <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add EntityMut::get_mut_by_id_unchecked Soulghost/bevy
3 participants
@alice-i-cecile @mockersf @BenjaminBrienen