Lifetime issues with component storages

[sunjay]

I originally posted about this in the users forum and then on Reddit.

Terminology Note: When I say "owned X" in the text below, I mean that you have ownership of a value X of type T, not a reference to that value (e.g. &T or &mut T)

You can look at this issue from two perspectives:

1. It is hard to get a SystemData containing references to storages.
   Example: You can get (ReadStorage<A>, ReadStorage<B>) by calling the system_data method on World but not (&ReadStorage<A>, &ReadStorage<B>)
2. It is not possible to use the Join trait with the owned versions of component storages.
   Example: You can do (&a, &b).join() but not (a, b).join()

Both of these issues exist for good reasons, but they make code like the following very hard to write:

fn iter_components<'a>(world: &'a World) -> impl Iterator<Item=(&Position, &Velocity)> + 'a {
    // Two *owned* storages allocated on the stack of this function
    let (positions, velocities) = world.system_data::<(ReadStorage<Position>, ReadStorage<Velocity>)>();

    // Returning a reference to stack allocated data (does not and should not work!)
    (&positions, &velocities).join()
}

This doesn't work and you get the following error: (copied from users forum post, see that for the full code example)

error[E0515]: cannot return value referencing local variable `velocities`       
  --> src/main.rs:22:5                                                          
   |                                                                            
22 |     (&positions, &velocities).join()                                       
   |     ^^^^^^^^^^^^^-----------^^^^^^^^                                       
   |     |            |                                                         
   |     |            `velocities` is borrowed here                             
   |     returns a value referencing data owned by the current function         
                                                                                
error[E0515]: cannot return value referencing local variable `positions`        
  --> src/main.rs:22:5                                                          
   |                                                                            
22 |     (&positions, &velocities).join()                                       
   |     ^----------^^^^^^^^^^^^^^^^^^^^^                                       
   |     ||                                                                     
   |     |`positions` is borrowed here                                          
   |     returns a value referencing data owned by the current function

If either of the two things I listed above weren't the case, this code could potentially compile:

1. If you could get a system data instance containing references to storages: (the storages would have to be owned within the world or something)

fn iter_components<'a>(world: &'a World) -> impl Iterator<Item=(&Position, &Velocity)> + 'a {
    // Two references to storages allocated somewhere by World
    // Notice the `&` before the `ReadStorage` types here
    let (positions, velocities) = world.system_data::<(&ReadStorage<Position>, &ReadStorage<Velocity>)>();

    // positions and velocities are both references, so we can use them here no problem
    (positions, velocities).join()
}

2. If it were possible to use the Join trait on owned versions of the storages:

fn iter_components<'a>(world: &'a World) -> impl Iterator<Item=(&Position, &Velocity)> + 'a {
    // Two *owned* storages allocated on the stack of this function
    let (positions, velocities) = world.system_data::<(ReadStorage<Position>, ReadStorage<Velocity>)>();

    // Storages are *moved* into this tuple that we call `join` on. This works!
    (positions, velocities).join()
}

A note about this: ReadStorage makes it pretty obvious that we are immutably borrowing here, but with WriteStorage we would probably need some methods like read_owned() and write_owned() to emulate what you could do with & and &mut before. See the following code for more details:

fn iter_components<'a>(world: &'a World) -> impl Iterator<Item=(&mut Position, &Velocity)> + 'a {
    // Two *owned* storages allocated on the stack of this function
    let (positions, velocities) = world.system_data::<(WriteStorage<Position>, WriteStorage<Velocity>)>();

    // You can join over either of these storages in two modes:
    // * `&positions` for read-only access to the components
    // * `&mut positions` for read-write access to the components
    // `write_owned()` could wrap the storage in a type that implements
    // the `Join` trait and allows access to mutable references
    // Instead of `read_owned()` we could default to the behaviour that
    // the owned version of a storage only allows reading

    // Storages are *moved* into this tuple that we call `join` on. This works!
    (positions.write_owned(), velocities.read_owned()).join()
}

I think this solution is probably the easiest to implement because it doesn't involve the World having to hold on to arbitrary storages for some extended amount of time.

The API I would suggest is:

• ReadStorage<T> and WriteStorage<T> implement Join and produce &T
• WriteStorage<T> provides a method (e.g. write_owned()) that returns an owned value that implements Join and produces &mut T (perhaps something similar to MaybeJoin?)

This may cause issues with World because it's probably not a good thing if a storage lives longer than expected. I am not familiar enough with the internals to comment on that.

[Xaeroxe]

Option 1 is much more serviceable in my opinion. Join::join takes self as a parameter, consuming it, which is very likely to cause confusion for users who might start operating under the mistaken belief they can't use join twice on a storage.

[sunjay]

Option 1 is much more serviceable

I'm happy with either. I don't actually know the internals well so my suggestion was just a guess :)

[torkleyy]

Thanks for filing this @sunjay!

I don't see a good way to implement Option 1. These values need to be held in memory because they protect memory from being accessed in an invalid manner.

What you can do though:

struct Iter<'a> {
    pos: WriteStorage<'a, Position>,
    vel: ReadStorage<'a, Velocity>,
}

impl<'a> Iter<'a> {
    fn iter(&mut self) -> impl Iterator<Item = (&mut Position, &Velocity)> + 'a {
        (&mut self.pos, &self.vel).join()
    }
}

fn iter_components<'a>(world: &'a World) -> Iter<'a> {
    let (pos, vel) = world.system_data::<(WriteStorage<Position>, WriteStorage<Velocity>)>();

    Iter {
        pos, vel,
    }
}

// Then:

iter_components(&world).iter().do_whatever();

This works because Rust will just keep the value in memory for as long as necessary and drop it once it's not borrowed anymore (so basically create the temporary for you).

As for the second option, that could be implemented, yes. I share @Xaeroxe's concern though.

I don't quite know what you're trying to achieve, but what I do usually is:

world.exec(|(vel: ReadStorage<Velocity>, pos: WriteStorage<Position>)| {
    (&vel, &mut pos)
        .join()
        .do_something()
});

If you want to hide the world and you plan on creating more of those convenience functions, one thing you can do is create RefJoin and MutJoin which wrap a T where &T / &mut T is Join and implement Join with that constraint. That would allow you to return (MutJoin(pos), RefJoin(vel)).join().

[sunjay]

I don't quite know what you're trying to achieve

I have a bunch of components that I want to move from one World to another. I want to do this several times in different places, so I want one function/struct/something that deals with iterating through all the components. It's hard to do that starting from a world without first defining a system data for all the components.

My goal is to be able to write a function with a signature similar to:

fn iter_components<'a>(world: &'a World) -> impl Iterator<Item=(&mut Position, &Velocity)> + 'a

If you want to hide the world and you plan on creating more of those convenience functions, one thing you can do is create RefJoin and MutJoin which wrap a T where &T / &mut T is Join and implement Join with that constraint. That would allow you to return (MutJoin(pos), RefJoin(vel)).join().

Could something like MutJoin or RefJoin be added to specs?

[torkleyy]

Ah, I see.

Could something like MutJoin or RefJoin be added to specs?

I'm not against it, but it's kind of a special case. I'd like to hear @Xaeroxe's and @WaDelma's opinion prior to merging it.