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

implement EntityIndexMap/Set #17449

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

implement EntityIndexMap/Set #17449

wants to merge 2 commits into from
+911 −0

Conversation

Victoronz
Copy link
Contributor

@Victoronz Victoronz commented Jan 20, 2025 •
edited
Loading

Objective

We do not have EntityIndexMap/EntityIndexSet.

Solution

indexmap version of #16912.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 20, 2025
@alice-i-cecile
Copy link
Member

As discussed on Discord, we should adopt #17447 to apply to the EntityIndexMap type once this is merged.

@Victoronz
Copy link
Contributor Author

Victoronz commented Jan 20, 2025 •
edited
Loading

Note: this currently only includes slicing via deref. It'd put this PR well over a thousand lines to add the wrappers for those as well, so I'll do that as a follow-up.
This means some types here seem useless to wrap: In actuality, they can be sliced, which in turn are able to then return EntitySetIterators.

@Victoronz Victoronz mentioned this pull request Jan 20, 2025
Merged
@BenjaminBrienen BenjaminBrienen added the C-Feature A new feature, making something new possible label Jan 20, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 20, 2025
@Victoronz
remove unsound DerefMut impls from EntityHashMap/EntityHashSet (#…
59657ed 
…17450)

# Objective

Noticed while doing #17449, I had left these `DerefMut` impls in.
Obtaining mutable references to those inner iterator types allows for
`mem::swap`, which can be used to swap an incorrectly behaving instance
into the wrappers.

## Solution

Remove them!
@BenjaminBrienen BenjaminBrienen 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 Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminBrienen BenjaminBrienen BenjaminBrienen approved these changes

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

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-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.
3 participants
@Victoronz @alice-i-cecile @BenjaminBrienen