Adds way of getting components of specific entity #299
Conversation
src/join.rs
Outdated
| impl<J: Join> JoinIter<J> { | ||
| /// Allows getting joined values for specific entity. | ||
| pub fn get(&mut self, e: Entity) -> Option<J::Type> { | ||
| if self.keys.contains(e.id()) { |
There was a problem hiding this comment.
Thinking about it again, maybe we should have a note saying you need to check if the entity is alive prior to doing this?
There was a problem hiding this comment.
Why would Index be preferred? Only thing that uses it is UnprotectedStorage which is not something normal library user would touch and all user facing API uses Entity.
So the problem with dead entities is that if the index is reused it would get that one instead of returning None? I would change it to check the liveliness then.
There was a problem hiding this comment.
I would change it to check the liveliness then.
But that would require to pass Entities.
There was a problem hiding this comment.
Entities or World depending where it's being called.
I just don't know if it's good idea to leave this for the user to check.
There was a problem hiding this comment.
You can get the entities from the world.
If it's a good idea, not sure, but it's better than using an entity without upholding the imposed guarantee.
src/join.rs
Outdated
| if self.keys.contains(e.id()) { | ||
| Some(unsafe { J::get(&mut self.values, e.id()) }) | ||
| pub fn get(&mut self, entity: Entity, entities: &Entities) -> Option<J::Type> { | ||
| if self.keys.contains(entity.id()) && entities.is_alive(entity) { |
There was a problem hiding this comment.
Could we at least also provide an _unchecked variant?
|
Also, please resolve the |
|
What I should change |
|
Reviewed 1 of 3 files at r4. src/join.rs, line 256 at r4 (raw file):
Logically this still gets the joined value for a given index, not an entity. Please change the parameter type and the documentation accordingly. Comments from Reviewable |
|
bors r+ Reviewed 2 of 3 files at r4. Comments from Reviewable |
299: Adds way of getting components of specific entity r=torkleyy a=WaDelma After randomly browsing through [rhusics](https://github.com/Rhuagh/rhusics) I noticed [piece of code](https://github.com/Rhuagh/rhusics/blob/259ff9e4ea61e98919472ea9ac77005177301c1d/src/ecs/collide/systems/spatial_collision.rs#L178) and thought about API that would make it cleaner/more efficient and this is the result. The code in question after this PR: ```rust let (left_shape, left_pose, left_next_pose) = (&shapes, &poses, &next_poses).join().get(left_entity).uwnrap(); let (right_shape, right_pose, right_next_pose) = (&shapes, &poses, &next_poses).join().get(right_entity).uwnrap(); ``` Depends on amethyst/hibitset#20 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/slide-rs/specs/299) <!-- Reviewable:end -->
|
Reviewed 1 of 1 files at r5. Comments from Reviewable |
Build succeeded |
After randomly browsing through rhusics I noticed piece of code and thought about API that would make it cleaner/more efficient and this is the result.
The code in question after this PR:
Depends on amethyst/hibitset#20
This change is