Borrow/reference issues


#1

I am trying to build a simple game while learning rust. I have a hash map of ‘Tiles’ and a vector of ‘Unit’ with the trait ‘Entity’ within a struct called ‘World’. Each ‘Tile’ has a reference to its adjacent tiles in a hash map and each ‘Unit’ has a reference to the tile it is currently occupying. The idea is that the ‘World’ struct owns everything.

struct World {
    map: HashMap<(i32,i32), RwLock<Tile>>,
    entities: Vec<Box<RwLock<Unit>>>,
}

struct Tile {
    occupier: Option<Box<RwLock<Unit>>>,
    position: (i32,i32),
    adjacent: HashMap<Direction, RwLock<Tile>>,
}

struct Unit {
    hp: u32,
    position: RwLock<Tile>,
}

#[derive(PartialEq, Eq, Hash)]
enum Direction {
    Up,
    UpRight,
    DownRight,
    Down,
    DownLeft,
    UpLeft,
}

I’m trying to implement a method for Unit which moves the unit to an adjacent tile:

impl Unit {
    fn move_entity(&mut self, dir: Direction) {
        let mut tile = self.position.write().unwrap();
        match tile.adjacent.get(&dir) {
            Some(adjtile) => {
                let ref mut occ = adjtile.write().unwrap().occupier;
                match *occ {
                    Option::Some(_) => unimplemented!(), // tile already occupied, error
                    Option::None => {
                        *occ = Option::Some(tile.occupier.unwrap());
                        tile.occupier = Option::None;
                    },
                }
            },
            None => unimplemented!(), // tile does not exist, error
        }
    }
}

However, the compiler complains that tile is borrowed in the match statement:

error[E0507]: cannot move out of borrowed content
  --> src\main.rs:67:45
   |
67 |                         *occ = Option::Some(tile.occupier.unwrap());
   |                                             ^^^^ cannot move out of borrowed content

error[E0502]: cannot borrow `tile` as mutable because it is also borrowed as immutable
  --> src\main.rs:68:25
   |
61 |         match tile.adjacent.get(&dir) {
   |               ---- immutable borrow occurs here
...
68 |                         tile.occupier = Option::None;
   |                         ^^^^ mutable borrow occurs here
...
73 |         }
   |         - immutable borrow ends here

I’m not sure how to go about resolving this.


#2

This is a bit tricky indeed.

Normally, Rust lets you borrow individual fields of a struct separately. However, in this case you’re accessing the struct fields through the RefMut wrapper that RwLock.write() returns, which has a Deref(Mut) implementation that lets you access the contained Tile. Rust isn’t clever enough to figure out that you still ought to be allowed to borrow struct fields separately.

There’s another problem that you’re trying to unwrap out of the borrowed Option which is not allowed. But since you’re swapping the options anyway, there’s a builtin function for that.

You can work around that by making the destructuring into fields explicit:

    fn move_entity(&mut self, dir: Direction) {
        let mut tile = self.position.write().unwrap();
        // Destructuring here!
        let Tile { ref mut occupier, ref mut adjacent, .. } = *tile;
        match adjacent.get(&dir) {
            Some(adjtile) => {
                let ref mut occ = adjtile.write().unwrap().occupier;
                match *occ {
                    Option::Some(_) => unimplemented!(),
                    Option::None => {
                        // Using swap() here. If you like to still panic on None,
                        // insert a check.
                        std::mem::swap(occ, occupier);
                    },
                }
...

#3

Oh, I see, that makes sense. Thanks for the help. Is there somewhere I can read more about this in more detail?

I was wondering if what I have written is a good implementation in rust, or if there is a more rust way to do what i’m doing?

Also, how did you get your code to be highlighted in your post?


#4

This is basically a bad approach to take in Rust. Rust favors composition over object-orientation, and for good reasons. Object-oriented data structures quickly become bloated and unable to scale. Try to keep data separated by their usage, else you’ll need to do some complex lifetime management.

Anyway, this is your issue:

let mut tile = self.position.write().unwrap();
match tile.adjacent.get(&dir) { }

At this point, you are performing an immutable borrow of the tile variable

*occ = Option::Some(tile.occupier.unwrap());
tile.occupier = Option::None;

Problem here is that you’re already immutably borrowing the tile value, so you cannot mutably borrow tile while it is already immutably borrowed. A workaround for your situation is to perform a pattern that is queuing future actions into a temporary location which is initially set to None.

let mut possible_value = None;
match self.item.get(&something) {
    // do stuff
    if value_needed { possible_value = Some(value); }
}

You can then wait until the first immutable borrow is finished, after the match has ended, and check to see if you need to later perform a mutable borrow to set this new state.

if let Some(value) = possible_value {
    self.item = value;
}

#5

I think you can also replace std::mem::swap usage with Option::take in this case, i.e.:

*occ = occupier.take();

#6

Indeed, that is nicer.