Problem with Rc and mutability

help
1

I have the following situation:

let mut e = scene.create_entity();

The type of e is std::rc::Rc<entity::Entity>.

The code that creates that variable is the following:

    pub fn create_entity(&mut self) -> Rc<Entity> {
        let e = Entity::new();
        self.next_id += 1;
        let rce = Rc::new(e);
        self.add(rce.clone());
        rce
    }

Next I have the following code:

        let mut ic = ImageComponent::new();
        ic.texture = Some(tm.get(&String::from("wabbit")));
        e.add(Rc::new(ic));

Where e.add() is actually

    pub fn add(&mut self, component: Rc<Component>) {
        self.components.add(component);
    }

And the compiler is complaining with the following error:

error[E0596]: cannot borrow immutable borrowed content as mutable
   --> src/engine.rs:346:9
    |
346 |         e.add(Rc::new(ic));
    |         ^ cannot borrow as mutable

I tried coning e but it didn't work. I tried with make_mut but to no avail.. I think I'm missing something obvious but I can't come up with the solution.

Many thanks in advance!

2

Rc does not permit borrowing the interior value as mutable. You'll need to wrap this into Rc<RefCell<Entity>>, and then get a (dynamic) mutable reference via the RefCell.

3

Damn.. RefCell smells of bad code design then.. :frowning:

4

Do you actually need Rc? :slight_smile:

5

Probably not. I suspect I added the Rc to make it easier to use the entity as that lets me clone() it and call its methods where I need them without having to make sure it's not being moved around.
I'm double checking as I'm writing this.

And the problem is probably that I want to use the Scene object to create an Entity so that the scene stores the Entity in a Vec, but I also want that create_entity function to give me back the Entity that has been just created because I need to do some stuff with it. So the Rc is actually handy.

I thought I might rewrite it without Rc the following way:

    pub fn create_entity(&mut self) -> &Entity {
        let e = Entity::new();
        self.next_id += 1;
        self.add(e);
        &e
    }

But then &e doesn't live long enough to be of any use. I might give back the id of the entity so that I can get it back from the Vec containing it later on, eventually.
Any other idea?

6

Yeah, if you can model your problem as the Scene object owning the Entity values, then you can return references to the Entity values from the Scene. In the example above, you can replace &e with something that does an indexed get on the underlying vec; since you're pushing to the vec, it'll be the last item (i.e. vec.last().unwrap())

7

I agree but I'm going to end up in more troubles anyway. I need to redesign quite a few systems. That same Entity is going to end up in a list of items that should be added to the scene and during the next update that entity ends up in the list of entities but at the same time it should end up in a list of entities to awake but of course I cannot add a reference of that entity to the second Vec as I'd borrow an entity that I need to consume from the other Vec as well.

8

You could add a reference to that Entity to the second vec so long as that 2nd vec isn't also owned by the Scene (directly or indirectly) so that you don't attempt to create a self-referential struct. Is there a clear single owner of all entities? If so, you should be able to sort this out without resorting to cloning.

But, you know your system better than I so ... :slight_smile:

9

The scene has a field that is a struct that contains the vec of Entity which is the real owner of all the entities. I have two more vecs that own the entities not yet added to the scene and those that should be removed (both at the next frame of the game). Then there is this "awake" vec that is just a list of those entities that have moved from the "to add" vec to the "entities" vec during the current frame.

It sounds doable. But I need to rewrite some other stuff before I can make sure that the system can work this way.

10

Yeah, it does sound doable - at any given point in time, an Entity is owned by a single vec (and you just move ownership around). Let us know how you make out.

11

How would you move something that is coming from a borrow in a for loop?

I have some code like this:

    pub fn update_lists(&mut self) {
        if self.to_add.len() > 0 {
            for to_add_it in &self.to_add {
                let entities_contains = self.entities.iter().any(|e| *e == *to_add_it);
                if !entities_contains {
                    self.entities.push(*to_add_it);
                    //scene.tag_lists.entity_added(to_add_it);
                    to_add_it.entity_added();
                }
            }
            self.unsorted = true;
        }
    }

But that self.entities.push(*to_add_it) isn't working as I'm trying to move that entity in the entities vec from the to_add vec but the element is coming from a borrow as I'm looping over the to_add vec.

12

You'll probably want to use drain to take the values out of self.to_add as you're looping over them - that will give you ownership over them inside the loop body, and you can add them to the target list. Are you conditionally leaving the elements in self.to_add though? It looks like it from the code above, but just wondering. If so, something like https://github.com/rust-lang/rust/issues/43244 would make that easier.

13

In general RefCell is not the only option. You could also use Cell for just some of the fields, which is zero overhead (unlike RefCell).

14

After some trial and errors it looks like it might be easier to just have a single vec of entities and add some flags to the struct of the entity itself and manage the states that way. It looks much easier than using different arrays even though different arrays are probably quicker to loop over.

15

That might be a good compromise indeed. If you don't have too many entities then performance may not be impacted drastically. This is not what the data oriented crowd would advise, but you can always redesign later if perf becomes an issue.