amethyst / specs

Specs - Parallel ECS
https://amethyst.github.io/specs/
Apache License 2.0
2.49k stars 219 forks source link

Implement Clone for JoinIter #593

Closed lcnr closed 4 years ago

lcnr commented 5 years ago

Description

Implement Clone for JoinIter when it is safe to do so.

Motivation

I am currently trying to build a small collision system which simply checks for each entity pair if they collide, JoinIter implemented Clone I would be able to write it like this:

impl<'s> System<'s> for CollisionSystem {
    type SystemData = (
        Entities<'s>,
        ReadStorage<'s, Collider>,
        ReadStorage<'s, Transform>,
    );

    fn run(&mut self, (entities, colliders, transforms): Self::SystemData) {
        let mut collisions = Vec::new();

        let mut iter = (&entities, &colliders, &transforms).join();
        while let Some(a) = iter.next() {
            for b in iter.clone() // check all following entities {
                if is_collision((a.1, a.2), (b.1, b.2)) {
                    self.collisions.push(Collision(a.0, b.0))
                }
            }
        }

        for collision in collisions {
            // ...
        }
    }
}

I currently collect the JoinIter into a temporary Vec which does work, even if it is somewhat unsatisfying. If there is a prettier solution, please tell me as I only recently started using specs/amethyst.

Drawbacks

Unresolved questions

How can we detect that the JoinIter iterates over a mutable collection, in which case Clone must not get implemented.


I would gladly try to implement this if the unresolved question can be solved

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to comment if this is still relevant.

lcnr commented 5 years ago

still relevant

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to comment if this is still relevant.

lcnr commented 5 years ago

relevant

SET001 commented 4 years ago

@lcnr I currently collect the JoinIter into a temporary Vec which does work can you show how this solution works? I'm having difficulties with this trying to implement collisions system.

lcnr commented 4 years ago

@SET001 Does the following work for you?

// by storing the join in a vec instead of using it directly we can use `Vec::iter`
// which does implement `Clone`.
let collision_entities = (&entities, &colliders, &transforms).join()
    .into_iter().collect::<Vec<_>>();

let mut iter = collision_entities.iter();
while let Some(a) = iter.next() {
    for b in iter.clone() // check all following entities {
        if is_collision((a.1, a.2), (b.1, b.2)) {
            self.collisions.push(Collision(a.0, b.0))
        }
    }
}
SET001 commented 4 years ago

@lcnr yeah, thanx. This is much better than what I had:

    let mut groupList:Vec<CompGroup> = Vec::new();
    for(_collision, position, entitiy) in (&mut collisions, &positions, &entities).join() {
      groupList.push(CompGroup{
        entitiy,
        position
      })
    }